From 78af3a7af2823e3de3ec588ee5e4ca9bb0c63709 Mon Sep 17 00:00:00 2001 From: Harsimar Sandhu <harsimar@Harsimars-MacBook-Pro.local> Date: Fri, 10 May 2024 01:42:52 +0530 Subject: [PATCH] Cleanup default_branch_protection_defaults feature flag This commit cleans up feature flag `default_branch_protection_defaults` Changelog: added --- app/models/project.rb | 20 +- .../protect_default_branch_service.rb | 12 +- .../_default_branch.html.haml | 6 +- .../repository/_default_branch.html.haml | 3 - .../default_branch_protection_defaults.yml | 9 - .../protect_default_branch_service.rb | 1 - .../protect_default_branch_service_spec.rb | 168 ++-- .../importer/protected_branch_importer.rb | 6 +- .../protected_branch_importer_spec.rb | 110 +-- spec/lib/gitlab/user_access_spec.rb | 61 +- spec/models/project_spec.rb | 238 ++---- spec/models/protected_branch_spec.rb | 90 +- spec/services/git/branch_push_service_spec.rb | 128 +-- .../protect_default_branch_service_spec.rb | 795 +++++------------- .../_default_branch.html.haml_spec.rb | 23 +- 15 files changed, 436 insertions(+), 1234 deletions(-) delete mode 100644 config/feature_flags/gitlab_com_derisk/default_branch_protection_defaults.yml diff --git a/app/models/project.rb b/app/models/project.rb index 63d0da63497eb..a67f340453ea6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -2914,27 +2914,15 @@ def deploy_token_revoke_url_for(token) end def default_branch_protected? - if Feature.enabled?(:default_branch_protection_defaults, self) - branch_protection = Gitlab::Access::DefaultBranchProtection.new(self.namespace.default_branch_protection_settings) + branch_protection = Gitlab::Access::DefaultBranchProtection.new(self.namespace.default_branch_protection_settings) - return !branch_protection.developer_can_push? - end - - branch_protection = Gitlab::Access::BranchProtection.new(self.namespace.default_branch_protection) - - branch_protection.fully_protected? || branch_protection.developer_can_merge? || branch_protection.developer_can_initial_push? + !branch_protection.developer_can_push? end def initial_push_to_default_branch_allowed_for_developer? - if Feature.enabled?(:default_branch_protection_defaults, self) - branch_protection = Gitlab::Access::DefaultBranchProtection.new(self.namespace.default_branch_protection_settings) - - return branch_protection.developer_can_push? || branch_protection.developer_can_initial_push? - end - - branch_protection = Gitlab::Access::BranchProtection.new(self.namespace.default_branch_protection) + branch_protection = Gitlab::Access::DefaultBranchProtection.new(self.namespace.default_branch_protection_settings) - !branch_protection.any? || branch_protection.developer_can_push? || branch_protection.developer_can_initial_push? + branch_protection.developer_can_push? || branch_protection.developer_can_initial_push? end def environments_for_scope(scope) diff --git a/app/services/projects/protect_default_branch_service.rb b/app/services/projects/protect_default_branch_service.rb index f265b0feb51f4..c96a0c9801c6a 100644 --- a/app/services/projects/protect_default_branch_service.rb +++ b/app/services/projects/protect_default_branch_service.rb @@ -10,13 +10,9 @@ class ProtectDefaultBranchService def initialize(project) @project = project - @default_branch_protection = if Feature.enabled?(:default_branch_protection_defaults, project) - Gitlab::Access::DefaultBranchProtection.new( - project.namespace.default_branch_protection_settings - ) - else - Gitlab::Access::BranchProtection.new(project.namespace.default_branch_protection) - end + @default_branch_protection = Gitlab::Access::DefaultBranchProtection.new( + project.namespace.default_branch_protection_settings + ) end def execute @@ -52,8 +48,6 @@ def code_owner_approval_required? end def allow_force_push? - return false unless Feature.enabled?(:default_branch_protection_defaults, project) - default_branch_protection.allow_force_push? end diff --git a/app/views/admin/application_settings/_default_branch.html.haml b/app/views/admin/application_settings/_default_branch.html.haml index 04569194b25c0..8b0f661d574c3 100644 --- a/app/views/admin/application_settings/_default_branch.html.haml +++ b/app/views/admin/application_settings/_default_branch.html.haml @@ -10,12 +10,8 @@ %span.form-text.text-muted = (s_("AdminSettings|If not specified at the group or instance level, the default is %{default_initial_branch_name}. Does not affect existing repositories.") % { default_initial_branch_name: fallback_branch_name }).html_safe - -# rubocop: disable Gitlab/FeatureFlagWithoutActor -- This is instance-wide feature flag, no actor here - - if Feature.enabled?(:default_branch_protection_defaults) = render 'shared/default_branch_protection_defaults', f: f, field: :application_setting, value: @application_setting.default_branch_protection_defaults - - else - = render 'shared/default_branch_protection', f: f - -# rubocop: enable Gitlab/FeatureFlagWithoutActor + = render_if_exists 'admin/application_settings/group_owners_can_manage_default_branch_protection_setting', form: f = f.submit _('Save changes'), pajamas_button: true diff --git a/app/views/groups/settings/repository/_default_branch.html.haml b/app/views/groups/settings/repository/_default_branch.html.haml index 0b0123c253a3e..ce51edc4f3fb0 100644 --- a/app/views/groups/settings/repository/_default_branch.html.haml +++ b/app/views/groups/settings/repository/_default_branch.html.haml @@ -18,10 +18,7 @@ %span.form-text.text-muted = (s_("GroupSettings|If not specified at the group or instance level, the default is %{default_initial_branch_name}. Does not affect existing repositories.") % { default_initial_branch_name: fallback_branch_name }).html_safe - - if Feature.enabled?(:default_branch_protection_defaults, @group) = render 'groups/settings/default_branch_protection_defaults', f: f, group: @group - - else - = render 'groups/settings/default_branch_protection', f: f, group: @group = f.hidden_field :redirect_target, value: "repository_settings" = f.submit _('Save changes'), pajamas_button: true diff --git a/config/feature_flags/gitlab_com_derisk/default_branch_protection_defaults.yml b/config/feature_flags/gitlab_com_derisk/default_branch_protection_defaults.yml deleted file mode 100644 index 2b3b574ae0077..0000000000000 --- a/config/feature_flags/gitlab_com_derisk/default_branch_protection_defaults.yml +++ /dev/null @@ -1,9 +0,0 @@ ---- -name: default_branch_protection_defaults -feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/408152 -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143796 -rollout_issue_url: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/17516 -milestone: '16.10' -group: group::compliance -type: gitlab_com_derisk -default_enabled: false diff --git a/ee/app/services/ee/projects/protect_default_branch_service.rb b/ee/app/services/ee/projects/protect_default_branch_service.rb index 8044703814cbf..41806f65ef2b7 100644 --- a/ee/app/services/ee/projects/protect_default_branch_service.rb +++ b/ee/app/services/ee/projects/protect_default_branch_service.rb @@ -21,7 +21,6 @@ def push_access_level override :code_owner_approval_required? def code_owner_approval_required? - return false unless ::Feature.enabled?(:default_branch_protection_defaults, project) return false unless ::License.feature_available?(:code_owner_approval_required) default_branch_protection.code_owner_approval_required? diff --git a/ee/spec/services/projects/protect_default_branch_service_spec.rb b/ee/spec/services/projects/protect_default_branch_service_spec.rb index 9bef5b3959118..df7116f1e44a8 100644 --- a/ee/spec/services/projects/protect_default_branch_service_spec.rb +++ b/ee/spec/services/projects/protect_default_branch_service_spec.rb @@ -14,158 +14,84 @@ end end - context 'when feature flag `default_branch_protection_defaults` is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) - end - - describe '#code_owner_approval_required?' do + describe '#code_owner_approval_required?' do + context 'when licensed feature is not available' do it 'is falsey' do expect(service.code_owner_approval_required?).to be_falsey end end - describe '#protect_branch?' do - context 'when project has security_policy_project' do - include_context 'has security_policy_project' - - it 'returns true' do - expect(service.protect_branch?).to eq(true) - end - end - - it { expect(service.protect_branch?).to eq(false) } - end - - describe '#push_access_level' do - context 'when project has security_policy_project' do - include_context 'has security_policy_project' - - it 'returns NO_ACCESS access level' do - expect(service.push_access_level).to eq(Gitlab::Access::NO_ACCESS) - end + context 'when licensed feature is available' do + before do + stub_licensed_features(code_owner_approval_required: true) + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protection_none) end - context 'when project does not have security_policy_project' do - before do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + it 'calls code_owner_approval_required? of Gitlab::Access::DefaultBranchProtection and returns correct value', + :aggregate_failures do + expect_next_instance_of(Gitlab::Access::DefaultBranchProtection) do |instance| + expect(instance).to receive(:code_owner_approval_required?) end - it 'returns DEVELOPER access level' do - expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) - end - end - end - - describe '#merge_access_level' do - context 'when project has security_policy_project' do - include_context 'has security_policy_project' - - it 'returns Maintainer access level' do - expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) - end - end - - context 'when project does not have security_policy_project' do - before do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - end - - it 'returns DEVELOPER access level' do - expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) - end + expect(service.code_owner_approval_required?).to be_falsey end end end - context 'when feature flag `default_branch_protection_defaults` is enabled' do - before do - stub_feature_flags(default_branch_protection_defaults: true) - end + describe '#protect_branch?' do + context 'when project has security_policy_project' do + include_context 'has security_policy_project' - describe '#code_owner_approval_required?' do - context 'when licensed feature is not available' do - it 'is falsey' do - expect(service.code_owner_approval_required?).to be_falsey - end + it 'returns true' do + expect(service.protect_branch?).to eq(true) end + end - context 'when licensed feature is available' do - before do - stub_licensed_features(code_owner_approval_required: true) - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protection_none) - end + it { expect(service.protect_branch?).to eq(false) } + end - it 'calls code_owner_approval_required? of Gitlab::Access::DefaultBranchProtection and returns correct value', - :aggregate_failures do - expect_next_instance_of(Gitlab::Access::DefaultBranchProtection) do |instance| - expect(instance).to receive(:code_owner_approval_required?) - end + describe '#push_access_level' do + context 'when project has security_policy_project' do + include_context 'has security_policy_project' - expect(service.code_owner_approval_required?).to be_falsey - end + it 'returns NO_ACCESS access level' do + expect(service.push_access_level).to eq(Gitlab::Access::NO_ACCESS) end end - describe '#protect_branch?' do - context 'when project has security_policy_project' do - include_context 'has security_policy_project' - - it 'returns true' do - expect(service.protect_branch?).to eq(true) - end + context 'when project does not have security_policy_project' do + before do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protection_partial) end - it { expect(service.protect_branch?).to eq(false) } - end - - describe '#push_access_level' do - context 'when project has security_policy_project' do - include_context 'has security_policy_project' - - it 'returns NO_ACCESS access level' do - expect(service.push_access_level).to eq(Gitlab::Access::NO_ACCESS) - end + it 'returns DEVELOPER access level' do + expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) end + end + end - context 'when project does not have security_policy_project' do - before do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protection_partial) - end + describe '#merge_access_level' do + context 'when project has security_policy_project' do + include_context 'has security_policy_project' - it 'returns DEVELOPER access level' do - expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) - end + it 'returns Maintainer access level' do + expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) end end - describe '#merge_access_level' do - context 'when project has security_policy_project' do - include_context 'has security_policy_project' - - it 'returns Maintainer access level' do - expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) - end + context 'when project does not have security_policy_project' do + before do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) end - context 'when project does not have security_policy_project' do - before do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) - end - - it 'returns DEVELOPER access level' do - expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) - end + it 'returns DEVELOPER access level' do + expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) end end end diff --git a/lib/gitlab/github_import/importer/protected_branch_importer.rb b/lib/gitlab/github_import/importer/protected_branch_importer.rb index 0582aa4fdc59c..d5acef78465f1 100644 --- a/lib/gitlab/github_import/importer/protected_branch_importer.rb +++ b/lib/gitlab/github_import/importer/protected_branch_importer.rb @@ -161,11 +161,7 @@ def default_branch_merge_access_level end def default_branch_protection - if Feature.enabled?(:default_branch_protection_defaults, project) - Gitlab::Access::DefaultBranchProtection.new(project.namespace.default_branch_protection_settings) - else - Gitlab::Access::BranchProtection.new(project.namespace.default_branch_protection) - end + Gitlab::Access::DefaultBranchProtection.new(project.namespace.default_branch_protection_settings) end def protected_on_gitlab? diff --git a/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb b/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb index 5803b8c74533e..04d477b9fcb7b 100644 --- a/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/protected_branch_importer_spec.rb @@ -120,58 +120,26 @@ context 'when "allow force pushes - everyone" rule is enabled' do let(:allow_force_pushes_on_github) { true } - context 'when feature flag `default_branch_protection_defaults` is disabled' do + context 'when default branch protection is applied' do before do - stub_feature_flags(default_branch_protection_defaults: false) + stub_application_setting(default_branch_protection_defaults: + Gitlab::Access::BranchProtection.protected_fully) end - context 'when default branch protection is applied' do - before do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_FULL) - end - - let(:expected_allow_force_push) { false } - - it_behaves_like 'create branch protection by the strictest ruleset' - end - - context 'when there is no default branch protection' do - before do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_NONE) - end + let(:expected_allow_force_push) { false } - let(:expected_allow_force_push) { allow_force_pushes_on_github } - - it_behaves_like 'create branch protection by the strictest ruleset' - end + it_behaves_like 'create branch protection by the strictest ruleset' end - context 'when feature flag `default_branch_protection_defaults` is enabled' do + context 'when there is no default branch protection' do before do - stub_feature_flags(default_branch_protection_defaults: true) - end - - context 'when default branch protection is applied' do - before do - stub_application_setting(default_branch_protection_defaults: - Gitlab::Access::BranchProtection.protected_fully) - end - - let(:expected_allow_force_push) { false } - - it_behaves_like 'create branch protection by the strictest ruleset' + stub_application_setting(default_branch_protection_defaults: + Gitlab::Access::BranchProtection.protection_none) end - context 'when there is no default branch protection' do - before do - stub_application_setting(default_branch_protection_defaults: - Gitlab::Access::BranchProtection.protection_none) - end - - let(:expected_allow_force_push) { allow_force_pushes_on_github } + let(:expected_allow_force_push) { allow_force_pushes_on_github } - it_behaves_like 'create branch protection by the strictest ruleset' - end + it_behaves_like 'create branch protection by the strictest ruleset' end end @@ -359,62 +327,28 @@ allow(project).to receive(:default_branch).and_return(branch_name) end - context 'when feature flag `default_branch_protection_defaults` is disabled' do + context 'when default branch protection is partially protected' do before do - stub_feature_flags(default_branch_protection_defaults: false) + stub_application_setting(default_branch_protection_defaults: + Gitlab::Access::BranchProtection.protection_partial) end - context 'when default branch protection = Gitlab::Access::PROTECTION_DEV_CAN_PUSH' do - before do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - end - - let(:expected_push_access_level) { Gitlab::Access::DEVELOPER } - let(:expected_merge_access_level) { Gitlab::Access::MAINTAINER } - - it_behaves_like 'create branch protection by the strictest ruleset' - end - - context 'when default branch protection = Gitlab::Access::PROTECTION_DEV_CAN_MERGE' do - before do - stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - end - - let(:expected_push_access_level) { Gitlab::Access::MAINTAINER } - let(:expected_merge_access_level) { Gitlab::Access::DEVELOPER } + let(:expected_push_access_level) { Gitlab::Access::DEVELOPER } + let(:expected_merge_access_level) { Gitlab::Access::MAINTAINER } - it_behaves_like 'create branch protection by the strictest ruleset' - end + it_behaves_like 'create branch protection by the strictest ruleset' end - context 'when feature flag `default_branch_protection_defaults` is enabled' do + context 'when default branch protection is protected against developer pushes' do before do - stub_feature_flags(default_branch_protection_defaults: true) + stub_application_setting(default_branch_protection_defaults: + Gitlab::Access::BranchProtection.protected_against_developer_pushes) end - context 'when default branch protection is partially protected' do - before do - stub_application_setting(default_branch_protection_defaults: - Gitlab::Access::BranchProtection.protection_partial) - end - - let(:expected_push_access_level) { Gitlab::Access::DEVELOPER } - let(:expected_merge_access_level) { Gitlab::Access::MAINTAINER } + let(:expected_push_access_level) { Gitlab::Access::MAINTAINER } + let(:expected_merge_access_level) { Gitlab::Access::DEVELOPER } - it_behaves_like 'create branch protection by the strictest ruleset' - end - - context 'when default branch protection is protected against developer pushes' do - before do - stub_application_setting(default_branch_protection_defaults: - Gitlab::Access::BranchProtection.protected_against_developer_pushes) - end - - let(:expected_push_access_level) { Gitlab::Access::MAINTAINER } - let(:expected_merge_access_level) { Gitlab::Access::DEVELOPER } - - it_behaves_like 'create branch protection by the strictest ruleset' - end + it_behaves_like 'create branch protection by the strictest ruleset' end end diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 894be0a783179..a5c01656b2904 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -56,60 +56,25 @@ expect(project_access.can_push_to_branch?('master')).to be_truthy end - context 'when feature flag `default_branch_protection_defaults` is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) - end - - context 'when the user is a developer' do - using RSpec::Parameterized::TableSyntax - - before do - empty_project.add_developer(user) - end + context 'when the user is a developer' do + using RSpec::Parameterized::TableSyntax - where(:default_branch_protection_level, :result) do - Gitlab::Access::PROTECTION_NONE | true - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false - Gitlab::Access::PROTECTION_FULL | false - end - - with_them do - it do - expect(empty_project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level).at_least(:once) - - expect(project_access.can_push_to_branch?('master')).to eq(result) - end - end - end - end - - context 'when feature flag `default_branch_protection_defaults` is enabled' do before do - stub_feature_flags(default_branch_protection_defaults: true) + empty_project.add_developer(user) end - context 'when the user is a developer' do - using RSpec::Parameterized::TableSyntax - - before do - empty_project.add_developer(user) - end - - where(:default_branch_protection_level, :result) do - Gitlab::Access::BranchProtection.protection_none | true - Gitlab::Access::BranchProtection.protection_partial | true - Gitlab::Access::BranchProtection.protected_against_developer_pushes | false - Gitlab::Access::BranchProtection.protected_fully | false - end + where(:default_branch_protection_level, :result) do + Gitlab::Access::BranchProtection.protection_none | true + Gitlab::Access::BranchProtection.protection_partial | true + Gitlab::Access::BranchProtection.protected_against_developer_pushes | false + Gitlab::Access::BranchProtection.protected_fully | false + end - with_them do - it do - expect(empty_project.namespace).to receive(:default_branch_protection_settings).and_return(default_branch_protection_level).at_least(:once) + with_them do + it do + expect(empty_project.namespace).to receive(:default_branch_protection_settings).and_return(default_branch_protection_level).at_least(:once) - expect(project_access.can_push_to_branch?('master')).to eq(result) - end + expect(project_access.can_push_to_branch?('master')).to eq(result) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index c79d13312a81a..23363a3b79e9f 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2959,111 +2959,53 @@ def has_external_wiki end end - context 'when feature flag `default_branch_protection_defaults` is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) - end - - describe '#default_branch_protected?' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:project) { create(:project, namespace: namespace) } - - subject { project.default_branch_protected? } - - where(:default_branch_protection_level, :result) do - Gitlab::Access::PROTECTION_NONE | false - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true - Gitlab::Access::PROTECTION_FULL | true - Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | true - end + describe '#default_branch_protected?' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } - with_them do - before do - expect(project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level) - end + subject { project.default_branch_protected? } - it { is_expected.to eq(result) } - end + where(:default_branch_protection_level, :result) do + Gitlab::Access::BranchProtection.protection_none | false + Gitlab::Access::BranchProtection.protection_partial | false + Gitlab::Access::BranchProtection.protected_against_developer_pushes | true + Gitlab::Access::BranchProtection.protected_fully | true + Gitlab::Access::BranchProtection.protected_after_initial_push | true end - describe 'initial_push_to_default_branch_allowed_for_developer?' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:project) { create(:project, namespace: namespace) } - - subject { project.initial_push_to_default_branch_allowed_for_developer? } - - where(:default_branch_protection_level, :result) do - Gitlab::Access::PROTECTION_NONE | true - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | true - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | false - Gitlab::Access::PROTECTION_FULL | false - Gitlab::Access::PROTECTION_DEV_CAN_INITIAL_PUSH | true + with_them do + before do + expect(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(default_branch_protection_level) end - with_them do - before do - expect(project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level) - end - - it { is_expected.to eq(result) } - end + it { is_expected.to eq(result) } end end - context 'when feature flag `default_branch_protection_defaults` is enabled' do - before do - stub_feature_flags(default_branch_protection_defaults: true) - end - - describe '#default_branch_protected?' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:project) { create(:project, namespace: namespace) } - - subject { project.default_branch_protected? } + describe 'initial_push_to_default_branch_allowed_for_developer?' do + let_it_be(:namespace) { create(:namespace) } + let_it_be(:project) { create(:project, namespace: namespace) } - where(:default_branch_protection_level, :result) do - Gitlab::Access::BranchProtection.protection_none | false - Gitlab::Access::BranchProtection.protection_partial | false - Gitlab::Access::BranchProtection.protected_against_developer_pushes | true - Gitlab::Access::BranchProtection.protected_fully | true - Gitlab::Access::BranchProtection.protected_after_initial_push | true - end + subject { project.initial_push_to_default_branch_allowed_for_developer? } - with_them do - before do - expect(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(default_branch_protection_level) - end - - it { is_expected.to eq(result) } - end + where(:default_branch_protection_level, :result) do + Gitlab::Access::BranchProtection.protection_none | true + Gitlab::Access::BranchProtection.protection_partial | true + Gitlab::Access::BranchProtection.protected_against_developer_pushes | false + Gitlab::Access::BranchProtection.protected_fully | false + Gitlab::Access::BranchProtection.protected_after_initial_push | true end - describe 'initial_push_to_default_branch_allowed_for_developer?' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:project) { create(:project, namespace: namespace) } - - subject { project.initial_push_to_default_branch_allowed_for_developer? } - - where(:default_branch_protection_level, :result) do - Gitlab::Access::BranchProtection.protection_none | true - Gitlab::Access::BranchProtection.protection_partial | true - Gitlab::Access::BranchProtection.protected_against_developer_pushes | false - Gitlab::Access::BranchProtection.protected_fully | false - Gitlab::Access::BranchProtection.protected_after_initial_push | true + with_them do + before do + expect(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(default_branch_protection_level) end - with_them do - before do - expect(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(default_branch_protection_level) - end - - it { is_expected.to eq(result) } - end + it { is_expected.to eq(result) } end end @@ -6074,108 +6016,50 @@ def has_external_wiki end end - context 'when feature flag `default_branch_protection_defaults` is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) - end - - context 'branch protection' do - let_it_be(:namespace) { create(:namespace) } - let_it_be(:project) { create(:project, :repository, namespace: namespace) } - - before do - create(:import_state, :started, project: project) - end - - it 'does not protect when branch protection is disabled' do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_NONE) - - project.after_import - - expect(project.protected_branches).to be_empty - end - - it "gives developer access to push when branch protection is set to 'developers can push'" do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - project.after_import - - expect(project.protected_branches).not_to be_empty - expect(project.default_branch).to eq(project.protected_branches.first.name) - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end - - it "gives developer access to merge when branch protection is set to 'developers can merge'" do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - project.after_import - - expect(project.protected_branches).not_to be_empty - expect(project.default_branch).to eq(project.protected_branches.first.name) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end - - it 'protects default branch' do - project.after_import + context 'branch protection' do + let_it_be(:namespace) { create(:namespace) } - expect(project.protected_branches).not_to be_empty - expect(project.default_branch).to eq(project.protected_branches.first.name) - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - end - end - end + let_it_be(:project) { create(:project, :repository, namespace: namespace) } - context 'when feature flag `default_branch_protection_defaults` is enabled' do before do - stub_feature_flags(default_branch_protection_defaults: true) + create(:import_state, :started, project: project) end - context 'branch protection' do - let_it_be(:namespace) { create(:namespace) } - - let_it_be(:project) { create(:project, :repository, namespace: namespace) } + it 'does not protect when branch protection is disabled' do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_none) - before do - create(:import_state, :started, project: project) - end - - it 'does not protect when branch protection is disabled' do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_none) - - project.after_import + project.after_import - expect(project.protected_branches).to be_empty - end + expect(project.protected_branches).to be_empty + end - it "gives developer access to push when branch protection is set to 'developers can push'" do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_partial) + it "gives developer access to push when branch protection is set to 'developers can push'" do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_partial) - project.after_import + project.after_import - expect(project.protected_branches).not_to be_empty - expect(project.default_branch).to eq(project.protected_branches.first.name) - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end - it "gives developer access to merge when branch protection is set to 'developers can merge'" do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + it "gives developer access to merge when branch protection is set to 'developers can merge'" do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) - project.after_import + project.after_import - expect(project.protected_branches).not_to be_empty - expect(project.default_branch).to eq(project.protected_branches.first.name) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end - it 'protects default branch' do - project.after_import + it 'protects default branch' do + project.after_import - expect(project.protected_branches).not_to be_empty - expect(project.default_branch).to eq(project.protected_branches.first.name) - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - end + expect(project.protected_branches).not_to be_empty + expect(project.default_branch).to eq(project.protected_branches.first.name) + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index 5923445630c33..d1f2a55577ad5 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -295,86 +295,38 @@ let(:project) { create(:project) } - context 'when feature flag `default_branch_protection_defaults` is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) + context 'when the group has set their own default_branch_protection level' do + where(:default_branch_protection_level, :result) do + Gitlab::Access::BranchProtection.protection_none | false + Gitlab::Access::BranchProtection.protection_partial | false + Gitlab::Access::BranchProtection.protected_against_developer_pushes | true + Gitlab::Access::BranchProtection.protected_fully | true end - context 'when the group has set their own default_branch_protection level' do - where(:default_branch_protection_level, :result) do - Gitlab::Access::PROTECTION_NONE | false - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true - Gitlab::Access::PROTECTION_FULL | true - end - - with_them do - it 'protects the default branch based on the default branch protection setting of the group' do - expect(project.namespace).to receive(:default_branch_protection).and_return(default_branch_protection_level) - - expect(described_class.protected?(project, 'master')).to eq(result) - end - end - end - - context 'when the group has not set their own default_branch_protection level' do - where(:default_branch_protection_level, :result) do - Gitlab::Access::PROTECTION_NONE | false - Gitlab::Access::PROTECTION_DEV_CAN_PUSH | false - Gitlab::Access::PROTECTION_DEV_CAN_MERGE | true - Gitlab::Access::PROTECTION_FULL | true - end - - with_them do - before do - stub_application_setting(default_branch_protection: default_branch_protection_level) - end + with_them do + it 'protects the default branch based on the default branch protection setting of the group' do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(default_branch_protection_level) - it 'protects the default branch based on the instance level default branch protection setting' do - expect(described_class.protected?(project, 'master')).to eq(result) - end + expect(described_class.protected?(project, 'master')).to eq(result) end end end - context 'when feature flag `default_branch_protection_defaults` is enabled' do - before do - stub_feature_flags(default_branch_protection_defaults: true) + context 'when the group has not set their own default_branch_protection level' do + where(:default_branch_protection_level, :result) do + Gitlab::Access::BranchProtection.protection_none | false + Gitlab::Access::BranchProtection.protection_partial | false + Gitlab::Access::BranchProtection.protected_against_developer_pushes | true + Gitlab::Access::BranchProtection.protected_fully | true end - context 'when the group has set their own default_branch_protection level' do - where(:default_branch_protection_level, :result) do - Gitlab::Access::BranchProtection.protection_none | false - Gitlab::Access::BranchProtection.protection_partial | false - Gitlab::Access::BranchProtection.protected_against_developer_pushes | true - Gitlab::Access::BranchProtection.protected_fully | true - end - - with_them do - it 'protects the default branch based on the default branch protection setting of the group' do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(default_branch_protection_level) - - expect(described_class.protected?(project, 'master')).to eq(result) - end - end - end - - context 'when the group has not set their own default_branch_protection level' do - where(:default_branch_protection_level, :result) do - Gitlab::Access::BranchProtection.protection_none | false - Gitlab::Access::BranchProtection.protection_partial | false - Gitlab::Access::BranchProtection.protected_against_developer_pushes | true - Gitlab::Access::BranchProtection.protected_fully | true + with_them do + before do + stub_application_setting(default_branch_protection_defaults: default_branch_protection_level) end - with_them do - before do - stub_application_setting(default_branch_protection_defaults: default_branch_protection_level) - end - - it 'protects the default branch based on the instance level default branch protection setting' do - expect(described_class.protected?(project, 'master')).to eq(result) - end + it 'protects the default branch based on the instance level default branch protection setting' do + expect(described_class.protected?(project, 'master')).to eq(result) end end end diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb index 2e9e810f5d30c..7e7db9a59ae49 100644 --- a/spec/services/git/branch_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -177,112 +177,52 @@ expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) end - context 'when feature flag `default_branch_protection_defaults` is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) - end - - it "with default branch protection disabled" do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_NONE) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - subject - expect(project.protected_branches).to be_empty - end - - it "with default branch protection set to 'developers can push'" do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - - subject - - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - end - - it "with an existing branch permission configured" do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + it "with default branch protection disabled" do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_none) - create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - expect(ProtectedBranches::CreateService).not_to receive(:new) - - subject - - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end - - it "with default branch protection set to 'developers can merge'" do - expect(project.namespace).to receive(:default_branch_protection).and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - subject - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + subject + expect(project.protected_branches).to be_empty end - context 'when feature flag `default_branch_protection_defaults` is enabled' do - before do - stub_feature_flags(default_branch_protection_defaults: true) - end - - it "with default branch protection disabled" do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_none) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - subject - expect(project.protected_branches).to be_empty - end + it "with default branch protection set to 'developers can push'" do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_partial) - it "with default branch protection set to 'developers can push'" do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_partial) - - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") - subject + subject - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - end + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + end - it "with an existing branch permission configured" do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_partial) + it "with an existing branch permission configured" do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protection_partial) - create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - expect(ProtectedBranches::CreateService).not_to receive(:new) + create(:protected_branch, :no_one_can_push, :developers_can_merge, project: project, name: 'master') + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + expect(ProtectedBranches::CreateService).not_to receive(:new) - subject + subject - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) - expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) + expect(project.protected_branches.last.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) + end - it "with default branch protection set to 'developers can merge'" do - expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + it "with default branch protection set to 'developers can merge'" do + expect(project.namespace).to receive(:default_branch_protection_settings).and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) - expect(project).to receive(:execute_hooks) - expect(project.default_branch).to eq("master") - subject - expect(project.protected_branches).not_to be_empty - expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) - expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) - end + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + subject + expect(project.protected_branches).not_to be_empty + expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) + expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) end end diff --git a/spec/services/projects/protect_default_branch_service_spec.rb b/spec/services/projects/protect_default_branch_service_spec.rb index ed9d2ede38624..79eb0487ff3c3 100644 --- a/spec/services/projects/protect_default_branch_service_spec.rb +++ b/spec/services/projects/protect_default_branch_service_spec.rb @@ -6,688 +6,345 @@ let(:service) { described_class.new(project) } let(:project) { create(:project) } - context 'when feature flag `default_branch_protection_defaults` is disabled' do + describe '#execute' do before do - stub_feature_flags(default_branch_protection_defaults: false) + allow(service) + .to receive(:protect_default_branch) end - describe '#execute' do - before do + context 'without a default branch' do + it 'does nothing' do allow(service) - .to receive(:protect_default_branch) - end - - context 'without a default branch' do - it 'does nothing' do - allow(service) - .to receive(:default_branch) - .and_return(nil) - - service.execute - - expect(service) - .not_to have_received(:protect_default_branch) - end - end - - context 'with a default branch' do - it 'protects the default branch' do - allow(service) - .to receive(:default_branch) - .and_return('master') + .to receive(:default_branch) + .and_return(nil) - service.execute + service.execute - expect(service) - .to have_received(:protect_default_branch) - end + expect(service) + .not_to have_received(:protect_default_branch) end end - describe '#protect_default_branch' do - before do + context 'with a default branch' do + it 'protects the default branch' do allow(service) .to receive(:default_branch) .and_return('master') - allow(project) - .to receive(:change_head) - .with('master') - - allow(service) - .to receive(:create_protected_branch) - end - - context 'when branch protection is needed' do - before do - allow(service) - .to receive(:protect_branch?) - .and_return(true) - - allow(service) - .to receive(:create_protected_branch) - end + service.execute - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'protects the default branch' do - service.protect_default_branch - - expect(service) - .to have_received(:create_protected_branch) - end - end - - context 'when branch protection is not needed' do - before do - allow(service) - .to receive(:protect_branch?) - .and_return(false) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'does not protect the default branch' do - service.protect_default_branch - - expect(service) - .not_to have_received(:create_protected_branch) - end - end - - context 'when protected branch does not exist' do - before do - allow(service) - .to receive(:protected_branch_exists?) - .and_return(false) - allow(service) - .to receive(:protect_branch?) - .and_return(true) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'protects the default branch' do - service.protect_default_branch - - expect(service) - .to have_received(:create_protected_branch) - end - end - - context 'when protected branch already exists' do - before do - allow(service) - .to receive(:protected_branch_exists?) - .and_return(true) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'does not protect the default branch' do - service.protect_default_branch - - expect(service) - .not_to have_received(:create_protected_branch) - end + expect(service) + .to have_received(:protect_default_branch) end end + end - describe '#create_protected_branch' do - it 'creates the protected branch' do - creator = instance_spy(User) - create_service = instance_spy(ProtectedBranches::CreateService) - access_level = Gitlab::Access::DEVELOPER - params = { - name: 'master', - push_access_levels_attributes: [{ access_level: access_level }], - merge_access_levels_attributes: [{ access_level: access_level }], - code_owner_approval_required: false, - allow_force_push: false - } - - allow(project) - .to receive(:creator) - .and_return(creator) - - allow(ProtectedBranches::CreateService) - .to receive(:new) - .with(project, creator, params) - .and_return(create_service) - - allow(service) - .to receive(:push_access_level) - .and_return(access_level) + describe '#protect_default_branch' do + before do + allow(service) + .to receive(:default_branch) + .and_return('master') - allow(service) - .to receive(:merge_access_level) - .and_return(access_level) + allow(project) + .to receive(:change_head) + .with('master') - allow(service) - .to receive(:code_owner_approval_required?) - .and_return(false) + allow(service) + .to receive(:create_protected_branch) + end + context 'when branch protection is needed' do + before do allow(service) - .to receive(:allow_force_push?) - .and_return(false) + .to receive(:protect_branch?) + .and_return(true) allow(service) - .to receive(:default_branch) - .and_return('master') - - allow(create_service) - .to receive(:execute) - .with(skip_authorization: true) - - service.create_protected_branch - - expect(create_service) - .to have_received(:execute) + .to receive(:create_protected_branch) end - end - describe '#protect_branch?' do - context 'when default branch protection is disabled' do - it 'returns false' do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_NONE) + it 'changes the HEAD of the project' do + service.protect_default_branch - expect(service.protect_branch?).to eq(false) - end + expect(project) + .to have_received(:change_head) end - context 'when default branch protection is enabled' do - before do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - allow(service) - .to receive(:default_branch) - .and_return('master') - end - - it 'returns false if the branch is already protected' do - allow(ProtectedBranch) - .to receive(:protected?) - .with(project, 'master') - .and_return(true) - - expect(service.protect_branch?).to eq(false) - end - - it 'returns true if the branch is not yet protected' do - allow(ProtectedBranch) - .to receive(:protected?) - .with(project, 'master') - .and_return(false) - - expect(service.protect_branch?).to eq(true) - end + it 'protects the default branch' do + service.protect_default_branch + + expect(service) + .to have_received(:create_protected_branch) end end - describe '#protected_branch_exists?' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - - let(:default_branch) { "default-branch" } - + context 'when branch protection is not needed' do before do - allow(project).to receive(:default_branch).and_return(default_branch) - create(:protected_branch, project: nil, group: group, name: default_branch) + allow(service) + .to receive(:protect_branch?) + .and_return(false) end - context 'when feature flag `group_protected_branches` disabled' do - before do - stub_feature_flags(group_protected_branches: false) - stub_feature_flags(allow_protected_branches_for_group: false) - end + it 'changes the HEAD of the project' do + service.protect_default_branch - it 'return false' do - expect(service.protected_branch_exists?).to eq(false) - end + expect(project) + .to have_received(:change_head) end - context 'when feature flag `group_protected_branches` enabled' do - before do - stub_feature_flags(group_protected_branches: true) - stub_feature_flags(allow_protected_branches_for_group: true) - end + it 'does not protect the default branch' do + service.protect_default_branch - it 'return true' do - expect(service.protected_branch_exists?).to eq(true) - end + expect(service) + .not_to have_received(:create_protected_branch) end end - describe '#default_branch' do - it 'returns the default branch of the project' do - allow(project) - .to receive(:default_branch) - .and_return('master') - - expect(service.default_branch).to eq('master') + context 'when protected branch does not exist' do + before do + allow(service) + .to receive(:protected_branch_exists?) + .and_return(false) + allow(service) + .to receive(:protect_branch?) + .and_return(true) end - end - describe '#push_access_level' do - context 'when developers can push' do - it 'returns the DEVELOPER access level' do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + it 'changes the HEAD of the project' do + service.protect_default_branch - expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) - end + expect(project) + .to have_received(:change_head) end - context 'when developers can not push' do - it 'returns the MAINTAINER access level' do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + it 'protects the default branch' do + service.protect_default_branch - expect(service.push_access_level).to eq(Gitlab::Access::MAINTAINER) - end + expect(service) + .to have_received(:create_protected_branch) end end - describe '#merge_access_level' do - context 'when developers can merge' do - it 'returns the DEVELOPER access level' do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_MERGE) - - expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) - end + context 'when protected branch already exists' do + before do + allow(service) + .to receive(:protected_branch_exists?) + .and_return(true) end - context 'when developers can not merge' do - it 'returns the MAINTAINER access level' do - allow(project.namespace) - .to receive(:default_branch_protection) - .and_return(Gitlab::Access::PROTECTION_DEV_CAN_PUSH) + it 'changes the HEAD of the project' do + service.protect_default_branch - expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) - end + expect(project) + .to have_received(:change_head) end - end - describe '#allow_force_push?' do - it 'is falsey' do - expect(service.allow_force_push?).to be_falsey + it 'does not protect the default branch' do + service.protect_default_branch + + expect(service) + .not_to have_received(:create_protected_branch) end end end - context 'when feature flag `default_branch_protection_defaults` is enabled' do - before do - stub_feature_flags(default_branch_protection_defaults: true) + describe '#create_protected_branch' do + it 'creates the protected branch' do + creator = instance_spy(User) + create_service = instance_spy(ProtectedBranches::CreateService) + access_level = Gitlab::Access::DEVELOPER + params = { + name: 'master', + push_access_levels_attributes: [{ access_level: access_level }], + merge_access_levels_attributes: [{ access_level: access_level }], + code_owner_approval_required: false, + allow_force_push: false + } + + allow(project) + .to receive(:creator) + .and_return(creator) + + allow(ProtectedBranches::CreateService) + .to receive(:new) + .with(project, creator, params) + .and_return(create_service) + + allow(service) + .to receive(:push_access_level) + .and_return(access_level) + + allow(service) + .to receive(:merge_access_level) + .and_return(access_level) + + allow(service) + .to receive(:default_branch) + .and_return('master') + + allow(service) + .to receive(:code_owner_approval_required?) + .and_return(false) + + allow(service) + .to receive(:allow_force_push?) + .and_return(false) + + allow(create_service) + .to receive(:execute) + .with(skip_authorization: true) + + service.create_protected_branch + + expect(create_service) + .to have_received(:execute) end + end - describe '#execute' do - before do - allow(service) - .to receive(:protect_default_branch) - end - - context 'without a default branch' do - it 'does nothing' do - allow(service) - .to receive(:default_branch) - .and_return(nil) - - service.execute - - expect(service) - .not_to have_received(:protect_default_branch) - end - end - - context 'with a default branch' do - it 'protects the default branch' do - allow(service) - .to receive(:default_branch) - .and_return('master') - - service.execute + describe '#protect_branch?' do + context 'when default branch protection is disabled' do + it 'returns false' do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protection_none) - expect(service) - .to have_received(:protect_default_branch) - end + expect(service.protect_branch?).to eq(false) end end - describe '#protect_default_branch' do + context 'when default branch protection is enabled' do before do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + allow(service) .to receive(:default_branch) .and_return('master') - - allow(project) - .to receive(:change_head) - .with('master') - - allow(service) - .to receive(:create_protected_branch) end - context 'when branch protection is needed' do - before do - allow(service) - .to receive(:protect_branch?) - .and_return(true) - - allow(service) - .to receive(:create_protected_branch) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'protects the default branch' do - service.protect_default_branch - - expect(service) - .to have_received(:create_protected_branch) - end - end - - context 'when branch protection is not needed' do - before do - allow(service) - .to receive(:protect_branch?) - .and_return(false) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'does not protect the default branch' do - service.protect_default_branch + it 'returns false if the branch is already protected' do + allow(ProtectedBranch) + .to receive(:protected?) + .with(project, 'master') + .and_return(true) - expect(service) - .not_to have_received(:create_protected_branch) - end + expect(service.protect_branch?).to eq(false) end - context 'when protected branch does not exist' do - before do - allow(service) - .to receive(:protected_branch_exists?) - .and_return(false) - allow(service) - .to receive(:protect_branch?) - .and_return(true) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'protects the default branch' do - service.protect_default_branch - - expect(service) - .to have_received(:create_protected_branch) - end - end - - context 'when protected branch already exists' do - before do - allow(service) - .to receive(:protected_branch_exists?) - .and_return(true) - end - - it 'changes the HEAD of the project' do - service.protect_default_branch - - expect(project) - .to have_received(:change_head) - end - - it 'does not protect the default branch' do - service.protect_default_branch + it 'returns true if the branch is not yet protected' do + allow(ProtectedBranch) + .to receive(:protected?) + .with(project, 'master') + .and_return(false) - expect(service) - .not_to have_received(:create_protected_branch) - end + expect(service.protect_branch?).to eq(true) end end + end - describe '#create_protected_branch' do - it 'creates the protected branch' do - creator = instance_spy(User) - create_service = instance_spy(ProtectedBranches::CreateService) - access_level = Gitlab::Access::DEVELOPER - params = { - name: 'master', - push_access_levels_attributes: [{ access_level: access_level }], - merge_access_levels_attributes: [{ access_level: access_level }], - code_owner_approval_required: false, - allow_force_push: false - } - - allow(project) - .to receive(:creator) - .and_return(creator) - - allow(ProtectedBranches::CreateService) - .to receive(:new) - .with(project, creator, params) - .and_return(create_service) - - allow(service) - .to receive(:push_access_level) - .and_return(access_level) - - allow(service) - .to receive(:merge_access_level) - .and_return(access_level) - - allow(service) - .to receive(:default_branch) - .and_return('master') - - allow(service) - .to receive(:code_owner_approval_required?) - .and_return(false) - - allow(service) - .to receive(:allow_force_push?) - .and_return(false) - - allow(create_service) - .to receive(:execute) - .with(skip_authorization: true) + describe '#protected_branch_exists?' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } - service.create_protected_branch + let(:default_branch) { "default-branch" } - expect(create_service) - .to have_received(:execute) - end + before do + allow(project).to receive(:default_branch).and_return(default_branch) + create(:protected_branch, project: nil, group: group, name: default_branch) end - describe '#protect_branch?' do - context 'when default branch protection is disabled' do - it 'returns false' do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protection_none) - - expect(service.protect_branch?).to eq(false) - end + context 'when feature flag `group_protected_branches` disabled' do + before do + stub_feature_flags(group_protected_branches: false) + stub_feature_flags(allow_protected_branches_for_group: false) end - context 'when default branch protection is enabled' do - before do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) - - allow(service) - .to receive(:default_branch) - .and_return('master') - end - - it 'returns false if the branch is already protected' do - allow(ProtectedBranch) - .to receive(:protected?) - .with(project, 'master') - .and_return(true) - - expect(service.protect_branch?).to eq(false) - end - - it 'returns true if the branch is not yet protected' do - allow(ProtectedBranch) - .to receive(:protected?) - .with(project, 'master') - .and_return(false) - - expect(service.protect_branch?).to eq(true) - end + it 'return false' do + expect(service.protected_branch_exists?).to eq(false) end end - describe '#protected_branch_exists?' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - - let(:default_branch) { "default-branch" } - + context 'when feature flag `group_protected_branches` enabled' do before do - allow(project).to receive(:default_branch).and_return(default_branch) - create(:protected_branch, project: nil, group: group, name: default_branch) - end - - context 'when feature flag `group_protected_branches` disabled' do - before do - stub_feature_flags(group_protected_branches: false) - stub_feature_flags(allow_protected_branches_for_group: false) - end - - it 'return false' do - expect(service.protected_branch_exists?).to eq(false) - end + stub_feature_flags(group_protected_branches: true) + stub_feature_flags(allow_protected_branches_for_group: true) end - context 'when feature flag `group_protected_branches` enabled' do - before do - stub_feature_flags(group_protected_branches: true) - stub_feature_flags(allow_protected_branches_for_group: true) - end - - it 'return true' do - expect(service.protected_branch_exists?).to eq(true) - end + it 'return true' do + expect(service.protected_branch_exists?).to eq(true) end end + end - describe '#default_branch' do - it 'returns the default branch of the project' do - allow(project) - .to receive(:default_branch) - .and_return('master') + describe '#default_branch' do + it 'returns the default branch of the project' do + allow(project) + .to receive(:default_branch) + .and_return('master') - expect(service.default_branch).to eq('master') - end + expect(service.default_branch).to eq('master') end + end - describe '#push_access_level' do - context 'when developers can push' do - it 'returns the DEVELOPER access level' do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protection_partial) - - expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) - end - end - - context 'when developers can not push' do - it 'returns the MAINTAINER access level' do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + describe '#push_access_level' do + context 'when developers can push' do + it 'returns the DEVELOPER access level' do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protection_partial) - expect(service.push_access_level).to eq(Gitlab::Access::MAINTAINER) - end + expect(service.push_access_level).to eq(Gitlab::Access::DEVELOPER) end end - describe '#merge_access_level' do - context 'when developers can merge' do - it 'returns the DEVELOPER access level' do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + context 'when developers can not push' do + it 'returns the MAINTAINER access level' do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) - expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) - end + expect(service.push_access_level).to eq(Gitlab::Access::MAINTAINER) end + end + end - context 'when developers can not merge' do - it 'returns the MAINTAINER access level' do - allow(project.namespace) - .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protection_partial) + describe '#merge_access_level' do + context 'when developers can merge' do + it 'returns the DEVELOPER access level' do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) - expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) - end + expect(service.merge_access_level).to eq(Gitlab::Access::DEVELOPER) end end - describe '#allow_force_push?' do - before do + context 'when developers can not merge' do + it 'returns the MAINTAINER access level' do allow(project.namespace) .to receive(:default_branch_protection_settings) - .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + .and_return(Gitlab::Access::BranchProtection.protection_partial) + + expect(service.merge_access_level).to eq(Gitlab::Access::MAINTAINER) end + end + end - it 'calls allow_force_push? method of Gitlab::Access::DefaultBranchProtection and returns correct value', - :aggregate_failures do - expect_next_instance_of(Gitlab::Access::DefaultBranchProtection) do |instance| - expect(instance).to receive(:allow_force_push?) - end + describe '#allow_force_push?' do + before do + allow(project.namespace) + .to receive(:default_branch_protection_settings) + .and_return(Gitlab::Access::BranchProtection.protected_against_developer_pushes) + end - expect(service.allow_force_push?).to be_falsey + it 'calls allow_force_push? method of Gitlab::Access::DefaultBranchProtection and returns correct value', + :aggregate_failures do + expect_next_instance_of(Gitlab::Access::DefaultBranchProtection) do |instance| + expect(instance).to receive(:allow_force_push?) end + + expect(service.allow_force_push?).to be_falsey end end diff --git a/spec/views/groups/settings/repository/_default_branch.html.haml_spec.rb b/spec/views/groups/settings/repository/_default_branch.html.haml_spec.rb index 0c6c3bc10e33c..b6bfca322b886 100644 --- a/spec/views/groups/settings/repository/_default_branch.html.haml_spec.rb +++ b/spec/views/groups/settings/repository/_default_branch.html.haml_spec.rb @@ -19,26 +19,9 @@ allow(group).to receive(:default_branch_protection).and_return({}) end - context 'when feature flag default_branch_protection_defaults is enabled' do - before do - stub_feature_flags(default_branch_protection_defaults: true) - end - - it 'renders default branch protection defaults correctly' do - render - expect(rendered).to render_template(partial: 'groups/settings/_default_branch_protection_defaults') - end - end - - context 'when feature flag default_branch_protection_defaults is disabled' do - before do - stub_feature_flags(default_branch_protection_defaults: false) - end - - it 'renders default branch protection settings correctly' do - render - expect(rendered).to render_template(partial: 'groups/settings/_default_branch_protection') - end + it 'renders default branch protection defaults correctly' do + render + expect(rendered).to render_template(partial: 'groups/settings/_default_branch_protection_defaults') end end end -- GitLab