diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 2926c7e3165..04f7ff4e02e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1235,8 +1235,18 @@ class MergeRequest < ApplicationRecord alias_method :wip_title, :draft_title def skipped_mergeable_checks(options = {}) + merge_when_checks_pass_strat = options[:auto_merge_strategy] == ::AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS + + skip_additional_checks = merge_when_checks_pass_strat && + ::Feature.enabled?(:additional_merge_when_checks_ready, project) + { - skip_ci_check: options.fetch(:auto_merge_requested, false) + skip_ci_check: options.fetch(:auto_merge_requested, false), + skip_approved_check: merge_when_checks_pass_strat, + skip_draft_check: skip_additional_checks, + skip_blocked_check: skip_additional_checks, + skip_discussions_check: skip_additional_checks, + skip_external_status_check: skip_additional_checks } end @@ -1285,31 +1295,28 @@ class MergeRequest < ApplicationRecord # skip_approved_check # skip_blocked_check # skip_external_status_check - def mergeable_state?(**mergeable_state_check_params) - additional_checks = execute_merge_checks( - self.class.mergeable_state_checks, - params: mergeable_state_check_params - ) - additional_checks.success? + def mergeable_state?(**params) + results = check_mergeability_states(checks: self.class.mergeable_state_checks, **params) + + results.success? end - def mergeable_git_state?(skip_rebase_check: false) - checks = execute_merge_checks( - self.class.mergeable_git_state_checks, - params: { - skip_rebase_check: skip_rebase_check - } - ) + # This runs only git related checks + def mergeable_git_state?(**params) + results = check_mergeability_states(checks: self.class.mergeable_git_state_checks, **params) - checks.success? + results.success? + end + + # This runs all the checks + def mergeability_checks_pass?(**params) + results = check_mergeability_states(checks: self.class.all_mergeability_checks, **params) + + results.success? end def all_mergeability_checks_results - execute_merge_checks( - self.class.all_mergeability_checks, - params: {}, - execute_all: true - ).payload[:results] + check_mergeability_states(checks: self.class.all_mergeability_checks, execute_all: true).payload[:results] end def ff_merge_possible? @@ -2241,6 +2248,14 @@ class MergeRequest < ApplicationRecord ) end + def check_mergeability_states(checks:, execute_all: false, **params) + execute_merge_checks( + checks, + params: params, + execute_all: execute_all + ) + end + def merge_base_pipelines return ::Ci::Pipeline.none unless diff_head_pipeline&.target_sha diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index 9d2d89e5665..61d2ab6e70a 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -58,16 +58,29 @@ module AutoMerge def available_for?(merge_request) strong_memoize("available_for_#{merge_request.id}") do - merge_request.can_be_merged_by?(current_user) && - merge_request.open? && - !merge_request.broken? && - overrideable_available_for_checks(merge_request) && - yield + if Feature.enabled?(:refactor_auto_merge, merge_request.project, type: :gitlab_com_derisk) + merge_request.can_be_merged_by?(current_user) && + merge_request.mergeability_checks_pass?(**skippable_available_for_checks(merge_request)) && + yield + else + merge_request.can_be_merged_by?(current_user) && + merge_request.open? && + !merge_request.broken? && + overrideable_available_for_checks(merge_request) && + yield + end end end private + def skippable_available_for_checks(merge_request) + merge_request.skipped_mergeable_checks( + auto_merge_requested: true, + auto_merge_strategy: strategy + ) + end + def overrideable_available_for_checks(merge_request) !merge_request.draft? && merge_request.mergeable_discussions_state? && diff --git a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb index db67a1a9f26..a669eea337a 100644 --- a/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb +++ b/app/services/auto_merge/merge_when_pipeline_succeeds_service.rb @@ -34,7 +34,7 @@ module AutoMerge def available_for?(merge_request) super do - check_availability(merge_request) + merge_request.auto_merge_available_when_pipeline_succeeds? end end @@ -44,10 +44,6 @@ module AutoMerge SystemNoteService.merge_when_pipeline_succeeds(merge_request, project, current_user, merge_request.diff_head_pipeline.sha) if merge_request.saved_change_to_auto_merge_enabled? end - def check_availability(merge_request) - merge_request.auto_merge_available_when_pipeline_succeeds? - end - def notify(merge_request) notification_service.async.merge_when_pipeline_succeeds(merge_request, current_user) if merge_request.saved_change_to_auto_merge_enabled? end diff --git a/app/services/auto_merge_service.rb b/app/services/auto_merge_service.rb index 912248d3a06..71a2a324136 100644 --- a/app/services/auto_merge_service.rb +++ b/app/services/auto_merge_service.rb @@ -4,6 +4,8 @@ class AutoMergeService < BaseService include Gitlab::Utils::StrongMemoize STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS = 'merge_when_pipeline_succeeds' + # Currently only EE but will be moved to CE in (https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146730) + STRATEGY_MERGE_WHEN_CHECKS_PASS = 'merge_when_checks_pass' STRATEGIES = [STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS].freeze class << self diff --git a/app/views/award_emoji/_awards_block.html.haml b/app/views/award_emoji/_awards_block.html.haml index a63ae29b64b..236e12c08d9 100644 --- a/app/views/award_emoji/_awards_block.html.haml +++ b/app/views/award_emoji/_awards_block.html.haml @@ -7,16 +7,16 @@ = yield - else - grouped_emojis = awardable.grouped_awards(with_thumbs: inline) - .awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.empty?), data: { award_url: toggle_award_url(awardable) } } + .awards.js-awards-block.gl-gap-3.gl-py-2{ class: ("hidden" if !inline && grouped_emojis.empty?), data: { award_url: toggle_award_url(awardable) } } - awards_sort(grouped_emojis).each do |emoji, awards| - = render Pajamas::ButtonComponent.new(button_options: { class: (award_state_class(awardable, awards, current_user) + ' award-control js-emoji-btn has-tooltip'), data: { title: award_user_list(awards, current_user) } }) do + = render Pajamas::ButtonComponent.new(button_options: { class: (award_state_class(awardable, awards, current_user) + ' js-emoji-btn has-tooltip'), data: { title: award_user_list(awards, current_user) } }) do = emoji_icon(emoji) %span.award-control-text.js-counter = awards.count - if can?(current_user, :award_emoji, awardable) .award-menu-holder.js-award-holder - = render Pajamas::ButtonComponent.new(button_options: { class: 'add-reaction-button award-control has-tooltip js-add-award btn-icon gl-relative', data: { title: _('Add reaction') }, aria: { label: _('Add reaction') } }) do + = render Pajamas::ButtonComponent.new(button_options: { class: 'add-reaction-button has-tooltip js-add-award btn-icon gl-relative', data: { title: _('Add reaction') }, aria: { label: _('Add reaction') } }) do = sprite_icon('slight-smile', css_class: 'reaction-control-icon-neutral award-control-icon-neutral gl-button-icon gl-icon') = sprite_icon('smiley', css_class: 'reaction-control-icon-positive award-control-icon-positive gl-button-icon gl-icon !gl-left-3') = sprite_icon('smile', css_class: 'reaction-control-icon-super-positive award-control-icon-super-positive gl-button-icon gl-icon !gl-left-3') diff --git a/config/feature_flags/gitlab_com_derisk/refactor_auto_merge.yml b/config/feature_flags/gitlab_com_derisk/refactor_auto_merge.yml new file mode 100644 index 00000000000..901997fd160 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/refactor_auto_merge.yml @@ -0,0 +1,9 @@ +--- +name: refactor_auto_merge +feature_issue_url: https://gitlab.com/groups/gitlab-org/-/epics/10874 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146153 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/443872 +milestone: '16.11' +group: group::code review +type: gitlab_com_derisk +default_enabled: false diff --git a/db/docs/batched_background_migrations/backfill_archived_and_traversal_ids_to_vulnerability_reads.yml b/db/docs/batched_background_migrations/backfill_archived_and_traversal_ids_to_vulnerability_reads.yml index cddb591e186..90df3367ebb 100644 --- a/db/docs/batched_background_migrations/backfill_archived_and_traversal_ids_to_vulnerability_reads.yml +++ b/db/docs/batched_background_migrations/backfill_archived_and_traversal_ids_to_vulnerability_reads.yml @@ -1,6 +1,6 @@ --- migration_job_name: BackfillArchivedAndTraversalIdsToVulnerabilityReads -description: Backfill project.archived and project.namespace.traversal_ids values to the denormalized columns of the same name on vulnerability_reads +description: Backfill project.archived and project.namespace.traversal_ids values to the denormalized columns of the same name on vulnerability_reads. No-oped and requeued in job RequeueBackfillArchivedAndTraversalIdsToVulnerabilityReads. feature_category: vulnerability_management introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144765 milestone: '16.10' diff --git a/db/docs/batched_background_migrations/requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads.yml b/db/docs/batched_background_migrations/requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads.yml new file mode 100644 index 00000000000..49617b1cffd --- /dev/null +++ b/db/docs/batched_background_migrations/requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads.yml @@ -0,0 +1,9 @@ +--- +migration_job_name: RequeueBackfillArchivedAndTraversalIdsToVulnerabilityReads +description: Backfill project.archived and project.namespace.traversal_ids values to the denormalized columns of the same name on vulnerability_reads. +feature_category: vulnerability_management +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144765 +milestone: '16.11' +queued_migration_version: 20240409140739 +finalize_after: '2024-03-15' +finalized_by: # version of the migration that finalized this BBM diff --git a/db/post_migrate/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb b/db/post_migrate/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb index f8e70a11b56..6a6069ef51e 100644 --- a/db/post_migrate/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb +++ b/db/post_migrate/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb @@ -11,17 +11,10 @@ class QueueBackfillArchivedAndTraversalIdsToVulnerabilityReads < Gitlab::Databas SUB_BATCH_SIZE = 100 def up - queue_batched_background_migration( - MIGRATION, - :vulnerability_reads, - :id, - job_interval: DELAY_INTERVAL, - batch_size: BATCH_SIZE, - sub_batch_size: SUB_BATCH_SIZE - ) + # no-op end def down - delete_batched_background_migration(MIGRATION, :vulnerability_reads, :id, []) + # no-op end end diff --git a/db/post_migrate/20240409140739_requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb b/db/post_migrate/20240409140739_requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb new file mode 100644 index 00000000000..dff9d23fcc0 --- /dev/null +++ b/db/post_migrate/20240409140739_requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class RequeueBackfillArchivedAndTraversalIdsToVulnerabilityReads < Gitlab::Database::Migration[2.2] + milestone '16.11' + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MIGRATION = "BackfillArchivedAndTraversalIdsToVulnerabilityReads" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 10_000 + SUB_BATCH_SIZE = 100 + + def up + # Clear previous background migration execution from QueueBackfillArchivedAndTraversalIdsToVulnerabilityReads + delete_batched_background_migration(MIGRATION, :vulnerability_reads, :id, []) + + queue_batched_background_migration( + MIGRATION, + :vulnerability_reads, + :id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :vulnerability_reads, :id, []) + end +end diff --git a/db/schema_migrations/20240409140739 b/db/schema_migrations/20240409140739 new file mode 100644 index 00000000000..515c0008a88 --- /dev/null +++ b/db/schema_migrations/20240409140739 @@ -0,0 +1 @@ +70e1cc677a0f2548bced6a430437e961e4e75ea6965a434427c4c653a50ffd0c \ No newline at end of file diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 0dc404224a0..046d63a59e4 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -1740,6 +1740,15 @@ For status, choose one: Generally available features should not have a status. +##### GitLab Duo Pro add-on + +The add-on belongs with other subscription tiers. Document it by using the phrase `with GitLab Duo Pro`. +For example: + +```markdown +**Tier:** Premium or Ultimate with GitLab Duo Pro +``` + ##### Duplicating tier, offering, or status on subheadings If a subheading has the same tier, offering, or status as its parent diff --git a/doc/user/ai_features.md b/doc/user/ai_features.md index 0795e61438a..f28ac223e37 100644 --- a/doc/user/ai_features.md +++ b/doc/user/ai_features.md @@ -71,8 +71,8 @@ To enable Beta and Experimental AI-powered features for GitLab versions where Gi - To use an HTTP/S proxy, both `gitLab_workhorse` and `gitLab_rails` must have the necessary [web proxy environment variables](https://docs.gitlab.com/omnibus/settings/environment-variables.html) set. - Check for restrictions on WebSocket (`wss://`) traffic to `wss://gitlab.com/-/cable` and other `.com` domains. - Network policy restrictions on `wss://` traffic can cause issues with some GitLab Duo - chat services. Consider policy updates to allow these services. + Network policy restrictions on `wss://` traffic can cause issues with some GitLab Duo Chat + services. Consider policy updates to allow these services. ### Disable GitLab Duo features for specific groups or projects or an entire instance diff --git a/doc/user/project/repository/code_suggestions/data.md b/doc/user/project/repository/code_suggestions/data.md index 5204cb43d36..b3b5c95cc50 100644 --- a/doc/user/project/repository/code_suggestions/data.md +++ b/doc/user/project/repository/code_suggestions/data.md @@ -7,8 +7,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Code Suggestions data usage DETAILS: -**Tier:** Premium, Ultimate -**Offering:** GitLab.com, Self-managed, GitLab Dedicated. GitLab Duo Pro required. +**Tier:** Premium or Ultimate with GitLab Duo Pro +**Offering:** GitLab.com, Self-managed, GitLab Dedicated Code Suggestions is powered by a generative AI model. diff --git a/doc/user/project/repository/code_suggestions/index.md b/doc/user/project/repository/code_suggestions/index.md index 7bd9f0e394d..99ae518e746 100644 --- a/doc/user/project/repository/code_suggestions/index.md +++ b/doc/user/project/repository/code_suggestions/index.md @@ -7,8 +7,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Code Suggestions DETAILS: -**Tier:** Premium, Ultimate -**Offering:** GitLab.com, Self-managed, GitLab Dedicated. GitLab Duo Pro required. +**Tier:** Premium or Ultimate with GitLab Duo Pro +**Offering:** GitLab.com, Self-managed, GitLab Dedicated > - [Introduced support for Google Vertex AI Codey APIs](https://gitlab.com/groups/gitlab-org/-/epics/10562) in GitLab 16.1. > - [Removed support for GitLab native model](https://gitlab.com/groups/gitlab-org/-/epics/10752) in GitLab 16.2. diff --git a/doc/user/project/repository/code_suggestions/supported_extensions.md b/doc/user/project/repository/code_suggestions/supported_extensions.md index f316ba44672..3bca4ec1f72 100644 --- a/doc/user/project/repository/code_suggestions/supported_extensions.md +++ b/doc/user/project/repository/code_suggestions/supported_extensions.md @@ -7,8 +7,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Supported extensions and languages DETAILS: -**Tier:** Premium, Ultimate -**Offering:** GitLab.com, Self-managed, GitLab Dedicated. GitLab Duo Pro required. +**Tier:** Premium or Ultimate with GitLab Duo Pro +**Offering:** GitLab.com, Self-managed, GitLab Dedicated Code Suggestions is available in the following editor extensions and for the following languages. diff --git a/doc/user/project/repository/code_suggestions/troubleshooting.md b/doc/user/project/repository/code_suggestions/troubleshooting.md index ac1dabc835b..e5c6291bf76 100644 --- a/doc/user/project/repository/code_suggestions/troubleshooting.md +++ b/doc/user/project/repository/code_suggestions/troubleshooting.md @@ -7,8 +7,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Troubleshooting Code Suggestions DETAILS: -**Tier:** Premium, Ultimate -**Offering:** GitLab.com, Self-managed, GitLab Dedicated. GitLab Duo Pro required. +**Tier:** Premium or Ultimate with GitLab Duo Pro +**Offering:** GitLab.com, Self-managed, GitLab Dedicated When working with GitLab Duo Code Suggestions, you might encounter the following issues. diff --git a/lib/gitlab/audit/ci_runner_token_author.rb b/lib/gitlab/audit/ci_runner_token_author.rb index 5f83725b5a3..2ace983688e 100644 --- a/lib/gitlab/audit/ci_runner_token_author.rb +++ b/lib/gitlab/audit/ci_runner_token_author.rb @@ -14,7 +14,7 @@ module Gitlab token = audit_event.details[:runner_registration_token] name = "Registration token: #{token}" else - raise ArgumentError, 'Runner token missing' + name = "Token not available" end super(id: -1, name: name) diff --git a/qa/qa/specs/features/api/3_create/repository/files_spec.rb b/qa/qa/specs/features/api/3_create/repository/files_spec.rb index 262155775d5..477d667da33 100644 --- a/qa/qa/specs/features/api/3_create/repository/files_spec.rb +++ b/qa/qa/specs/features/api/3_create/repository/files_spec.rb @@ -15,7 +15,7 @@ module QA # this file path deliberately includes a subdirectory which matches the file name to verify file/dir matching logic let(:file_path) { CGI.escape("føo/#{file_name}/føo/#{file_name}") } - it 'user creates a project with a file and deletes them afterwards', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347745' do + it 'user creates a project with a file and deletes them afterwards', :blocking, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347745' do create_project_request = Runtime::API::Request.new(@api_client, '/projects') post create_project_request.url, path: project_name, name: project_name diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/branch_with_unusual_name_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/branch_with_unusual_name_spec.rb index 30bccf0167b..2a285d72d5e 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/branch_with_unusual_name_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/branch_with_unusual_name_spec.rb @@ -11,7 +11,7 @@ module QA end context 'when branch name contains slash, hash, double dash, and capital letter' do - it 'renders repository file tree correctly', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347715' do + it 'renders repository file tree correctly', :blocking, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347715' do create(:commit, project: project, branch: branch_name, diff --git a/qa/qa/specs/features/browser_ui/3_create/snippet/snippet_index_page_spec.rb b/qa/qa/specs/features/browser_ui/3_create/snippet/snippet_index_page_spec.rb index aa794ebe115..230fbe412ab 100644 --- a/qa/qa/specs/features/browser_ui/3_create/snippet/snippet_index_page_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/snippet/snippet_index_page_spec.rb @@ -2,7 +2,7 @@ module QA RSpec.describe 'Create', product_group: :source_code do - describe 'Snippet index page' do + describe 'Snippet index page', :blocking do let(:personal_snippet_with_single_file) do create(:snippet, title: "Personal snippet with one file-#{SecureRandom.hex(8)}") end diff --git a/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb b/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb index 2a763d08276..73885763df1 100644 --- a/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/source_editor/source_editor_toolbar_spec.rb @@ -10,7 +10,7 @@ module QA Flow::Login.sign_in end - it 'can preview markdown side-by-side while editing', + it 'can preview markdown side-by-side while editing', :blocking, testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/367749' do project.visit! Page::Project::Show.perform do |project| diff --git a/spec/features/merge_request/user_resolves_wip_mr_spec.rb b/spec/features/merge_request/user_resolves_wip_mr_spec.rb index dd1e73cf57f..6dda5b606c4 100644 --- a/spec/features/merge_request/user_resolves_wip_mr_spec.rb +++ b/spec/features/merge_request/user_resolves_wip_mr_spec.rb @@ -30,10 +30,12 @@ RSpec.describe 'Merge request > User resolves Draft', :js, feature_category: :co end context 'when there is active pipeline for merge request' do - before do - create(:ci_build, pipeline: pipeline) + let(:feature_flags_state) { true } - stub_feature_flags(merge_blocked_component: false) + before do + stub_feature_flags(merge_when_checks_pass: feature_flags_state, merge_blocked_component: feature_flags_state) + + create(:ci_build, pipeline: pipeline) sign_in(user) visit project_merge_request_path(project, merge_request) @@ -41,8 +43,17 @@ RSpec.describe 'Merge request > User resolves Draft', :js, feature_category: :co end it 'retains merge request data after clicking Resolve WIP status' do + # rubocop:disable RSpec/AvoidConditionalStatements -- remove when Auto merge goes to Foss + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/146730 + expected_message = if Gitlab.ee? + "Set to auto-merge" + else + "Merge blocked:" + end + # rubocop:enable RSpec/AvoidConditionalStatements + expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}") - expect(page).to have_content "Merge blocked: Select Mark as ready to remove it from Draft status." + expect(page).to have_content expected_message page.within('.mr-state-widget') do click_button('Mark as ready') @@ -54,7 +65,28 @@ RSpec.describe 'Merge request > User resolves Draft', :js, feature_category: :co # merge request widget refreshes, which masks missing elements # that should already be present. expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}") - expect(page).not_to have_content("Merge blocked: Select Mark as ready to remove it from Draft status.") + expect(page).to have_content("Set to auto-merge") + end + + context 'when the new merge_when_checks_pass and merge blocked components are disabled' do + let(:feature_flags_state) { false } + + it 'retains merge request data after clicking Resolve WIP status' do + expect(page.find('.ci-widget-content')).to have_content("Pipeline ##{pipeline.id}") + expect(page).to have_content "Merge blocked: Select Mark as ready to remove it from Draft status." + + page.within('.mr-state-widget') do + click_button('Mark as ready') + end + + wait_for_requests + + # If we don't disable the wait here, the test will wait until the + # merge request widget refreshes, which masks missing elements + # that should already be present. + expect(page.find('.ci-widget-content', wait: 0)).to have_content("Pipeline ##{pipeline.id}") + expect(page).not_to have_content("Merge blocked") + end end end end diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index 5448d6867cb..0928394b571 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -331,12 +331,31 @@ RSpec.describe 'Merge request > User sees merge widget', :js, feature_category: visit project_merge_request_path(project_only_mwps, merge_request_in_only_mwps_project) end - it 'is allowed to merge' do - # Wait for the `ci_status` and `merge_check` requests - wait_for_requests + context 'when using merge when pipeline succeeds' do + before do + stub_feature_flags(merge_when_checks_pass: false) + end - expect(page).not_to have_selector('.accept-merge-request') + it 'is not allowed to merge' do + # Wait for the `ci_status` and `merge_check` requests + wait_for_requests + + expect(page).not_to have_selector('.accept-merge-request') + end end + # TODO When moving merge when checks pass to FOSS + # context 'when using merge when checks pass' do + # before do + # stub_feature_flags(merge_when_checks_pass: true) + # end + # + # it 'is not allowed to set auto merge' do + # # Wait for the `ci_status` and `merge_check` requests + # wait_for_requests + # + # expect(page).to have_selector('.accept-merge-request') + # end + # end end context 'view merge request with MWPS enabled but automatically merge fails' do diff --git a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb index 89664c57084..6718760e920 100644 --- a/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb +++ b/spec/lib/gitlab/audit/ci_runner_token_author_spec.rb @@ -34,8 +34,8 @@ RSpec.describe Gitlab::Audit::CiRunnerTokenAuthor do {} end - it 'raises ArgumentError' do - expect { subject }.to raise_error ArgumentError, 'Runner token missing' + it 'returns token not available' do + is_expected.to have_attributes(id: -1, name: 'Token not available') end end end diff --git a/spec/migrations/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb b/spec/migrations/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb index 8fc5c6b9c40..d23ad26fb11 100644 --- a/spec/migrations/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb +++ b/spec/migrations/20240214163238_queue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb @@ -6,20 +6,14 @@ require_migration! RSpec.describe QueueBackfillArchivedAndTraversalIdsToVulnerabilityReads, feature_category: :vulnerability_management do let!(:batched_migration) { described_class::MIGRATION } - it 'schedules a new batched migration' do + it 'does not schedule a new batched migration' do reversible_migration do |migration| migration.before -> { expect(batched_migration).not_to have_scheduled_batched_migration } migration.after -> { - expect(batched_migration).to have_scheduled_batched_migration( - table_name: :vulnerability_reads, - column_name: :id, - interval: described_class::DELAY_INTERVAL, - batch_size: described_class::BATCH_SIZE, - sub_batch_size: described_class::SUB_BATCH_SIZE - ) + expect(batched_migration).not_to have_scheduled_batched_migration } end end diff --git a/spec/migrations/20240409140739_requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb b/spec/migrations/20240409140739_requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb new file mode 100644 index 00000000000..c4eef2ebbcb --- /dev/null +++ b/spec/migrations/20240409140739_requeue_backfill_archived_and_traversal_ids_to_vulnerability_reads_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe RequeueBackfillArchivedAndTraversalIdsToVulnerabilityReads, feature_category: :vulnerability_management do + let!(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :vulnerability_reads, + column_name: :id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 57914866c28..b2983194175 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -3660,15 +3660,40 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev describe '#skipped_mergeable_checks' do subject { build_stubbed(:merge_request).skipped_mergeable_checks(options) } + let(:feature_flag) { true } + + before do + stub_feature_flags(additional_merge_when_checks_ready: feature_flag) + end + where(:options, :skip_ci_check) do {} | false { auto_merge_requested: false } | false { auto_merge_requested: true } | true end - with_them do it { is_expected.to include(skip_ci_check: skip_ci_check) } end + + context 'when auto_merge_requested is true' do + let(:options) { { auto_merge_requested: true, auto_merge_strategy: auto_merge_strategy } } + + where(:auto_merge_strategy, :skip_approved_check, :skip_draft_check, :skip_blocked_check, + :skip_discussions_check, :skip_external_status_check, :feature_flag) do + '' | false | false | false | false | false | true + AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS | false | false | false | false | false | true + AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS | true | true | true | true | true | true + AutoMergeService::STRATEGY_MERGE_WHEN_CHECKS_PASS | true | false | false | false | false | false + end + + with_them do + it do + is_expected.to include(skip_approved_check: skip_approved_check, skip_draft_check: skip_draft_check, + skip_blocked_check: skip_blocked_check, skip_discussions_check: skip_discussions_check, + skip_external_status_check: skip_external_status_check) + end + end + end end describe '#check_mergeability' do @@ -6285,6 +6310,26 @@ RSpec.describe MergeRequest, factory_default: :keep, feature_category: :code_rev end end + describe '#mergeability_checks_pass?' do + let(:merge_request) { build_stubbed(:merge_request) } + let(:result) { instance_double(ServiceResponse, success?: { results: ['result'] }) } + + it 'executes MergeRequests::Mergeability::RunChecksService with all mergeability checks and returns a boolean' do + expect_next_instance_of( + MergeRequests::Mergeability::RunChecksService, + merge_request: merge_request, + params: {} + ) do |svc| + expect(svc) + .to receive(:execute) + .with(described_class.all_mergeability_checks, execute_all: false) + .and_return(result) + end + + expect(merge_request.mergeability_checks_pass?).to be_truthy + end + end + describe '#only_allow_merge_if_pipeline_succeeds?' do let(:merge_request) { build_stubbed(:merge_request) } diff --git a/spec/services/auto_merge/base_service_spec.rb b/spec/services/auto_merge/base_service_spec.rb index f4b3430200c..5d3ce648984 100644 --- a/spec/services/auto_merge/base_service_spec.rb +++ b/spec/services/auto_merge/base_service_spec.rb @@ -305,22 +305,65 @@ RSpec.describe AutoMerge::BaseService, feature_category: :code_review_workflow d describe '#available_for?' do using RSpec::Parameterized::TableSyntax - subject(:available_for) { service.available_for?(merge_request) { true } } + subject(:available_for) { service.available_for?(merge_request) { yield_result } } let(:merge_request) { create(:merge_request) } + let(:yield_result) { true } + let(:checks_pass) { true } + let(:can_be_merged) { true } - where(:can_be_merged, :open, :broken, :discussions, :blocked, :draft, :result) do - true | true | false | true | false | false | true - false | true | false | true | false | false | false - true | false | false | true | false | false | false - true | true | true | true | false | false | false - true | true | false | false | false | false | false - true | true | false | true | true | false | false - true | true | false | true | false | true | false + context 'when can_be_merged is true' do + before do + allow(merge_request).to receive(:can_be_merged_by?).and_return(can_be_merged) + allow(merge_request).to receive(:mergeability_checks_pass?).and_return(checks_pass) + end + + context 'when the mergeabilty checks pass is true' do + context 'when the yield is true' do + it 'returns true' do + expect(available_for).to be_truthy + end + end + + context 'when the yield is false' do + let(:yield_result) { false } + + it 'returns false' do + expect(available_for).to be_falsey + end + end + end + + context 'when the mergeabilty checks pass is false' do + let(:checks_pass) { false } + + context 'when the yield is true' do + it 'returns false' do + expect(available_for).to be_falsey + end + end + + context 'when the yield is false' do + let(:yield_result) { false } + + it 'returns false' do + expect(available_for).to be_falsey + end + end + end end - with_them do + context 'when can_be_merged is false' do + let(:can_be_merged) { false } + + it 'returns false' do + expect(available_for).to be_falsey + end + end + + context 'when refactor_auto_merge is disabled' do before do + stub_feature_flags(refactor_auto_merge: false) allow(merge_request).to receive(:can_be_merged_by?).and_return(can_be_merged) allow(merge_request).to receive(:open?).and_return(open) allow(merge_request).to receive(:broken?).and_return(broken) @@ -329,8 +372,20 @@ RSpec.describe AutoMerge::BaseService, feature_category: :code_review_workflow d allow(merge_request).to receive(:merge_blocked_by_other_mrs?).and_return(blocked) end - it 'returns the expected results' do - expect(available_for).to eq(result) + where(:can_be_merged, :open, :broken, :discussions, :blocked, :draft, :result) do + true | true | false | true | false | false | true + false | true | false | true | false | false | false + true | false | false | true | false | false | false + true | true | true | true | false | false | false + true | true | false | false | false | false | false + true | true | false | true | true | false | false + true | true | false | true | false | true | false + end + + with_them do + it 'returns the expected results' do + expect(available_for).to eq(result) + end end end end diff --git a/spec/services/merge_requests/merge_orchestration_service_spec.rb b/spec/services/merge_requests/merge_orchestration_service_spec.rb index 7e6eeec2a2d..faec5ac2fde 100644 --- a/spec/services/merge_requests/merge_orchestration_service_spec.rb +++ b/spec/services/merge_requests/merge_orchestration_service_spec.rb @@ -102,7 +102,7 @@ RSpec.describe MergeRequests::MergeOrchestrationService, feature_category: :code context 'when merge request is not mergeable' do before do - allow(merge_request).to receive(:mergeable?) { false } + merge_request.update!(merge_status: 'cannot_be_merged') end it { is_expected.to eq(false) } diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index e253c06125f..aee39f8beb2 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -553,7 +553,13 @@ RSpec.describe MergeRequests::UpdateService, :mailer, feature_category: :code_re head_pipeline_of: merge_request ) - expect(AutoMerge::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user, { sha: merge_request.diff_head_sha }) + expected_class = if Gitlab.ee? + AutoMerge::MergeWhenChecksPassService + else + AutoMerge::MergeWhenPipelineSucceedsService + end + + expect(expected_class).to receive(:new).with(project, user, { sha: merge_request.diff_head_sha }) .and_return(service_mock) allow(service_mock).to receive(:available_for?) { true } expect(service_mock).to receive(:execute).with(merge_request)