Merge branch 'issue_34247_save_count_on_db' into 'master'

Persist blocking issues count on database

See merge request gitlab-org/gitlab!37368

db/migrate/20200720191134_add_blocking_issues_count_to_issues.rb

+# frozen_string_literal: true
+
+class AddBlockingIssuesCountToIssues < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_issue_on_project_id_state_id_and_blocking_issues_count'
+
+  disable_ddl_transaction!
+
+  def up
+    unless column_exists?(:issues, :blocking_issues_count)
+      with_lock_retries do
+        add_column :issues, :blocking_issues_count, :integer, default: 0, null: false
+      end
+    end
+
+    add_concurrent_index :issues, [:project_id, :state_id, :blocking_issues_count], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :issues, INDEX_NAME
+
+    with_lock_retries do
+      remove_column :issues, :blocking_issues_count
+    end
+  end
+end

db/schema_migrations/20200720191134

+08566b4094de37395f3ebc3f70eedc9ed6052a78090cf296e66ce4ef167e6bce
\ No newline at end of file

db/structure.sql

@@ -12552,6 +12552,7 @@ CREATE TABLE public.issues (
     external_key character varying(255),
     sprint_id bigint,
     issue_type smallint DEFAULT 0 NOT NULL,
+    blocking_issues_count integer DEFAULT 0 NOT NULL,
     CONSTRAINT check_fba63f706d CHECK ((lock_version IS NOT NULL))
 );
 
@@ -19737,6 +19738,8 @@ CREATE INDEX index_issue_metrics ON public.issue_metrics USING btree (issue_id);
 
 CREATE INDEX index_issue_metrics_on_issue_id_and_timestamps ON public.issue_metrics USING btree (issue_id, first_mentioned_in_commit_at, first_associated_with_milestone_at, first_added_to_board_at);
 
+CREATE INDEX index_issue_on_project_id_state_id_and_blocking_issues_count ON public.issues USING btree (project_id, state_id, blocking_issues_count);
+
 CREATE INDEX index_issue_tracker_data_on_service_id ON public.issue_tracker_data USING btree (service_id);
 
 CREATE UNIQUE INDEX index_issue_user_mentions_on_note_id ON public.issue_user_mentions USING btree (note_id) WHERE (note_id IS NOT NULL);

ee/app/models/ee/issue.rb

@@ -210,6 +210,12 @@ module EE
       end
     end
 
+    def update_blocking_issues_count!
+      blocking_count = IssueLink.blocking_issues_count_for(self)
+
+      update!(blocking_issues_count: blocking_count)
+    end
+
     private
 
     def blocking_issues_ids

ee/app/models/issue_link.rb

@@ -18,6 +18,9 @@ class IssueLink < ApplicationRecord
   TYPE_BLOCKS = 'blocks'
   TYPE_IS_BLOCKED_BY = 'is_blocked_by'
 
+  after_create :refresh_blocking_issue_cache
+  after_destroy :refresh_blocking_issue_cache
+
   enum link_type: { TYPE_RELATES_TO => 0, TYPE_BLOCKS => 1, TYPE_IS_BLOCKED_BY => 2 }
 
   class << self
@@ -43,6 +46,17 @@ class IssueLink < ApplicationRecord
 
   private
 
+  def blocking_issue
+    case link_type
+    when TYPE_BLOCKS then source
+    when TYPE_IS_BLOCKED_BY then target
+    end
+  end
+
+  def refresh_blocking_issue_cache
+    blocking_issue&.update_blocking_issues_count!
+  end
+
   class << self
     def blocked_and_blocking_issues_union(issue_ids)
       from_union([
@@ -99,6 +113,10 @@ class IssueLink < ApplicationRecord
           .group(:blocked_issue_id)
       ], remove_duplicates: false).select('blocked_issue_id, SUM(count) AS count').group('blocked_issue_id')
     end
+
+    def blocking_issues_count_for(issue)
+      blocking_issues_for_collection(issue.id)[0]&.count.to_i
+    end
   end
 
   def check_self_relation

ee/spec/models/issue_link_spec.rb

@@ -51,6 +51,75 @@ RSpec.describe IssueLink do
     end
   end
 
