Merge branch 'small-corrections-to-group-push-rules' into 'master'

Add small alterations for better readability for group push rules

See merge request gitlab-org/gitlab!33163

ee/app/policies/ee/project_policy.rb

@@ -99,7 +99,13 @@ module EE
         end
       end
 
-      with_scope :subject
+      condition(:can_change_reject_unsigned_commits) do
+        admin? ||
+          (can?(:maintainer_access) &&
+            reject_unsigned_commits_disabled_globally? &&
+            reject_unsigned_commits_disabled_by_group?)
+      end
+
       condition(:commit_committer_check_disabled_by_group) do
         if group_push_rule_present?
           !subject.group.push_rule.commit_committer_check
@@ -113,6 +119,13 @@ module EE
         @subject.feature_available?(:commit_committer_check)
       end
 
+      condition(:can_change_commit_commiter_check) do
+        admin? ||
+          (can?(:maintainer_access) &&
+            commit_committer_check_disabled_globally? &&
+            commit_committer_check_disabled_by_group?)
+      end
+
       with_scope :subject
       condition(:reject_unsigned_commits_available) do
         @subject.feature_available?(:reject_unsigned_commits)
@@ -301,23 +314,17 @@ module EE
 
       rule { ~can?(:push_code) }.prevent :push_code_to_protected_branches
 
-      rule { admin | (reject_unsigned_commits_disabled_globally & reject_unsigned_commits_disabled_by_group & can?(:maintainer_access)) }.enable :change_reject_unsigned_commits
+      rule { can_change_reject_unsigned_commits }.enable :change_reject_unsigned_commits
 
       rule { reject_unsigned_commits_available }.enable :read_reject_unsigned_commits
 
       rule { ~reject_unsigned_commits_available }.prevent :change_reject_unsigned_commits
 
-      rule { admin | (commit_committer_check_disabled_globally & commit_committer_check_disabled_by_group & can?(:maintainer_access)) }.policy do
-        enable :change_commit_committer_check
-      end
+      rule { can_change_commit_commiter_check }.enable :change_commit_committer_check
 
-      rule { commit_committer_check_available }.policy do
-        enable :read_commit_committer_check
-      end
+      rule { commit_committer_check_available }.enable :read_commit_committer_check
 
-      rule { ~commit_committer_check_available }.policy do
-        prevent :change_commit_committer_check
-      end
+      rule { ~commit_committer_check_available }.prevent :change_commit_committer_check
 
       rule { owner | reporter }.enable :build_read_project
 

ee/app/views/groups/push_rules/edit.html.haml

-- page_title "Push Rules"
+- page_title _("Push Rules")
 %h3
   = _("Pre-defined push rules.")
 %p.light

ee/spec/controllers/projects/push_rules_controller_spec.rb

@@ -39,8 +39,8 @@ describe Projects::PushRulesController do
     shared_examples 'updateable setting' do |rule_attr, updates, new_value|
       it "#{updates ? 'updates' : 'does not update'} the setting" do
         patch :update, params: { namespace_id: project.namespace, project_id: project, id: 1, push_rule: { rule_attr => new_value } }
-
         be_new, be_old = new_value ? [be_truthy, be_falsy] : [be_falsy, be_truthy]
+
         expect(project.reload_push_rule.public_send(rule_attr)).to(updates ? be_new : be_old)
       end
     end

ee/spec/services/projects/create_service_spec.rb

@@ -223,67 +223,59 @@ describe Projects::CreateService, '#execute' do
       end
     end
 
-    context 'group push rules' do
-      before do
-        stub_licensed_features(push_rules: true)
+    context 'when there are no push rules' do
+      it 'does not create push rule' do
+        expect(create_project(user, opts).push_rule).to be_nil
       end
+    end
+  end
 
-      context 'project created within a group' do
-        let(:group) { create(:group) }
-        let(:opts) do
-          {
-            name: "GitLab",
-            namespace_id: group.id
-          }
-        end
+  context 'group push rules' do
+    before do
+      stub_licensed_features(push_rules: true)
+    end
 
-        before do
-          group.add_owner(user)
-        end
+    context 'project created within a group' do
+      let(:group) { create(:group) }
+      let(:opts) do
+        {
+          name: "GitLab",
+          namespace_id: group.id
+        }
+      end
 
-        context 'when group has push rule defined' do
-          let(:push_rule) { create(:push_rule_without_project, force_push_regex: 'testing me') }
+      before do
+        group.add_owner(user)
+      end
 
-          before do
-            group.update!(push_rule: push_rule)
-            group.add_owner(user)
-          end
+      context 'when group has push rule defined' do
+        let(:group_push_rule) { create(:push_rule_without_project, force_push_regex: 'testing me') }
 
-          it 'creates push rule from group push rule' do
-            project = create_project(user, opts)
-            project_push_rule = project.push_rule
-
-            expect(project_push_rule).to have_attributes(
-              force_push_regex: push_rule.force_push_regex,
-              deny_delete_tag: push_rule.deny_delete_tag,
-              delete_branch_regex: push_rule.delete_branch_regex,
-              commit_message_regex: push_rule.commit_message_regex,
-              is_sample: false
-            )
-            expect(project.project_setting.push_rule_id).to eq(project_push_rule.id)
-          end
+        before do
+          group.update!(push_rule: group_push_rule)
+        end
 
-          context 'when feature flag is switched off' do
-            let!(:sample) { create(:push_rule_sample) }
-
-            before do
-              stub_feature_flags(group_push_rules: false)
-            end
-
-            it 'creates push rule from sample' do
-              expect(create_project(user, opts).push_rule).to have_attributes(
-                force_push_regex: sample.force_push_regex,
-                deny_delete_tag: sample.deny_delete_tag,
-                delete_branch_regex: sample.delete_branch_regex,
-                commit_message_regex: sample.commit_message_regex
-              )
-            end
-          end
+        it 'creates push rule from group push rule' do
+          project = create_project(user, opts)
+          project_push_rule = project.push_rule
+
+          expect(project_push_rule).to have_attributes(
+            force_push_regex: group_push_rule.force_push_regex,
+            deny_delete_tag: group_push_rule.deny_delete_tag,
+            delete_branch_regex: group_push_rule.delete_branch_regex,
+            commit_message_regex: group_push_rule.commit_message_regex,
+            is_sample: false
+          )
+          expect(project.project_setting.push_rule_id).to eq(project_push_rule.id)
         end
 
-        context 'when group has not push rule defined' do
+        context 'when feature flag is switched off' do
           let!(:sample) { create(:push_rule_sample) }
 
+          before do
+            stub_feature_flags(group_push_rules: false)
+          end
+
           it 'creates push rule from sample' do
             expect(create_project(user, opts).push_rule).to have_attributes(
               force_push_regex: sample.force_push_regex,
@@ -294,11 +286,18 @@ describe Projects::CreateService, '#execute' do
           end
         end
       end
-    end
 
-    context 'when there are no push rules' do
-      it 'does not create push rule' do
-        expect(create_project(user, opts).push_rule).to be_nil
+      context 'when group does not have push rule defined' do
+        let!(:sample) { create(:push_rule_sample) }
+
+        it 'creates push rule from sample' do
+          expect(create_project(user, opts).push_rule).to have_attributes(
+            force_push_regex: sample.force_push_regex,
+            deny_delete_tag: sample.deny_delete_tag,
+            delete_branch_regex: sample.delete_branch_regex,
+            commit_message_regex: sample.commit_message_regex
+          )
+        end
       end
     end
   end