diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 02c84ce33d8c52cded412081cea89a33162eeaa5..f0ddd62e9964dcde91ffd78ed0a45bbd930a4c15 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -362,6 +362,7 @@ def project_params(attributes: []) def project_params_attributes [ + :allow_merge_on_skipped_pipeline, :avatar, :build_allow_git_fetch, :build_coverage_regex, diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 88a9adfc3934994eb8ddd5d07c5d91f866e5b695..f0e2e4c7ae335dffb5ccde555b3534c0716582a9 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -95,6 +95,8 @@ class ProjectType < BaseObject description: 'Status of Jira import background job of the project' field :only_allow_merge_if_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true, description: 'Indicates if merge requests of the project can only be merged with successful jobs' + field :allow_merge_on_skipped_pipeline, GraphQL::BOOLEAN_TYPE, null: true, + description: 'If `only_allow_merge_if_pipeline_succeeds` is true, indicates if merge requests of the project can also be merged with skipped jobs' field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true, description: 'Indicates if users can request member access to the project' field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true, diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1453b1f11d234b6d5b6ba4530b0f6eb631bc111f..caf7b554427dac52fa64016fd8f86cc0aa6c8b97 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1169,6 +1169,7 @@ def can_be_merged_via_command_line_by?(user) def mergeable_ci_state? return true unless project.only_allow_merge_if_pipeline_succeeds? return false unless actual_head_pipeline + return true if project.allow_merge_on_skipped_pipeline? && actual_head_pipeline.skipped? actual_head_pipeline.success? end diff --git a/app/models/project.rb b/app/models/project.rb index a70f016a9f3a535a257afe285b64dcbd93067828..bdcef14d705d072aebb1d69782a4a00a2c61d9d3 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -198,7 +198,7 @@ class Project < ApplicationRecord has_one :error_tracking_setting, inverse_of: :project, class_name: 'ErrorTracking::ProjectErrorTrackingSetting' has_one :metrics_setting, inverse_of: :project, class_name: 'ProjectMetricsSetting' has_one :grafana_integration, inverse_of: :project - has_one :project_setting, ->(project) { where_or_create_by(project: project) }, inverse_of: :project + has_one :project_setting, inverse_of: :project, autosave: true has_one :alerting_setting, inverse_of: :project, class_name: 'Alerting::ProjectAlertingSetting' # Merge Requests for target project should be removed with it @@ -376,6 +376,7 @@ class Project < ApplicationRecord delegate :default_git_depth, :default_git_depth=, to: :ci_cd_settings, prefix: :ci delegate :forward_deployment_enabled, :forward_deployment_enabled=, :forward_deployment_enabled?, to: :ci_cd_settings delegate :actual_limits, :actual_plan_name, to: :namespace, allow_nil: true + delegate :allow_merge_on_skipped_pipeline, :allow_merge_on_skipped_pipeline?, :allow_merge_on_skipped_pipeline=, to: :project_setting # Validations validates :creator, presence: true, on: :create @@ -727,6 +728,10 @@ def initialize(attributes = nil) super end + def project_setting + super.presence || build_project_setting + end + def all_pipelines if builds_enabled? super diff --git a/app/models/project_setting.rb b/app/models/project_setting.rb index 7c93faf3928be23b6ce67fe8bb0cb9948b76363b..9022d3e879d0c4c96c5688bfc7982fd2fcb10246 100644 --- a/app/models/project_setting.rb +++ b/app/models/project_setting.rb @@ -4,10 +4,6 @@ class ProjectSetting < ApplicationRecord belongs_to :project, inverse_of: :project_setting self.primary_key = :project_id - - def self.where_or_create_by(attrs) - where(primary_key => safe_find_or_create_by(attrs)) - end end ProjectSetting.prepend_if_ee('EE::ProjectSetting') diff --git a/app/views/projects/_merge_request_merge_checks_settings.html.haml b/app/views/projects/_merge_request_merge_checks_settings.html.haml index d3fcb52422b1eb1cee976d005c17a1e528f2f4e4..9cebb191346f80e5783ccddd3a97f0504f78204b 100644 --- a/app/views/projects/_merge_request_merge_checks_settings.html.haml +++ b/app/views/projects/_merge_request_merge_checks_settings.html.haml @@ -13,6 +13,13 @@ help_page_path('ci/merge_request_pipelines/index.md', anchor: 'pipelines-for-merge-requests'), target: '_blank' + .form-check.mb-2 + .gl-pl-6 + = form.check_box :allow_merge_on_skipped_pipeline, class: 'form-check-input' + = form.label :allow_merge_on_skipped_pipeline, class: 'form-check-label' do + = s_('ProjectSettings|Skipped pipelines are considered successful') + .text-secondary + = s_('ProjectSettings|This introduces the risk of merging changes that will not pass the pipeline.') .form-check.mb-2 = form.check_box :only_allow_merge_if_all_discussions_are_resolved, class: 'form-check-input' = form.label :only_allow_merge_if_all_discussions_are_resolved, class: 'form-check-label' do diff --git a/changelogs/unreleased/allow_skipped.yml b/changelogs/unreleased/allow_skipped.yml new file mode 100644 index 0000000000000000000000000000000000000000..b920298f42e24c91f6cade4e83483beaceb826cf --- /dev/null +++ b/changelogs/unreleased/allow_skipped.yml @@ -0,0 +1,5 @@ +--- +title: Add setting to allow merge on skipped pipeline +merge_request: 27490 +author: Mathieu Parent +type: added diff --git a/db/migrate/20200325094612_add_allow_merge_on_skipped_pipeline_to_project_settings.rb b/db/migrate/20200325094612_add_allow_merge_on_skipped_pipeline_to_project_settings.rb new file mode 100644 index 0000000000000000000000000000000000000000..8575dd2f080d27cf6c8868794264349f7a512372 --- /dev/null +++ b/db/migrate/20200325094612_add_allow_merge_on_skipped_pipeline_to_project_settings.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddAllowMergeOnSkippedPipelineToProjectSettings < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + add_column :project_settings, :allow_merge_on_skipped_pipeline, :boolean + end +end diff --git a/db/structure.sql b/db/structure.sql index 0018eeb4eaba81ef1f3b288f4d8cd0f180b39860..a7ed83620cdd32667a3e403ca7141921bd4dae79 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5340,6 +5340,7 @@ CREATE TABLE public.project_settings ( updated_at timestamp with time zone NOT NULL, push_rule_id bigint, show_default_award_emojis boolean DEFAULT true, + allow_merge_on_skipped_pipeline boolean, CONSTRAINT check_bde223416c CHECK ((show_default_award_emojis IS NOT NULL)) ); @@ -13613,6 +13614,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200323134519 20200324093258 20200324115359 +20200325094612 20200325104755 20200325104756 20200325104833 diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index b01af67204955dda2487124cc1e9c1f948dbc1cb..cbe79658f385849d9ea8910c452752a1fa5b6496 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -8265,6 +8265,12 @@ type Project { statuses: [AlertManagementStatus!] ): AlertManagementAlertConnection + """ + If `only_allow_merge_if_pipeline_succeeds` is true, indicates if merge + requests of the project can also be merged with skipped jobs + """ + allowMergeOnSkippedPipeline: Boolean + """ Indicates the archived status of the project """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index c5d172c035d9c0f6b3ad25fb0725e0daec134a7c..40dad1852897a71a0930569f84b1fbcd44fe3c67 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -24673,6 +24673,20 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "allowMergeOnSkippedPipeline", + "description": "If `only_allow_merge_if_pipeline_succeeds` is true, indicates if merge requests of the project can also be merged with skipped jobs", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "archived", "description": "Indicates the archived status of the project", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a3f84642e134ff7477e8c417b7c38338a9bf633b..6f404e47fb4266a660934715c8a06d7345a04f8a 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -1252,6 +1252,7 @@ Information about pagination in a connection. | --- | ---- | ---------- | | `alertManagementAlert` | AlertManagementAlert | A single Alert Management alert of the project | | `alertManagementAlertStatusCounts` | AlertManagementAlertStatusCountsType | Counts of alerts by status for the project | +| `allowMergeOnSkippedPipeline` | Boolean | If `only_allow_merge_if_pipeline_succeeds` is true, indicates if merge requests of the project can also be merged with skipped jobs | | `archived` | Boolean | Indicates the archived status of the project | | `autocloseReferencedIssues` | Boolean | Indicates if issues referenced by merge requests and commits within the default branch are closed automatically | | `avatarUrl` | String | URL to avatar image file of the project | diff --git a/doc/api/projects.md b/doc/api/projects.md index ed6f9360f9ac261c82322b3c057839846732e5a6..b9ba632cd9e427db4e16f5778f3286b4fe5add27 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -158,6 +158,7 @@ When the user is authenticated and `simple` is not set this returns something li "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -248,6 +249,7 @@ When the user is authenticated and `simple` is not set this returns something li "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -407,6 +409,7 @@ This endpoint supports [keyset pagination](README.md#keyset-based-pagination) fo "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -497,6 +500,7 @@ This endpoint supports [keyset pagination](README.md#keyset-based-pagination) fo "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -623,6 +627,7 @@ Example response: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -708,6 +713,7 @@ Example response: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -861,6 +867,7 @@ GET /projects/:id ], "repository_storage": "default", "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "printing_merge_requests_link_enabled": true, @@ -1044,6 +1051,7 @@ POST /projects | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, jobs can be viewed by non-project-members | | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | +| `allow_merge_on_skipped_pipeline` | boolean | no | Set whether or not merge requests can be merged with skipped jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `merge_method` | string | no | Set the [merge method](#project-merge-method) used | | `autoclose_referenced_issues` | boolean | no | Set whether auto-closing referenced issues on default branch | @@ -1113,6 +1121,7 @@ POST /projects/user/:user_id | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, jobs can be viewed by non-project-members | | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | +| `allow_merge_on_skipped_pipeline` | boolean | no | Set whether or not merge requests can be merged with skipped jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `merge_method` | string | no | Set the [merge method](#project-merge-method) used | | `autoclose_referenced_issues` | boolean | no | Set whether auto-closing referenced issues on default branch | @@ -1183,6 +1192,7 @@ PUT /projects/:id | `import_url` | string | no | URL to import repository from | | `public_builds` | boolean | no | If `true`, jobs can be viewed by non-project-members | | `only_allow_merge_if_pipeline_succeeds` | boolean | no | Set whether merge requests can only be merged with successful jobs | +| `allow_merge_on_skipped_pipeline` | boolean | no | Set whether or not merge requests can be merged with skipped jobs | | `only_allow_merge_if_all_discussions_are_resolved` | boolean | no | Set whether merge requests can only be merged when all the discussions are resolved | | `merge_method` | string | no | Set the [merge method](#project-merge-method) used | | `autoclose_referenced_issues` | boolean | no | Set whether auto-closing referenced issues on default branch | @@ -1317,6 +1327,7 @@ Example responses: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -1408,6 +1419,7 @@ Example response: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -1498,6 +1510,7 @@ Example response: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -1680,6 +1693,7 @@ Example response: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, @@ -1789,6 +1803,7 @@ Example response: "public_jobs": true, "shared_with_groups": [], "only_allow_merge_if_pipeline_succeeds": false, + "allow_merge_on_skipped_pipeline": false, "only_allow_merge_if_all_discussions_are_resolved": false, "remove_source_branch_after_merge": false, "request_access_enabled": false, diff --git a/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md b/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md index 5eedd0cf8a763f2b0bb8befb43ed2abd01885445..d45ccdc9be9d6edb97f38c5059a4c960bc04b334 100644 --- a/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md +++ b/doc/user/project/merge_requests/merge_when_pipeline_succeeds.md @@ -59,6 +59,19 @@ merge request from the UI, until you make all relevant jobs pass. ![Only allow merge if pipeline succeeds message](img/merge_when_pipeline_succeeds_only_if_succeeds_msg.png) +### Skipped pipelines + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/211482) in GitLab 13.1. + +When the **Pipelines must succeed** checkbox is checked, [skipped pipelines](../../../ci/yaml/README.md#skip-pipeline) prevent +merge requests from being merged. To change this behavior: + +1. Navigate to your project's **Settings > General** page. +1. Expand the **Merge requests** section. +1. In the **Merge checks** subsection, ensure **Pipelines must succeed** is checked. +1. In the **Merge checks** subsection, select the **Skipped pipelines are considered successful** checkbox. +1. Press **Save** for the changes to take effect. + ### Limitations When this setting is enabled, a merge request is prevented from being merged if there is no pipeline. This may conflict with some use cases where [`only/except`](../../../ci/yaml/README.md#onlyexcept-advanced) rules are used and they don't generate any pipelines. diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index b6fa419da58fc4dbf73218291f5c42c98e5e5d70..55a57501858b28f3292f40d1cd95af6dd3d958b9 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -93,6 +93,7 @@ class Project < BasicProjectDetails SharedGroupWithProject.represent(project.project_group_links, options) end expose :only_allow_merge_if_pipeline_succeeds + expose :allow_merge_on_skipped_pipeline expose :request_access_enabled expose :only_allow_merge_if_all_discussions_are_resolved expose :remove_source_branch_after_merge @@ -119,6 +120,7 @@ def self.preload_relation(projects_relation, options = {}) # MR describing the solution: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/20555 super(projects_relation).preload(:group) .preload(:ci_cd_settings) + .preload(:project_setting) .preload(:container_expiration_policy) .preload(:auto_devops) .preload(project_group_links: { group: :route }, diff --git a/lib/api/helpers/projects_helpers.rb b/lib/api/helpers/projects_helpers.rb index 5afdb34da971544563a5545199658a563ec397af..8a115d42929c69dcd0d199d751defa708b24b3c9 100644 --- a/lib/api/helpers/projects_helpers.rb +++ b/lib/api/helpers/projects_helpers.rb @@ -44,6 +44,7 @@ module ProjectsHelpers optional :public_builds, type: Boolean, desc: 'Perform public builds' optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' optional :only_allow_merge_if_pipeline_succeeds, type: Boolean, desc: 'Only allow to merge if builds succeed' + optional :allow_merge_on_skipped_pipeline, type: Boolean, desc: 'Allow to merge if pipeline is skipped' optional :only_allow_merge_if_all_discussions_are_resolved, type: Boolean, desc: 'Only allow to merge if all discussions are resolved' optional :tag_list, type: Array[String], desc: 'The list of tags for a project' # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 @@ -92,6 +93,7 @@ module ProjectsHelpers def self.update_params_at_least_one_of [ + :allow_merge_on_skipped_pipeline, :autoclose_referenced_issues, :auto_devops_enabled, :auto_devops_deploy_strategy, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 210f51a73dbf45996f2d07cbdb54e6d613356b26..a1b3105fcc2b976408504ea0e44c254cd9472e28 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -17341,6 +17341,9 @@ msgstr "" msgid "ProjectSettings|Show link to create/view merge request when pushing from the command line" msgstr "" +msgid "ProjectSettings|Skipped pipelines are considered successful" +msgstr "" + msgid "ProjectSettings|Snippets" msgstr "" @@ -17356,6 +17359,9 @@ msgstr "" msgid "ProjectSettings|These checks must pass before merge requests can be merged" msgstr "" +msgid "ProjectSettings|This introduces the risk of merging changes that will not pass the pipeline." +msgstr "" + msgid "ProjectSettings|This setting is applied on the server level and can be overridden by an admin." msgstr "" diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 235d54e3a16fe3c2498f79ec1affec3b4b3c9164..849d8fcc1c42c0aa9693282cf0cbf79f5b0bedda 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -424,6 +424,7 @@ project: - deploy_tokens - settings - ci_cd_settings +- project_settings - import_export_upload - repository_languages - pool_repository diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 97fd30837d44c0a04e7ed3c1a051ddfb4a3bea49..0d112bfdb2a20f65b87e017555ca2faa2ca074da 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -703,6 +703,8 @@ Badge: - type ProjectCiCdSetting: - group_runners_enabled +ProjectSetting: +- allow_merge_on_skipped_pipeline ProtectedEnvironment: - id - project_id diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 8c50966078f4c845e45a2e64c4fab4d974f1730e..c70ddac5da6bbfa97abf0d0e49d2a92f2609e7cc 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2517,12 +2517,13 @@ def set_compare(merge_request) end describe '#mergeable_ci_state?' do - let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } let(:pipeline) { create(:ci_empty_pipeline) } - subject { build(:merge_request, target_project: project) } - context 'when it is only allowed to merge when build is green' do + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } + + subject { build(:merge_request, target_project: project) } + context 'and a failed pipeline is associated' do before do pipeline.update(status: 'failed', sha: subject.diff_head_sha) @@ -2544,7 +2545,7 @@ def set_compare(merge_request) context 'and a skipped pipeline is associated' do before do pipeline.update(status: 'skipped', sha: subject.diff_head_sha) - allow(subject).to receive(:head_pipeline) { pipeline } + allow(subject).to receive(:head_pipeline).and_return(pipeline) end it { expect(subject.mergeable_ci_state?).to be_falsey } @@ -2552,7 +2553,48 @@ def set_compare(merge_request) context 'when no pipeline is associated' do before do - allow(subject).to receive(:head_pipeline) { nil } + allow(subject).to receive(:head_pipeline).and_return(nil) + end + + it { expect(subject.mergeable_ci_state?).to be_falsey } + end + end + + context 'when it is only allowed to merge when build is green or skipped' do + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true, allow_merge_on_skipped_pipeline: true) } + + subject { build(:merge_request, target_project: project) } + + context 'and a failed pipeline is associated' do + before do + pipeline.update!(status: 'failed', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_falsey } + end + + context 'and a successful pipeline is associated' do + before do + pipeline.update!(status: 'success', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'and a skipped pipeline is associated' do + before do + pipeline.update!(status: 'skipped', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'when no pipeline is associated' do + before do + allow(subject).to receive(:head_pipeline).and_return(nil) end it { expect(subject.mergeable_ci_state?).to be_falsey } @@ -2560,7 +2602,9 @@ def set_compare(merge_request) end context 'when merges are not restricted to green builds' do - subject { build(:merge_request, target_project: create(:project, only_allow_merge_if_pipeline_succeeds: false)) } + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: false) } + + subject { build(:merge_request, target_project: project) } context 'and a failed pipeline is associated' do before do @@ -2578,6 +2622,23 @@ def set_compare(merge_request) it { expect(subject.mergeable_ci_state?).to be_truthy } end + + context 'and a skipped pipeline is associated' do + before do + pipeline.update!(status: 'skipped', sha: subject.diff_head_sha) + allow(subject).to receive(:head_pipeline).and_return(pipeline) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end + + context 'when no pipeline is associated' do + before do + allow(subject).to receive(:head_pipeline).and_return(nil) + end + + it { expect(subject.mergeable_ci_state?).to be_truthy } + end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 787d778b483bf9d240301e955fda845b0338c498..1a19f6f8814c0413b0e8216e3e0af8556bb788e4 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -183,9 +183,9 @@ expect(project.pages_metadatum).to be_persisted end - it 'automatically creates a project setting row' do + it 'automatically builds a project setting row' do expect(project.project_setting).to be_an_instance_of(ProjectSetting) - expect(project.project_setting).to be_persisted + expect(project.project_setting).to be_new_record end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 6f6737864b6f89a70a0961df9e25ece89415cf46..5af4ad020fc35618a31481965e2a6ff941fff39c 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -764,7 +764,8 @@ resolve_outdated_diff_discussions: false, remove_source_branch_after_merge: true, autoclose_referenced_issues: true, - only_allow_merge_if_pipeline_succeeds: false, + only_allow_merge_if_pipeline_succeeds: true, + allow_merge_on_skipped_pipeline: true, request_access_enabled: true, only_allow_merge_if_all_discussions_are_resolved: false, ci_config_path: 'a/custom/path', @@ -912,6 +913,22 @@ expect(json_response['only_allow_merge_if_pipeline_succeeds']).to be_truthy end + it 'sets a project as not allowing merge when pipeline is skipped' do + project_params = attributes_for(:project, allow_merge_on_skipped_pipeline: false) + + post api('/projects', user), params: project_params + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_falsey + end + + it 'sets a project as allowing merge when pipeline is skipped' do + project_params = attributes_for(:project, allow_merge_on_skipped_pipeline: true) + + post api('/projects', user), params: project_params + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_truthy + end + it 'sets a project as allowing merge even if discussions are unresolved' do project = attributes_for(:project, only_allow_merge_if_all_discussions_are_resolved: false) @@ -1245,16 +1262,36 @@ it 'sets a project as allowing merge even if build fails' do project = attributes_for(:project, only_allow_merge_if_pipeline_succeeds: false) + post api("/projects/user/#{user.id}", admin), params: project + expect(json_response['only_allow_merge_if_pipeline_succeeds']).to be_falsey end it 'sets a project as allowing merge only if pipeline succeeds' do project = attributes_for(:project, only_allow_merge_if_pipeline_succeeds: true) + post api("/projects/user/#{user.id}", admin), params: project + expect(json_response['only_allow_merge_if_pipeline_succeeds']).to be_truthy end + it 'sets a project as not allowing merge when pipeline is skipped' do + project = attributes_for(:project, allow_merge_on_skipped_pipeline: false) + + post api("/projects/user/#{user.id}", admin), params: project + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_falsey + end + + it 'sets a project as allowing merge when pipeline is skipped' do + project = attributes_for(:project, allow_merge_on_skipped_pipeline: true) + + post api("/projects/user/#{user.id}", admin), params: project + + expect(json_response['allow_merge_on_skipped_pipeline']).to be_truthy + end + it 'sets a project as allowing merge even if discussions are unresolved' do project = attributes_for(:project, only_allow_merge_if_all_discussions_are_resolved: false) @@ -1394,6 +1431,7 @@ expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name) expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) + expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) end end @@ -1461,6 +1499,7 @@ expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access) expect(json_response['shared_with_groups'][0]).to have_key('expires_at') expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds) + expect(json_response['allow_merge_on_skipped_pipeline']).to eq(project.allow_merge_on_skipped_pipeline) expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['ci_default_git_depth']).to eq(project.ci_default_git_depth) expect(json_response['merge_method']).to eq(project.merge_method.to_s) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 729e5d9c23d5d383abd923cbc29fe5b4e70baa35..e70ee05ed31a43ba009e0ad39cbbd981b17d7648 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -43,10 +43,10 @@ create_project(user, opts) end - it 'creates associated project settings' do + it 'builds associated project settings' do project = create_project(user, opts) - expect(project.project_setting).to be_persisted + expect(project.project_setting).to be_new_record end end