Add migration connecting push rules with groups

Update group model

Fix order of creation

Add changelog entry

Fix structure file

Move changelog gile

Move changelog file

Add cr remarks

Add cr remarks

Add cr remarks

Add cr remarks

Add cr remarks

changelogs/unreleased/34370-configure-group-push-rules-db-new-approach.yml

+---
+title: Add push rules association for groups
+merge_request: 29144
+author:
+type: added

db/migrate/20200407120000_add_push_rule_id_to_groups.rb

+# frozen_string_literal: true
+
+class AddPushRuleIdToGroups < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :namespaces, :push_rule_id, :bigint
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :namespaces, :push_rule_id
+    end
+  end
+end

db/migrate/20200407121321_add_push_rule_foreign_key_to_groups.rb

+# frozen_string_literal: true
+
+class AddPushRuleForeignKeyToGroups < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :namespaces, :push_rule_id, unique: true
+    add_concurrent_foreign_key :namespaces, :push_rules, column: :push_rule_id, on_delete: :nullify
+  end
+
+  def down
+    remove_foreign_key_if_exists :namespaces, column: :push_rule_id
+    remove_concurrent_index :namespaces, :push_rule_id
+  end
+end

db/structure.sql

@@ -4051,7 +4051,8 @@ CREATE TABLE public.namespaces (
     mentions_disabled boolean,
     default_branch_protection smallint,
     unlock_membership_to_ldap boolean,
-    max_personal_access_token_lifetime integer
+    max_personal_access_token_lifetime integer,
+    push_rule_id bigint
 );
 
 CREATE SEQUENCE public.namespaces_id_seq
@@ -9667,6 +9668,8 @@ CREATE INDEX index_namespaces_on_path_trigram ON public.namespaces USING gin (pa
 
 CREATE INDEX index_namespaces_on_plan_id ON public.namespaces USING btree (plan_id);
 
+CREATE UNIQUE INDEX index_namespaces_on_push_rule_id ON public.namespaces USING btree (push_rule_id);
+
 CREATE INDEX index_namespaces_on_require_two_factor_authentication ON public.namespaces USING btree (require_two_factor_authentication);
 
 CREATE UNIQUE INDEX index_namespaces_on_runners_token ON public.namespaces USING btree (runners_token);
@@ -10623,6 +10626,9 @@ ALTER TABLE ONLY public.merge_requests
 ALTER TABLE ONLY public.ci_group_variables
     ADD CONSTRAINT fk_33ae4d58d8 FOREIGN KEY (group_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.namespaces
+    ADD CONSTRAINT fk_3448c97865 FOREIGN KEY (push_rule_id) REFERENCES public.push_rules(id) ON DELETE SET NULL;
+
 ALTER TABLE ONLY public.epics
     ADD CONSTRAINT fk_3654b61b03 FOREIGN KEY (author_id) REFERENCES public.users(id) ON DELETE CASCADE;
 
@@ -13160,6 +13166,8 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200406193427
 20200407094005
 20200407094923
+20200407120000
+20200407121321
 20200407171133
 20200407171417
 20200408110856

ee/app/models/ee/group.rb

@@ -49,6 +49,8 @@ module EE
 
       belongs_to :file_template_project, class_name: "Project"
 
+      belongs_to :push_rule
+
       # Use +checked_file_template_project+ instead, which implements important
       # visibility checks
       private :file_template_project
@@ -344,6 +346,18 @@ module EE
       ::PersonalAccessTokens::Groups::UpdateLifetimeService.new(self).execute
     end
 
+    def predefined_push_rule
+      strong_memoize(:predefined_push_rule) do
+        if push_rule.present?
+          push_rule
+        elsif has_parent?
+          parent.predefined_push_rule
+        else
+          PushRule.global
+        end
+      end
+    end
+
     private
 
     def custom_project_templates_group_allowed

ee/spec/models/group_spec.rb

@@ -18,6 +18,7 @@ describe Group do
     it { is_expected.to have_many(:ip_restrictions) }
     it { is_expected.to have_one(:dependency_proxy_setting) }
     it { is_expected.to have_one(:deletion_schedule) }
+    it { is_expected.to belong_to(:push_rule) }
   end
 
   describe 'scopes' do
@@ -412,6 +413,45 @@ describe Group do
     end
   end
 
+  describe '#predefined_push_rule' do
+    context 'group with no associated push_rules record' do
+      let!(:sample) { create(:push_rule_sample) }
+
+      it 'returns instance push rule' do
+        expect(group.predefined_push_rule).to eq(sample)
+      end
+    end
+
+    context 'group with associated push_rules record' do
+      context 'with its own push rule' do
+        let(:push_rule) { create(:push_rule )}
+
+        it 'returns its own push rule' do
+          group.update(push_rule: push_rule)
+
+          expect(group.predefined_push_rule).to eq(push_rule)
+        end
+      end
+
+      context 'with push rule from ancestor' do
+        let(:group) { create(:group, push_rule: push_rule) }
+        let(:push_rule) { create(:push_rule) }
+        let(:subgroup_1) { create(:group, parent: group) }
+        let!(:subgroup_1_1) { create(:group, parent: subgroup_1) }
+
+        it 'returns push rule from closest ancestor' do
+          expect(subgroup_1_1.predefined_push_rule).to eq(push_rule)
+        end
+      end
+    end
+
+    context 'there are no push rules' do
+      it 'returns nil' do
+        expect(group.predefined_push_rule).to be_nil
+      end
+    end
+  end
+
   describe '#checked_file_template_project' do
     let(:valid_project) { create(:project, namespace: group) }