Extract group push rule update service

This commit extracts group push rule create and update logic to
dedicated service classes. This change allows to centralise auditing and
logging of actions from Group Push Rule controller and API.

ee/app/controllers/groups/push_rules_controller.rb

 # frozen_string_literal: true
+
 class Groups::PushRulesController < Groups::ApplicationController
   include Gitlab::Utils::StrongMemoize
   include PushRulesHelper
@@ -16,10 +17,14 @@ class Groups::PushRulesController < Groups::ApplicationController
   end
 
   def update
-    if @push_rule.update(push_rule_params)
+    service_response = PushRules::CreateOrUpdateService
+      .new(container: group, current_user: current_user, params: push_rule_params)
+      .execute
+
+    if service_response.success?
       flash[:notice] = _('Push Rule updated successfully.')
     else
-      flash[:alert] = @push_rule.errors.full_messages.join(', ').html_safe
+      flash[:alert] = service_response.message
     end
 
     redirect_to edit_group_push_rules_path(group)
@@ -45,8 +50,7 @@ class Groups::PushRulesController < Groups::ApplicationController
 
   def push_rule
     strong_memoize(:push_rule) do
-      group.update(push_rule: PushRule.create!) unless group.push_rule
-      group.push_rule
+      group.push_rule || group.build_push_rule
     end
   end
 

ee/app/models/ee/group.rb

@@ -58,7 +58,7 @@ module EE
 
       belongs_to :file_template_project, class_name: "Project"
 
-      belongs_to :push_rule
+      belongs_to :push_rule, inverse_of: :group
 
       # Use +checked_file_template_project+ instead, which implements important
       # visibility checks

ee/app/models/push_rule.rb

@@ -20,6 +20,7 @@ class PushRule < ApplicationRecord
   ].freeze
 
   belongs_to :project, inverse_of: :push_rule
+  has_one :group, inverse_of: :push_rule, autosave: true
 
   validates :max_file_size, numericality: { greater_than_or_equal_to: 0, only_integer: true }
   validates(*REGEX_COLUMNS, untrusted_regexp: true)

ee/lib/api/group_push_rule.rb

@@ -18,6 +18,22 @@ module API
           not_found! unless can?(current_user, :change_push_rules, user_group)
         end
 
+        def create_or_update_push_rule
+          service_response = PushRules::CreateOrUpdateService.new(
+            container: user_group,
+            current_user: current_user,
+            params: declared_params(include_missing: false)
+          ).execute
+
+          push_rule = service_response.payload[:push_rule]
+
+          if service_response.success?
+            present(push_rule, with: EE::API::Entities::GroupPushRule, user: current_user)
+          else
+            render_validation_error!(push_rule)
+          end
+        end
+
         params :push_rule_params do
           optional :deny_delete_tag, type: Boolean, desc: 'Deny deleting a tag'
           optional :member_check, type: Boolean, desc: 'Restrict commits by author (email) to existing GitLab users'
@@ -59,11 +75,8 @@ module API
         use :push_rule_params
       end
       post ":id/push_rule" do
-        render_api_error!(_('Group push rule exists, try updating'), 422) if user_group.push_rule
-
-        allowed_params = declared_params(include_missing: false)
-        user_group.update!(push_rule: PushRule.create!(allowed_params))
-        present user_group.push_rule, with: EE::API::Entities::GroupPushRule, user: current_user
+        unprocessable_entity!('Group push rule exists, try updating') if user_group.push_rule
+        create_or_update_push_rule
       end
 
       desc 'Edit push rule of a group' do
@@ -74,14 +87,8 @@ module API
         use :push_rule_params
       end
       put ":id/push_rule" do
-        push_rule = user_group.push_rule
-        not_found! unless push_rule
-
-        if push_rule.update(declared_params(include_missing: false))
-          present push_rule, with: EE::API::Entities::GroupPushRule, user: current_user
-        else
-          render_validation_error!(push_rule)
-        end
+        not_found!('Push Rule') unless user_group.push_rule
+        create_or_update_push_rule
       end
 
       desc 'Deletes group push rule' do

ee/spec/models/group_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe Group do
     it { is_expected.to have_many(:compliance_management_frameworks) }
     it { is_expected.to have_one(:deletion_schedule) }
     it { is_expected.to have_one(:group_wiki_repository) }
-    it { is_expected.to belong_to(:push_rule) }
+    it { is_expected.to belong_to(:push_rule).inverse_of(:group) }
     it { is_expected.to have_many(:saml_group_links) }
     it { is_expected.to have_many(:epics) }
     it { is_expected.to have_many(:epic_boards).inverse_of(:group) }

ee/spec/models/push_rule_spec.rb

@@ -12,6 +12,7 @@ RSpec.describe PushRule do
 
   describe "Associations" do
     it { is_expected.to belong_to(:project).inverse_of(:push_rule) }
+    it { is_expected.to have_one(:group).inverse_of(:push_rule).autosave(true) }
   end
 
   describe "Validation" do

ee/spec/requests/api/group_push_rule_spec.rb

@@ -303,7 +303,7 @@ RSpec.describe API::GroupPushRule, 'GroupPushRule', api: true do
             put api("/groups/#{group_without_push_rule.id}/push_rule", admin), params: attributes_for_update
 
             expect(response).to have_gitlab_http_status(:not_found)
-            expect(json_response['message']).to include('Not Found')
+            expect(json_response['message']).to include('Push Rule Not Found')
           end
         end
 

locale/gitlab.pot

@@ -14676,9 +14676,6 @@ msgstr ""
 msgid "Group project URLs are prefixed with the group namespace"
 msgstr ""
 
-msgid "Group push rule exists, try updating"
-msgstr ""
-
 msgid "Group requires separate account"
 msgstr ""
 

spec/lib/gitlab/import_export/all_models.yml

@@ -740,3 +740,5 @@ status_page_published_incident:
 - issue
 issuable_sla:
   - issue
+push_rule:
+  - group