Merge branch '34370-configure-group-push-rules-db-new-approach' into 'master'

Resolve "Configure push rules at a group level" - db and models part

See merge request gitlab-org/gitlab!29144

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) }