Merge branch '218040-expose-issue-blocked' into 'master'

Expose issue blocked in GraphQL

See merge request gitlab-org/gitlab!36428

doc/api/graphql/reference/gitlab_schema.graphql

@@ -4150,6 +4150,11 @@ type EpicIssue implements Noteable {
   """
   author: User!
 
+  """
+  Indicates the issue is blocked
+  """
+  blocked: Boolean!
+
   """
   Timestamp of when the issue was closed
   """
@@ -5767,6 +5772,11 @@ type Issue implements Noteable {
   """
   author: User!
 
+  """
+  Indicates the issue is blocked
+  """
+  blocked: Boolean!
+
   """
   Timestamp of when the issue was closed
   """

doc/api/graphql/reference/gitlab_schema.json

@@ -11590,6 +11590,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "blocked",
+              "description": "Indicates the issue is blocked",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "Boolean",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "closedAt",
               "description": "Timestamp of when the issue was closed",
@@ -15862,6 +15880,24 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "blocked",
+              "description": "Indicates the issue is blocked",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "SCALAR",
+                  "name": "Boolean",
+                  "ofType": null
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "closedAt",
               "description": "Timestamp of when the issue was closed",

doc/api/graphql/reference/index.md

@@ -705,6 +705,7 @@ Relationship between an epic and an issue
 | Name  | Type  | Description |
 | ---   |  ---- | ----------  |
 | `author` | User! | User that created the issue |
+| `blocked` | Boolean! | Indicates the issue is blocked |
 | `closedAt` | Time | Timestamp of when the issue was closed |
 | `confidential` | Boolean! | Indicates the issue is confidential |
 | `createdAt` | Time! | Timestamp of when the issue was created |
@@ -864,6 +865,7 @@ Represents a Group Member
 | Name  | Type  | Description |
 | ---   |  ---- | ----------  |
 | `author` | User! | User that created the issue |
+| `blocked` | Boolean! | Indicates the issue is blocked |
 | `closedAt` | Time | Timestamp of when the issue was closed |
 | `confidential` | Boolean! | Indicates the issue is confidential |
 | `createdAt` | Time! | Timestamp of when the issue was created |

ee/app/graphql/ee/gitlab_schema.rb

@@ -6,6 +6,7 @@ module EE
 
     prepended do
       lazy_resolve ::Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate, :epic_aggregate
+      lazy_resolve ::Gitlab::Graphql::Aggregations::Issues::LazyBlockAggregate, :block_aggregate
     end
   end
 end

ee/app/graphql/ee/types/issue_type.rb

@@ -17,6 +17,12 @@ module EE
               description: 'Weight of the issue',
               resolve: -> (obj, _args, _ctx) { obj.supports_weight? ? obj.weight : nil }
 
+        field :blocked, GraphQL::BOOLEAN_TYPE, null: false,
+              description: 'Indicates the issue is blocked',
+              resolve: -> (obj, _args, ctx) {
+                ::Gitlab::Graphql::Aggregations::Issues::LazyBlockAggregate.new(ctx, obj.id)
+              }
+
         field :health_status,
           ::Types::HealthStatusEnum,
           null: true,

ee/app/models/issue_link.rb

@@ -82,6 +82,23 @@ class IssueLink < ApplicationRecord
           .group(:blocking_issue_id)
       ], remove_duplicates: false).select('blocking_issue_id, SUM(count) AS count').group('blocking_issue_id')
     end
+
+    def blocked_issues_for_collection(issues_ids)
+      from_union([
+        select('COUNT(*), issue_links.source_id AS blocked_issue_id')
+          .joins(:target)
+          .where(issues: { state_id: Issue.available_states[:opened] })
+          .where(link_type: TYPE_IS_BLOCKED_BY)
+          .where(source_id: issues_ids)
+          .group(:blocked_issue_id),
+        select('COUNT(*), issue_links.target_id AS blocked_issue_id')
+          .joins(:source)
+          .where(issues: { state_id: Issue.available_states[:opened] })
+          .where(link_type: TYPE_BLOCKS)
+          .where(target_id: issues_ids)
+          .group(:blocked_issue_id)
+      ], remove_duplicates: false).select('blocked_issue_id, SUM(count) AS count').group('blocked_issue_id')
+    end
   end
 
   def check_self_relation