+  context 'callbacks' do
+    let_it_be(:target) { create(:issue) }
+    let_it_be(:source) { create(:issue) }
+
+    describe '.after_create_commit' do
+      context 'with TYPE_BLOCKS relation' do
+        it 'updates blocking issues count' do
+          expect(source).to receive(:update_blocking_issues_count!)
+          expect(target).not_to receive(:update_blocking_issues_count!)
+
+          create(:issue_link, target: target, source: source, link_type: described_class::TYPE_BLOCKS)
+        end
+      end
+
+      context 'with TYPE_IS_BLOCKED_BY' do
+        it 'updates blocking issues count' do
+          expect(source).not_to receive(:update_blocking_issues_count!)
+          expect(target).to receive(:update_blocking_issues_count!)
+
+          create(:issue_link, target: target, source: source, link_type: described_class::TYPE_IS_BLOCKED_BY)
+        end
+      end
+
+      context 'with TYPE_RELATES_TO' do
+        it 'does not update blocking_issues_count' do
+          expect(source).not_to receive(:update_blocking_issues_count!)
+          expect(target).not_to receive(:update_blocking_issues_count!)
+
+          create(:issue_link, target: target, source: source, link_type: described_class::TYPE_RELATES_TO)
+        end
+      end
+    end
+
+    describe '.after_destroy_commit' do
+      context 'with TYPE_BLOCKS relation' do
+        it 'updates blocking issues count' do
+          link = create(:issue_link, target: target, source: source, link_type: described_class::TYPE_BLOCKS)
+
+          expect(source).to receive(:update_blocking_issues_count!)
+          expect(target).not_to receive(:update_blocking_issues_count!)
+
+          link.destroy!
+        end
+      end
+
+      context 'with TYPE_IS_BLOCKED_BY' do
+        it 'updates blocking issues count' do
+          link = create(:issue_link, target: target, source: source, link_type: described_class::TYPE_IS_BLOCKED_BY)
+
+          expect(source).not_to receive(:update_blocking_issues_count!)
+          expect(target).to receive(:update_blocking_issues_count!)
+
+          link.destroy!
+        end
+      end
+
+      context 'with TYPE_RELATES_TO' do
+        it 'does not update blocking_issues_count' do
+          link = create(:issue_link, target: target, source: source, link_type: described_class::TYPE_RELATES_TO)
+
+          expect(source).not_to receive(:update_blocking_issues_count!)
+          expect(target).not_to receive(:update_blocking_issues_count!)
+
+          link.destroy!
+        end
+      end
+    end
+  end
+
   describe '.blocked_issue_ids' do
     it 'returns only ids of issues which are blocked' do
       link1 = create(:issue_link, link_type: described_class::TYPE_BLOCKS)
@@ -85,7 +154,7 @@ RSpec.describe IssueLink do
     end
   end
 
-  describe 'collections' do
+  context 'blocking issues count' do
     let_it_be(:blocked_issue_1) { create(:issue) }
     let_it_be(:project) { blocked_issue_1.project }
     let_it_be(:blocked_issue_2) { create(:issue, project: project) }
@@ -117,6 +186,14 @@ RSpec.describe IssueLink do
         expect(result_by(results, blocked_issue_3.id).count).to eq(1)
       end
     end
+
+    describe '.blocking_issues_count_for' do
+      it 'returns blocked issues count for single issue' do
+        blocking_count = described_class.blocking_issues_count_for(blocking_issue_1)
+
+        expect(blocking_count).to eq(2)
+      end
+    end
   end
 
   def result_by(results, id)

ee/spec/models/issue_spec.rb

@@ -737,5 +737,22 @@ RSpec.describe Issue do
         expect(subject).to be_falsey
       end
     end
+
+    describe '#update_blocking_issues_count' do
+      it 'updates blocking issues count' do
+        issue = create(:issue, project: project)
+        blocked_issue_1 = create(:issue, project: project)
+        blocked_issue_2 = create(:issue, project: project)
+        blocked_issue_3 = create(:issue, project: project)
+        create(:issue_link, source: issue, target: blocked_issue_1, link_type: IssueLink::TYPE_BLOCKS)
+        create(:issue_link, source: blocked_issue_2, target: issue, link_type: IssueLink::TYPE_IS_BLOCKED_BY)
+        create(:issue_link, source: issue, target: blocked_issue_3, link_type: IssueLink::TYPE_BLOCKS)
+        # Set to 0 for proper testing, this is being set by IssueLink callbacks.
+        issue.update(blocking_issues_count: 0)
+
+        expect { issue.update_blocking_issues_count! }
+          .to change { issue.blocking_issues_count }.from(0).to(3)
+      end
+    end
   end
 end

ee/spec/services/issue_links/destroy_service_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe IssueLinks::DestroyService do
   describe '#execute' do
-    let(:project) { create(:project) }
+    let(:project) { create(:project_empty_repo) }
     let(:user) { create(:user) }
 
     subject { described_class.new(issue_link, user).execute }

lib/gitlab/import_export/project/import_export.yml

@@ -205,6 +205,7 @@ excluded_attributes:
     - :state_id
     - :duplicated_to_id
     - :promoted_to_epic_id
+    - :blocking_issues_count
   merge_request:
     - :milestone_id
     - :sprint_id