diff options
51 files changed, 1084 insertions, 182 deletions
diff --git a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue index ac00af2ab34..124780df8a5 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue @@ -174,6 +174,7 @@ export default { this.$emit('open-form', this.discussion.id); this.isFormRendered = true; }, + toggleResolvedStatus() { this.isResolving = true; @@ -234,6 +235,7 @@ export default { :note="firstNote" :markdown-preview-path="markdownPreviewPath" :is-resolving="isResolving" + :noteable-id="noteableId" :class="{ 'gl-bg-blue-50': isDiscussionActive }" @error="$emit('update-note-error', $event)" > @@ -276,6 +278,7 @@ export default { :note="note" :markdown-preview-path="markdownPreviewPath" :is-resolving="isResolving" + :noteable-id="noteableId" :class="{ 'gl-bg-blue-50': isDiscussionActive }" @error="$emit('update-note-error', $event)" /> @@ -307,6 +310,8 @@ export default { v-model="discussionComment" :is-saving="loading" :markdown-preview-path="markdownPreviewPath" + :noteable-id="noteableId" + :discussion-id="discussion.id" @submit-form="mutate" @cancel-form="hideForm" > diff --git a/app/assets/javascripts/design_management/components/design_notes/design_note.vue b/app/assets/javascripts/design_management/components/design_notes/design_note.vue index 5fb5989e11a..e629f74ba02 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_note.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_note.vue @@ -45,6 +45,10 @@ export default { required: false, default: '', }, + noteableId: { + type: String, + required: true, + }, }, data() { return { @@ -160,6 +164,7 @@ export default { :is-saving="loading" :markdown-preview-path="markdownPreviewPath" :is-new-comment="false" + :noteable-id="noteableId" class="gl-mt-5" @submit-form="mutate" @cancel-form="hideForm" diff --git a/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue b/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue index 1b6458668f5..4faeba3983b 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_reply_form.vue @@ -1,7 +1,11 @@ <script> import { GlButton, GlModal } from '@gitlab/ui'; +import $ from 'jquery'; import { helpPagePath } from '~/helpers/help_page_helper'; import { s__ } from '~/locale'; +import Autosave from '~/autosave'; +import { isLoggedIn } from '~/lib/utils/common_utils'; +import { getIdFromGraphQLId, isGid } from '~/graphql_shared/utils'; import MarkdownField from '~/vue_shared/components/markdown/field.vue'; export default { @@ -30,10 +34,20 @@ export default { required: false, default: true, }, + noteableId: { + type: String, + required: true, + }, + discussionId: { + type: String, + required: false, + default: 'new', + }, }, data() { return { formText: this.value, + isLoggedIn: isLoggedIn(), }; }, computed: { @@ -64,13 +78,19 @@ export default { markdownDocsPath() { return helpPagePath('user/markdown'); }, + shortDiscussionId() { + return isGid(this.discussionId) ? getIdFromGraphQLId(this.discussionId) : this.discussionId; + }, }, mounted() { this.focusInput(); }, methods: { submitForm() { - if (this.hasValue) this.$emit('submit-form'); + if (this.hasValue) { + this.$emit('submit-form'); + this.autosaveDiscussion.reset(); + } }, cancelComment() { if (this.hasValue && this.formText !== this.value) { @@ -79,8 +99,22 @@ export default { this.$emit('cancel-form'); } }, + confirmCancelCommentModal() { + this.$emit('cancel-form'); + this.autosaveDiscussion.reset(); + }, focusInput() { this.$refs.textarea.focus(); + this.initAutosaveComment(); + }, + initAutosaveComment() { + if (this.isLoggedIn) { + this.autosaveDiscussion = new Autosave($(this.$refs.textarea), [ + s__('DesignManagement|Discussion'), + getIdFromGraphQLId(this.noteableId), + this.shortDiscussionId, + ]); + } }, }, }; @@ -124,7 +158,7 @@ export default { type="submit" data-track-action="click_button" data-qa-selector="save_comment_button" - @click="$emit('submit-form')" + @click="submitForm" > {{ buttonText }} </gl-button> @@ -144,7 +178,7 @@ export default { :ok-title="modalSettings.okTitle" :cancel-title="modalSettings.cancelTitle" modal-id="cancel-comment-modal" - @ok="$emit('cancel-form')" + @ok="confirmCancelCommentModal" >{{ modalSettings.content }} </gl-modal> </form> diff --git a/app/assets/javascripts/design_management/pages/design/index.vue b/app/assets/javascripts/design_management/pages/design/index.vue index 1825ce7f092..228ad637b9e 100644 --- a/app/assets/javascripts/design_management/pages/design/index.vue +++ b/app/assets/javascripts/design_management/pages/design/index.vue @@ -418,6 +418,7 @@ export default { v-model="comment" :is-saving="loading" :markdown-preview-path="markdownPreviewPath" + :noteable-id="design.id" @submit-form="mutate" @cancel-form="closeCommentForm" /> </apollo-mutation diff --git a/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql b/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql index 75e1daaafbf..98b51e8c2c4 100644 --- a/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql +++ b/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql @@ -12,7 +12,6 @@ query getJobs($fullPath: ID!, $after: String, $first: Int = 30, $statuses: [CiJo nodes { artifacts { nodes { - id downloadPath fileType } diff --git a/app/assets/javascripts/pages/projects/settings/merge_requests/index.js b/app/assets/javascripts/pages/projects/settings/merge_requests/index.js new file mode 100644 index 00000000000..739e666644c --- /dev/null +++ b/app/assets/javascripts/pages/projects/settings/merge_requests/index.js @@ -0,0 +1,10 @@ +import groupsSelect from '~/groups_select'; +import UserCallout from '~/user_callout'; +import UsersSelect from '~/users_select'; + +// eslint-disable-next-line no-new +new UsersSelect(); +groupsSelect(); + +// eslint-disable-next-line no-new +new UserCallout({ className: 'js-mr-approval-callout' }); diff --git a/app/assets/javascripts/persistent_user_callouts.js b/app/assets/javascripts/persistent_user_callouts.js index 04f0caa1ca3..a2b9a3bf889 100644 --- a/app/assets/javascripts/persistent_user_callouts.js +++ b/app/assets/javascripts/persistent_user_callouts.js @@ -18,6 +18,7 @@ const PERSISTENT_USER_CALLOUTS = [ '.js-project-usage-limitations-callout', '.js-namespace-storage-alert', '.js-web-hook-disabled-callout', + '.js-merge-request-settings-callout', ]; const initCallouts = () => { diff --git a/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_jobs.query.graphql b/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_jobs.query.graphql index 0ed8f596d3d..641ec7a3cf6 100644 --- a/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_jobs.query.graphql +++ b/app/assets/javascripts/pipelines/graphql/queries/get_pipeline_jobs.query.graphql @@ -12,7 +12,6 @@ query getPipelineJobs($fullPath: ID!, $iid: ID!, $after: String) { nodes { artifacts { nodes { - id downloadPath fileType } diff --git a/app/assets/javascripts/vue_shared/security_reports/graphql/fragments/job_artifacts.fragment.graphql b/app/assets/javascripts/vue_shared/security_reports/graphql/fragments/job_artifacts.fragment.graphql index 981d01cc81a..829b9d9f9d8 100644 --- a/app/assets/javascripts/vue_shared/security_reports/graphql/fragments/job_artifacts.fragment.graphql +++ b/app/assets/javascripts/vue_shared/security_reports/graphql/fragments/job_artifacts.fragment.graphql @@ -6,7 +6,6 @@ fragment JobArtifacts on Pipeline { name artifacts { nodes { - id downloadPath fileType } diff --git a/app/assets/javascripts/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql b/app/assets/javascripts/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql index 9c5090cfc28..2e80db30e9a 100644 --- a/app/assets/javascripts/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql +++ b/app/assets/javascripts/vue_shared/security_reports/graphql/queries/security_report_merge_request_download_paths.query.graphql @@ -15,7 +15,6 @@ query securityReportDownloadPaths( name artifacts { nodes { - id downloadPath fileType } diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss index ab426f388c6..a63ce66e681 100644 --- a/app/assets/stylesheets/framework/highlight.scss +++ b/app/assets/stylesheets/framework/highlight.scss @@ -31,8 +31,7 @@ width: 100%; padding-left: 10px; padding-right: 10px; - white-space: break-spaces; - word-break: break-word; + white-space: pre; &:empty::before { content: '\200b'; diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 6c909b8d9fa..f65c45d6d89 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -383,6 +383,10 @@ input[type='checkbox']:hover { .line_holder { pre { padding: 0; // This overrides the existing style that will add space between each line. + .line { + @include gl-word-break-word; + white-space: break-spaces; + } } svg { diff --git a/app/controllers/projects/settings/merge_requests_controller.rb b/app/controllers/projects/settings/merge_requests_controller.rb new file mode 100644 index 00000000000..93e10695767 --- /dev/null +++ b/app/controllers/projects/settings/merge_requests_controller.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Projects + module Settings + class MergeRequestsController < Projects::ApplicationController + layout 'project_settings' + + before_action :merge_requests_enabled? + before_action :present_project, only: [:edit] + before_action :authorize_admin_project! + + feature_category :code_review + + def update + result = ::Projects::UpdateService.new(@project, current_user, project_params).execute + + if result[:status] == :success + flash[:notice] = format(_("Project '%{project_name}' was successfully updated."), project_name: @project.name) + redirect_to project_settings_merge_requests_path(@project) + else + # Refresh the repo in case anything changed + @repository = @project.repository.reset + + flash[:alert] = result[:message] + @project.reset + render 'show' + end + end + + private + + def merge_requests_enabled? + render_404 unless @project.merge_requests_enabled? + end + + def project_params + params.require(:project) + .permit(project_params_attributes) + end + + def project_setting_attributes + %i[ + squash_option + allow_editing_commit_messages + mr_default_target_self + ] + end + + def project_params_attributes + [ + :allow_merge_on_skipped_pipeline, + :resolve_outdated_diff_discussions, + :only_allow_merge_if_all_discussions_are_resolved, + :only_allow_merge_if_pipeline_succeeds, + :printing_merge_request_link_enabled, + :remove_source_branch_after_merge, + :merge_method, + :merge_commit_template_or_default, + :squash_commit_template_or_default, + :suggestion_commit_message + ] + [project_setting_attributes: project_setting_attributes] + end + end + end +end + +Projects::Settings::MergeRequestsController.prepend_mod_with('Projects::Settings::MergeRequestsController') diff --git a/app/graphql/types/ci/job_artifact_type.rb b/app/graphql/types/ci/job_artifact_type.rb index 6346d50de3a..a6ab445702c 100644 --- a/app/graphql/types/ci/job_artifact_type.rb +++ b/app/graphql/types/ci/job_artifact_type.rb @@ -6,9 +6,6 @@ module Types class JobArtifactType < BaseObject graphql_name 'CiJobArtifact' - field :id, Types::GlobalIDType[::Ci::JobArtifact], null: false, - description: 'ID of the artifact.' - field :download_path, GraphQL::Types::String, null: true, description: "URL for downloading the artifact's file." @@ -19,12 +16,6 @@ module Types description: 'File name of the artifact.', method: :filename - field :size, GraphQL::Types::Int, null: false, - description: 'Size of the artifact in bytes.' - - field :expire_at, Types::TimeType, null: true, - description: 'Expiry date of the artifact.' - def download_path ::Gitlab::Routing.url_helpers.download_project_job_artifacts_path( object.project, diff --git a/app/helpers/users/callouts_helper.rb b/app/helpers/users/callouts_helper.rb index b08de4edb62..a2c3aa649e6 100644 --- a/app/helpers/users/callouts_helper.rb +++ b/app/helpers/users/callouts_helper.rb @@ -10,6 +10,7 @@ module Users REGISTRATION_ENABLED_CALLOUT = 'registration_enabled_callout' UNFINISHED_TAG_CLEANUP_CALLOUT = 'unfinished_tag_cleanup_callout' SECURITY_NEWSLETTER_CALLOUT = 'security_newsletter_callout' + MERGE_REQUEST_SETTINGS_MOVED_CALLOUT = 'merge_request_settings_moved_callout' REGISTRATION_ENABLED_CALLOUT_ALLOWED_CONTROLLER_PATHS = [/^root/, /^dashboard\S*/, /^admin\S*/].freeze WEB_HOOK_DISABLED = 'web_hook_disabled' @@ -74,6 +75,10 @@ module Users user_dismissed?(WEB_HOOK_DISABLED, last_failure, project: project) end + def show_merge_request_settings_callout? + !user_dismissed?(MERGE_REQUEST_SETTINGS_MOVED_CALLOUT) + end + private def user_dismissed?(feature_name, ignore_dismissal_earlier_than = nil, project: nil) diff --git a/app/models/users/callout.rb b/app/models/users/callout.rb index c9b6b859d45..03841ee48fa 100644 --- a/app/models/users/callout.rb +++ b/app/models/users/callout.rb @@ -60,7 +60,8 @@ module Users namespace_storage_limit_banner_warning_threshold: 56, # EE-only namespace_storage_limit_banner_alert_threshold: 57, # EE-only namespace_storage_limit_banner_error_threshold: 58, # EE-only - project_quality_summary_feedback: 59 # EE-only + project_quality_summary_feedback: 59, # EE-only + merge_request_settings_moved_callout: 60 } validates :feature_name, diff --git a/app/services/users/migrate_records_to_ghost_user_service.rb b/app/services/users/migrate_records_to_ghost_user_service.rb new file mode 100644 index 00000000000..6e2f478ea72 --- /dev/null +++ b/app/services/users/migrate_records_to_ghost_user_service.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +# When a user is destroyed, some of their associated records are +# moved to a "Ghost User", to prevent these associated records from +# being destroyed. +# +# For example, all the issues/MRs a user has created are _not_ destroyed +# when the user is destroyed. +module Users + class MigrateRecordsToGhostUserService + extend ActiveSupport::Concern + + DestroyError = Class.new(StandardError) + + attr_reader :ghost_user, :user, :initiator_user, :hard_delete + + def initialize(user, initiator_user) + @user = user + @initiator_user = initiator_user + @ghost_user = User.ghost + end + + def execute(hard_delete: false) + @hard_delete = hard_delete + + migrate_records + post_migrate_records + end + + private + + def migrate_records + return if hard_delete + + migrate_issues + migrate_merge_requests + migrate_notes + migrate_abuse_reports + migrate_award_emoji + migrate_snippets + migrate_reviews + end + + def post_migrate_records + delete_snippets + + # Rails attempts to load all related records into memory before + # destroying: https://github.com/rails/rails/issues/22510 + # This ensures we delete records in batches. + user.destroy_dependent_associations_in_batches(exclude: [:snippets]) + user.nullify_dependent_associations_in_batches + + # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing + user_data = user.destroy + user.namespace.destroy + + user_data + end + + def delete_snippets + response = Snippets::BulkDestroyService.new(initiator_user, user.snippets).execute(skip_authorization: true) + raise DestroyError, response.message if response.error? + end + + def migrate_issues + batched_migrate(Issue, :author_id) + batched_migrate(Issue, :last_edited_by_id) + end + + def migrate_merge_requests + batched_migrate(MergeRequest, :author_id) + batched_migrate(MergeRequest, :merge_user_id) + end + + def migrate_notes + batched_migrate(Note, :author_id) + end + + def migrate_abuse_reports + user.reported_abuse_reports.update_all(reporter_id: ghost_user.id) + end + + def migrate_award_emoji + user.award_emoji.update_all(user_id: ghost_user.id) + end + + def migrate_snippets + snippets = user.snippets.only_project_snippets + snippets.update_all(author_id: ghost_user.id) + end + + def migrate_reviews + batched_migrate(Review, :author_id) + end + + # rubocop:disable CodeReuse/ActiveRecord + def batched_migrate(base_scope, column, batch_size: 50) + loop do + update_count = base_scope.where(column => user.id).limit(batch_size).update_all(column => ghost_user.id) + break if update_count == 0 + end + end + # rubocop:enable CodeReuse/ActiveRecord + end +end + +Users::MigrateRecordsToGhostUserService.prepend_mod_with('Users::MigrateRecordsToGhostUserService') diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index a7dd69a9607..fda17284f83 100644 --- a/app/views/projects/edit.html.haml +++ b/app/views/projects/edit.html.haml @@ -26,23 +26,13 @@ %template.js-project-permissions-form-data{ type: "application/json" }= project_permissions_panel_data(@project).to_json.html_safe .js-project-permissions-form{ data: visibility_confirm_modal_data(@project, reduce_visibility_form_id) } -%section.rspec-merge-request-settings.settings.merge-requests-feature.no-animate#js-merge-request-settings{ class: [('expanded' if expanded), ('hidden' if @project.project_feature.send(:merge_requests_access_level) == 0)], data: { qa_selector: 'merge_request_settings_content' } } - .settings-header - %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Merge requests') - = render Pajamas::ButtonComponent.new(button_options: { class: 'js-settings-toggle' }) do - = expanded ? _('Collapse') : _('Expand') - = render_if_exists 'projects/merge_request_settings_description_text' - - .settings-content - = render_if_exists 'shared/promotions/promote_mr_features' - - = gitlab_ui_form_for @project, html: { multipart: true, class: "merge-request-settings-form js-mr-settings-form" }, authenticity_token: true do |f| - %input{ name: 'update_section', type: 'hidden', value: 'js-merge-request-settings' } - = render 'projects/merge_request_settings', form: f - = f.submit _('Save changes'), class: "btn gl-button btn-confirm rspec-save-merge-request-changes", data: { qa_selector: 'save_merge_request_changes_button' } - -= render_if_exists 'projects/merge_request_approvals_settings', expanded: expanded - +- if show_merge_request_settings_callout? + %section.settings.expanded + = render Pajamas::AlertComponent.new(variant: :info, + title: _('Merge requests and approvals settings have moved.'), + alert_options: { class: 'js-merge-request-settings-callout gl-my-5', data: { feature_id: Users::CalloutsHelper::MERGE_REQUEST_SETTINGS_MOVED_CALLOUT, dismiss_endpoint: callouts_path, defer_links: 'true' } }) do |c| + = c.body do + = _('On the left sidebar, select %{merge_requests_link} to view them.').html_safe % { merge_requests_link: link_to('Settings > Merge requests', project_settings_merge_requests_path(@project)).html_safe } %section.settings.no-animate{ class: ('expanded' if expanded), data: { qa_selector: 'badges_settings_content' } } .settings-header diff --git a/app/views/projects/settings/merge_requests/show.html.haml b/app/views/projects/settings/merge_requests/show.html.haml new file mode 100644 index 00000000000..886e276dea5 --- /dev/null +++ b/app/views/projects/settings/merge_requests/show.html.haml @@ -0,0 +1,18 @@ +- breadcrumb_title _('Merge requests') +- page_title _('Merge requests') +- @content_class = 'limit-container-width' unless fluid_layout + +%section.rspec-merge-request-settings.settings.merge-requests-feature.no-animate#js-merge-request-settings.expanded{ class: [('hidden' if @project.project_feature.send(:merge_requests_access_level) == 0)], data: { qa_selector: 'merge_request_settings_content' } } + .settings-header + %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Merge requests') + = render_if_exists 'projects/merge_request_settings_description_text' + + .settings-content + = render_if_exists 'shared/promotions/promote_mr_features' + + = gitlab_ui_form_for @project, url: project_settings_merge_requests_path(@project), html: { multipart: true, class: "merge-request-settings-form js-mr-settings-form" }, authenticity_token: true do |f| + %input{ name: 'update_section', type: 'hidden', value: 'js-merge-request-settings' } + = render 'projects/merge_request_settings', form: f + = f.submit _('Save changes'), class: "btn gl-button btn-confirm rspec-save-merge-request-changes", data: { qa_selector: 'save_merge_request_changes_button' } + += render_if_exists 'projects/settings/merge_requests/merge_request_approvals_settings', expanded: true diff --git a/config/routes/project.rb b/config/routes/project.rb index e1ee0c8f28b..79ca13e3d8c 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -159,6 +159,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resource :packages_and_registries, only: [:show] do get '/cleanup_image_tags', to: 'packages_and_registries#cleanup_tags' end + resource :merge_requests, only: [:show, :update] end resources :usage_quotas, only: [:index] diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index f1ed7c5fc04..87900d1451b 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -10267,11 +10267,8 @@ CI/CD variables for a GitLab instance. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="cijobartifactdownloadpath"></a>`downloadPath` | [`String`](#string) | URL for downloading the artifact's file. | -| <a id="cijobartifactexpireat"></a>`expireAt` | [`Time`](#time) | Expiry date of the artifact. | | <a id="cijobartifactfiletype"></a>`fileType` | [`JobArtifactFileType`](#jobartifactfiletype) | File type of the artifact. | -| <a id="cijobartifactid"></a>`id` | [`CiJobArtifactID!`](#cijobartifactid) | ID of the artifact. | | <a id="cijobartifactname"></a>`name` | [`String`](#string) | File name of the artifact. | -| <a id="cijobartifactsize"></a>`size` | [`Int!`](#int) | Size of the artifact in bytes. | ### `CiJobTokenScopeType` @@ -20962,6 +20959,7 @@ Name of the feature that the callout is for. | <a id="usercalloutfeaturenameenumgeo_migrate_hashed_storage"></a>`GEO_MIGRATE_HASHED_STORAGE` | Callout feature name for geo_migrate_hashed_storage. | | <a id="usercalloutfeaturenameenumgke_cluster_integration"></a>`GKE_CLUSTER_INTEGRATION` | Callout feature name for gke_cluster_integration. | | <a id="usercalloutfeaturenameenumgold_trial_billings"></a>`GOLD_TRIAL_BILLINGS` | Callout feature name for gold_trial_billings. | +| <a id="usercalloutfeaturenameenummerge_request_settings_moved_callout"></a>`MERGE_REQUEST_SETTINGS_MOVED_CALLOUT` | Callout feature name for merge_request_settings_moved_callout. | | <a id="usercalloutfeaturenameenummr_experience_survey"></a>`MR_EXPERIENCE_SURVEY` | Callout feature name for mr_experience_survey. | | <a id="usercalloutfeaturenameenumnamespace_storage_limit_banner_alert_threshold"></a>`NAMESPACE_STORAGE_LIMIT_BANNER_ALERT_THRESHOLD` | Callout feature name for namespace_storage_limit_banner_alert_threshold. | | <a id="usercalloutfeaturenameenumnamespace_storage_limit_banner_error_threshold"></a>`NAMESPACE_STORAGE_LIMIT_BANNER_ERROR_THRESHOLD` | Callout feature name for namespace_storage_limit_banner_error_threshold. | @@ -21288,12 +21286,6 @@ A `CiBuildID` is a global ID. It is encoded as a string. An example `CiBuildID` is: `"gid://gitlab/Ci::Build/1"`. -### `CiJobArtifactID` - -A `CiJobArtifactID` is a global ID. It is encoded as a string. - -An example `CiJobArtifactID` is: `"gid://gitlab/Ci::JobArtifact/1"`. - ### `CiPipelineID` A `CiPipelineID` is a global ID. It is encoded as a string. diff --git a/doc/user/discussions/img/create-new-issue_v15.png b/doc/user/discussions/img/create-new-issue_v15.png Binary files differdeleted file mode 100644 index 779196b6ba4..00000000000 --- a/doc/user/discussions/img/create-new-issue_v15.png +++ /dev/null diff --git a/doc/user/discussions/img/create_new_issue_v15_4.png b/doc/user/discussions/img/create_new_issue_v15_4.png Binary files differnew file mode 100644 index 00000000000..3720b601cc5 --- /dev/null +++ b/doc/user/discussions/img/create_new_issue_v15_4.png diff --git a/doc/user/discussions/img/unresolved_threads_v15.png b/doc/user/discussions/img/unresolved_threads_v15.png Binary files differdeleted file mode 100644 index 113af20effc..00000000000 --- a/doc/user/discussions/img/unresolved_threads_v15.png +++ /dev/null diff --git a/doc/user/discussions/img/unresolved_threads_v15_4.png b/doc/user/discussions/img/unresolved_threads_v15_4.png Binary files differnew file mode 100644 index 00000000000..1d1669de0f1 --- /dev/null +++ b/doc/user/discussions/img/unresolved_threads_v15_4.png diff --git a/doc/user/discussions/index.md b/doc/user/discussions/index.md index 3fb0be6480c..7540ae8450f 100644 --- a/doc/user/discussions/index.md +++ b/doc/user/discussions/index.md @@ -309,15 +309,15 @@ To resolve a thread: At the top of the page, the number of unresolved threads is updated: - + ### Move all unresolved threads in a merge request to an issue If you have multiple unresolved threads in a merge request, you can create an issue to resolve them separately. In the merge request, at the top of the page, -select **Create issue to resolve all threads** (**{issue-new}**): +click the ellipsis icon button (**{ellipsis_v}**) in the threads control and then select **Create issue to resolve all threads**: - + All threads are marked as resolved, and a link is added from the merge request to the newly created issue. diff --git a/lib/sidebars/projects/menus/merge_requests_menu.rb b/lib/sidebars/projects/menus/merge_requests_menu.rb index fe501667d37..3e543872d36 100644 --- a/lib/sidebars/projects/menus/merge_requests_menu.rb +++ b/lib/sidebars/projects/menus/merge_requests_menu.rb @@ -59,9 +59,9 @@ module Sidebars override :active_routes def active_routes if context.project.issues_enabled? - { controller: :merge_requests } + { controller: 'projects/merge_requests' } else - { controller: [:merge_requests, :milestones] } + { controller: ['projects/merge_requests', :milestones] } end end end diff --git a/lib/sidebars/projects/menus/settings_menu.rb b/lib/sidebars/projects/menus/settings_menu.rb index 23be751ff10..f422d56ce4f 100644 --- a/lib/sidebars/projects/menus/settings_menu.rb +++ b/lib/sidebars/projects/menus/settings_menu.rb @@ -13,6 +13,7 @@ module Sidebars add_item(webhooks_menu_item) add_item(access_tokens_menu_item) add_item(repository_menu_item) + add_item(merge_requests_menu_item) add_item(ci_cd_menu_item) add_item(packages_and_registries_menu_item) add_item(pages_menu_item) @@ -150,6 +151,17 @@ module Sidebars item_id: :usage_quotas ) end + + def merge_requests_menu_item + return unless context.project.merge_requests_enabled? + + ::Sidebars::MenuItem.new( + title: _('Merge requests'), + link: project_settings_merge_requests_path(context.project), + active_routes: { path: 'projects/settings/merge_requests#show' }, + item_id: :merge_requests + ) + end end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 856076adfb3..7d88c32e472 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -13345,6 +13345,9 @@ msgstr "" msgid "DesignManagement|Discard comment" msgstr "" +msgid "DesignManagement|Discussion" +msgstr "" + msgid "DesignManagement|Download design" msgstr "" @@ -24759,6 +24762,9 @@ msgstr "" msgid "Merge requests" msgstr "" +msgid "Merge requests and approvals settings have moved." +msgstr "" + msgid "Merge requests are a place to propose changes you've made to a project and discuss those changes with others" msgstr "" @@ -27029,6 +27035,9 @@ msgid_plural "On %{end_date}, your trial will end and %{namespace_name} will be msgstr[0] "" msgstr[1] "" +msgid "On the left sidebar, select %{merge_requests_link} to view them." +msgstr "" + msgid "On track" msgstr "" diff --git a/qa/qa/flow/merge_request.rb b/qa/qa/flow/merge_request.rb index cd8bac69fed..24abfa9e356 100644 --- a/qa/qa/flow/merge_request.rb +++ b/qa/qa/flow/merge_request.rb @@ -6,8 +6,7 @@ module QA extend self def enable_merge_trains - Page::Project::Menu.perform(&:go_to_general_settings) - Page::Project::Settings::Main.perform(&:expand_merge_requests_settings) + Page::Project::Menu.perform(&:go_to_merge_request_settings) Page::Project::Settings::MergeRequest.perform(&:enable_merge_train) end diff --git a/qa/qa/page/project/settings/main.rb b/qa/qa/page/project/settings/main.rb index 52ed630ac66..ca5d13abdae 100644 --- a/qa/qa/page/project/settings/main.rb +++ b/qa/qa/page/project/settings/main.rb @@ -13,11 +13,14 @@ module QA view 'app/views/projects/edit.html.haml' do element :advanced_settings_content - element :merge_request_settings_content element :visibility_features_permissions_content element :badges_settings_content end + view 'app/views/projects/settings/merge_requests/show.html.haml' do + element :merge_request_settings_content + end + view 'app/views/projects/settings/_general.html.haml' do element :project_name_field element :save_naming_topics_avatar_button @@ -42,12 +45,6 @@ module QA end end - def expand_merge_requests_settings(&block) - expand_content(:merge_request_settings_content) do - MergeRequest.perform(&block) - end - end - def expand_visibility_project_features_permissions(&block) expand_content(:visibility_features_permissions_content) do VisibilityFeaturesPermissions.perform(&block) diff --git a/qa/qa/page/project/settings/merge_request.rb b/qa/qa/page/project/settings/merge_request.rb index dd9c94ebbb7..d862979aeec 100644 --- a/qa/qa/page/project/settings/merge_request.rb +++ b/qa/qa/page/project/settings/merge_request.rb @@ -7,7 +7,7 @@ module QA class MergeRequest < QA::Page::Base include QA::Page::Settings::Common - view 'app/views/projects/edit.html.haml' do + view 'app/views/projects/settings/merge_requests/show.html.haml' do element :save_merge_request_changes_button end diff --git a/qa/qa/page/project/sub_menus/settings.rb b/qa/qa/page/project/sub_menus/settings.rb index 53a5eaf60c5..2ed4c28afb7 100644 --- a/qa/qa/page/project/sub_menus/settings.rb +++ b/qa/qa/page/project/sub_menus/settings.rb @@ -77,6 +77,14 @@ module QA end end + def go_to_merge_request_settings + hover_settings do + within_submenu do + click_element(:sidebar_menu_item_link, menu_item: 'Merge requests') + end + end + end + private def hover_settings diff --git a/qa/qa/specs/features/browser_ui/3_create/merge_request/rebase_merge_request_spec.rb b/qa/qa/specs/features/browser_ui/3_create/merge_request/rebase_merge_request_spec.rb index 2280cc971a7..c7296b6eea2 100644 --- a/qa/qa/specs/features/browser_ui/3_create/merge_request/rebase_merge_request_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/merge_request/rebase_merge_request_spec.rb @@ -12,11 +12,9 @@ module QA it 'user rebases source branch of merge request', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347735' do merge_request.project.visit! - Page::Project::Menu.perform(&:go_to_general_settings) - Page::Project::Settings::Main.perform do |main| - main.expand_merge_requests_settings do |settings| - settings.enable_ff_only - end + Page::Project::Menu.perform(&:go_to_merge_request_settings) + Page::Project::Settings::MergeRequest.perform do |settings| + settings.enable_ff_only end Resource::Repository::ProjectPush.fabricate! do |push| diff --git a/qa/qa/specs/features/shared_examples/merge_with_code_owner_shared_examples.rb b/qa/qa/specs/features/shared_examples/merge_with_code_owner_shared_examples.rb index 4bbad9bf3e5..01b229192cc 100644 --- a/qa/qa/specs/features/shared_examples/merge_with_code_owner_shared_examples.rb +++ b/qa/qa/specs/features/shared_examples/merge_with_code_owner_shared_examples.rb @@ -8,11 +8,9 @@ module QA # Require one approval from any eligible user on any branch # This will confirm that this type of unrestricted approval is # also satisfied when a code owner grants approval - Page::Project::Menu.perform(&:go_to_general_settings) - Page::Project::Settings::Main.perform do |main| - main.expand_merge_request_approvals_settings do |settings| - settings.set_default_number_of_approvals_required(1) - end + Page::Project::Menu.perform(&:go_to_merge_request_settings) + Page::Project::Settings::MergeRequest.perform do |settings| + settings.set_default_number_of_approvals_required(1) end Resource::Repository::Commit.fabricate_via_api! do |commit| diff --git a/spec/controllers/projects/settings/merge_requests_controller_spec.rb b/spec/controllers/projects/settings/merge_requests_controller_spec.rb new file mode 100644 index 00000000000..106ec62bea0 --- /dev/null +++ b/spec/controllers/projects/settings/merge_requests_controller_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Settings::MergeRequestsController do + let(:project) { create(:project_empty_repo, :public) } + let(:user) { create(:user) } + + before do + project.add_maintainer(user) + sign_in(user) + end + + describe 'GET show' do + it 'renders show with 200 status code' do + get :show, params: { namespace_id: project.namespace, project_id: project } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:show) + end + end + + describe '#update', :enable_admin_mode do + render_views + + let(:admin) { create(:admin) } + + before do + sign_in(admin) + end + + it 'updates Fast Forward Merge attributes' do + controller.instance_variable_set(:@project, project) + + params = { + merge_method: :ff + } + + put :update, + params: { + namespace_id: project.namespace, + project_id: project.id, + project: params + } + + expect(response).to redirect_to project_settings_merge_requests_path(project) + params.each do |param, value| + expect(project.public_send(param)).to eq(value) + end + end + end +end diff --git a/spec/features/projects/settings/merge_requests_settings_spec.rb b/spec/features/projects/settings/merge_requests_settings_spec.rb new file mode 100644 index 00000000000..ba84d8b6d1a --- /dev/null +++ b/spec/features/projects/settings/merge_requests_settings_spec.rb @@ -0,0 +1,261 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Projects > Settings > Merge requests' do + include ProjectForksHelper + + let(:user) { create(:user) } + let(:project) { create(:project, :public, namespace: user.namespace, path: 'gitlab', name: 'sample') } + + before do + sign_in(user) + + visit(project_settings_merge_requests_path(project)) + end + + it 'shows "Merge commit" strategy' do + page.within '.merge-request-settings-form' do + expect(page).to have_content 'Merge commit' + end + end + + it 'shows "Merge commit with semi-linear history " strategy' do + page.within '.merge-request-settings-form' do + expect(page).to have_content 'Merge commit with semi-linear history' + end + end + + it 'shows "Fast-forward merge" strategy' do + page.within '.merge-request-settings-form' do + expect(page).to have_content 'Fast-forward merge' + end + end + + it 'shows Squash commit options', :aggregate_failures do + page.within '.merge-request-settings-form' do + expect(page).to have_content 'Do not allow' + expect(page).to have_content 'Squashing is never performed and the checkbox is hidden.' + + expect(page).to have_content 'Allow' + expect(page).to have_content 'Checkbox is visible and unselected by default.' + + expect(page).to have_content 'Encourage' + expect(page).to have_content 'Checkbox is visible and selected by default.' + + expect(page).to have_content 'Require' + end + end + + context 'when Merge Request and Pipelines are initially enabled', :js do + context 'when Pipelines are initially enabled' do + it 'shows the Merge Requests settings' do + expect(page).to have_content 'Pipelines must succeed' + expect(page).to have_content 'All threads must be resolved' + + visit edit_project_path(project) + + within('.sharing-permissions-form') do + within('[data-for="project[project_feature_attributes][merge_requests_access_level]"]') do + find('.gl-toggle').click + end + end + + find('[data-testid="project-features-save-button"]').send_keys(:return) + + visit project_settings_merge_requests_path(project) + + expect(page).to have_content('Not Found') + end + end + + context 'when Pipelines are initially disabled', :js do + before do + project.project_feature.update_attribute('builds_access_level', ProjectFeature::DISABLED) + + visit project_settings_merge_requests_path(project) + end + + it 'shows the Merge Requests settings that do not depend on Builds feature' do + expect(page).to have_content 'Pipelines must succeed' + expect(page).to have_content 'All threads must be resolved' + + visit edit_project_path(project) + + within('.sharing-permissions-form') do + within('.project-feature-controls[data-for="project[project_feature_attributes][builds_access_level]"]') do + find('.gl-toggle').click + end + end + + find('[data-testid="project-features-save-button"]').send_keys(:return) + + visit project_settings_merge_requests_path(project) + + expect(page).to have_content 'Pipelines must succeed' + expect(page).to have_content 'All threads must be resolved' + end + end + end + + context 'when Merge Request are initially disabled', :js do + before do + project.project_feature.update_attribute('merge_requests_access_level', ProjectFeature::DISABLED) + + visit(project_settings_merge_requests_path(project)) + end + + it 'does not show the Merge Requests settings' do + expect(page).to have_content('Not Found') + + visit edit_project_path(project) + + within('.sharing-permissions-form') do + within('[data-for="project[project_feature_attributes][merge_requests_access_level]"]') do + find('.gl-toggle').click + end + end + + find('[data-testid="project-features-save-button"]').send_keys(:return) + + visit project_settings_merge_requests_path(project) + + expect(page).to have_content 'Pipelines must succeed' + expect(page).to have_content 'All threads must be resolved' + end + end + + describe 'Checkbox to enable merge request link', :js do + it 'is initially checked' do + checkbox = find_field('project_printing_merge_request_link_enabled') + expect(checkbox).to be_checked + end + + it 'when unchecked sets :printing_merge_request_link_enabled to false' do + uncheck('project_printing_merge_request_link_enabled') + within('.merge-request-settings-form') do + find('.rspec-save-merge-request-changes') + click_on('Save changes') + end + + find('.flash-notice') + checkbox = find_field('project_printing_merge_request_link_enabled') + + expect(checkbox).not_to be_checked + + project.reload + expect(project.printing_merge_request_link_enabled).to be(false) + end + end + + describe 'Checkbox to remove source branch after merge', :js do + it 'is initially checked' do + checkbox = find_field('project_remove_source_branch_after_merge') + expect(checkbox).to be_checked + end + + it 'when unchecked sets :remove_source_branch_after_merge to false' do + uncheck('project_remove_source_branch_after_merge') + within('.merge-request-settings-form') do + find('.rspec-save-merge-request-changes') + click_on('Save changes') + end + + find('.flash-notice') + checkbox = find_field('project_remove_source_branch_after_merge') + + expect(checkbox).not_to be_checked + + project.reload + expect(project.remove_source_branch_after_merge).to be(false) + end + end + + describe 'Squash commits when merging', :js do + it 'initially has :squash_option set to :default_off' do + radio = find_field('project_project_setting_attributes_squash_option_default_off') + expect(radio).to be_checked + end + + it 'allows :squash_option to be set to :default_on' do + choose('project_project_setting_attributes_squash_option_default_on') + + within('.merge-request-settings-form') do + find('.rspec-save-merge-request-changes') + click_on('Save changes') + end + + wait_for_requests + + radio = find_field('project_project_setting_attributes_squash_option_default_on') + + expect(radio).to be_checked + expect(project.reload.project_setting.squash_option).to eq('default_on') + end + + it 'allows :squash_option to be set to :always' do + choose('project_project_setting_attributes_squash_option_always') + + within('.merge-request-settings-form') do + find('.rspec-save-merge-request-changes') + click_on('Save changes') + end + + wait_for_requests + + radio = find_field('project_project_setting_attributes_squash_option_always') + + expect(radio).to be_checked + expect(project.reload.project_setting.squash_option).to eq('always') + end + + it 'allows :squash_option to be set to :never' do + choose('project_project_setting_attributes_squash_option_never') + + within('.merge-request-settings-form') do + find('.rspec-save-merge-request-changes') + click_on('Save changes') + end + + wait_for_requests + + radio = find_field('project_project_setting_attributes_squash_option_never') + + expect(radio).to be_checked + expect(project.reload.project_setting.squash_option).to eq('never') + end + end + + describe 'target project settings' do + context 'when project is a fork' do + let_it_be(:upstream) { create(:project, :public) } + + let(:project) { fork_project(upstream, user) } + + it 'allows to change merge request target project behavior' do + expect(page).to have_content 'The default target project for merge requests' + + radio = find_field('project_project_setting_attributes_mr_default_target_self_false') + expect(radio).to be_checked + + choose('project_project_setting_attributes_mr_default_target_self_true') + + within('.merge-request-settings-form') do + find('.rspec-save-merge-request-changes') + click_on('Save changes') + end + + wait_for_requests + + radio = find_field('project_project_setting_attributes_mr_default_target_self_true') + + expect(radio).to be_checked + expect(project.reload.project_setting.mr_default_target_self).to be_truthy + end + end + + it 'does not show target project section' do + expect(page).not_to have_content 'The default target project for merge requests' + end + end +end diff --git a/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb b/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb index 6aa59f72d2a..c76b4d0af88 100644 --- a/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb +++ b/spec/features/projects/settings/user_manages_merge_requests_settings_spec.rb @@ -9,29 +9,29 @@ RSpec.describe 'Projects > Settings > User manages merge request settings' do before do sign_in(user) - visit edit_project_path(project) + visit project_settings_merge_requests_path(project) end it 'shows "Merge commit" strategy' do - page.within '#js-merge-request-settings' do + page.within '.merge-request-settings-form' do expect(page).to have_content 'Merge commit' end end it 'shows "Merge commit with semi-linear history " strategy' do - page.within '#js-merge-request-settings' do + page.within '.merge-request-settings-form' do expect(page).to have_content 'Merge commit with semi-linear history' end end it 'shows "Fast-forward merge" strategy' do - page.within '#js-merge-request-settings' do + page.within '.merge-request-settings-form' do expect(page).to have_content 'Fast-forward merge' end end it 'shows Squash commit options', :aggregate_failures do - page.within '#js-merge-request-settings' do + page.within '.merge-request-settings-form' do expect(page).to have_content 'Do not allow' expect(page).to have_content 'Squashing is never performed and the checkbox is hidden.' @@ -52,30 +52,33 @@ RSpec.describe 'Projects > Settings > User manages merge request settings' do expect(page).to have_content 'Pipelines must succeed' expect(page).to have_content 'All threads must be resolved' - within('.sharing-permissions-form') do - find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .gl-toggle').click - find('[data-testid="project-features-save-button"]').send_keys(:return) - end + visit edit_project_path(project) + + find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .gl-toggle').click + find('[data-testid="project-features-save-button"]').send_keys(:return) + + visit project_settings_merge_requests_path(project) - expect(page).not_to have_content 'Pipelines must succeed' - expect(page).not_to have_content 'All threads must be resolved' + expect(page).to have_content "Page Not Found" end end context 'when Pipelines are initially disabled', :js do before do project.project_feature.update_attribute('builds_access_level', ProjectFeature::DISABLED) - visit edit_project_path(project) + visit project_settings_merge_requests_path(project) end it 'shows the Merge Requests settings that do not depend on Builds feature' do expect(page).to have_content 'Pipelines must succeed' expect(page).to have_content 'All threads must be resolved' - within('.sharing-permissions-form') do - find('.project-feature-controls[data-for="project[project_feature_attributes][builds_access_level]"] .gl-toggle').click - find('[data-testid="project-features-save-button"]').send_keys(:return) - end + visit edit_project_path(project) + + find('.project-feature-controls[data-for="project[project_feature_attributes][builds_access_level]"] .gl-toggle').click + find('[data-testid="project-features-save-button"]').send_keys(:return) + + visit project_settings_merge_requests_path(project) expect(page).to have_content 'Pipelines must succeed' expect(page).to have_content 'All threads must be resolved' @@ -86,18 +89,22 @@ RSpec.describe 'Projects > Settings > User manages merge request settings' do context 'when Merge Request are initially disabled', :js do before do project.project_feature.update_attribute('merge_requests_access_level', ProjectFeature::DISABLED) - visit edit_project_path(project) + visit project_settings_merge_requests_path(project) end it 'does not show the Merge Requests settings' do expect(page).not_to have_content 'Pipelines must succeed' expect(page).not_to have_content 'All threads must be resolved' + visit edit_project_path(project) + within('.sharing-permissions-form') do find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .gl-toggle').click find('[data-testid="project-features-save-button"]').send_keys(:return) end + visit project_settings_merge_requests_path(project) + expect(page).to have_content 'Pipelines must succeed' expect(page).to have_content 'All threads must be resolved' end diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb index fc78b5b5769..5cb12544066 100644 --- a/spec/features/projects/settings/visibility_settings_spec.rb +++ b/spec/features/projects/settings/visibility_settings_spec.rb @@ -28,26 +28,12 @@ RSpec.describe 'Projects > Settings > Visibility settings', :js do expect(visibility_select_container).to have_content 'Only accessible by project members. Membership must be explicitly granted to each user.' end - context 'merge requests select' do - it 'hides merge requests section' do - find('.project-feature-controls[data-for="project[project_feature_attributes][merge_requests_access_level]"] .gl-toggle').click - - expect(page).to have_selector('.merge-requests-feature', visible: false) - end - - context 'given project with merge_requests_disabled access level' do - let(:project) { create(:project, :merge_requests_disabled, namespace: user.namespace) } - - it 'hides merge requests section' do - expect(page).to have_selector('.merge-requests-feature', visible: false) - end - end - end - context 'builds select' do it 'hides builds select section' do find('.project-feature-controls[data-for="project[project_feature_attributes][builds_access_level]"] .gl-toggle').click + visit project_settings_merge_requests_path(project) + expect(page).to have_selector('.builds-feature', visible: false) end @@ -55,6 +41,8 @@ RSpec.describe 'Projects > Settings > Visibility settings', :js do let(:project) { create(:project, :builds_disabled, namespace: user.namespace) } it 'hides builds select section' do + visit project_settings_merge_requests_path(project) + expect(page).to have_selector('.builds-feature', visible: false) end end diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 9dcf3c5ab99..cbd9340b737 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -418,8 +418,7 @@ RSpec.describe 'Project' do visit path end - it_behaves_like 'dirty submit form', [{ form: '.js-general-settings-form', input: 'input[name="project[name]"]' }, - { form: '.rspec-merge-request-settings', input: '#project_printing_merge_request_link_enabled' }] + it_behaves_like 'dirty submit form', [{ form: '.js-general-settings-form', input: 'input[name="project[name]"]' }] end describe 'view for a user without an access to a repo' do diff --git a/spec/frontend/design_management/components/design_notes/design_note_spec.js b/spec/frontend/design_management/components/design_notes/design_note_spec.js index 28833b4af5c..df511586c10 100644 --- a/spec/frontend/design_management/components/design_notes/design_note_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_note_spec.js @@ -43,6 +43,7 @@ describe('Design note component', () => { wrapper = shallowMountExtended(DesignNote, { propsData: { note: {}, + noteableId: 'gid://gitlab/DesignManagement::Design/6', ...props, }, data() { diff --git a/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js b/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js index f7ce742b933..e36f5c79e3e 100644 --- a/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_reply_form_spec.js @@ -1,5 +1,6 @@ import { mount } from '@vue/test-utils'; import { nextTick } from 'vue'; +import Autosave from '~/autosave'; import DesignReplyForm from '~/design_management/components/design_notes/design_reply_form.vue'; const showModal = jest.fn(); @@ -13,6 +14,7 @@ const GlModal = { describe('Design reply form component', () => { let wrapper; + let originalGon; const findTextarea = () => wrapper.find('textarea'); const findSubmitButton = () => wrapper.findComponent({ ref: 'submitButton' }); @@ -24,6 +26,7 @@ describe('Design reply form component', () => { propsData: { value: '', isSaving: false, + noteableId: 'gid://gitlab/DesignManagement::Design/6', ...props, }, stubs: { GlModal }, @@ -31,8 +34,14 @@ describe('Design reply form component', () => { }); } + beforeEach(() => { + originalGon = window.gon; + window.gon.current_user_id = 1; + }); + afterEach(() => { wrapper.destroy(); + window.gon = originalGon; }); it('textarea has focus after component mount', () => { @@ -66,6 +75,25 @@ describe('Design reply form component', () => { expect(findSubmitButton().html()).toMatchSnapshot(); }); + it.each` + discussionId | shortDiscussionId + ${undefined} | ${'new'} + ${'gid://gitlab/DiffDiscussion/123'} | ${123} + `( + 'initializes autosave support on discussion with proper key', + async ({ discussionId, shortDiscussionId }) => { + createComponent({ discussionId }); + await nextTick(); + + // We discourage testing `wrapper.vm` properties but + // since `autosave` library instantiates on component + // there's no other way to test whether instantiation + // happened correctly or not. + expect(wrapper.vm.autosaveDiscussion).toBeInstanceOf(Autosave); + expect(wrapper.vm.autosaveDiscussion.key).toBe(`autosave/Discussion/6/${shortDiscussionId}`); + }, + ); + describe('when form has no text', () => { beforeEach(() => { createComponent({ @@ -120,28 +148,37 @@ describe('Design reply form component', () => { }); it('emits submitForm event on Comment button click', async () => { + const autosaveResetSpy = jest.spyOn(wrapper.vm.autosaveDiscussion, 'reset'); + findSubmitButton().vm.$emit('click'); await nextTick(); expect(wrapper.emitted('submit-form')).toBeTruthy(); + expect(autosaveResetSpy).toHaveBeenCalled(); }); it('emits submitForm event on textarea ctrl+enter keydown', async () => { + const autosaveResetSpy = jest.spyOn(wrapper.vm.autosaveDiscussion, 'reset'); + findTextarea().trigger('keydown.enter', { ctrlKey: true, }); await nextTick(); expect(wrapper.emitted('submit-form')).toBeTruthy(); + expect(autosaveResetSpy).toHaveBeenCalled(); }); it('emits submitForm event on textarea meta+enter keydown', async () => { + const autosaveResetSpy = jest.spyOn(wrapper.vm.autosaveDiscussion, 'reset'); + findTextarea().trigger('keydown.enter', { metaKey: true, }); await nextTick(); expect(wrapper.emitted('submit-form')).toBeTruthy(); + expect(autosaveResetSpy).toHaveBeenCalled(); }); it('emits input event on changing textarea content', async () => { @@ -180,10 +217,13 @@ describe('Design reply form component', () => { }); it('emits cancelForm event on modal Ok button click', () => { + const autosaveResetSpy = jest.spyOn(wrapper.vm.autosaveDiscussion, 'reset'); + findTextarea().trigger('keyup.esc'); findModal().vm.$emit('ok'); expect(wrapper.emitted('cancel-form')).toBeTruthy(); + expect(autosaveResetSpy).toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/pipelines/mock_data.js b/spec/frontend/pipelines/mock_data.js index 24514d99078..57d1511d859 100644 --- a/spec/frontend/pipelines/mock_data.js +++ b/spec/frontend/pipelines/mock_data.js @@ -528,7 +528,6 @@ export const mockPipelineJobsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/101', downloadPath: '/root/ci-project/-/jobs/620/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', @@ -581,7 +580,6 @@ export const mockPipelineJobsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/102', downloadPath: '/root/ci-project/-/jobs/619/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', diff --git a/spec/frontend/vue_shared/security_reports/mock_data.js b/spec/frontend/vue_shared/security_reports/mock_data.js index a0e31243365..a9ad675e538 100644 --- a/spec/frontend/vue_shared/security_reports/mock_data.js +++ b/spec/frontend/vue_shared/security_reports/mock_data.js @@ -356,14 +356,12 @@ export const securityReportMergeRequestDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/101', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1399/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/102', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1399/artifacts/download?file_type=secret_detection', fileType: 'SECRET_DETECTION', @@ -380,14 +378,12 @@ export const securityReportMergeRequestDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/103', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1400/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/104', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1400/artifacts/download?file_type=sast', fileType: 'SAST', @@ -404,14 +400,12 @@ export const securityReportMergeRequestDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/105', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1401/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/106', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1401/artifacts/download?file_type=sast', fileType: 'SAST', @@ -428,21 +422,18 @@ export const securityReportMergeRequestDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/107', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1402/artifacts/download?file_type=archive', fileType: 'ARCHIVE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/108', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1402/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/109', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1402/artifacts/download?file_type=metadata', fileType: 'METADATA', @@ -477,14 +468,12 @@ export const securityReportPipelineDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/110', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1399/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/111', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1399/artifacts/download?file_type=secret_detection', fileType: 'SECRET_DETECTION', @@ -501,14 +490,12 @@ export const securityReportPipelineDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/112', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1400/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/113', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1400/artifacts/download?file_type=sast', fileType: 'SAST', @@ -525,14 +512,12 @@ export const securityReportPipelineDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/114', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1401/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/115', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1401/artifacts/download?file_type=sast', fileType: 'SAST', @@ -549,21 +534,18 @@ export const securityReportPipelineDownloadPathsQueryResponse = { artifacts: { nodes: [ { - id: 'gid://gitlab/Ci::JobArtifact/116', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1402/artifacts/download?file_type=archive', fileType: 'ARCHIVE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/117', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1402/artifacts/download?file_type=trace', fileType: 'TRACE', __typename: 'CiJobArtifact', }, { - id: 'gid://gitlab/Ci::JobArtifact/118', downloadPath: '/gitlab-org/secrets-detection-test/-/jobs/1402/artifacts/download?file_type=metadata', fileType: 'METADATA', diff --git a/spec/graphql/types/ci/job_artifact_type_spec.rb b/spec/graphql/types/ci/job_artifact_type_spec.rb index 3e054faf0c9..58b5f9cfcb7 100644 --- a/spec/graphql/types/ci/job_artifact_type_spec.rb +++ b/spec/graphql/types/ci/job_artifact_type_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['CiJobArtifact'] do it 'has the correct fields' do - expected_fields = [:id, :download_path, :file_type, :name, :size, :expire_at] + expected_fields = [:download_path, :file_type, :name] expect(described_class).to have_graphql_fields(*expected_fields) end diff --git a/spec/lib/sidebars/projects/menus/settings_menu_spec.rb b/spec/lib/sidebars/projects/menus/settings_menu_spec.rb index 904b9f041b1..0733e0c6521 100644 --- a/spec/lib/sidebars/projects/menus/settings_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/settings_menu_spec.rb @@ -133,6 +133,12 @@ RSpec.describe Sidebars::Projects::Menus::SettingsMenu do end end + describe 'Merge requests' do + let(:item_id) { :merge_requests } + + it_behaves_like 'access rights checks' + end + describe 'Packages and registries' do let(:item_id) { :packages_and_registries } let(:packages_enabled) { false } diff --git a/spec/services/users/migrate_records_to_ghost_user_service_spec.rb b/spec/services/users/migrate_records_to_ghost_user_service_spec.rb new file mode 100644 index 00000000000..04310b977c0 --- /dev/null +++ b/spec/services/users/migrate_records_to_ghost_user_service_spec.rb @@ -0,0 +1,258 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::MigrateRecordsToGhostUserService do + let!(:user) { create(:user) } + let(:service) { described_class.new(user, admin) } + + let_it_be(:admin) { create(:admin) } + let_it_be(:project) { create(:project, :repository) } + + context "when migrating a user's associated records to the ghost user" do + context 'for issues' do + context 'when deleted user is present as both author and edited_user' do + include_examples 'migrating records to the ghost user', Issue, [:author, :last_edited_by] do + let(:created_record) do + create(:issue, project: project, author: user, last_edited_by: user) + end + end + end + + context 'when deleted user is present only as edited_user' do + include_examples 'migrating records to the ghost user', Issue, [:last_edited_by] do + let(:created_record) { create(:issue, project: project, author: create(:user), last_edited_by: user) } + end + end + + context "when deleted user is the assignee" do + let!(:issue) { create(:issue, project: project, assignees: [user]) } + + it 'migrates the issue so that it is "Unassigned"' do + service.execute + + migrated_issue = Issue.find_by_id(issue.id) + expect(migrated_issue).to be_present + expect(migrated_issue.assignees).to be_empty + end + end + end + + context 'for merge requests' do + context 'when deleted user is present as both author and merge_user' do + include_examples 'migrating records to the ghost user', MergeRequest, [:author, :merge_user] do + let(:created_record) do + create(:merge_request, source_project: project, + author: user, + merge_user: user, + target_branch: "first") + end + end + end + + context 'when deleted user is present only as both merge_user' do + include_examples 'migrating records to the ghost user', MergeRequest, [:merge_user] do + let(:created_record) do + create(:merge_request, source_project: project, + merge_user: user, + target_branch: "first") + end + end + end + + context "when deleted user is the assignee" do + let!(:merge_request) { create(:merge_request, source_project: project, assignees: [user]) } + + it 'migrates the merge request so that it is "Unassigned"' do + service.execute + + migrated_merge_request = MergeRequest.find_by_id(merge_request.id) + expect(migrated_merge_request).to be_present + expect(migrated_merge_request.assignees).to be_empty + end + end + end + + context 'for notes' do + include_examples 'migrating records to the ghost user', Note do + let(:created_record) { create(:note, project: project, author: user) } + end + end + + context 'for abuse reports' do + include_examples 'migrating records to the ghost user', AbuseReport do + let(:created_record) { create(:abuse_report, reporter: user, user: create(:user)) } + end + end + + context 'for award emoji' do + include_examples 'migrating records to the ghost user', AwardEmoji, [:user] do + let(:created_record) { create(:award_emoji, user: user) } + + context "when the awardable already has an award emoji of the same name assigned to the ghost user" do + let(:awardable) { create(:issue) } + let!(:existing_award_emoji) { create(:award_emoji, user: User.ghost, name: "thumbsup", awardable: awardable) } + let!(:award_emoji) { create(:award_emoji, user: user, name: "thumbsup", awardable: awardable) } + + it "migrates the award emoji regardless" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record.user).to eq(User.ghost) + end + + it "does not leave the migrated award emoji in an invalid state" do + service.execute + + migrated_record = AwardEmoji.find_by_id(award_emoji.id) + + expect(migrated_record).to be_valid + end + end + end + end + + context 'for snippets' do + include_examples 'migrating records to the ghost user', Snippet do + let(:created_record) { create(:snippet, project: project, author: user) } + end + end + + context 'for reviews' do + include_examples 'migrating records to the ghost user', Review, [:author] do + let(:created_record) { create(:review, author: user) } + end + end + end + + context 'on post-migrate cleanups' do + it 'destroys the user and personal namespace' do + namespace = user.namespace + + allow(user).to receive(:destroy).and_call_original + + service.execute + + expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound) + expect { Namespace.find(namespace.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + + it 'deletes user associations in batches' do + expect(user).to receive(:destroy_dependent_associations_in_batches) + + service.execute + end + + context 'for batched nullify' do + it 'nullifies related associations in batches' do + expect(user).to receive(:nullify_dependent_associations_in_batches).and_call_original + + service.execute + end + + it 'nullifies last_updated_issues, closed_issues, resource_label_events' do + issue = create(:issue, closed_by: user, updated_by: user) + resource_label_event = create(:resource_label_event, user: user) + + service.execute + + issue.reload + resource_label_event.reload + + expect(issue.closed_by).to be_nil + expect(issue.updated_by).to be_nil + expect(resource_label_event.user).to be_nil + end + end + + context 'for snippets' do + let(:gitlab_shell) { Gitlab::Shell.new } + + it 'does not include snippets when deleting in batches' do + expect(user).to receive(:destroy_dependent_associations_in_batches).with({ exclude: [:snippets] }) + + service.execute + end + + it 'calls the bulk snippet destroy service for the user personal snippets' do + repo1 = create(:personal_snippet, :repository, author: user).snippet_repository + repo2 = create(:project_snippet, :repository, project: project, author: user).snippet_repository + + aggregate_failures do + expect(gitlab_shell.repository_exists?(repo1.shard_name, "#{repo1.disk_path}.git")).to be(true) + expect(gitlab_shell.repository_exists?(repo2.shard_name, "#{repo2.disk_path}.git")).to be(true) + end + + # Call made when destroying user personal projects + expect(Snippets::BulkDestroyService).not_to( + receive(:new).with(admin, project.snippets).and_call_original) + + # Call to remove user personal snippets and for + # project snippets where projects are not user personal + # ones + expect(Snippets::BulkDestroyService).to( + receive(:new).with(admin, user.snippets.only_personal_snippets).and_call_original) + + service.execute + + aggregate_failures do + expect(gitlab_shell.repository_exists?(repo1.shard_name, "#{repo1.disk_path}.git")).to be(false) + expect(gitlab_shell.repository_exists?(repo2.shard_name, "#{repo2.disk_path}.git")).to be(true) + end + end + + it 'calls the bulk snippet destroy service with hard delete option if it is present' do + # this avoids getting into Projects::DestroyService as it would + # call Snippets::BulkDestroyService first! + allow(user).to receive(:personal_projects).and_return([]) + + expect_next_instance_of(Snippets::BulkDestroyService) do |bulk_destroy_service| + expect(bulk_destroy_service).to receive(:execute).with({ skip_authorization: true }).and_call_original + end + + service.execute(hard_delete: true) + end + + it 'does not delete project snippets that the user is the author of' do + repo = create(:project_snippet, :repository, author: user).snippet_repository + + service.execute + + expect(gitlab_shell.repository_exists?(repo.shard_name, "#{repo.disk_path}.git")).to be(true) + expect(User.ghost.snippets).to include(repo.snippet) + end + + context 'when an error is raised deleting snippets' do + it 'does not delete user' do + snippet = create(:personal_snippet, :repository, author: user) + + bulk_service = double + allow(Snippets::BulkDestroyService).to receive(:new).and_call_original + allow(Snippets::BulkDestroyService).to receive(:new).with(admin, user.snippets).and_return(bulk_service) + allow(bulk_service).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + + aggregate_failures do + expect { service.execute }.to( + raise_error(Users::MigrateRecordsToGhostUserService::DestroyError, 'foo' )) + expect(snippet.reload).not_to be_nil + expect( + gitlab_shell.repository_exists?(snippet.repository_storage, + "#{snippet.disk_path}.git") + ).to be(true) + end + end + end + end + + context 'when hard_delete option is given' do + it 'will not ghost certain records' do + issue = create(:issue, author: user) + + service.execute(hard_delete: true) + + expect(Issue).not_to exist(issue.id) + end + end + end +end diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 6543fc327d2..3b89ecf8995 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -109,6 +109,7 @@ RSpec.shared_context 'project navbar structure' do _('Webhooks'), _('Access Tokens'), _('Repository'), + _('Merge requests'), _('CI/CD'), _('Packages and registries'), _('Monitor'), diff --git a/spec/support/shared_examples/users/migrate_records_to_ghost_user_service_shared_examples.rb b/spec/support/shared_examples/users/migrate_records_to_ghost_user_service_shared_examples.rb new file mode 100644 index 00000000000..eb03f0888b9 --- /dev/null +++ b/spec/support/shared_examples/users/migrate_records_to_ghost_user_service_shared_examples.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'migrating records to the ghost user' do |record_class, fields| + record_class_name = record_class.to_s.titleize.downcase + + let(:project) do + case record_class + when MergeRequest + create(:project, :repository) + else + create(:project) + end + end + + before do + project.add_developer(user) + end + + context "for a #{record_class_name} the user has created" do + let!(:record) { created_record } + let(:migrated_fields) { fields || [:author] } + + it "does not delete the #{record_class_name}" do + service.execute + + expect(record_class.find_by_id(record.id)).to be_present + end + + it 'migrates all associated fields to the "Ghost user"' do + service.execute + + migrated_record = record_class.find_by_id(record.id) + + migrated_fields.each do |field| + expect(migrated_record.public_send(field)).to eq(User.ghost) + end + end + end +end diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb index a85ddf7a005..2935e4395ba 100644 --- a/spec/views/projects/edit.html.haml_spec.rb +++ b/spec/views/projects/edit.html.haml_spec.rb @@ -28,62 +28,6 @@ RSpec.describe 'projects/edit' do end end - context 'merge suggestions settings' do - it 'displays a placeholder if none is set' do - render - - expect(rendered).to have_field('project[suggestion_commit_message]', placeholder: "Apply %{suggestions_count} suggestion(s) to %{files_count} file(s)") - end - - it 'displays the user entered value' do - project.update!(suggestion_commit_message: 'refactor: changed %{file_paths}') - - render - - expect(rendered).to have_field('project[suggestion_commit_message]', with: 'refactor: changed %{file_paths}') - end - end - - context 'merge commit template' do - it 'displays default template if none is set' do - render - - expect(rendered).to have_field('project[merge_commit_template_or_default]', with: <<~MSG.rstrip) - Merge branch '%{source_branch}' into '%{target_branch}' - - %{title} - - %{issues} - - See merge request %{reference} - MSG - end - - it 'displays the user entered value' do - project.update!(merge_commit_template: '%{title}') - - render - - expect(rendered).to have_field('project[merge_commit_template_or_default]', with: '%{title}') - end - end - - context 'squash template' do - it 'displays default template if none is set' do - render - - expect(rendered).to have_field('project[squash_commit_template_or_default]', with: '%{title}') - end - - it 'displays the user entered value' do - project.update!(squash_commit_template: '%{first_multiline_commit}') - - render - - expect(rendered).to have_field('project[squash_commit_template_or_default]', with: '%{first_multiline_commit}') - end - end - context 'forking' do before do assign(:project, project) diff --git a/spec/views/projects/settings/merge_requests/show.html.haml_spec.rb b/spec/views/projects/settings/merge_requests/show.html.haml_spec.rb new file mode 100644 index 00000000000..821f430eb10 --- /dev/null +++ b/spec/views/projects/settings/merge_requests/show.html.haml_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'projects/settings/merge_requests/show' do + include Devise::Test::ControllerHelpers + include ProjectForksHelper + + let(:project) { create(:project) } + let(:user) { create(:admin) } + + before do + assign(:project, project) + + allow(controller).to receive(:current_user).and_return(user) + allow(view).to receive_messages(current_user: user, + can?: true, + current_application_settings: Gitlab::CurrentSettings.current_application_settings) + end + + describe 'merge suggestions settings' do + it 'displays a placeholder if none is set' do + render + + placeholder = "Apply %{suggestions_count} suggestion(s) to %{files_count} file(s)" + + expect(rendered).to have_field('project[suggestion_commit_message]', placeholder: placeholder) + end + + it 'displays the user entered value' do + project.update!(suggestion_commit_message: 'refactor: changed %{file_paths}') + + render + + expect(rendered).to have_field('project[suggestion_commit_message]', with: 'refactor: changed %{file_paths}') + end + end + + describe 'merge commit template' do + it 'displays default template if none is set' do + render + + expect(rendered).to have_field('project[merge_commit_template_or_default]', with: <<~MSG.rstrip) + Merge branch '%{source_branch}' into '%{target_branch}' + + %{title} + + %{issues} + + See merge request %{reference} + MSG + end + + it 'displays the user entered value' do + project.update!(merge_commit_template: '%{title}') + + render + + expect(rendered).to have_field('project[merge_commit_template_or_default]', with: '%{title}') + end + end + + describe 'squash template' do + it 'displays default template if none is set' do + render + + expect(rendered).to have_field('project[squash_commit_template_or_default]', with: '%{title}') + end + + it 'displays the user entered value' do + project.update!(squash_commit_template: '%{first_multiline_commit}') + + render + + expect(rendered).to have_field('project[squash_commit_template_or_default]', with: '%{first_multiline_commit}') + end + end +end |