ee/changelogs/unreleased/218040-expose-issue-blocked.yml

+---
+title: Expose blocked field on GraphQL issue type
+merge_request: 36428
+author:
+type: changed

ee/lib/gitlab/graphql/aggregations/issues/lazy_block_aggregate.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module Aggregations
+      module Issues
+        class LazyBlockAggregate
+          attr_reader :issue_id, :lazy_state
+
+          def initialize(query_ctx, issue_id)
+            @issue_id = issue_id
+
+            # Initialize the loading state for this query,
+            # or get the previously-initiated state
+            @lazy_state = query_ctx[:lazy_block_aggregate] ||= {
+              pending_ids: Set.new,
+              loaded_objects: {}
+            }
+            # Register this ID to be loaded later:
+            @lazy_state[:pending_ids] << issue_id
+          end
+
+          # Return the loaded record, hitting the database if needed
+          def block_aggregate
+            # Check if the record was already loaded
+            if @lazy_state[:pending_ids].present?
+              load_records_into_loaded_objects
+            end
+
+            !!@lazy_state[:loaded_objects][@issue_id]
+          end
+
+          private
+
+          def load_records_into_loaded_objects
+            # The record hasn't been loaded yet, so
+            # hit the database with all pending IDs to prevent N+1
+            pending_ids = @lazy_state[:pending_ids].to_a
+            blocked = IssueLink.blocked_issues_for_collection(pending_ids).compact.flatten
+
+            blocked.each do |o|
+              @lazy_state[:loaded_objects][o.blocked_issue_id] = true
+            end
+
+            @lazy_state[:pending_ids].clear
+          end
+        end
+      end
+    end
+  end
+end

ee/spec/graphql/types/issue_type_spec.rb

@@ -10,4 +10,67 @@ RSpec.describe GitlabSchema.types['Issue'] do
   it { expect(described_class).to have_graphql_field(:weight) }
 
   it { expect(described_class).to have_graphql_field(:health_status) }
+
+  it { expect(described_class).to have_graphql_field(:blocked) }
+
+  context 'N+1 queries' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:group) { create(:group) }
+    let_it_be(:project) { create(:project, :public, group: group) }
+    let_it_be(:project_path) { project.full_path }
+    let!(:blocking_issue1) { create(:issue, project: project) }
+    let!(:blocked_issue1) { create(:issue, project: project) }
+    let!(:issue_link1) { create :issue_link, source: blocking_issue1, target: blocked_issue1, link_type: IssueLink::TYPE_BLOCKS }
+
+    shared_examples 'avoids N+1 queries on blocked' do
+      specify do
+        control_count = ActiveRecord::QueryRecorder.new { GitlabSchema.execute(query, context: { current_user: user }) }.count
+
+        blocked_issue2 = create(:issue, project: project)
+        blocking_issue2 = create(:issue, project: project)
+        create :issue_link, source: blocked_issue2, target: blocking_issue2, link_type: IssueLink::TYPE_IS_BLOCKED_BY
+
+        # added the +1 due to an existing N+1 with issues
+        expect { GitlabSchema.execute(query, context: { current_user: user }) }.not_to exceed_query_limit(control_count + 1)
+      end
+    end
+
+    context 'group issues' do
+      let(:query) do
+        %(
+          query{
+            group(fullPath:"#{group.full_path}"){
+              issues{
+                nodes{
+                  title
+                  blocked
+                }
+              }
+            }
+          }
+        )
+      end
+
+      it_behaves_like 'avoids N+1 queries on blocked'
+    end
+
+    context 'project issues' do
+      let(:query) do
+        %(
+          query{
+            project(fullPath:"#{project_path}"){
+              issues{
+                nodes{
+                  title
+                  blocked
+                }
+              }
+            }
+          }
+        )
+      end
+
+      it_behaves_like 'avoids N+1 queries on blocked'
+    end
+  end
 end

