diff options
91 files changed, 2908 insertions, 214 deletions
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bd0f9184cd6..c359670759e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -208,6 +208,17 @@ Naming/RescuedExceptionsVariableName: RSpec/ContextWording: Enabled: false +RSpec/EmptyLineAfterSharedExample: + Exclude: + - 'ee/spec/mailers/notify_spec.rb' + - 'ee/spec/services/quick_actions/interpret_service_spec.rb' + - 'spec/controllers/repositories/git_http_controller_spec.rb' + - 'spec/finders/projects/serverless/functions_finder_spec.rb' + - 'spec/lib/gitlab/hook_data/issuable_builder_spec.rb' + - 'spec/lib/gitlab/legacy_github_import/importer_spec.rb' + - 'spec/models/event_spec.rb' + - 'spec/support/shared_examples/models/issuable_hook_data_shared_examples.rb' + # Offense count: 879 # Cop supports --auto-correct. # Configuration parameters: EnforcedStyle. diff --git a/app/assets/javascripts/alert_management/components/alert_management_list.vue b/app/assets/javascripts/alert_management/components/alert_management_list.vue index b4c68767ab3..741f04cfd9b 100644 --- a/app/assets/javascripts/alert_management/components/alert_management_list.vue +++ b/app/assets/javascripts/alert_management/components/alert_management_list.vue @@ -5,17 +5,21 @@ import { GlLoadingIcon, GlTable, GlAlert, - GlNewDropdown, - GlNewDropdownItem, + GlIcon, + GlDropdown, + GlDropdownItem, GlTabs, GlTab, GlBadge, } from '@gitlab/ui'; +import createFlash from '~/flash'; import { s__ } from '~/locale'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; import getAlerts from '../graphql/queries/getAlerts.query.graphql'; import { ALERTS_STATUS, ALERTS_STATUS_TABS } from '../constants'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; +import updateAlertStatus from '../graphql/mutations/update_alert_status.graphql'; +import { capitalizeFirstCharacter } from '~/lib/utils/text_utility'; const tdClass = 'table-col d-flex d-md-table-cell align-items-center'; @@ -53,6 +57,8 @@ export default { }, { key: 'status', + thClass: 'w-15p', + trClass: 'w-15p', label: s__('AlertManagement|Status'), tdClass: `${tdClass} rounded-bottom text-capitalize`, }, @@ -70,8 +76,9 @@ export default { GlAlert, GlDeprecatedButton, TimeAgo, - GlNewDropdown, - GlNewDropdownItem, + GlDropdown, + GlDropdownItem, + GlIcon, GlTabs, GlTab, GlBadge, @@ -140,6 +147,25 @@ export default { filterALertsByStatus(tabIndex) { this.statusFilter = this.$options.statusTabs[tabIndex].status; }, + capitalizeFirstCharacter, + updateAlertStatus(status, iid) { + this.$apollo + .mutate({ + mutation: updateAlertStatus, + variables: { + iid, + status: status.toUpperCase(), + projectPath: this.projectPath, + }, + }) + .catch(() => { + createFlash( + s__( + 'AlertManagement|There was an error while updating the status of the alert. Please try again.', + ), + ); + }); + }, }, }; </script> @@ -190,11 +216,26 @@ export default { </template> <template #cell(status)="{ item }"> - <gl-new-dropdown :text="item.status"> - <gl-new-dropdown-item v-for="(label, field) in $options.statuses" :key="field"> - {{ label }} - </gl-new-dropdown-item> - </gl-new-dropdown> + <gl-dropdown + :text="capitalizeFirstCharacter(item.status.toLowerCase())" + class="w-100" + right + > + <gl-dropdown-item + v-for="(label, field) in $options.statuses" + :key="field" + @click="updateAlertStatus(label, item.iid)" + > + <span class="d-flex"> + <gl-icon + class="flex-shrink-0 append-right-4" + :class="{ invisible: label.toUpperCase() !== item.status }" + name="mobile-issue-close" + /> + {{ label }} + </span> + </gl-dropdown-item> + </gl-dropdown> </template> <template #empty> diff --git a/app/assets/javascripts/alert_management/graphql/mutations/update_alert_status.graphql b/app/assets/javascripts/alert_management/graphql/mutations/update_alert_status.graphql new file mode 100644 index 00000000000..009ae0b2930 --- /dev/null +++ b/app/assets/javascripts/alert_management/graphql/mutations/update_alert_status.graphql @@ -0,0 +1,9 @@ +mutation ($projectPath: ID!, $status: AlertManagementStatus!, $iid: String!) { + updateAlertStatus(input: { iid: $iid, status: $status, projectPath: $projectPath }) { + errors + alert { + iid, + status, + } + } +} diff --git a/app/assets/javascripts/alert_management/list.js b/app/assets/javascripts/alert_management/list.js index ba3e60354de..cae6a536b56 100644 --- a/app/assets/javascripts/alert_management/list.js +++ b/app/assets/javascripts/alert_management/list.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createDefaultClient from '~/lib/graphql'; +import { defaultDataIdFromObject } from 'apollo-cache-inmemory'; import { parseBoolean } from '~/lib/utils/common_utils'; import AlertManagementList from './components/alert_management_list.vue'; @@ -17,7 +18,20 @@ export default () => { userCanEnableAlertManagement = parseBoolean(userCanEnableAlertManagement); const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(), + defaultClient: createDefaultClient( + {}, + { + cacheConfig: { + dataIdFromObject: object => { + // eslint-disable-next-line no-underscore-dangle + if (object.__typename === 'AlertManagementAlert') { + return object.iid; + } + return defaultDataIdFromObject(object); + }, + }, + }, + ), }); return new Vue({ diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 98c660077bb..941365d9d1d 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -224,6 +224,7 @@ export default { methods: { ...mapActions(['startTaskList']), ...mapActions('diffs', [ + 'moveToNeighboringCommit', 'setBaseConfig', 'fetchDiffFiles', 'fetchDiffFilesMeta', @@ -344,9 +345,16 @@ export default { break; } }); + + if (this.commit && this.glFeatures.mrCommitNeighborNav) { + Mousetrap.bind('c', () => this.moveToNeighboringCommit({ direction: 'next' })); + Mousetrap.bind('x', () => this.moveToNeighboringCommit({ direction: 'previous' })); + } }, removeEventListeners() { Mousetrap.unbind(['[', 'k', ']', 'j']); + Mousetrap.unbind('c'); + Mousetrap.unbind('x'); }, jumpToFile(step) { const targetIndex = this.currentDiffIndex + step; diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index 9d4edd84f25..ee93ca020e8 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -1,10 +1,18 @@ <script> +import { mapActions } from 'vuex'; +import { GlButtonGroup, GlButton, GlIcon, GlTooltipDirective } from '@gitlab/ui'; + +import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; + import UserAvatarLink from '~/vue_shared/components/user_avatar/user_avatar_link.vue'; import Icon from '~/vue_shared/components/icon.vue'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; + import CommitPipelineStatus from '~/projects/tree/components/commit_pipeline_status_component.vue'; + import initUserPopovers from '../../user_popovers'; +import { setUrlParams } from '../../lib/utils/url_utility'; /** * CommitItem @@ -18,7 +26,16 @@ import initUserPopovers from '../../user_popovers'; * coexist, but there is an issue to remove the duplication. * https://gitlab.com/gitlab-org/gitlab-foss/issues/51613 * + * EXCEPTION WARNING + * 1. The commit navigation buttons (next neighbor, previous neighbor) + * are not duplicated because: + * - We don't have the same data available on the Rails side (yet, + * without backend work) + * - This Vue component should always be what's used when in the + * context of an MR diff, so the HAML should never have any idea + * about navigating among commits. */ + export default { components: { UserAvatarLink, @@ -26,7 +43,14 @@ export default { ClipboardButton, TimeAgoTooltip, CommitPipelineStatus, + GlButtonGroup, + GlButton, + GlIcon, + }, + directives: { + GlTooltip: GlTooltipDirective, }, + mixins: [glFeatureFlagsMixin()], props: { commit: { type: Object, @@ -54,12 +78,28 @@ export default { authorAvatar() { return this.author.avatar_url || this.commit.author_gravatar_url; }, + nextCommitUrl() { + return this.commit.next_commit_id + ? setUrlParams({ commit_id: this.commit.next_commit_id }) + : ''; + }, + previousCommitUrl() { + return this.commit.prev_commit_id + ? setUrlParams({ commit_id: this.commit.prev_commit_id }) + : ''; + }, + hasNeighborCommits() { + return this.commit.next_commit_id || this.commit.prev_commit_id; + }, }, created() { this.$nextTick(() => { initUserPopovers(this.$el.querySelectorAll('.js-user-link')); }); }, + methods: { + ...mapActions('diffs', ['moveToNeighboringCommit']), + }, }; </script> @@ -123,6 +163,41 @@ export default { class="btn btn-default" /> </div> + <div + v-if="hasNeighborCommits && glFeatures.mrCommitNeighborNav" + class="commit-nav-buttons ml-3" + > + <gl-button-group> + <gl-button + :href="previousCommitUrl" + :disabled="!commit.prev_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'previous' })" + > + <span + v-if="!commit.prev_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute" + :title="__('You\'re at the first commit')" + ></span> + <gl-icon name="chevron-left" /> + {{ __('Prev') }} + </gl-button> + <gl-button + :href="nextCommitUrl" + :disabled="!commit.next_commit_id" + @click.prevent="moveToNeighboringCommit({ direction: 'next' })" + > + <span + v-if="!commit.next_commit_id" + v-gl-tooltip + class="h-100 w-100 position-absolute" + :title="__('You\'re at the last commit')" + ></span> + {{ __('Next') }} + <gl-icon name="chevron-right" /> + </gl-button> + </gl-button-group> + </div> </div> </div> </li> diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index dfa65656739..1422959a10a 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -392,6 +392,10 @@ img.emoji { } /** COMMON CLASSES **/ +/** + 🚨 Do not use these classes — they are deprecated and being removed. 🚨 + See https://gitlab.com/gitlab-org/gitlab/-/issues/217418 for more details. +**/ .prepend-top-0 { margin-top: 0; } .prepend-top-2 { margin-top: 2px; } .prepend-top-4 { margin-top: $gl-padding-4; } diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 230f390aa90..9a69afc6044 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -208,6 +208,18 @@ } } +.commit-nav-buttons { + a.btn, + button { + // See: https://gitlab.com/gitlab-org/gitlab-ui/-/issues/730 + &:last-child > svg { + margin-left: 0.25rem; + margin-right: 0; + } + } +} + + .clipboard-group, .commit-sha-group { display: inline-flex; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 398d46316f7..87b69291559 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -33,6 +33,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true) push_frontend_feature_flag(:merge_ref_head_comments, @project) push_frontend_feature_flag(:accessibility_merge_request_widget, @project) + push_frontend_feature_flag(:mr_commit_neighbor_nav, @project, default_enabled: true) end before_action do diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 1b512419e5c..3f970f7c4a1 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -25,7 +25,8 @@ module Ci RUNNER_FEATURES = { upload_multiple_artifacts: -> (build) { build.publishes_artifacts_reports? }, - refspecs: -> (build) { build.merge_request_ref? } + refspecs: -> (build) { build.merge_request_ref? }, + artifacts_exclude: -> (build) { build.supports_artifacts_exclude? } }.freeze DEFAULT_RETRIES = { @@ -920,6 +921,11 @@ module Ci failure_reason: :data_integrity_failure) end + def supports_artifacts_exclude? + options&.dig(:artifacts, :exclude)&.any? && + Gitlab::Ci::Features.artifacts_exclude_enabled? + end + def degradation_threshold var = yaml_variables.find { |v| v[:key] == DEGRADATION_THRESHOLD_VARIABLE_NAME } var[:value]&.to_i if var diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index 247ba0822fe..933a0b167e2 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -146,6 +146,14 @@ module Noteable target_id: id ) end + + def after_note_created(_note) + # no-op + end + + def after_note_destroyed(_note) + # no-op + end end Noteable.extend(Noteable::ClassMethods) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index e2d1a433935..8116f7a256f 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -14,6 +14,7 @@ class Namespace < ApplicationRecord include IgnorableColumns ignore_column :plan_id, remove_with: '13.1', remove_after: '2020-06-22' + ignore_column :trial_ends_on, remove_with: '13.2', remove_after: '2020-07-22' # Prevent users from creating unreasonably deep level of nesting. # The number 20 was taken based on maximum nesting level of diff --git a/app/models/note.rb b/app/models/note.rb index 9d0cd30f5dc..d174ba8fe83 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -159,6 +159,8 @@ class Note < ApplicationRecord after_save :touch_noteable, unless: :importing? after_destroy :expire_etag_cache after_save :store_mentions!, if: :any_mentionable_attributes_changed? + after_commit :notify_after_create, on: :create + after_commit :notify_after_destroy, on: :destroy class << self def model_name @@ -509,6 +511,14 @@ class Note < ApplicationRecord noteable_object end + def notify_after_create + noteable&.after_note_created(self) + end + + def notify_after_destroy + noteable&.after_note_destroyed(self) + end + def banzai_render_context(field) super.merge(noteable: noteable, system_note: system?) end diff --git a/app/presenters/ci/build_runner_presenter.rb b/app/presenters/ci/build_runner_presenter.rb index 61fcbaf691a..f63fd3c50c4 100644 --- a/app/presenters/ci/build_runner_presenter.rb +++ b/app/presenters/ci/build_runner_presenter.rb @@ -52,7 +52,7 @@ module Ci def create_archive(artifacts) return unless artifacts[:untracked] || artifacts[:paths] - { + archive = { artifact_type: :archive, artifact_format: :zip, name: artifacts[:name], @@ -61,6 +61,12 @@ module Ci when: artifacts[:when], expire_in: artifacts[:expire_in] } + + if artifacts.dig(:exclude).present? && ::Gitlab::Ci::Features.artifacts_exclude_enabled? + archive.merge(exclude: artifacts[:exclude]) + else + archive + end end def create_reports(reports, expire_in:) diff --git a/app/services/design_management/delete_designs_service.rb b/app/services/design_management/delete_designs_service.rb new file mode 100644 index 00000000000..e69f07db5bf --- /dev/null +++ b/app/services/design_management/delete_designs_service.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module DesignManagement + class DeleteDesignsService < DesignService + include RunsDesignActions + include OnSuccessCallbacks + + def initialize(project, user, params = {}) + super + + @designs = params.fetch(:designs) + end + + def execute + return error('Forbidden!') unless can_delete_designs? + + version = delete_designs! + + success(version: version) + end + + def commit_message + n = designs.size + + <<~MSG + Removed #{n} #{'designs'.pluralize(n)} + + #{formatted_file_list} + MSG + end + + private + + attr_reader :designs + + def delete_designs! + DesignManagement::Version.with_lock(project.id, repository) do + run_actions(build_actions) + end + end + + def can_delete_designs? + Ability.allowed?(current_user, :destroy_design, issue) + end + + def build_actions + designs.map { |d| design_action(d) } + end + + def design_action(design) + on_success { counter.count(:delete) } + + DesignManagement::DesignAction.new(design, :delete) + end + + def counter + ::Gitlab::UsageDataCounters::DesignsCounter + end + + def formatted_file_list + designs.map { |design| "- #{design.full_path}" }.join("\n") + end + end +end + +DesignManagement::DeleteDesignsService.prepend_if_ee('EE::DesignManagement::DeleteDesignsService') diff --git a/app/services/design_management/design_service.rb b/app/services/design_management/design_service.rb new file mode 100644 index 00000000000..54e53609646 --- /dev/null +++ b/app/services/design_management/design_service.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module DesignManagement + class DesignService < ::BaseService + def initialize(project, user, params = {}) + super + + @issue = params.fetch(:issue) + end + + # Accessors common to all subclasses: + + attr_reader :issue + + def target_branch + repository.root_ref || "master" + end + + def collection + issue.design_collection + end + + def repository + collection.repository + end + + def project + issue.project + end + end +end diff --git a/app/services/design_management/generate_image_versions_service.rb b/app/services/design_management/generate_image_versions_service.rb new file mode 100644 index 00000000000..213aac164ff --- /dev/null +++ b/app/services/design_management/generate_image_versions_service.rb @@ -0,0 +1,99 @@ +# frozen_string_literal: true + +module DesignManagement + # This service generates smaller image versions for `DesignManagement::Design` + # records within a given `DesignManagement::Version`. + class GenerateImageVersionsService < DesignService + # We limit processing to only designs with file sizes that don't + # exceed `MAX_DESIGN_SIZE`. + # + # Note, we may be able to remove checking this limit, if when we come to + # implement a file size limit for designs, there are no designs that + # exceed 40MB on GitLab.com + # + # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/22860#note_281780387 + MAX_DESIGN_SIZE = 40.megabytes.freeze + + def initialize(version) + super(version.project, version.author, issue: version.issue) + + @version = version + end + + def execute + # rubocop: disable CodeReuse/ActiveRecord + version.actions.includes(:design).each do |action| + generate_image(action) + end + # rubocop: enable CodeReuse/ActiveRecord + + success(version: version) + end + + private + + attr_reader :version + + def generate_image(action) + raw_file = get_raw_file(action) + + unless raw_file + log_error("No design file found for Action: #{action.id}") + return + end + + # Skip attempting to process images that would be rejected by CarrierWave. + return unless DesignManagement::DesignV432x230Uploader::MIME_TYPE_WHITELIST.include?(raw_file.content_type) + + # Store and process the file + action.image_v432x230.store!(raw_file) + action.save! + rescue CarrierWave::UploadError => e + Gitlab::ErrorTracking.track_exception(e, project_id: project.id, design_id: action.design_id, version_id: action.version_id) + log_error(e.message) + end + + # Returns the `CarrierWave::SanitizedFile` of the original design file + def get_raw_file(action) + raw_files_by_path[action.design.full_path] + end + + # Returns the `Carrierwave:SanitizedFile` instances for all of the original + # design files, mapping to { design.filename => `Carrierwave::SanitizedFile` }. + # + # As design files are stored in Git LFS, the only way to retrieve their original + # files is to first fetch the LFS pointer file data from the Git design repository. + # The LFS pointer file data contains an "OID" that lets us retrieve `LfsObject` + # records, which have an Uploader (`LfsObjectUploader`) for the original design file. + def raw_files_by_path + @raw_files_by_path ||= begin + LfsObject.for_oids(blobs_by_oid.keys).each_with_object({}) do |lfs_object, h| + blob = blobs_by_oid[lfs_object.oid] + file = lfs_object.file.file + # The `CarrierWave::SanitizedFile` is loaded without knowing the `content_type` + # of the file, due to the file not having an extension. + # + # Set the content_type from the `Blob`. + file.content_type = blob.content_type + h[blob.path] = file + end + end + end + + # Returns the `Blob`s that correspond to the design files in the repository. + # + # All design `Blob`s are LFS Pointer files, and are therefore small amounts + # of data to load. + # + # `Blob`s whose size are above a certain threshold: `MAX_DESIGN_SIZE` + # are filtered out. + def blobs_by_oid + @blobs ||= begin + items = version.designs.map { |design| [version.sha, design.full_path] } + blobs = repository.blobs_at(items) + blobs.reject! { |blob| blob.lfs_size > MAX_DESIGN_SIZE } + blobs.index_by(&:lfs_oid) + end + end + end +end diff --git a/app/services/design_management/on_success_callbacks.rb b/app/services/design_management/on_success_callbacks.rb new file mode 100644 index 00000000000..be55890a02d --- /dev/null +++ b/app/services/design_management/on_success_callbacks.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module DesignManagement + module OnSuccessCallbacks + def on_success(&block) + success_callbacks.push(block) + end + + def success(*_) + while cb = success_callbacks.pop + cb.call + end + + super + end + + private + + def success_callbacks + @success_callbacks ||= [] + end + end +end diff --git a/app/services/design_management/runs_design_actions.rb b/app/services/design_management/runs_design_actions.rb new file mode 100644 index 00000000000..4bd6bb45658 --- /dev/null +++ b/app/services/design_management/runs_design_actions.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module DesignManagement + module RunsDesignActions + NoActions = Class.new(StandardError) + + # this concern requires the following methods to be implemented: + # current_user, target_branch, repository, commit_message + # + # Before calling `run_actions`, you should ensure the repository exists, by + # calling `repository.create_if_not_exists`. + # + # @raise [NoActions] if actions are empty + def run_actions(actions) + raise NoActions if actions.empty? + + sha = repository.multi_action(current_user, + branch_name: target_branch, + message: commit_message, + actions: actions.map(&:gitaly_action)) + + ::DesignManagement::Version + .create_for_designs(actions, sha, current_user) + .tap { |version| post_process(version) } + end + + private + + def post_process(version) + version.run_after_commit_or_now do + ::DesignManagement::NewVersionWorker.perform_async(id) + end + end + end +end diff --git a/app/services/design_management/save_designs_service.rb b/app/services/design_management/save_designs_service.rb new file mode 100644 index 00000000000..a09c19bc885 --- /dev/null +++ b/app/services/design_management/save_designs_service.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +module DesignManagement + class SaveDesignsService < DesignService + include RunsDesignActions + include OnSuccessCallbacks + + MAX_FILES = 10 + + def initialize(project, user, params = {}) + super + + @files = params.fetch(:files) + end + + def execute + return error("Not allowed!") unless can_create_designs? + return error("Only #{MAX_FILES} files are allowed simultaneously") if files.size > MAX_FILES + + uploaded_designs, version = upload_designs! + skipped_designs = designs - uploaded_designs + + success({ designs: uploaded_designs, version: version, skipped_designs: skipped_designs }) + rescue ::ActiveRecord::RecordInvalid => e + error(e.message) + end + + private + + attr_reader :files + + def upload_designs! + ::DesignManagement::Version.with_lock(project.id, repository) do + actions = build_actions + + [actions.map(&:design), actions.presence && run_actions(actions)] + end + end + + # Returns `Design` instances that correspond with `files`. + # New `Design`s will be created where a file name does not match + # an existing `Design` + def designs + @designs ||= files.map do |file| + collection.find_or_create_design!(filename: file.original_filename) + end + end + + def build_actions + files.zip(designs).flat_map do |(file, design)| + Array.wrap(build_design_action(file, design)) + end + end + + def build_design_action(file, design) + content = file_content(file, design.full_path) + return if design_unchanged?(design, content) + + action = new_file?(design) ? :create : :update + on_success { ::Gitlab::UsageDataCounters::DesignsCounter.count(action) } + + DesignManagement::DesignAction.new(design, action, content) + end + + # Returns true if the design file is the same as its latest version + def design_unchanged?(design, content) + content == existing_blobs[design]&.data + end + + def commit_message + <<~MSG + Updated #{files.size} #{'designs'.pluralize(files.size)} + + #{formatted_file_list} + MSG + end + + def formatted_file_list + filenames.map { |name| "- #{name}" }.join("\n") + end + + def filenames + @filenames ||= files.map(&:original_filename) + end + + def can_create_designs? + Ability.allowed?(current_user, :create_design, issue) + end + + def new_file?(design) + !existing_blobs[design] + end + + def file_content(file, full_path) + transformer = ::Lfs::FileTransformer.new(project, repository, target_branch) + transformer.new_file(full_path, file.to_io).content + end + + # Returns the latest blobs for the designs as a Hash of `{ Design => Blob }` + def existing_blobs + @existing_blobs ||= begin + items = designs.map { |d| ['HEAD', d.full_path] } + + repository.blobs_at(items).each_with_object({}) do |blob, h| + design = designs.find { |d| d.full_path == blob.path } + + h[design] = blob + end + end + end + end +end + +DesignManagement::SaveDesignsService.prepend_if_ee('EE::DesignManagement::SaveDesignsService') diff --git a/app/services/lfs/file_transformer.rb b/app/services/lfs/file_transformer.rb index 88f59b820a4..69d33e1c873 100644 --- a/app/services/lfs/file_transformer.rb +++ b/app/services/lfs/file_transformer.rb @@ -5,8 +5,7 @@ module Lfs # return a transformed result with `content` and `encoding` to commit. # # The `repository` passed to the initializer can be a Repository or - # a DesignManagement::Repository (an EE-specific class that inherits - # from Repository). + # class that inherits from Repository. # # The `repository_type` property will be one of the types named in # `Gitlab::GlRepository.types`, and is recorded on the `LfsObjectsProject` diff --git a/app/services/notes/post_process_service.rb b/app/services/notes/post_process_service.rb index 53b3b57f4af..bc86118a150 100644 --- a/app/services/notes/post_process_service.rb +++ b/app/services/notes/post_process_service.rb @@ -16,10 +16,18 @@ module Notes return if @note.for_personal_snippet? @note.create_cross_references! + ::SystemNoteService.design_discussion_added(@note) if create_design_discussion_system_note? + execute_note_hooks end end + private + + def create_design_discussion_system_note? + @note && @note.for_design? && @note.start_of_discussion? + end + def hook_data Gitlab::DataBuilder::Note.build(@note, @note.author) end diff --git a/app/services/projects/hashed_storage/base_repository_service.rb b/app/services/projects/hashed_storage/base_repository_service.rb index d81aa4de9f1..065bf8725be 100644 --- a/app/services/projects/hashed_storage/base_repository_service.rb +++ b/app/services/projects/hashed_storage/base_repository_service.rb @@ -8,13 +8,15 @@ module Projects class BaseRepositoryService < BaseService include Gitlab::ShellAdapter - attr_reader :old_disk_path, :new_disk_path, :old_storage_version, :logger, :move_wiki + attr_reader :old_disk_path, :new_disk_path, :old_storage_version, + :logger, :move_wiki, :move_design def initialize(project:, old_disk_path:, logger: nil) @project = project @logger = logger || Gitlab::AppLogger @old_disk_path = old_disk_path @move_wiki = has_wiki? + @move_design = has_design? end protected @@ -23,6 +25,10 @@ module Projects gitlab_shell.repository_exists?(project.repository_storage, "#{old_wiki_disk_path}.git") end + def has_design? + gitlab_shell.repository_exists?(project.repository_storage, "#{old_design_disk_path}.git") + end + def move_repository(from_name, to_name) from_exists = gitlab_shell.repository_exists?(project.repository_storage, "#{from_name}.git") to_exists = gitlab_shell.repository_exists?(project.repository_storage, "#{to_name}.git") @@ -58,12 +64,18 @@ module Projects project.clear_memoization(:wiki) end + if move_design + result &&= move_repository(old_design_disk_path, new_design_disk_path) + project.clear_memoization(:design_repository) + end + result end def rollback_folder_move move_repository(new_disk_path, old_disk_path) move_repository(new_wiki_disk_path, old_wiki_disk_path) + move_repository(new_design_disk_path, old_design_disk_path) if move_design end def try_to_set_repository_read_only! @@ -87,8 +99,18 @@ module Projects def new_wiki_disk_path @new_wiki_disk_path ||= "#{new_disk_path}#{wiki_path_suffix}" end + + def design_path_suffix + @design_path_suffix ||= ::Gitlab::GlRepository::DESIGN.path_suffix + end + + def old_design_disk_path + @old_design_disk_path ||= "#{old_disk_path}#{design_path_suffix}" + end + + def new_design_disk_path + @new_design_disk_path ||= "#{new_disk_path}#{design_path_suffix}" + end end end end - -Projects::HashedStorage::BaseRepositoryService.prepend_if_ee('EE::Projects::HashedStorage::BaseRepositoryService') diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 309eab59463..60e5b7e2639 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -135,7 +135,8 @@ module Projects return if project.hashed_storage?(:repository) move_repo_folder(@new_path, @old_path) - move_repo_folder("#{@new_path}.wiki", "#{@old_path}.wiki") + move_repo_folder(new_wiki_repo_path, old_wiki_repo_path) + move_repo_folder(new_design_repo_path, old_design_repo_path) end def move_repo_folder(from_name, to_name) @@ -157,8 +158,9 @@ module Projects # Disk path is changed; we need to ensure we reload it project.reload_repository! - # Move wiki repo also if present - move_repo_folder("#{@old_path}.wiki", "#{@new_path}.wiki") + # Move wiki and design repos also if present + move_repo_folder(old_wiki_repo_path, new_wiki_repo_path) + move_repo_folder(old_design_repo_path, new_design_repo_path) end def move_project_uploads(project) @@ -170,6 +172,22 @@ module Projects @new_namespace.full_path ) end + + def old_wiki_repo_path + "#{old_path}#{::Gitlab::GlRepository::WIKI.path_suffix}" + end + + def new_wiki_repo_path + "#{new_path}#{::Gitlab::GlRepository::WIKI.path_suffix}" + end + + def old_design_repo_path + "#{old_path}#{::Gitlab::GlRepository::DESIGN.path_suffix}" + end + + def new_design_repo_path + "#{new_path}#{::Gitlab::GlRepository::DESIGN.path_suffix}" + end end end diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index 21cd84117d7..0082b7bfed5 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -58,6 +58,10 @@ module Projects if project.wiki.repository_exists? mirror_repository(type: Gitlab::GlRepository::WIKI) end + + if project.design_repository.exists? + mirror_repository(type: ::Gitlab::GlRepository::DESIGN) + end end def mirror_repository(type: Gitlab::GlRepository::PROJECT) @@ -106,6 +110,13 @@ module Projects wiki.disk_path, "#{new_project_path}.wiki") end + + if design_repository.exists? + GitlabShellWorker.perform_async(:mv_repository, + old_repository_storage, + design_repository.disk_path, + "#{new_project_path}.design") + end end end @@ -140,5 +151,3 @@ module Projects end end end - -Projects::UpdateRepositoryStorageService.prepend_if_ee('EE::Projects::UpdateRepositoryStorageService') diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 1b9f5971f73..6bf04c55415 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -245,6 +245,34 @@ module SystemNoteService def auto_resolve_prometheus_alert(noteable, project, author) ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).auto_resolve_prometheus_alert end + + # Parameters: + # - version [DesignManagement::Version] + # + # Example Note text: + # + # "added [1 designs](link-to-version)" + # "changed [2 designs](link-to-version)" + # + # Returns [Array<Note>]: the created Note objects + def design_version_added(version) + ::SystemNotes::DesignManagementService.new(noteable: version.issue, project: version.issue.project, author: version.author).design_version_added(version) + end + + # Called when a new discussion is created on a design + # + # discussion_note - DiscussionNote + # + # Example Note text: + # + # "started a discussion on screen.png" + # + # Returns the created Note object + def design_discussion_added(discussion_note) + design = discussion_note.noteable + + ::SystemNotes::DesignManagementService.new(noteable: design.issue, project: design.project, author: discussion_note.author).design_discussion_added(discussion_note) + end end SystemNoteService.prepend_if_ee('EE::SystemNoteService') diff --git a/app/services/system_notes/design_management_service.rb b/app/services/system_notes/design_management_service.rb new file mode 100644 index 00000000000..a773877e25b --- /dev/null +++ b/app/services/system_notes/design_management_service.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module SystemNotes + class DesignManagementService < ::SystemNotes::BaseService + include ActionView::RecordIdentifier + + # Parameters: + # - version [DesignManagement::Version] + # + # Example Note text: + # + # "added [1 designs](link-to-version)" + # "changed [2 designs](link-to-version)" + # + # Returns [Array<Note>]: the created Note objects + def design_version_added(version) + events = DesignManagement::Action.events + link_href = designs_path(version: version.id) + + version.designs_by_event.map do |(event_name, designs)| + note_data = self.class.design_event_note_data(events[event_name]) + icon_name = note_data[:icon] + n = designs.size + + body = "%s [%d %s](%s)" % [note_data[:past_tense], n, 'design'.pluralize(n), link_href] + + create_note(NoteSummary.new(noteable, project, author, body, action: icon_name)) + end + end + + # Called when a new discussion is created on a design + # + # discussion_note - DiscussionNote + # + # Example Note text: + # + # "started a discussion on screen.png" + # + # Returns the created Note object + def design_discussion_added(discussion_note) + design = discussion_note.noteable + + body = _('started a discussion on %{design_link}') % { + design_link: '[%s](%s)' % [ + design.filename, + designs_path(vueroute: design.filename, anchor: dom_id(discussion_note)) + ] + } + + action = :designs_discussion_added + + create_note(NoteSummary.new(noteable, project, author, body, action: action)) + end + + # Take one of the `DesignManagement::Action.events` and + # return: + # * an English past-tense verb. + # * the name of an icon used in renderin a system note + # + # We do not currently internationalize our system notes, + # instead we just produce English-language descriptions. + # See: https://gitlab.com/gitlab-org/gitlab/issues/30408 + # See: https://gitlab.com/gitlab-org/gitlab/issues/14056 + def self.design_event_note_data(event) + case event + when DesignManagement::Action.events[:creation] + { icon: 'designs_added', past_tense: 'added' } + when DesignManagement::Action.events[:modification] + { icon: 'designs_modified', past_tense: 'updated' } + when DesignManagement::Action.events[:deletion] + { icon: 'designs_removed', past_tense: 'removed' } + else + raise "Unknown event: #{event}" + end + end + + private + + def designs_path(params = {}) + url_helpers.designs_project_issue_path(project, noteable, params) + end + end +end diff --git a/app/services/wikis/create_attachment_service.rb b/app/services/wikis/create_attachment_service.rb index 6ef6cbc3c12..82179459345 100644 --- a/app/services/wikis/create_attachment_service.rb +++ b/app/services/wikis/create_attachment_service.rb @@ -5,12 +5,15 @@ module Wikis ATTACHMENT_PATH = 'uploads' MAX_FILENAME_LENGTH = 255 - delegate :wiki, to: :project + attr_reader :container + + delegate :wiki, to: :container delegate :repository, to: :wiki - def initialize(*args) - super + def initialize(container:, current_user: nil, params: {}) + super(nil, current_user, params) + @container = container @file_name = clean_file_name(params[:file_name]) @file_path = File.join(ATTACHMENT_PATH, SecureRandom.hex, @file_name) if @file_name @commit_message ||= "Upload attachment #{@file_name}" @@ -51,7 +54,7 @@ module Wikis end def validate_permissions! - unless can?(current_user, :create_wiki, project) + unless can?(current_user, :create_wiki, container) raise_error('You are not allowed to push to the wiki') end end diff --git a/app/views/help/_shortcuts.html.haml b/app/views/help/_shortcuts.html.haml index aa63127049c..bd5424c30c6 100644 --- a/app/views/help/_shortcuts.html.haml +++ b/app/views/help/_shortcuts.html.haml @@ -316,6 +316,18 @@ %tbody %tr %th + %th= _('Merge Request Commits') + %tr + %td.shortcut + %kbd c + %td= _('Next commit') + %tr + %td.shortcut + %kbd x + %td= _('Previous commit') + %tbody + %tr + %th %th= _('Web IDE') %tr %td.shortcut diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 8b659034fe6..b42eef32a76 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -1,6 +1,8 @@ -#----------------------------------------------------------------- WARNING: Please keep changes up-to-date with the following files: - `assets/javascripts/diffs/components/commit_item.vue` + + EXCEPTION WARNING - see above `.vue` file for de-sync drift -#----------------------------------------------------------------- - view_details = local_assigns.fetch(:view_details, false) - merge_request = local_assigns.fetch(:merge_request, nil) diff --git a/changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml b/changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml new file mode 100644 index 00000000000..a21cafe5e14 --- /dev/null +++ b/changelogs/unreleased/cat-duplicate-ci-pipelines-index-215790.yml @@ -0,0 +1,5 @@ +--- +title: Fix duplicate index removal on ci_pipelines.project_id +merge_request: 31043 +author: +type: fixed diff --git a/changelogs/unreleased/feature-next-prev-commit-buttons.yml b/changelogs/unreleased/feature-next-prev-commit-buttons.yml new file mode 100644 index 00000000000..3a53f803349 --- /dev/null +++ b/changelogs/unreleased/feature-next-prev-commit-buttons.yml @@ -0,0 +1,5 @@ +--- +title: Add Previous and Next buttons for commit-by-commit navigation +merge_request: 28596 +author: +type: added diff --git a/changelogs/unreleased/mwaw-214581-star-metrics-dashboard-endpoint.yml b/changelogs/unreleased/mwaw-214581-star-metrics-dashboard-endpoint.yml new file mode 100644 index 00000000000..3b539cd3140 --- /dev/null +++ b/changelogs/unreleased/mwaw-214581-star-metrics-dashboard-endpoint.yml @@ -0,0 +1,5 @@ +--- +title: New API endpoint for starring metrics dashboards +merge_request: 31316 +author: +type: added diff --git a/changelogs/unreleased/update-ado-image-to-0-14-0.yml b/changelogs/unreleased/update-ado-image-to-0-14-0.yml new file mode 100644 index 00000000000..320625d7f0a --- /dev/null +++ b/changelogs/unreleased/update-ado-image-to-0-14-0.yml @@ -0,0 +1,6 @@ +--- +title: Update auto-deploy-image to v0.14.0 with helm 2.16.6, --atomic deployments + and improved kubernetes 1.16 support +merge_request: 31505 +author: +type: changed diff --git a/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb b/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb index dbfe3758cda..9e78457b007 100644 --- a/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb +++ b/db/migrate/20191229140154_drop_index_ci_pipelines_on_project_id.rb @@ -8,10 +8,13 @@ class DropIndexCiPipelinesOnProjectId < ActiveRecord::Migration[5.2] disable_ddl_transaction! def up - remove_concurrent_index :ci_pipelines, :project_id + remove_concurrent_index_by_name :ci_pipelines, 'index_ci_pipelines_on_project_id' + + # extra (duplicate) index that already existed on some installs + remove_concurrent_index_by_name :ci_pipelines, 'ci_pipelines_project_id_idx' end def down - add_concurrent_index :ci_pipelines, :project_id + add_concurrent_index :ci_pipelines, :project_id, name: 'index_ci_pipelines_on_project_id' end end diff --git a/doc/api/api_resources.md b/doc/api/api_resources.md index 34b4bf934bc..f0f3e58c61f 100644 --- a/doc/api/api_resources.md +++ b/doc/api/api_resources.md @@ -72,6 +72,7 @@ The following API resources are available in the project context: | [Search](search.md) | `/projects/:id/search` (also available for groups and standalone) | | [Services](services.md) | `/projects/:id/services` | | [Tags](tags.md) | `/projects/:id/repository/tags` | +| [User-starred metrics dashboards](metrics_user_starred_dashboards.md ) | `/projects/:id/metrics/user_starred_dashboards` | | [Visual Review discussions](visual_review_discussions.md) **(STARTER)** | `/projects/:id/merge_requests/:merge_request_id/visual_review_discussions` | | [Vulnerabilities](vulnerabilities.md) **(ULTIMATE)** | `/vulnerabilities/:id` | | [Vulnerability exports](vulnerability_exports.md) **(ULTIMATE)** | `/projects/:id/vulnerability_exports` | diff --git a/doc/api/metrics_user_starred_dashboards.md b/doc/api/metrics_user_starred_dashboards.md new file mode 100644 index 00000000000..94bc1d3b4d1 --- /dev/null +++ b/doc/api/metrics_user_starred_dashboards.md @@ -0,0 +1,34 @@ +# User-starred metrics dashboards API + +The starred dashboard feature makes navigating to frequently-used dashboards easier +by displaying favorited dashboards at the top of the select list. + +## Add a star to a dashboard + +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31316) in GitLab 13.0. + +```plaintext +POST /projects/:id/metrics/user_starred_dashboards +``` + +Parameters: + +| Attribute | Type | Required | Description | +|:---------------|:---------------|:---------|:-----------------------------------------------------------------------------| +| `dashboard_path` | string | yes | URL-encoded path to file defining the dashboard which should be marked as favorite. | + +```shell +curl --header 'Private-Token: <your_access_token>' https://gitlab.example.com/api/v4/projects/20/metrics/user_starred_dashboards \ + --data-urlencode "dashboard_path=config/prometheus/dashboards/common_metrics.yml" +``` + +Example Response: + +```json +{ + "id": 5, + "dashboard_path": "config/prometheus/common_metrics.yml", + "user_id": 1, + "project_id": 20 +} +``` diff --git a/doc/development/fe_guide/style/scss.md b/doc/development/fe_guide/style/scss.md index b5e73050d6f..abfe985c389 100644 --- a/doc/development/fe_guide/style/scss.md +++ b/doc/development/fe_guide/style/scss.md @@ -17,7 +17,7 @@ led by the [GitLab UI WG](https://gitlab.com/gitlab-com/www-gitlab-com/-/merge_r #### Where are utility classes defined? - [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/) -- [`common.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/framework/common.scss) (old) +- [`common.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/framework/common.scss) (old, many classes are now deprecated) - [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/app/assets/stylesheets/utilities.scss) (new) #### Where should I put new utility classes? diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index cb9b76f6e5e..8eb192a62cf 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -63,7 +63,6 @@ file and ask your manager to review and merge. ```yaml projects: gitlab: reviewer go - gitlab-foss: reviewer go ``` ## Code style and format diff --git a/doc/topics/autodevops/customize.md b/doc/topics/autodevops/customize.md index 526a5f9be04..ac9b2ded720 100644 --- a/doc/topics/autodevops/customize.md +++ b/doc/topics/autodevops/customize.md @@ -305,6 +305,7 @@ applications. |-----------------------------------------|------------------------------------| | `ADDITIONAL_HOSTS` | Fully qualified domain names specified as a comma-separated list that are added to the Ingress hosts. | | `<ENVIRONMENT>_ADDITIONAL_HOSTS` | For a specific environment, the fully qualified domain names specified as a comma-separated list that are added to the Ingress hosts. This takes precedence over `ADDITIONAL_HOSTS`. | +| `AUTO_DEVOPS_ATOMIC_RELEASE` | As of GitLab 13.0, Auto DevOps uses [`--atomic`](https://v2.helm.sh/docs/helm/#options-43) for Helm deployments by default. Set this variable to `false` to disable the use of `--atomic` | | `AUTO_DEVOPS_BUILD_IMAGE_CNB_ENABLED` | When set to a non-empty value and no `Dockerfile` is present, Auto Build builds your application using Cloud Native Buildpacks instead of Herokuish. [More details](stages.md#auto-build-using-cloud-native-buildpacks-beta). | | `AUTO_DEVOPS_BUILD_IMAGE_EXTRA_ARGS` | Extra arguments to be passed to the `docker build` command. Note that using quotes won't prevent word splitting. [More details](#passing-arguments-to-docker-build). | | `AUTO_DEVOPS_BUILD_IMAGE_FORWARDED_CI_VARIABLES` | A [comma-separated list of CI variable names](#passing-secrets-to-docker-build) to be passed to the `docker build` command as secrets. | diff --git a/doc/topics/autodevops/upgrading_postgresql.md b/doc/topics/autodevops/upgrading_postgresql.md index ccb009905eb..2f50a897481 100644 --- a/doc/topics/autodevops/upgrading_postgresql.md +++ b/doc/topics/autodevops/upgrading_postgresql.md @@ -126,7 +126,7 @@ volumes](https://kubernetes.io/docs/concepts/storage/persistent-volumes/) used to store the underlying data for PostgreSQL is marked as `Delete` when the pods and pod claims that use the volume is deleted. -This is signficant as, when you opt into the newer 8.2.1 PostgreSQL, the older 0.7.1 PostgreSQL is +This is significant as, when you opt into the newer 8.2.1 PostgreSQL, the older 0.7.1 PostgreSQL is deleted causing the persistent volumes to be deleted as well. You can verify this by using the following command: @@ -165,12 +165,14 @@ volume](#retain-persistent-volumes). TIP: **Tip:** You can also [scope](../../ci/environments.md#scoping-environments-with-specs) the -`AUTO_DEVOPS_POSTGRES_CHANNEL` and `POSTGRES_VERSION` variables to -specific environments, e.g. `staging`. +`AUTO_DEVOPS_POSTGRES_CHANNEL`, `AUTO_DEVOPS_POSTGRES_DELETE_V1` and +`POSTGRES_VERSION` variables to specific environments, e.g. `staging`. 1. Set `AUTO_DEVOPS_POSTGRES_CHANNEL` to `2`. This opts into using the newer 8.2.1-based PostgreSQL, and removes the older 0.7.1-based PostgreSQL. +1. Set `AUTO_DEVOPS_POSTGRES_DELETE_V1` to a non-empty value. This flag is a + safeguard to prevent accidental deletion of databases. 1. Set `POSTGRES_VERSION` to `9.6.16`. This is the minimum PostgreSQL version supported. 1. Set `PRODUCTION_REPLICAS` to `0`. For other environments, use diff --git a/doc/user/permissions.md b/doc/user/permissions.md index 3f34695d011..baa1f9dc544 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -70,6 +70,7 @@ The following table depicts the various user permission levels in a project. | View confidential issues | (*2*) | ✓ | ✓ | ✓ | ✓ | | View [Releases](project/releases/index.md) | ✓ (*6*) | ✓ | ✓ | ✓ | ✓ | | View requirements **(ULTIMATE)** | ✓ | ✓ | ✓ | ✓ | ✓ | +| Manage user-starred metrics dashboards (*7*) | ✓ | ✓ | ✓ | ✓ | ✓ | | Assign issues | | ✓ | ✓ | ✓ | ✓ | | Label issues | | ✓ | ✓ | ✓ | ✓ | | Set issue weight | | ✓ | ✓ | ✓ | ✓ | @@ -170,6 +171,7 @@ The following table depicts the various user permission levels in a project. 1. Not allowed for Guest, Reporter, Developer, Maintainer, or Owner. See [Protected Branches](./project/protected_branches.md). 1. If the [branch is protected](./project/protected_branches.md#using-the-allowed-to-merge-and-allowed-to-push-settings), this depends on the access Developers and Maintainers are given. 1. Guest users can access GitLab [**Releases**](project/releases/index.md) for downloading assets but are not allowed to download the source code nor see repository information like tags and commits. +1. Actions are limited only to records owned (referenced) by user. ## Project features permissions diff --git a/doc/user/project/integrations/img/panel_context_menu_v12_10.png b/doc/user/project/integrations/img/panel_context_menu_v12_10.png Binary files differdeleted file mode 100644 index 096262fea91..00000000000 --- a/doc/user/project/integrations/img/panel_context_menu_v12_10.png +++ /dev/null diff --git a/doc/user/project/integrations/img/panel_context_menu_v13_0.png b/doc/user/project/integrations/img/panel_context_menu_v13_0.png Binary files differnew file mode 100644 index 00000000000..2d7cb923981 --- /dev/null +++ b/doc/user/project/integrations/img/panel_context_menu_v13_0.png diff --git a/doc/user/project/integrations/prometheus.md b/doc/user/project/integrations/prometheus.md index 48164a53727..2ef29caa118 100644 --- a/doc/user/project/integrations/prometheus.md +++ b/doc/user/project/integrations/prometheus.md @@ -663,10 +663,11 @@ When viewing a custom dashboard of a project, you can view the original From each of the panels in the dashboard, you can access the context menu by clicking the **{ellipsis_v}** **More actions** dropdown box above the upper right corner of the panel to take actions related to the chart's data. - + The options are: +- [Expand panel](#expand-panel) - [View logs](#view-logs-ultimate) - [Download CSV](#downloading-data-as-csv) - [Copy link to chart](#embedding-gitlab-managed-kubernetes-metrics) @@ -689,6 +690,16 @@ You can create annotations by making requests to the  +### Expand panel + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3100) in GitLab 13.0. + +To view a larger version of a visualization, expand the panel by clicking the +**{ellipsis_v}** **More actions** icon and selecting **Expand panel**. + +To return to the metrics dashboard, click the **Back** button in your +browser, or pressing the <kbd>Esc</kbd> key. + ### View Logs **(ULTIMATE)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/122013) in GitLab 12.8. @@ -857,6 +868,8 @@ Metric charts may also be hidden:  +You can open the link directly into your browser for a [detailed view of the data](#expand-panel). + ### Embedding metrics in issue templates It is also possible to embed either the default dashboard metrics or individual metrics in issue templates. For charts to render side-by-side, links to the entire metrics dashboard or individual metrics should be separated by either a comma or a space. diff --git a/lib/api/api.rb b/lib/api/api.rb index 5816d2db534..433edc1daba 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -162,6 +162,7 @@ module API mount ::API::MergeRequestDiffs mount ::API::MergeRequests mount ::API::Metrics::Dashboard::Annotations + mount ::API::Metrics::UserStarredDashboards mount ::API::Namespaces mount ::API::Notes mount ::API::Discussions diff --git a/lib/api/entities/job_request/artifacts.rb b/lib/api/entities/job_request/artifacts.rb index c6871fdd875..0d27f5a9189 100644 --- a/lib/api/entities/job_request/artifacts.rb +++ b/lib/api/entities/job_request/artifacts.rb @@ -7,6 +7,7 @@ module API expose :name expose :untracked expose :paths + expose :exclude, expose_nil: false expose :when expose :expire_in expose :artifact_type diff --git a/lib/api/entities/metrics/user_starred_dashboard.rb b/lib/api/entities/metrics/user_starred_dashboard.rb new file mode 100644 index 00000000000..d774160e3ea --- /dev/null +++ b/lib/api/entities/metrics/user_starred_dashboard.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module API + module Entities + module Metrics + class UserStarredDashboard < Grape::Entity + expose :id, :dashboard_path, :user_id, :project_id + end + end + end +end diff --git a/lib/api/metrics/user_starred_dashboards.rb b/lib/api/metrics/user_starred_dashboards.rb new file mode 100644 index 00000000000..478403b40ad --- /dev/null +++ b/lib/api/metrics/user_starred_dashboards.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module API + module Metrics + class UserStarredDashboards < Grape::API + desc 'Marks selected metrics dashboard as starred' do + success Entities::Metrics::UserStarredDashboard + end + + params do + requires :dashboard_path, type: String, allow_blank: false, coerce_with: ->(val) { CGI.unescape(val) }, + desc: 'Url encoded path to a file defining the dashboard to which the star should be added' + end + + resource :projects do + post ':id/metrics/user_starred_dashboards' do + result = ::Metrics::UsersStarredDashboards::CreateService.new(current_user, user_project, params[:dashboard_path]).execute + + if result.success? + present result.payload, with: Entities::Metrics::UserStarredDashboard + else + error!({ errors: result.message }, 400) + end + end + end + end + end +end diff --git a/lib/api/wikis.rb b/lib/api/wikis.rb index a42cb9b49af..884e3019a2d 100644 --- a/lib/api/wikis.rb +++ b/lib/api/wikis.rb @@ -123,9 +123,11 @@ module API post ":id/wikis/attachments" do authorize! :create_wiki, user_project - result = ::Wikis::CreateAttachmentService.new(user_project, - current_user, - commit_params(declared_params(include_missing: false))).execute + result = ::Wikis::CreateAttachmentService.new( + container: user_project, + current_user: current_user, + params: commit_params(declared_params(include_missing: false)) + ).execute if result[:status] == :success status(201) diff --git a/lib/gitlab/ci/config/entry/artifacts.rb b/lib/gitlab/ci/config/entry/artifacts.rb index ea9d35f2ccc..a9a9636637f 100644 --- a/lib/gitlab/ci/config/entry/artifacts.rb +++ b/lib/gitlab/ci/config/entry/artifacts.rb @@ -12,7 +12,7 @@ module Gitlab include ::Gitlab::Config::Entry::Validatable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[name untracked paths reports when expire_in expose_as].freeze + ALLOWED_KEYS = %i[name untracked paths reports when expire_in expose_as exclude].freeze EXPOSE_AS_REGEX = /\A\w[-\w ]*\z/.freeze EXPOSE_AS_ERROR_MESSAGE = "can contain only letters, digits, '-', '_' and spaces" @@ -35,6 +35,8 @@ module Gitlab }, if: :expose_as_present? validates :expose_as, type: String, length: { maximum: 100 }, if: :expose_as_present? validates :expose_as, format: { with: EXPOSE_AS_REGEX, message: EXPOSE_AS_ERROR_MESSAGE }, if: :expose_as_present? + validates :exclude, array_of_strings: true, if: :exclude_enabled? + validates :exclude, absence: { message: 'feature is disabled' }, unless: :exclude_enabled? validates :reports, type: Hash validates :when, inclusion: { in: %w[on_success on_failure always], @@ -57,6 +59,10 @@ module Gitlab !@config[:expose_as].nil? end + + def exclude_enabled? + ::Gitlab::Ci::Features.artifacts_exclude_enabled? + end end end end diff --git a/lib/gitlab/ci/features.rb b/lib/gitlab/ci/features.rb new file mode 100644 index 00000000000..a8edb42194f --- /dev/null +++ b/lib/gitlab/ci/features.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + ## + # Ci::Features is a class that aggregates all CI/CD feature flags in one place. + # + module Features + def self.artifacts_exclude_enabled? + ::Feature.enabled?(:ci_artifacts_exclude, default_enabled: false) + end + end + end +end diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index 7f98f0074d8..46f4aa3118f 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -1,5 +1,5 @@ .auto-deploy: - image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v0.13.0" + image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v0.14.0" review: extends: .auto-deploy diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index bf14bd4ab4b..08006657b07 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -502,7 +502,7 @@ module Gitlab update_column_in_batches(table, column, default_after_type_cast, &block) end - add_not_null_constraint(table, column) unless allow_null + change_column_null(table, column, false) unless allow_null # We want to rescue _all_ exceptions here, even those that don't inherit # from StandardError. rescue Exception => error # rubocop: disable all diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 0e227370741..981fc6f2b1f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1796,6 +1796,9 @@ msgstr "" msgid "AlertManagement|There was an error displaying the alerts. Confirm your endpoint's configuration details to ensure alerts appear." msgstr "" +msgid "AlertManagement|There was an error while updating the status of the alert. Please try again." +msgstr "" + msgid "AlertManagement|Tool" msgstr "" @@ -13076,6 +13079,9 @@ msgstr "" msgid "Merge Request Approvals" msgstr "" +msgid "Merge Request Commits" +msgstr "" + msgid "Merge Requests" msgstr "" @@ -13934,6 +13940,9 @@ msgstr "" msgid "Next" msgstr "" +msgid "Next commit" +msgstr "" + msgid "Next file in diff" msgstr "" @@ -15607,6 +15616,9 @@ msgstr "" msgid "Press %{key}-C to copy" msgstr "" +msgid "Prev" +msgstr "" + msgid "Prevent adding new members to project membership within this group" msgstr "" @@ -15643,6 +15655,9 @@ msgstr "" msgid "Previous Artifacts" msgstr "" +msgid "Previous commit" +msgstr "" + msgid "Previous file in diff" msgstr "" @@ -24618,6 +24633,12 @@ msgstr "" msgid "You're about to reduce the visibility of the project %{strong_start}%{project_name}%{strong_end}." msgstr "" +msgid "You're at the first commit" +msgstr "" + +msgid "You're at the last commit" +msgstr "" + msgid "You're not allowed to %{tag_start}edit%{tag_end} files in this project directly. Please fork this project, make your changes there, and submit a merge request." msgstr "" diff --git a/rubocop/cop/rspec/empty_line_after_shared_example.rb b/rubocop/cop/rspec/empty_line_after_shared_example.rb new file mode 100644 index 00000000000..5d09565bd5a --- /dev/null +++ b/rubocop/cop/rspec/empty_line_after_shared_example.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'rubocop/rspec/final_end_location' +require 'rubocop/rspec/blank_line_separation' +require 'rubocop/rspec/language' + +module RuboCop + module Cop + module RSpec + # Checks if there is an empty line after shared example blocks. + # + # @example + # # bad + # RSpec.describe Foo do + # it_behaves_like 'do this first' + # it_behaves_like 'does this' do + # end + # it_behaves_like 'does that' do + # end + # it_behaves_like 'do some more' + # end + # + # # good + # RSpec.describe Foo do + # it_behaves_like 'do this first' + # it_behaves_like 'does this' do + # end + # + # it_behaves_like 'does that' do + # end + # + # it_behaves_like 'do some more' + # end + # + # # fair - it's ok to have non-separated without blocks + # RSpec.describe Foo do + # it_behaves_like 'do this first' + # it_behaves_like 'does this' + # end + # + class EmptyLineAfterSharedExample < RuboCop::Cop::Cop + include RuboCop::RSpec::BlankLineSeparation + include RuboCop::RSpec::Language + + MSG = 'Add an empty line after `%<example>s` block.' + + def_node_matcher :shared_examples, + (SharedGroups::ALL + Includes::ALL).block_pattern + + def on_block(node) + shared_examples(node) do + break if last_child?(node) + + missing_separating_line(node) do |location| + add_offense(node, + location: location, + message: format(MSG, example: node.method_name)) + end + end + end + end + end + end +end diff --git a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb index 9a3fee5b43a..6bfec1b314e 100644 --- a/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb +++ b/spec/controllers/projects/design_management/designs/resized_image_controller_spec.rb @@ -14,16 +14,6 @@ describe Projects::DesignManagement::Designs::ResizedImageController do let(:sha) { design.versions.first.sha } before do - # TODO these tests are being temporarily skipped unless run in EE, - # as we are in the process of moving Design Management to FOSS in 13.0 - # in steps. In the current step the services have not yet been moved, - # and the `design` factory used in this test uses the `:with_smaller_image_versions` - # trait, which calls `GenerateImageVersionsService` to generate the - # smaller image versions. - # - # See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283. - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - enable_design_management end diff --git a/spec/frontend/alert_management/components/alert_management_list_spec.js b/spec/frontend/alert_management/components/alert_management_list_spec.js index 662c39fc84d..df845884b03 100644 --- a/spec/frontend/alert_management/components/alert_management_list_spec.js +++ b/spec/frontend/alert_management/components/alert_management_list_spec.js @@ -4,16 +4,20 @@ import { GlTable, GlAlert, GlLoadingIcon, - GlNewDropdown, + GlDropdown, GlBadge, GlTab, + GlDropdownItem, } from '@gitlab/ui'; import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; +import createFlash from '~/flash'; import AlertManagementList from '~/alert_management/components/alert_management_list.vue'; import { ALERTS_STATUS_TABS } from '../../../../app/assets/javascripts/alert_management/constants'; - +import updateAlertStatus from '~/alert_management/graphql/mutations/update_alert_status.graphql'; import mockAlerts from '../mocks/alerts.json'; +jest.mock('~/flash'); + describe('AlertManagementList', () => { let wrapper; @@ -21,10 +25,11 @@ describe('AlertManagementList', () => { const findAlerts = () => wrapper.findAll('table tbody tr'); const findAlert = () => wrapper.find(GlAlert); const findLoader = () => wrapper.find(GlLoadingIcon); - const findStatusDropdown = () => wrapper.find(GlNewDropdown); + const findStatusDropdown = () => wrapper.find(GlDropdown); const findStatusFilterTabs = () => wrapper.findAll(GlTab); const findNumberOfAlertsBadge = () => wrapper.findAll(GlBadge); const findDateFields = () => wrapper.findAll(TimeAgo); + const findFirstStatusOption = () => findStatusDropdown().find(GlDropdownItem); function mountComponent({ props = { @@ -51,6 +56,7 @@ describe('AlertManagementList', () => { }, mocks: { $apollo: { + mutate: jest.fn(), queries: { alerts: { loading, @@ -191,6 +197,7 @@ describe('AlertManagementList', () => { alerts: [ { iid: 1, + status: 'acknowledged', startedAt: '2020-03-17T23:18:14.996Z', endedAt: '2020-04-17T23:18:14.996Z', }, @@ -209,6 +216,7 @@ describe('AlertManagementList', () => { alerts: [ { iid: 1, + status: 'acknowledged', startedAt: null, endedAt: null, }, @@ -221,4 +229,52 @@ describe('AlertManagementList', () => { }); }); }); + + describe('updating the alert status', () => { + const iid = '1527542'; + const mockUpdatedMutationResult = { + data: { + updateAlertStatus: { + errors: [], + alert: { + iid, + status: 'acknowledged', + }, + }, + }, + }; + + beforeEach(() => { + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alerts: mockAlerts, errored: false }, + loading: false, + }); + }); + + it('calls `$apollo.mutate` with `updateAlertStatus` mutation and variables containing `iid`, `status`, & `projectPath`', () => { + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue(mockUpdatedMutationResult); + findFirstStatusOption().vm.$emit('click'); + + expect(wrapper.vm.$apollo.mutate).toHaveBeenCalledWith({ + mutation: updateAlertStatus, + variables: { + iid, + status: 'TRIGGERED', + projectPath: 'gitlab-org/gitlab', + }, + }); + }); + + it('calls `createFlash` when request fails', () => { + jest.spyOn(wrapper.vm.$apollo, 'mutate').mockReturnValue(Promise.reject(new Error())); + findFirstStatusOption().vm.$emit('click'); + + setImmediate(() => { + expect(createFlash).toHaveBeenCalledWith( + 'There was an error while updating the status of the alert. Please try again.', + ); + }); + }); + }); }); diff --git a/spec/frontend/alert_management/mocks/alerts.json b/spec/frontend/alert_management/mocks/alerts.json index e0b8fa55507..5e6b96aed71 100644 --- a/spec/frontend/alert_management/mocks/alerts.json +++ b/spec/frontend/alert_management/mocks/alerts.json @@ -9,8 +9,8 @@ "status": "triggered" }, { - "iid": "1527542", - "title": "Some otherr alert Some otherr alert Some otherr alert Some otherr alert Some otherr alert Some otherr alert", + "iid": "1527543", + "title": "Some other alert Some other alert Some other alert Some other alert Some other alert Some other alert", "severity": "MEDIUM", "eventCount": 1, "startedAt": "2020-04-17T23:18:14.996Z", @@ -18,7 +18,7 @@ "status": "acknowledged" }, { - "iid": "1527542", + "iid": "1527544", "title": "SyntaxError: Invalid or unexpected token", "severity": "LOW", "eventCount": 4, diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 1a95f586344..57e3a93c6f4 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -28,8 +28,14 @@ describe('diffs/components/app', () => { let wrapper; let mock; - function createComponent(props = {}, extendStore = () => {}) { + function createComponent(props = {}, extendStore = () => {}, provisions = {}) { const localVue = createLocalVue(); + const provide = { + ...provisions, + glFeatures: { + ...(provisions.glFeatures || {}), + }, + }; localVue.use(Vuex); @@ -52,6 +58,7 @@ describe('diffs/components/app', () => { showSuggestPopover: true, ...props, }, + provide, store, methods: { isLatestVersion() { @@ -82,7 +89,10 @@ describe('diffs/components/app', () => { window.mrTabs = oldMrTabs; // reset component - wrapper.destroy(); + if (wrapper) { + wrapper.destroy(); + wrapper = null; + } mock.restore(); }); @@ -455,76 +465,109 @@ describe('diffs/components/app', () => { }); describe('keyboard shortcut navigation', () => { - const mappings = { - '[': -1, - k: -1, - ']': +1, - j: +1, - }; - let spy; + let spies = []; + let jumpSpy; + let moveSpy; + + function setup(componentProps, featureFlags) { + createComponent( + componentProps, + ({ state }) => { + state.diffs.commit = { id: 'SHA123' }; + }, + { glFeatures: { mrCommitNeighborNav: true, ...featureFlags } }, + ); + + moveSpy = jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + jumpSpy = jest.fn(); + spies = [jumpSpy, moveSpy]; + wrapper.setMethods({ + jumpToFile: jumpSpy, + }); + } describe('visible app', () => { - beforeEach(() => { - spy = jest.fn(); + it.each` + key | name | spy | args | featureFlags + ${'['} | ${'jumpToFile'} | ${0} | ${[-1]} | ${{}} + ${'k'} | ${'jumpToFile'} | ${0} | ${[-1]} | ${{}} + ${']'} | ${'jumpToFile'} | ${0} | ${[+1]} | ${{}} + ${'j'} | ${'jumpToFile'} | ${0} | ${[+1]} | ${{}} + ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'previous' }]} | ${{ mrCommitNeighborNav: true }} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'next' }]} | ${{ mrCommitNeighborNav: true }} + `( + 'calls `$name()` with correct parameters whenever the "$key" key is pressed', + ({ key, spy, args, featureFlags }) => { + setup({ shouldShow: true }, featureFlags); - createComponent({ - shouldShow: true, - }); - wrapper.setMethods({ - jumpToFile: spy, - }); - }); + return wrapper.vm.$nextTick().then(() => { + expect(spies[spy]).not.toHaveBeenCalled(); + + Mousetrap.trigger(key); + + expect(spies[spy]).toHaveBeenCalledWith(...args); + }); + }, + ); + + it.each` + key | name | spy | featureFlags + ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${{ mrCommitNeighborNav: false }} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${{ mrCommitNeighborNav: false }} + `( + 'does not call `$name()` even when the correct key is pressed if the feature flag is disabled', + ({ key, spy, featureFlags }) => { + setup({ shouldShow: true }, featureFlags); - it.each(Object.keys(mappings))( - 'calls `jumpToFile()` with correct parameter whenever pre-defined %s is pressed', - key => { return wrapper.vm.$nextTick().then(() => { - expect(spy).not.toHaveBeenCalled(); + expect(spies[spy]).not.toHaveBeenCalled(); Mousetrap.trigger(key); - expect(spy).toHaveBeenCalledWith(mappings[key]); + expect(spies[spy]).not.toHaveBeenCalled(); }); }, ); - it('does not call `jumpToFile()` when unknown key is pressed', done => { - wrapper.vm - .$nextTick() - .then(() => { - Mousetrap.trigger('d'); + it.each` + key | name | spy | allowed + ${'d'} | ${'jumpToFile'} | ${0} | ${['[', ']', 'j', 'k']} + ${'r'} | ${'moveToNeighboringCommit'} | ${1} | ${['x', 'c']} + `( + `does not call \`$name()\` when a key that is not one of \`$allowed\` is pressed`, + ({ key, spy }) => { + setup({ shouldShow: true }, { mrCommitNeighborNav: true }); - expect(spy).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); - }); + return wrapper.vm.$nextTick().then(() => { + Mousetrap.trigger(key); + + expect(spies[spy]).not.toHaveBeenCalled(); + }); + }, + ); }); - describe('hideen app', () => { + describe('hidden app', () => { beforeEach(() => { - spy = jest.fn(); + setup({ shouldShow: false }, { mrCommitNeighborNav: true }); - createComponent({ - shouldShow: false, - }); - wrapper.setMethods({ - jumpToFile: spy, + return wrapper.vm.$nextTick().then(() => { + Mousetrap.reset(); }); }); - it('stops calling `jumpToFile()` when application is hidden', done => { - wrapper.vm - .$nextTick() - .then(() => { - Object.keys(mappings).forEach(key => { - Mousetrap.trigger(key); + it.each` + key | name | spy + ${'['} | ${'jumpToFile'} | ${0} + ${'k'} | ${'jumpToFile'} | ${0} + ${']'} | ${'jumpToFile'} | ${0} + ${'j'} | ${'jumpToFile'} | ${0} + ${'x'} | ${'moveToNeighboringCommit'} | ${1} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} + `('stops calling `$name()` when the app is hidden', ({ key, spy }) => { + Mousetrap.trigger(key); - expect(spy).not.toHaveBeenCalled(); - }); - }) - .then(done) - .catch(done.fail); + expect(spies[spy]).not.toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index 6bb3a0dcf21..0df951d43a7 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -13,6 +13,8 @@ const TEST_AUTHOR_EMAIL = 'test+test@gitlab.com'; const TEST_AUTHOR_GRAVATAR = `${TEST_HOST}/avatar/test?s=40`; const TEST_SIGNATURE_HTML = '<a>Legit commit</a>'; const TEST_PIPELINE_STATUS_PATH = `${TEST_HOST}/pipeline/status`; +const NEXT_COMMIT_URL = `${TEST_HOST}/?commit_id=next`; +const PREV_COMMIT_URL = `${TEST_HOST}/?commit_id=prev`; describe('diffs/components/commit_item', () => { let wrapper; @@ -30,12 +32,24 @@ describe('diffs/components/commit_item', () => { const getCommitActionsElement = () => wrapper.find('.commit-actions'); const getCommitPipelineStatus = () => wrapper.find(CommitPipelineStatus); - const mountComponent = propsData => { + const getCommitNavButtonsElement = () => wrapper.find('.commit-nav-buttons'); + const getNextCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:last-child'); + const getPrevCommitNavElement = () => + getCommitNavButtonsElement().find('.btn-group > *:first-child'); + + const mountComponent = (propsData, featureFlags = {}) => { wrapper = mount(Component, { propsData: { commit, ...propsData, }, + provide: { + glFeatures: { + mrCommitNeighborNav: true, + ...featureFlags, + }, + }, stubs: { CommitPipelineStatus: true, }, @@ -173,4 +187,132 @@ describe('diffs/components/commit_item', () => { expect(getCommitPipelineStatus().exists()).toBe(true); }); }); + + describe('without neighbor commits', () => { + beforeEach(() => { + mountComponent({ commit: { ...commit, prev_commit_id: null, next_commit_id: null } }); + }); + + it('does not render any navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(false); + }); + }); + + describe('with neighbor commits', () => { + let mrCommit; + + beforeEach(() => { + mrCommit = { + ...commit, + next_commit_id: 'next', + prev_commit_id: 'prev', + }; + + mountComponent({ commit: mrCommit }); + }); + + it('renders the commit navigation buttons', () => { + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + mountComponent({ + commit: { ...mrCommit, next_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + + mountComponent({ + commit: { ...mrCommit, prev_commit_id: null }, + }); + expect(getCommitNavButtonsElement().exists()).toEqual(true); + }); + + it('does not render the commit navigation buttons if the `mrCommitNeighborNav` feature flag is disabled', () => { + mountComponent({ commit: mrCommit }, { mrCommitNeighborNav: false }); + + expect(getCommitNavButtonsElement().exists()).toEqual(false); + }); + + describe('prev commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getPrevCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(PREV_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getPrevCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ + direction: 'previous', + }); + }); + }); + + it('renders a disabled button when there is no prev commit', () => { + mountComponent({ commit: { ...mrCommit, prev_commit_id: null } }); + + const button = getPrevCommitNavElement(); + + expect(button.element.tagName).toEqual('BUTTON'); + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + + describe('next commit', () => { + const { location } = window; + + beforeAll(() => { + delete window.location; + window.location = { href: `${TEST_HOST}?commit_id=${mrCommit.id}` }; + }); + + beforeEach(() => { + jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); + }); + + afterAll(() => { + window.location = location; + }); + + it('uses the correct href', () => { + const link = getNextCommitNavElement(); + + expect(link.element.getAttribute('href')).toEqual(NEXT_COMMIT_URL); + }); + + it('triggers the correct Vuex action on click', () => { + const link = getNextCommitNavElement(); + + link.trigger('click'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.vm.moveToNeighboringCommit).toHaveBeenCalledWith({ direction: 'next' }); + }); + }); + + it('renders a disabled button when there is no next commit', () => { + mountComponent({ commit: { ...mrCommit, next_commit_id: null } }); + + const button = getNextCommitNavElement(); + + expect(button.element.tagName).toEqual('BUTTON'); + expect(button.element.hasAttribute('disabled')).toEqual(true); + }); + }); + }); }); diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index 53469159110..8cfd07df777 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -124,5 +124,55 @@ describe Gitlab::Ci::Config::Entry::Artifacts do end end end + + describe 'excluded artifacts' do + context 'when configuration is valid and the feature is enabled' do + before do + stub_feature_flags(ci_artifacts_exclude: true) + end + + context 'when configuration is valid' do + let(:config) { { untracked: true, exclude: ['some/directory/'] } } + + it 'correctly parses the configuration' do + expect(entry).to be_valid + expect(entry.value).to eq config + end + end + + context 'when configuration is not valid' do + let(:config) { { untracked: true, exclude: 1234 } } + + it 'returns an error' do + expect(entry).not_to be_valid + expect(entry.errors) + .to include 'artifacts exclude should be an array of strings' + end + end + end + + context 'when artifacts/exclude feature is disabled' do + before do + stub_feature_flags(ci_artifacts_exclude: false) + end + + context 'when configuration has been provided' do + let(:config) { { untracked: true, exclude: ['some/directory/'] } } + + it 'returns an error' do + expect(entry).not_to be_valid + expect(entry.errors).to include 'artifacts exclude feature is disabled' + end + end + + context 'when configuration is not present' do + let(:config) { { untracked: true } } + + it 'is a valid configuration' do + expect(entry).to be_valid + end + end + end + end end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 969a674a308..c93bb901981 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1364,6 +1364,24 @@ module Gitlab expect { described_class.new(config) }.to raise_error(described_class::ValidationError) end + + it 'populates a build options with complete artifacts configuration' do + stub_feature_flags(ci_artifacts_exclude: true) + + config = <<~YAML + test: + script: echo "Hello World" + artifacts: + paths: + - my/test + exclude: + - my/test/something + YAML + + attributes = Gitlab::Ci::YamlProcessor.new(config).build_attributes('test') + + expect(attributes.dig(*%i[options artifacts exclude])).to eq(%w[my/test/something]) + end end describe "release" do diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 73b9dcc5bf9..9ed4db673a6 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -647,7 +647,7 @@ describe Gitlab::Database::MigrationHelpers do expect(model).to receive(:update_column_in_batches) .with(:projects, :foo, 10) - expect(model).not_to receive(:add_not_null_constraint) + expect(model).not_to receive(:change_column_null) model.add_column_with_default(:projects, :foo, :integer, default: 10, @@ -658,8 +658,8 @@ describe Gitlab::Database::MigrationHelpers do expect(model).to receive(:update_column_in_batches) .with(:projects, :foo, 10) - expect(model).to receive(:add_not_null_constraint) - .with(:projects, :foo) + expect(model).to receive(:change_column_null) + .with(:projects, :foo, false) model.add_column_with_default(:projects, :foo, :integer, default: 10) end @@ -678,8 +678,8 @@ describe Gitlab::Database::MigrationHelpers do end it 'removes the added column whenever changing a column NULL constraint fails' do - expect(model).to receive(:add_not_null_constraint) - .with(:projects, :foo) + expect(model).to receive(:change_column_null) + .with(:projects, :foo, false) .and_raise(ActiveRecord::ActiveRecordError) expect(model).to receive(:remove_column) @@ -699,7 +699,7 @@ describe Gitlab::Database::MigrationHelpers do allow(model).to receive(:transaction).and_yield allow(model).to receive(:column_for).with(:user_details, :foo).and_return(column) allow(model).to receive(:update_column_in_batches).with(:user_details, :foo, 10, batch_column_name: :user_id) - allow(model).to receive(:add_not_null_constraint).with(:user_details, :foo) + allow(model).to receive(:change_column_null).with(:user_details, :foo, false) allow(model).to receive(:change_column_default).with(:user_details, :foo, 10) expect(model).to receive(:add_column) @@ -721,7 +721,7 @@ describe Gitlab::Database::MigrationHelpers do allow(model).to receive(:transaction).and_yield allow(model).to receive(:column_for).with(:projects, :foo).and_return(column) allow(model).to receive(:update_column_in_batches).with(:projects, :foo, 10) - allow(model).to receive(:add_not_null_constraint).with(:projects, :foo) + allow(model).to receive(:change_column_null).with(:projects, :foo, false) allow(model).to receive(:change_column_default).with(:projects, :foo, 10) expect(model).to receive(:add_column) diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 6e1dba6945d..3c66902bb2e 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -70,6 +70,7 @@ describe Notify do it_behaves_like 'an email starting a new thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -116,6 +117,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -155,6 +157,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with a labels subscriptions link in its footer' @@ -200,6 +203,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -214,6 +218,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -248,6 +253,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' @@ -300,6 +306,7 @@ describe Notify do it_behaves_like 'an email starting a new thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -344,6 +351,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like "an unsubscribeable thread" it_behaves_like 'appearance header and footer enabled' @@ -409,6 +417,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'an email with a labels subscriptions link in its footer' @@ -436,6 +445,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -466,6 +476,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -502,6 +513,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -533,6 +545,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -694,6 +707,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { project_snippet } end + it_behaves_like 'a user cannot unsubscribe through footer link' it 'has the correct subject' do @@ -936,6 +950,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { commit } end + it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'appearance header and footer enabled' @@ -962,6 +977,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -988,6 +1004,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -1060,6 +1077,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { commit } end + it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like 'a user cannot unsubscribe through footer link' it_behaves_like 'appearance header and footer enabled' @@ -1092,6 +1110,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { merge_request } end + it_behaves_like 'it should show Gmail Actions View Merge request link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' @@ -1124,6 +1143,7 @@ describe Notify do it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do let(:model) { issue } end + it_behaves_like 'it should show Gmail Actions View Issue link' it_behaves_like 'an unsubscribeable thread' it_behaves_like 'appearance header and footer enabled' diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 985bb9d3194..02558ef21ee 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -4088,6 +4088,28 @@ describe Ci::Build do it { is_expected.to include(:upload_multiple_artifacts) } end + + context 'when artifacts exclude is defined and the is feature enabled' do + let(:options) do + { artifacts: { exclude: %w[something] } } + end + + context 'when a feature flag is enabled' do + before do + stub_feature_flags(ci_artifacts_exclude: true) + end + + it { is_expected.to include(:artifacts_exclude) } + end + + context 'when a feature flag is disabled' do + before do + stub_feature_flags(ci_artifacts_exclude: false) + end + + it { is_expected.not_to include(:artifacts_exclude) } + end + end end describe '#supported_runner?' do diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 4900743ccb5..6dd295ca915 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -105,6 +105,38 @@ describe Note do end end + describe 'callbacks' do + describe '#notify_after_create' do + it 'calls #after_note_created on the noteable' do + note = build(:note) + + expect(note).to receive(:notify_after_create).and_call_original + expect(note.noteable).to receive(:after_note_created).with(note) + + note.save! + end + end + + describe '#notify_after_destroy' do + it 'calls #after_note_destroyed on the noteable' do + note = create(:note) + + expect(note).to receive(:notify_after_destroy).and_call_original + expect(note.noteable).to receive(:after_note_destroyed).with(note) + + note.destroy + end + + it 'does not error if noteable is nil' do + note = create(:note) + + expect(note).to receive(:notify_after_destroy).and_call_original + expect(note).to receive(:noteable).at_least(:once).and_return(nil) + expect { note.destroy }.not_to raise_error + end + end + end + describe "Commit notes" do before do allow(Gitlab::Git::KeepAround).to receive(:execute).and_call_original diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb index 6b2b031dbd0..cfa2e77e7cf 100644 --- a/spec/presenters/ci/build_runner_presenter_spec.rb +++ b/spec/presenters/ci/build_runner_presenter_spec.rb @@ -38,6 +38,47 @@ describe Ci::BuildRunnerPresenter do expect(presenter.artifacts).to be_empty end end + + context 'when artifacts exclude is defined' do + let(:build) do + create(:ci_build, options: { artifacts: { paths: %w[abc], exclude: %w[cde] } }) + end + + context 'when the feature is enabled' do + before do + stub_feature_flags(ci_artifacts_exclude: true) + end + + it 'includes the list of excluded paths' do + expect(presenter.artifacts.first).to include( + artifact_type: :archive, + artifact_format: :zip, + paths: %w[abc], + exclude: %w[cde] + ) + end + end + + context 'when the feature is disabled' do + before do + stub_feature_flags(ci_artifacts_exclude: false) + end + + it 'does not include the list of excluded paths' do + expect(presenter.artifacts.first).not_to have_key(:exclude) + end + end + end + + context 'when artifacts exclude is not defined' do + let(:build) do + create(:ci_build, options: { artifacts: { paths: %w[abc] } }) + end + + it 'does not include an empty list of excluded paths' do + expect(presenter.artifacts.first).not_to have_key(:exclude) + end + end end context "with reports" do diff --git a/spec/requests/api/metrics/user_starred_dashboards_spec.rb b/spec/requests/api/metrics/user_starred_dashboards_spec.rb new file mode 100644 index 00000000000..4b3c6407ef7 --- /dev/null +++ b/spec/requests/api/metrics/user_starred_dashboards_spec.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::Metrics::UserStarredDashboards do + let_it_be(:user) { create(:user) } + let!(:project) { create(:project, :private, :repository, :custom_repo, namespace: user.namespace, files: { dashboard => dashboard_yml }) } + let(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } + let(:dashboard) { '.gitlab/dashboards/find&seek.yml' } + let(:params) do + { + user: user, + project: project, + dashboard_path: CGI.escape(dashboard) + } + end + + describe 'POST /projects/:id/metrics/user_starred_dashboards' do + let(:url) { "/projects/#{project.id}/metrics/user_starred_dashboards" } + + before do + project.add_reporter(user) + end + + context 'with correct permissions' do + context 'with valid parameters' do + context 'dashboard_path as url param url escaped' do + it 'creates a new annotation', :aggregate_failures do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['project_id']).to eq(project.id) + expect(json_response['user_id']).to eq(user.id) + expect(json_response['dashboard_path']).to eq(dashboard) + end + end + + context 'dashboard_path in request body unescaped' do + let(:params) do + { + user: user, + project: project, + dashboard_path: dashboard + } + end + + it 'creates a new annotation', :aggregate_failures do + post api(url, user), params: params + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['project_id']).to eq(project.id) + expect(json_response['user_id']).to eq(user.id) + expect(json_response['dashboard_path']).to eq(dashboard) + end + end + end + + context 'with invalid parameters' do + it 'returns error message' do + post api(url, user), params: { dashboard_path: '' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('dashboard_path is empty') + end + + context 'user is missing' do + it 'returns 404 not found' do + post api(url, nil), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'project is missing' do + it 'returns 404 not found' do + post api("/projects/#{project.id + 1}/user_starred_dashboards", user), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + end + + context 'without correct permissions' do + it 'returns 404 not found' do + post api(url, create(:user)), params: params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end +end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index bc2da8a2b9a..5327a3a77c5 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -998,6 +998,53 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end + describe 'a job with excluded artifacts' do + context 'when excluded paths are defined' do + let(:job) do + create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'test', + stage: 'deploy', stage_idx: 1, + options: { artifacts: { paths: ['abc'], exclude: ['cde'] } }) + end + + context 'when a runner supports this feature' do + it 'exposes excluded paths when the feature is enabled' do + stub_feature_flags(ci_artifacts_exclude: true) + + request_job info: { features: { artifacts_exclude: true } } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.dig('artifacts').first).to include('exclude' => ['cde']) + end + + it 'does not expose excluded paths when the feature is disabled' do + stub_feature_flags(ci_artifacts_exclude: false) + + request_job info: { features: { artifacts_exclude: true } } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.dig('artifacts').first).not_to have_key('exclude') + end + end + + context 'when a runner does not support this feature' do + it 'does not expose the build at all' do + stub_feature_flags(ci_artifacts_exclude: true) + + request_job + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + + it 'does not expose excluded paths when these are empty' do + request_job + + expect(response).to have_gitlab_http_status(:created) + expect(json_response.dig('artifacts').first).not_to have_key('exclude') + end + end + def request_job(token = runner.token, **params) new_params = params.merge(token: token, last_update: last_update) post api('/jobs/request'), params: new_params, headers: { 'User-Agent' => user_agent } diff --git a/spec/rubocop/cop/rspec/empty_line_after_shared_example_spec.rb b/spec/rubocop/cop/rspec/empty_line_after_shared_example_spec.rb new file mode 100644 index 00000000000..cee593fe535 --- /dev/null +++ b/spec/rubocop/cop/rspec/empty_line_after_shared_example_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require_relative '../../../../rubocop/cop/rspec/empty_line_after_shared_example' + +describe RuboCop::Cop::RSpec::EmptyLineAfterSharedExample do + subject(:cop) { described_class.new } + + it 'flags a missing empty line after `it_behaves_like` block' do + expect_offense(<<-RUBY) + RSpec.describe Foo do + it_behaves_like 'does this' do + end + ^^^ Add an empty line after `it_behaves_like` block. + it_behaves_like 'does that' do + end + end + RUBY + + expect_correction(<<-RUBY) + RSpec.describe Foo do + it_behaves_like 'does this' do + end + + it_behaves_like 'does that' do + end + end + RUBY + end + + it 'ignores one-line shared examples before shared example blocks' do + expect_no_offenses(<<-RUBY) + RSpec.describe Foo do + it_behaves_like 'does this' + it_behaves_like 'does that' do + end + end + RUBY + end + + it 'flags a missing empty line after `shared_examples`' do + expect_offense(<<-RUBY) + RSpec.context 'foo' do + shared_examples do + end + ^^^ Add an empty line after `shared_examples` block. + shared_examples 'something gets done' do + end + end + RUBY + + expect_correction(<<-RUBY) + RSpec.context 'foo' do + shared_examples do + end + + shared_examples 'something gets done' do + end + end + RUBY + end + + it 'ignores consecutive one-liners' do + expect_no_offenses(<<-RUBY) + RSpec.describe Foo do + it_behaves_like 'do this' + it_behaves_like 'do that' + end + RUBY + end + + it 'flags mixed one-line and multi-line shared examples' do + expect_offense(<<-RUBY) + RSpec.context 'foo' do + it_behaves_like 'do this' + it_behaves_like 'do that' + it_behaves_like 'does this' do + end + ^^^ Add an empty line after `it_behaves_like` block. + it_behaves_like 'do this' + it_behaves_like 'do that' + end + RUBY + end +end diff --git a/spec/services/design_management/delete_designs_service_spec.rb b/spec/services/design_management/delete_designs_service_spec.rb new file mode 100644 index 00000000000..2c0c1570cb4 --- /dev/null +++ b/spec/services/design_management/delete_designs_service_spec.rb @@ -0,0 +1,195 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe DesignManagement::DeleteDesignsService do + include DesignManagementTestHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:user) { create(:user) } + let(:designs) { create_designs } + + subject(:service) { described_class.new(project, user, issue: issue, designs: designs) } + + # Defined as a method so that the reponse is not cached. We also construct + # a new service executor each time to avoid the intermediate cached values + # it constructs during its execution. + def run_service(delenda = nil) + service = described_class.new(project, user, issue: issue, designs: delenda || designs) + service.execute + end + + let(:response) { run_service } + + shared_examples 'a service error' do + it 'returns an error', :aggregate_failures do + expect(response).to include(status: :error) + end + end + + shared_examples 'a top-level error' do + let(:expected_error) { StandardError } + it 'raises an en expected error', :aggregate_failures do + expect { run_service }.to raise_error(expected_error) + end + end + + shared_examples 'a success' do + it 'returns successfully', :aggregate_failures do + expect(response).to include(status: :success) + end + + it 'saves the user as the author' do + version = response[:version] + + expect(version.author).to eq(user) + end + end + + before do + enable_design_management(enabled) + project.add_developer(user) + end + + describe "#execute" do + context "when the feature is not available" do + let(:enabled) { false } + + it_behaves_like "a service error" + end + + context "when the feature is available" do + let(:enabled) { true } + + it 'is able to delete designs' do + expect(service.send(:can_delete_designs?)).to be true + end + + context 'no designs were passed' do + let(:designs) { [] } + + it_behaves_like "a top-level error" + + it 'does not log any events' do + counter = ::Gitlab::UsageDataCounters::DesignsCounter + expect { run_service rescue nil }.not_to change { counter.totals } + end + end + + context 'one design is passed' do + before do + create_designs(2) + end + + let!(:designs) { create_designs(1) } + + it 'removes that design' do + expect { run_service }.to change { issue.designs.current.count }.from(3).to(2) + end + + it 'logs a deletion event' do + counter = ::Gitlab::UsageDataCounters::DesignsCounter + expect { run_service }.to change { counter.read(:delete) }.by(1) + end + + it 'informs the new-version-worker' do + expect(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer) + + run_service + end + + it 'creates a new version' do + expect { run_service }.to change { DesignManagement::Version.where(issue: issue).count }.by(1) + end + + it 'returns the new version' do + version = response[:version] + + expect(version).to eq(DesignManagement::Version.for_issue(issue).ordered.first) + end + + it_behaves_like "a success" + + it 'removes the design from the current design list' do + run_service + + expect(issue.designs.current).not_to include(designs.first) + end + + it 'marks the design as deleted' do + expect { run_service } + .to change { designs.first.deleted? }.from(false).to(true) + end + end + + context 'more than one design is passed' do + before do + create_designs(1) + end + + let!(:designs) { create_designs(2) } + + it 'removes those designs' do + expect { run_service } + .to change { issue.designs.current.count }.from(3).to(1) + end + + it 'logs the correct number of deletion events' do + counter = ::Gitlab::UsageDataCounters::DesignsCounter + expect { run_service }.to change { counter.read(:delete) }.by(2) + end + + it_behaves_like "a success" + + context 'after executing the service' do + let(:deleted_designs) { designs.map(&:reset) } + + let!(:version) { run_service[:version] } + + it 'removes the removed designs from the current design list' do + expect(issue.designs.current).not_to include(*deleted_designs) + end + + it 'does not make the designs impossible to find' do + expect(issue.designs).to include(*deleted_designs) + end + + it 'associates the new version with all the designs' do + current_versions = deleted_designs.map { |d| d.most_recent_action.version } + expect(current_versions).to all(eq version) + end + + it 'marks all deleted designs as deleted' do + expect(deleted_designs).to all(be_deleted) + end + + it 'marks all deleted designs with the same deletion version' do + expect(deleted_designs.map { |d| d.most_recent_action.version_id }.uniq) + .to have_attributes(size: 1) + end + end + end + + describe 'scalability' do + before do + run_service(create_designs(1)) # ensure project, issue, etc are created + end + + it 'makes the same number of DB requests for one design as for several' do + one = create_designs(1) + many = create_designs(5) + + baseline = ActiveRecord::QueryRecorder.new { run_service(one) } + + expect { run_service(many) }.not_to exceed_query_limit(baseline) + end + end + end + end + + private + + def create_designs(how_many = 2) + create_list(:design, how_many, :with_lfs_file, issue: issue) + end +end diff --git a/spec/services/design_management/design_user_notes_count_service_spec.rb b/spec/services/design_management/design_user_notes_count_service_spec.rb index f4808542995..62211a4dd0f 100644 --- a/spec/services/design_management/design_user_notes_count_service_spec.rb +++ b/spec/services/design_management/design_user_notes_count_service_spec.rb @@ -23,15 +23,8 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_ end end - # TODO These tests are being temporarily skipped unless run in EE, - # as we are in the process of moving Design Management to FOSS in 13.0 - # in steps. In the current step the services have not yet been moved. - # - # See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283. describe 'cache invalidation' do it 'changes when a new note is created' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - new_note_attrs = attributes_for(:diff_note_on_design, noteable: design) expect do @@ -40,8 +33,6 @@ describe DesignManagement::DesignUserNotesCountService, :use_clean_rails_memory_ end it 'changes when a note is destroyed' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - note = create(:diff_note_on_design, noteable: design) expect do diff --git a/spec/services/design_management/generate_image_versions_service_spec.rb b/spec/services/design_management/generate_image_versions_service_spec.rb new file mode 100644 index 00000000000..cd021c8d7d3 --- /dev/null +++ b/spec/services/design_management/generate_image_versions_service_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DesignManagement::GenerateImageVersionsService do + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:version) { create(:design, :with_lfs_file, issue: issue).versions.first } + let_it_be(:action) { version.actions.first } + + describe '#execute' do + it 'generates the image' do + expect { described_class.new(version).execute } + .to change { action.reload.image_v432x230.file } + .from(nil).to(CarrierWave::SanitizedFile) + end + + it 'skips generating image versions if the mime type is not whitelisted' do + stub_const('DesignManagement::DesignV432x230Uploader::MIME_TYPE_WHITELIST', []) + + described_class.new(version).execute + + expect(action.reload.image_v432x230.file).to eq(nil) + end + + it 'skips generating image versions if the design file size is too large' do + stub_const("#{described_class.name}::MAX_DESIGN_SIZE", 1.byte) + + described_class.new(version).execute + + expect(action.reload.image_v432x230.file).to eq(nil) + end + + it 'returns the status' do + result = described_class.new(version).execute + + expect(result[:status]).to eq(:success) + end + + it 'returns the version' do + result = described_class.new(version).execute + + expect(result[:version]).to eq(version) + end + + it 'logs if the raw image cannot be found' do + version.designs.first.update(filename: 'foo.png') + + expect(Gitlab::AppLogger).to receive(:error).with("No design file found for Action: #{action.id}") + + described_class.new(version).execute + end + + context 'when an error is encountered when generating the image versions' do + before do + expect_next_instance_of(DesignManagement::DesignV432x230Uploader) do |uploader| + expect(uploader).to receive(:cache!).and_raise(CarrierWave::DownloadError, 'foo') + end + end + + it 'logs the error' do + expect(Gitlab::AppLogger).to receive(:error).with('foo') + + described_class.new(version).execute + end + + it 'tracks the error' do + expect(Gitlab::ErrorTracking).to receive(:track_exception).with( + instance_of(CarrierWave::DownloadError), + project_id: project.id, version_id: version.id, design_id: version.designs.first.id + ) + + described_class.new(version).execute + end + end + end +end diff --git a/spec/services/design_management/save_designs_service_spec.rb b/spec/services/design_management/save_designs_service_spec.rb new file mode 100644 index 00000000000..013d5473860 --- /dev/null +++ b/spec/services/design_management/save_designs_service_spec.rb @@ -0,0 +1,356 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe DesignManagement::SaveDesignsService do + include DesignManagementTestHelpers + include ConcurrentHelpers + + let_it_be(:developer) { create(:user) } + let(:project) { issue.project } + let(:issue) { create(:issue) } + let(:user) { developer } + let(:files) { [rails_sample] } + let(:design_repository) { ::Gitlab::GlRepository::DESIGN.repository_resolver.call(project) } + let(:rails_sample_name) { 'rails_sample.jpg' } + let(:rails_sample) { sample_image(rails_sample_name) } + let(:dk_png) { sample_image('dk.png') } + + def sample_image(filename) + fixture_file_upload("spec/fixtures/#{filename}") + end + + before do + project.add_developer(developer) + end + + def run_service(files_to_upload = nil) + design_files = files_to_upload || files + design_files.each(&:rewind) + + service = described_class.new(project, user, + issue: issue, + files: design_files) + service.execute + end + + # Randomly alter the content of files. + # This allows the files to be updated by the service, as unmodified + # files are rejected. + def touch_files(files_to_touch = nil) + design_files = files_to_touch || files + + design_files.each do |f| + f.tempfile.write(SecureRandom.random_bytes) + end + end + + let(:response) { run_service } + + shared_examples 'a service error' do + it 'returns an error', :aggregate_failures do + expect(response).to match(a_hash_including(status: :error)) + end + end + + shared_examples 'an execution error' do + it 'returns an error', :aggregate_failures do + expect { service.execute }.to raise_error(some_error) + end + end + + describe '#execute' do + context 'when the feature is not available' do + before do + enable_design_management(false) + end + + it_behaves_like 'a service error' + end + + context 'when the feature is available' do + before do + enable_design_management(true) + end + + describe 'repository existence' do + def repository_exists + # Expire the memoized value as the service creates it's own instance + design_repository.expire_exists_cache + design_repository.exists? + end + + it 'creates a design repository when it did not exist' do + expect { run_service }.to change { repository_exists }.from(false).to(true) + end + end + + it 'updates the creation count' do + counter = Gitlab::UsageDataCounters::DesignsCounter + expect { run_service }.to change { counter.read(:create) }.by(1) + end + + it 'creates a commit in the repository' do + run_service + + expect(design_repository.commit).to have_attributes( + author: user, + message: include(rails_sample_name) + ) + end + + it 'can run the same command in parallel' do + blocks = Array.new(10).map do + unique_files = %w(rails_sample.jpg dk.png) + .map { |name| RenameableUpload.unique_file(name) } + + -> { run_service(unique_files) } + end + + expect { run_parallel(blocks) }.to change(DesignManagement::Version, :count).by(10) + end + + it 'causes diff_refs not to be nil' do + expect(response).to include( + designs: all(have_attributes(diff_refs: be_present)) + ) + end + + it 'creates a design & a version for the filename if it did not exist' do + expect(issue.designs.size).to eq(0) + + updated_designs = response[:designs] + + expect(updated_designs.size).to eq(1) + expect(updated_designs.first.versions.size).to eq(1) + end + + it 'saves the user as the author' do + updated_designs = response[:designs] + + expect(updated_designs.first.versions.first.author).to eq(user) + end + + describe 'saving the file to LFS' do + before do + expect_next_instance_of(Lfs::FileTransformer) do |transformer| + expect(transformer).to receive(:lfs_file?).and_return(true) + end + end + + it 'saves the design to LFS' do + expect { run_service }.to change { LfsObject.count }.by(1) + end + + it 'saves the repository_type of the LfsObjectsProject as design' do + expect do + run_service + end.to change { project.lfs_objects_projects.count }.from(0).to(1) + + expect(project.lfs_objects_projects.first.repository_type).to eq('design') + end + end + + context 'when a design is being updated' do + before do + run_service + touch_files + end + + it 'creates a new version for the existing design and updates the file' do + expect(issue.designs.size).to eq(1) + expect(DesignManagement::Version.for_designs(issue.designs).size).to eq(1) + + updated_designs = response[:designs] + + expect(updated_designs.size).to eq(1) + expect(updated_designs.first.versions.size).to eq(2) + end + + it 'increments the update counter' do + counter = Gitlab::UsageDataCounters::DesignsCounter + expect { run_service }.to change { counter.read(:update) }.by 1 + end + + context 'when uploading a new design' do + it 'does not link the new version to the existing design' do + existing_design = issue.designs.first + + updated_designs = run_service([dk_png])[:designs] + + expect(existing_design.versions.reload.size).to eq(1) + expect(updated_designs.size).to eq(1) + expect(updated_designs.first.versions.size).to eq(1) + end + end + end + + context 'when a design has not changed since its previous version' do + before do + run_service + end + + it 'does not create a new version' do + expect { run_service }.not_to change { issue.design_versions.count } + end + + it 'returns the design in `skipped_designs` instead of `designs`' do + response = run_service + + expect(response[:designs]).to be_empty + expect(response[:skipped_designs].size).to eq(1) + end + end + + context 'when doing a mixture of updates and creations' do + let(:files) { [rails_sample, dk_png] } + + before do + # Create just the first one, which we will later update. + run_service([files.first]) + touch_files([files.first]) + end + + it 'counts one creation and one update' do + counter = Gitlab::UsageDataCounters::DesignsCounter + expect { run_service } + .to change { counter.read(:create) }.by(1) + .and change { counter.read(:update) }.by(1) + end + + it 'creates a single commit' do + commit_count = -> do + design_repository.expire_all_method_caches + design_repository.commit_count + end + + expect { run_service }.to change { commit_count.call }.by(1) + end + + it 'enqueues just one new version worker' do + expect(::DesignManagement::NewVersionWorker) + .to receive(:perform_async).once.with(Integer) + + run_service + end + end + + context 'when uploading multiple files' do + let(:files) { [rails_sample, dk_png] } + + it 'returns information about both designs in the response' do + expect(response).to include(designs: have_attributes(size: 2), status: :success) + end + + it 'creates 2 designs with a single version' do + expect { run_service }.to change { issue.designs.count }.from(0).to(2) + + expect(DesignManagement::Version.for_designs(issue.designs).size).to eq(1) + end + + it 'increments the creation count by 2' do + counter = Gitlab::UsageDataCounters::DesignsCounter + expect { run_service }.to change { counter.read(:create) }.by 2 + end + + it 'enqueues a new version worker' do + expect(::DesignManagement::NewVersionWorker) + .to receive(:perform_async).once.with(Integer) + + run_service + end + + it 'creates a single commit' do + commit_count = -> do + design_repository.expire_all_method_caches + design_repository.commit_count + end + + expect { run_service }.to change { commit_count.call }.by(1) + end + + it 'only does 5 gitaly calls', :request_store, :sidekiq_might_not_need_inline do + allow(::DesignManagement::NewVersionWorker).to receive(:perform_async).with(Integer) + service = described_class.new(project, user, issue: issue, files: files) + # Some unrelated calls that are usually cached or happen only once + service.__send__(:repository).create_if_not_exists + service.__send__(:repository).has_visible_content? + + request_count = -> { Gitlab::GitalyClient.get_request_count } + + # An exists?, a check for existing blobs, default branch, an after_commit + # callback on LfsObjectsProject + expect { service.execute }.to change(&request_count).by(4) + end + + context 'when uploading too many files' do + let(:files) { Array.new(DesignManagement::SaveDesignsService::MAX_FILES + 1) { dk_png } } + + it 'returns the correct error' do + expect(response[:message]).to match(/only \d+ files are allowed simultaneously/i) + end + end + end + + context 'when the user is not allowed to upload designs' do + let(:user) { create(:user) } + + it_behaves_like 'a service error' + end + + describe 'failure modes' do + let(:service) { described_class.new(project, user, issue: issue, files: files) } + let(:response) { service.execute } + + before do + expect(service).to receive(:run_actions).and_raise(some_error) + end + + context 'when creating the commit fails' do + let(:some_error) { Gitlab::Git::BaseError } + + it_behaves_like 'an execution error' + end + + context 'when creating the versions fails' do + let(:some_error) { ActiveRecord::RecordInvalid } + + it_behaves_like 'a service error' + end + end + + context "when a design already existed in the repo but we didn't know about it in the database" do + let(:filename) { rails_sample_name } + + before do + path = File.join(build(:design, issue: issue, filename: filename).full_path) + design_repository.create_if_not_exists + design_repository.create_file(user, path, 'something fake', + branch_name: 'master', + message: 'Somehow created without being tracked in db') + end + + it 'creates the design and a new version for it' do + first_updated_design = response[:designs].first + + expect(first_updated_design.filename).to eq(filename) + expect(first_updated_design.versions.size).to eq(1) + end + end + + describe 'scalability', skip: 'See: https://gitlab.com/gitlab-org/gitlab/-/issues/213169' do + before do + run_service([sample_image('banana_sample.gif')]) # ensure project, issue, etc are created + end + + it 'runs the same queries for all requests, regardless of number of files' do + one = [dk_png] + two = [rails_sample, dk_png] + + baseline = ActiveRecord::QueryRecorder.new { run_service(one) } + + expect { run_service(two) }.not_to exceed_query_limit(baseline) + end + end + end + end +end diff --git a/spec/services/git/wiki_push_service_spec.rb b/spec/services/git/wiki_push_service_spec.rb index 2b2f8835d85..cdb1dc5a435 100644 --- a/spec/services/git/wiki_push_service_spec.rb +++ b/spec/services/git/wiki_push_service_spec.rb @@ -320,7 +320,7 @@ describe Git::WikiPushService, services: true do file_content: 'some stuff', branch_name: 'master' } - ::Wikis::CreateAttachmentService.new(project, project.owner, params).execute + ::Wikis::CreateAttachmentService.new(container: project, current_user: project.owner, params: params).execute end def update_page(title) diff --git a/spec/services/lfs/file_transformer_spec.rb b/spec/services/lfs/file_transformer_spec.rb index 9973d64930b..13d9c369c42 100644 --- a/spec/services/lfs/file_transformer_spec.rb +++ b/spec/services/lfs/file_transformer_spec.rb @@ -81,6 +81,23 @@ describe Lfs::FileTransformer do expect(LfsObject.last.file.read).to eq file_content end + + context 'when repository is a design repository' do + let(:file_path) { "/#{DesignManagement.designs_directory}/test_file.lfs" } + let(:repository) { project.design_repository } + + it "creates an LfsObject with the file's content" do + subject.new_file(file_path, file) + + expect(LfsObject.last.file.read).to eq(file_content) + end + + it 'saves the correct repository_type to LfsObjectsProject' do + subject.new_file(file_path, file) + + expect(project.lfs_objects_projects.first.repository_type).to eq('design') + end + end end context "when doesn't use LFS" do diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index c461dd700ec..2ae4fbeb900 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -342,6 +342,60 @@ describe Notes::CreateService do end end + context 'design note' do + subject(:service) { described_class.new(project, user, params) } + + let_it_be(:design) { create(:design, :with_file) } + let_it_be(:project) { design.project } + let_it_be(:user) { project.owner } + let_it_be(:params) do + { + type: 'DiffNote', + noteable: design, + note: "A message", + position: { + old_path: design.full_path, + new_path: design.full_path, + position_type: 'image', + width: '100', + height: '100', + x: '50', + y: '50', + base_sha: design.diff_refs.base_sha, + start_sha: design.diff_refs.base_sha, + head_sha: design.diff_refs.head_sha + } + } + end + + it 'can create diff notes for designs' do + note = service.execute + + expect(note).to be_a(DiffNote) + expect(note).to be_persisted + expect(note.noteable).to eq(design) + end + + it 'sends a notification about this note', :sidekiq_might_not_need_inline do + notifier = double + allow(::NotificationService).to receive(:new).and_return(notifier) + + expect(notifier) + .to receive(:new_note) + .with have_attributes(noteable: design) + + service.execute + end + + it 'correctly builds the position of the note' do + note = service.execute + + expect(note.position.new_path).to eq(design.full_path) + expect(note.position.old_path).to eq(design.full_path) + expect(note.position.diff_refs).to eq(design.diff_refs) + end + end + context 'note with emoji only' do it 'creates regular note' do opts = { diff --git a/spec/services/notes/post_process_service_spec.rb b/spec/services/notes/post_process_service_spec.rb index 99db7897664..d564cacd2d8 100644 --- a/spec/services/notes/post_process_service_spec.rb +++ b/spec/services/notes/post_process_service_spec.rb @@ -43,5 +43,32 @@ describe Notes::PostProcessService do described_class.new(@note).execute end end + + context 'when the noteable is a design' do + let_it_be(:noteable) { create(:design, :with_file) } + let_it_be(:discussion_note) { create_note } + + subject { described_class.new(note).execute } + + def create_note(in_reply_to: nil) + create(:diff_note_on_design, noteable: noteable, in_reply_to: in_reply_to) + end + + context 'when the note is the start of a new discussion' do + let(:note) { discussion_note } + + it 'creates a new system note' do + expect { subject }.to change { Note.system.count }.by(1) + end + end + + context 'when the note is a reply within a discussion' do + let_it_be(:note) { create_note(in_reply_to: discussion_note) } + + it 'does not create a new system note' do + expect { subject }.not_to change { Note.system.count } + end + end + end end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 2134166cd6d..7d6fd9430eb 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -709,6 +709,57 @@ describe NotificationService, :mailer do end end end + + context 'when notified of a new design diff note' do + include DesignManagementTestHelpers + + let_it_be(:design) { create(:design, :with_file) } + let_it_be(:project) { design.project } + let_it_be(:dev) { create(:user) } + let_it_be(:stranger) { create(:user) } + let_it_be(:note) do + create(:diff_note_on_design, + noteable: design, + note: "Hello #{dev.to_reference}, G'day #{stranger.to_reference}") + end + let(:mailer) { double(deliver_later: true) } + + context 'design management is enabled' do + before do + enable_design_management + project.add_developer(dev) + allow(Notify).to receive(:note_design_email) { mailer } + end + + it 'sends new note notifications' do + expect(subject).to receive(:send_new_note_notifications).with(note) + + subject.new_note(note) + end + + it 'sends a mail to the developer' do + expect(Notify) + .to receive(:note_design_email).with(dev.id, note.id, 'mentioned') + + subject.new_note(note) + end + + it 'does not notify non-developers' do + expect(Notify) + .not_to receive(:note_design_email).with(stranger.id, note.id) + + subject.new_note(note) + end + end + + context 'design management is disabled' do + it 'does not notify the user' do + expect(Notify).not_to receive(:note_design_email) + + subject.new_note(note) + end + end + end end describe '#send_new_release_notifications', :deliver_mails_inline, :sidekiq_inline do diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 71be335c11d..f1eaf8324e0 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -6,7 +6,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do include GitHelpers let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :legacy_storage, :repository, :wiki_repo) } + let(:project) { create(:project, :legacy_storage, :repository, :wiki_repo, :design_repo) } let(:legacy_storage) { Storage::LegacyProject.new(project) } let(:hashed_storage) { Storage::Hashed.new(project) } @@ -45,11 +45,12 @@ describe Projects::HashedStorage::MigrateRepositoryService do end context 'when succeeds' do - it 'renames project and wiki repositories' do + it 'renames project, wiki and design repositories' do service.execute expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy + expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_truthy end it 'updates project to be hashed and not read-only' do @@ -59,9 +60,10 @@ describe Projects::HashedStorage::MigrateRepositoryService do expect(project.repository_read_only).to be_falsey end - it 'move operation is called for both repositories' do + it 'move operation is called for all repositories' do expect_move_repository(old_disk_path, new_disk_path) expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") + expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") service.execute end @@ -86,6 +88,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey + expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_falsey expect(project.repository_read_only?).to be_falsey end @@ -97,6 +100,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do it 'does not try to move nil repository over existing' do expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, old_disk_path, new_disk_path) expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") + expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") service.execute end diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb index 6dcd2ff4555..1c0f446d9cf 100644 --- a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb @@ -6,7 +6,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis include GitHelpers let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :repository, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } + let(:project) { create(:project, :repository, :wiki_repo, :design_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } let(:legacy_storage) { Storage::LegacyProject.new(project) } let(:hashed_storage) { Storage::Hashed.new(project) } @@ -45,11 +45,12 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis end context 'when succeeds' do - it 'renames project and wiki repositories' do + it 'renames project, wiki and design repositories' do service.execute expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy + expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_truthy end it 'updates project to be legacy and not read-only' do @@ -62,6 +63,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis it 'move operation is called for both repositories' do expect_move_repository(old_disk_path, new_disk_path) expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") + expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") service.execute end @@ -86,6 +88,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey + expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_falsey expect(project.repository_read_only?).to be_falsey end @@ -97,6 +100,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis it 'does not try to move nil repository over existing' do expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, old_disk_path, new_disk_path) expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") + expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") service.execute end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index f17ddb22d22..274af5f03bf 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -359,6 +359,90 @@ describe Projects::TransferService do end end + describe 'transferring a design repository' do + subject { described_class.new(project, user) } + + before do + group.add_owner(user) + end + + def design_repository + project.design_repository + end + + it 'does not create a design repository' do + expect(subject.execute(group)).to be true + + project.clear_memoization(:design_repository) + + expect(design_repository.exists?).to be false + end + + describe 'when the project has a design repository' do + let(:project_repo_path) { "#{project.path}#{::Gitlab::GlRepository::DESIGN.path_suffix}" } + let(:old_full_path) { "#{user.namespace.full_path}/#{project_repo_path}" } + let(:new_full_path) { "#{group.full_path}/#{project_repo_path}" } + + context 'with legacy storage' do + let(:project) { create(:project, :repository, :legacy_storage, :design_repo, namespace: user.namespace) } + + it 'moves the repository' do + expect(subject.execute(group)).to be true + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: new_full_path, + full_path: new_full_path + ) + end + + it 'does not move the repository when an error occurs', :aggregate_failures do + allow(subject).to receive(:execute_system_hooks).and_raise('foo') + expect { subject.execute(group) }.to raise_error('foo') + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: old_full_path, + full_path: old_full_path + ) + end + end + + context 'with hashed storage' do + let(:project) { create(:project, :repository, namespace: user.namespace) } + + it 'does not move the repository' do + old_disk_path = design_repository.disk_path + + expect(subject.execute(group)).to be true + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: old_disk_path, + full_path: new_full_path + ) + end + + it 'does not move the repository when an error occurs' do + old_disk_path = design_repository.disk_path + + allow(subject).to receive(:execute_system_hooks).and_raise('foo') + expect { subject.execute(group) }.to raise_error('foo') + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: old_disk_path, + full_path: old_full_path + ) + end + end + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index b58dac9a6ce..4d63216d1c8 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -141,5 +141,18 @@ describe Projects::UpdateRepositoryStorageService do end end end + + context 'with design repository' do + include_examples 'moves repository to another storage', 'design' do + let(:project) { create(:project, :repository, repository_read_only: true) } + let(:repository) { project.design_repository } + let(:destination) { 'test_second_storage' } + let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } + + before do + project.design_repository.create_if_not_exists + end + end + end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index f3e6cb3cc52..66f9b5d092f 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -6,6 +6,7 @@ describe SystemNoteService do include Gitlab::Routing include RepoHelpers include AssetsHelpers + include DesignManagementTestHelpers let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, group: group) } @@ -636,4 +637,28 @@ describe SystemNoteService do described_class.auto_resolve_prometheus_alert(noteable, project, author) end end + + describe '.design_version_added' do + let(:version) { create(:design_version) } + + it 'calls DesignManagementService' do + expect_next_instance_of(SystemNotes::DesignManagementService) do |service| + expect(service).to receive(:design_version_added).with(version) + end + + described_class.design_version_added(version) + end + end + + describe '.design_discussion_added' do + let(:discussion_note) { create(:diff_note_on_design) } + + it 'calls DesignManagementService' do + expect_next_instance_of(SystemNotes::DesignManagementService) do |service| + expect(service).to receive(:design_discussion_added).with(discussion_note) + end + + described_class.design_discussion_added(discussion_note) + end + end end diff --git a/spec/services/system_notes/design_management_service_spec.rb b/spec/services/system_notes/design_management_service_spec.rb new file mode 100644 index 00000000000..08511e62341 --- /dev/null +++ b/spec/services/system_notes/design_management_service_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SystemNotes::DesignManagementService do + let(:project) { create(:project) } + let(:issue) { create(:issue, project: project) } + + let(:instance) { described_class.new(noteable: instance_noteable, project: instance_project, author: instance_author) } + + describe '#design_version_added' do + let(:instance_noteable) { version.issue } + let(:instance_project) { version.issue.project } + let(:instance_author) { version.author } + + subject { instance.design_version_added(version) } + + # default (valid) parameters: + let(:n_designs) { 3 } + let(:designs) { create_list(:design, n_designs, issue: issue) } + let(:user) { build(:user) } + let(:version) do + create(:design_version, issue: issue, designs: designs) + end + + before do + # Avoid needing to call into gitaly + allow(version).to receive(:author).and_return(user) + end + + context 'with one kind of event' do + before do + DesignManagement::Action + .where(design: designs).update_all(event: :modification) + end + + it 'makes just one note' do + expect(subject).to contain_exactly(Note) + end + + it 'adds a new system note' do + expect { subject }.to change { Note.system.count }.by(1) + end + end + + context 'with a mixture of events' do + let(:n_designs) { DesignManagement::Action.events.size } + + before do + designs.each_with_index do |design, i| + design.actions.update_all(event: i) + end + end + + it 'makes one note for each kind of event' do + expect(subject).to have_attributes(size: n_designs) + end + + it 'adds a system note for each kind of event' do + expect { subject }.to change { Note.system.count }.by(n_designs) + end + end + + describe 'icons' do + where(:action) do + [ + [:creation], + [:modification], + [:deletion] + ] + end + + with_them do + before do + version.actions.update_all(event: action) + end + + subject(:metadata) do + instance.design_version_added(version) + .first.system_note_metadata + end + + it 'has a valid action' do + expect(::SystemNoteHelper::ICON_NAMES_BY_ACTION) + .to include(metadata.action) + end + end + end + + context 'it succeeds' do + where(:action, :icon, :human_description) do + [ + [:creation, 'designs_added', 'added'], + [:modification, 'designs_modified', 'updated'], + [:deletion, 'designs_removed', 'removed'] + ] + end + + with_them do + before do + version.actions.update_all(event: action) + end + + let(:anchor_tag) { %r{ <a[^>]*>#{link}</a>} } + let(:href) { instance.send(:designs_path, { version: version.id }) } + let(:link) { "#{n_designs} designs" } + + subject(:note) { instance.design_version_added(version).first } + + it 'has the correct data' do + expect(note) + .to be_system + .and have_attributes( + system_note_metadata: have_attributes(action: icon), + note: include(human_description) + .and(include link) + .and(include href), + note_html: a_string_matching(anchor_tag) + ) + end + end + end + end + + describe '#design_discussion_added' do + let(:instance_noteable) { design.issue } + let(:instance_project) { design.issue.project } + let(:instance_author) { discussion_note.author } + + subject { instance.design_discussion_added(discussion_note) } + + let(:design) { create(:design, :with_file, issue: issue) } + let(:author) { create(:user) } + let(:discussion_note) do + create(:diff_note_on_design, noteable: design, author: author) + end + let(:action) { 'designs_discussion_added' } + + it_behaves_like 'a system note' do + let(:noteable) { discussion_note.noteable.issue } + end + + it 'adds a new system note' do + expect { subject }.to change { Note.system.count }.by(1) + end + + it 'has the correct note text' do + href = instance.send(:designs_path, + { vueroute: design.filename, anchor: ActionView::RecordIdentifier.dom_id(discussion_note) } + ) + + expect(subject.note).to eq("started a discussion on [#{design.filename}](#{href})") + end + end +end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 9b92590cb63..4894cf12372 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -895,6 +895,36 @@ describe TodoService do end end + describe 'Designs' do + include DesignManagementTestHelpers + + let(:issue) { create(:issue, project: project) } + let(:design) { create(:design, issue: issue) } + + before do + enable_design_management + + project.add_guest(author) + project.add_developer(john_doe) + end + + let(:note) do + build(:diff_note_on_design, + noteable: design, + author: author, + note: "Hey #{john_doe.to_reference}") + end + + it 'creates a todo for mentioned user on new diff note' do + service.new_note(note, author) + + should_create_todo(user: john_doe, + target: design, + action: Todo::MENTIONED, + note: note) + end + end + describe '#update_note' do let(:noteable) { create(:issue, project: project) } let(:note) { create(:note, project: project, note: mentions, noteable: noteable) } diff --git a/spec/services/wikis/create_attachment_service_spec.rb b/spec/services/wikis/create_attachment_service_spec.rb index 7a73a0a555f..4adfaa24874 100644 --- a/spec/services/wikis/create_attachment_service_spec.rb +++ b/spec/services/wikis/create_attachment_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Wikis::CreateAttachmentService do - let(:project) { create(:project, :wiki_repo) } + let(:container) { create(:project, :wiki_repo) } let(:user) { create(:user) } let(:file_name) { 'filename.txt' } let(:file_path_regex) { %r{#{described_class::ATTACHMENT_PATH}/\h{32}/#{file_name}} } @@ -15,25 +15,21 @@ describe Wikis::CreateAttachmentService do end let(:opts) { file_opts } - subject(:service) { described_class.new(project, user, opts) } + subject(:service) { described_class.new(container: container, current_user: user, params: opts) } before do - project.add_developer(user) + container.add_developer(user) end describe 'initialization' do context 'author commit info' do it 'does not raise error if user is nil' do - service = described_class.new(project, nil, opts) + service = described_class.new(container: container, current_user: nil, params: opts) expect(service.instance_variable_get(:@author_email)).to be_nil expect(service.instance_variable_get(:@author_name)).to be_nil end - it 'fills file_path from the repository uploads folder' do - expect(service.instance_variable_get(:@file_path)).to match(file_path_regex) - end - context 'when no author info provided' do it 'fills author_email and author_name from current_user info' do expect(service.instance_variable_get(:@author_email)).to eq user.email @@ -73,7 +69,7 @@ describe Wikis::CreateAttachmentService do context 'branch name' do context 'when no branch provided' do it 'sets the branch from the wiki default_branch' do - expect(service.instance_variable_get(:@branch_name)).to eq project.wiki.default_branch + expect(service.instance_variable_get(:@branch_name)).to eq container.wiki.default_branch end end @@ -151,7 +147,7 @@ describe Wikis::CreateAttachmentService do context 'when user' do shared_examples 'wiki attachment user validations' do it 'returns error' do - result = described_class.new(project, user2, opts).execute + result = described_class.new(container: container, current_user: user2, params: opts).execute expect(result[:status]).to eq :error expect(result[:message]).to eq 'You are not allowed to push to the wiki' @@ -172,54 +168,5 @@ describe Wikis::CreateAttachmentService do end end - describe '#execute' do - let(:wiki) { project.wiki } - - subject(:service_execute) { service.execute[:result] } - - context 'creates branch if it does not exists' do - let(:branch_name) { 'new_branch' } - let(:opts) { file_opts.merge(branch_name: branch_name) } - - it do - expect(wiki.repository.branches).to be_empty - expect { service.execute }.to change { wiki.repository.branches.count }.by(1) - expect(wiki.repository.branches.first.name).to eq branch_name - end - end - - it 'adds file to the repository' do - expect(wiki.repository.ls_files('HEAD')).to be_empty - - service.execute - - files = wiki.repository.ls_files('HEAD') - expect(files.count).to eq 1 - expect(files.first).to match(file_path_regex) - end - - context 'returns' do - before do - allow(SecureRandom).to receive(:hex).and_return('fixed_hex') - - service_execute - end - - it 'returns the file name' do - expect(service_execute[:file_name]).to eq file_name - end - - it 'returns the path where file was stored' do - expect(service_execute[:file_path]).to eq 'uploads/fixed_hex/filename.txt' - end - - it 'returns the branch where the file was pushed' do - expect(service_execute[:branch]).to eq wiki.default_branch - end - - it 'returns the commit id' do - expect(service_execute[:commit]).not_to be_empty - end - end - end + it_behaves_like 'Wikis::CreateAttachmentService#execute', :project end diff --git a/spec/support/helpers/wiki_helpers.rb b/spec/support/helpers/wiki_helpers.rb index 86eb1793707..e6818ff8f0c 100644 --- a/spec/support/helpers/wiki_helpers.rb +++ b/spec/support/helpers/wiki_helpers.rb @@ -14,7 +14,10 @@ module WikiHelpers file_content: File.read(expand_fixture_path(file_name)) } - ::Wikis::CreateAttachmentService.new(project, user, opts) - .execute[:result][:file_path] + ::Wikis::CreateAttachmentService.new( + container: project, + current_user: user, + params: opts + ).execute[:result][:file_path] end end diff --git a/spec/support/shared_examples/services/wikis/create_attachment_service_shared_examples.rb b/spec/support/shared_examples/services/wikis/create_attachment_service_shared_examples.rb new file mode 100644 index 00000000000..541e332e3a1 --- /dev/null +++ b/spec/support/shared_examples/services/wikis/create_attachment_service_shared_examples.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'Wikis::CreateAttachmentService#execute' do |container_type| + let(:container) { create(container_type, :wiki_repo) } + let(:wiki) { container.wiki } + + let(:user) { create(:user) } + let(:file_name) { 'filename.txt' } + let(:file_path_regex) { %r{#{described_class::ATTACHMENT_PATH}/\h{32}/#{file_name}} } + + let(:file_opts) do + { + file_name: file_name, + file_content: 'Content of attachment' + } + end + let(:opts) { file_opts } + + let(:service) { Wikis::CreateAttachmentService.new(container: container, current_user: user, params: opts) } + + subject(:service_execute) { service.execute[:result] } + + before do + container.add_developer(user) + end + + context 'creates branch if it does not exists' do + let(:branch_name) { 'new_branch' } + let(:opts) { file_opts.merge(branch_name: branch_name) } + + it do + expect(wiki.repository.branches).to be_empty + expect { service.execute }.to change { wiki.repository.branches.count }.by(1) + expect(wiki.repository.branches.first.name).to eq branch_name + end + end + + it 'adds file to the repository' do + expect(wiki.repository.ls_files('HEAD')).to be_empty + + service.execute + + files = wiki.repository.ls_files('HEAD') + expect(files.count).to eq 1 + expect(files.first).to match(file_path_regex) + end + + context 'returns' do + before do + allow(SecureRandom).to receive(:hex).and_return('fixed_hex') + + service_execute + end + + it 'returns related information', :aggregate_failures do + expect(service_execute[:file_name]).to eq file_name + expect(service_execute[:file_path]).to eq 'uploads/fixed_hex/filename.txt' + expect(service_execute[:branch]).to eq wiki.default_branch + expect(service_execute[:commit]).not_to be_empty + end + end +end diff --git a/spec/workers/design_management/new_version_worker_spec.rb b/spec/workers/design_management/new_version_worker_spec.rb index 76497dde464..ef7cd8de108 100644 --- a/spec/workers/design_management/new_version_worker_spec.rb +++ b/spec/workers/design_management/new_version_worker_spec.rb @@ -3,14 +3,6 @@ require 'spec_helper' describe DesignManagement::NewVersionWorker do - # TODO a number of these tests are being temporarily skipped unless run in EE, - # as we are in the process of moving Design Management to FOSS in 13.0 - # in steps. In the current step the services have not yet been moved, and - # certain services are called within these tests: - # - `SystemNoteService` - # - `DesignManagement::GenerateImageVersionsService` - # - # See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283. describe '#perform' do let(:worker) { described_class.new } @@ -18,16 +10,12 @@ describe DesignManagement::NewVersionWorker do let(:version_id) { -1 } it 'does not create system notes' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect(SystemNoteService).not_to receive(:design_version_added) worker.perform(version_id) end it 'does not invoke GenerateImageVersionsService' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect(DesignManagement::GenerateImageVersionsService).not_to receive(:new) worker.perform(version_id) @@ -45,14 +33,10 @@ describe DesignManagement::NewVersionWorker do let_it_be(:version) { create(:design_version, :with_lfs_file, designs_count: 2) } it 'creates a system note' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect { worker.perform(version.id) }.to change { Note.system.count }.by(1) end it 'invokes GenerateImageVersionsService' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect_next_instance_of(DesignManagement::GenerateImageVersionsService) do |service| expect(service).to receive(:execute) end @@ -61,8 +45,6 @@ describe DesignManagement::NewVersionWorker do end it 'does not log anything' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect(Sidekiq.logger).not_to receive(:warn) worker.perform(version.id) @@ -77,14 +59,10 @@ describe DesignManagement::NewVersionWorker do end it 'creates two system notes' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect { worker.perform(version.id) }.to change { Note.system.count }.by(2) end it 'calls design_version_added' do - skip 'See https://gitlab.com/gitlab-org/gitlab/-/issues/212566#note_327724283' unless Gitlab.ee? - expect(SystemNoteService).to receive(:design_version_added).with(version) worker.perform(version.id) |