ee/spec/lib/gitlab/graphql/aggregations/issues/lazy_block_aggregate_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::Aggregations::Issues::LazyBlockAggregate do
+  let(:query_ctx) do
+    {}
+  end
+
+  let(:issue_id) { 37 }
+  let(:blocks_issue_id) { 18 }
+  let(:blocking_issue_id) { 38 }
+
+  describe '#initialize' do
+    it 'adds the issue_id to the lazy state' do
+      subject = described_class.new(query_ctx, issue_id)
+
+      expect(subject.lazy_state[:pending_ids]).to match [issue_id]
+      expect(subject.issue_id).to match issue_id
+    end
+  end
+
+  describe '#block_aggregate' do
+    subject { described_class.new(query_ctx, issue_id) }
+
+    before do
+      subject.instance_variable_set(:@lazy_state, fake_state)
+    end
+
+    context 'if the record has already been loaded' do
+      let(:fake_state) do
+        { pending_ids: Set.new, loaded_objects: { issue_id => true } }
+      end
+
+      it 'does not make the query again' do
+        expect(IssueLink).not_to receive(:blocked_issues_for_collection)
+
+        subject.block_aggregate
+      end
+    end
+
+    context 'if the record has not been loaded' do
+      let(:other_issue_id) { 39 }
+      let(:fake_state) do
+        { pending_ids: Set.new([issue_id]), loaded_objects: {} }
+      end
+
+      let(:fake_data) do
+        [
+            double(blocked_issue_id: 1745, count: 1.0),
+            nil # nil for unblocked issues
+        ]
+      end
+
+      before do
+        expect(IssueLink).to receive(:blocked_issues_for_collection).and_return(fake_data)
+      end
+
+      it 'clears the pending IDs' do
+        subject.block_aggregate
+
+        expect(subject.lazy_state[:pending_ids]).to be_empty
+      end
+    end
+  end
+end

ee/spec/models/issue_link_spec.rb

@@ -85,21 +85,41 @@ RSpec.describe IssueLink do
     end
   end
 
-  describe '.blocking_issues_for_collection' do
-    it 'returns blocking issues count grouped by issue id' do
-      issue_1 = create(:issue)
-      issue_2 = create(:issue)
-      issue_3 = create(:issue)
-      blocking_issue_1 = create(:issue, project: issue_1.project)
-      blocking_issue_2 = create(:issue, project: issue_2.project)
-      create(:issue_link, source: blocking_issue_1, target: issue_1, link_type: IssueLink::TYPE_BLOCKS)
-      create(:issue_link, source: issue_2, target: blocking_issue_1, link_type: IssueLink::TYPE_IS_BLOCKED_BY)
-      create(:issue_link, source: blocking_issue_2, target: issue_3, link_type: IssueLink::TYPE_BLOCKS)
-
-      results = described_class.blocking_issues_for_collection([blocking_issue_1, blocking_issue_2])
-
-      expect(results.find { |link| link.blocking_issue_id == blocking_issue_1.id }.count).to eq(2)
-      expect(results.find { |link| link.blocking_issue_id == blocking_issue_2.id }.count).to eq(1)
+  describe 'collections' 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) }
+    let_it_be(:blocked_issue_3) { create(:issue, project: project) }
+    let_it_be(:blocking_issue_1) { create(:issue, project: project) }
+    let_it_be(:blocking_issue_2) { create(:issue, project: project) }
+
+    before :all do
+      create(:issue_link, source: blocking_issue_1, target: blocked_issue_1, link_type: IssueLink::TYPE_BLOCKS)
+      create(:issue_link, source: blocked_issue_2, target: blocking_issue_1, link_type: IssueLink::TYPE_IS_BLOCKED_BY)
+      create(:issue_link, source: blocking_issue_2, target: blocked_issue_3, link_type: IssueLink::TYPE_BLOCKS)
     end
+
+    describe '.blocking_issues_for_collection' do
+      it 'returns blocking issues count grouped by issue id' do
+        results = described_class.blocking_issues_for_collection([blocking_issue_1, blocking_issue_2])
+
+        expect(results.find { |link| link.blocking_issue_id == blocking_issue_1.id }.count).to eq(2)
+        expect(results.find { |link| link.blocking_issue_id == blocking_issue_2.id }.count).to eq(1)
+      end
+    end
+
+    describe '.blocked_issues_for_collection' do
+      it 'returns blocked issues count grouped by issue id' do
+        results = described_class.blocked_issues_for_collection([blocked_issue_1, blocked_issue_2, blocked_issue_3])
+
+        expect(result_by(results, blocked_issue_1.id).count).to eq(1)
+        expect(result_by(results, blocked_issue_2.id).count).to eq(1)
+        expect(result_by(results, blocked_issue_3.id).count).to eq(1)
+      end
+    end
+  end
+
+  def result_by(results, id)
+    results.find { |link| link.blocked_issue_id == id }
   end
 end

ee/spec/requests/api/graphql/project/issues_spec.rb

@@ -47,4 +47,67 @@ RSpec.describe 'getting an issue list for a project' do
       end
     end
   end
+
+  describe 'blocked' do
+    let_it_be(:group) { create(:group, :public) }
+    let_it_be(:project) { create(:project, namespace: group) }
+    let_it_be(:unrelated_issue) { create(:issue, project: project) }
+    let_it_be(:blocked_issue1) { create(:issue, project: project) }
+    let_it_be(:blocking_issue1) { create(:issue, project: project) }
+    let_it_be(:blocked_issue2) { create(:issue, project: project) }
+    let_it_be(:blocking_issue2) { create(:issue, :confidential, project: project) }
+
+    let_it_be(:issue_link1) { create(:issue_link, source: blocked_issue1, target: blocking_issue1, link_type: 'is_blocked_by') }
+    let_it_be(:issue_link2) { create(:issue_link, source: blocking_issue2, target: blocked_issue2, link_type: 'blocks') }
+
+    let(:query) do
+      graphql_query_for('project', { fullPath: project.full_path }, query_graphql_field('issues', {}, issue_links_aggregates_query))
+    end
+
+    let(:single_issue_query) do
+      graphql_query_for('project', { fullPath: project.full_path }, query_graphql_field('issues', { iid: blocked_issue1.iid.to_s }, issue_links_aggregates_query))
+    end
+
+    let(:issue_links_aggregates_query) do
+      <<~QUERY
+        nodes {
+          id
+          blocked
+        }
+      QUERY
+    end
+
+    before do
+      group.add_developer(current_user)
+    end
+
+    context 'working query' do
+      before do
+        post_graphql(single_issue_query, current_user: current_user)
+      end
+
+      it_behaves_like 'a working graphql query'
+    end
+
+    it 'uses the LazyBlockAggregate service' do
+      expect(::Gitlab::Graphql::Aggregations::Issues::LazyBlockAggregate).to receive(:new)
+
+      post_graphql(single_issue_query, current_user: current_user)
+    end
+
+    it 'returns the correct results', :aggregate_failures do
+      post_graphql(query, current_user: current_user)
+
+      result = graphql_data.dig('project', 'issues', 'nodes')
+
+      expect(find_result(result, blocked_issue1)).to eq true
+      expect(find_result(result, blocked_issue2)).to eq true
+      expect(find_result(result, blocking_issue1)).to eq false
+      expect(find_result(result, blocking_issue2)).to eq false
+    end
+  end
+
+  def find_result(result, issue)
+    result.find { |r| r['id'] == issue.to_global_id.to_s }['blocked']
+  end
 end