Merge branch 'fix-blocked-status' into 'master'

Fix `blocked` status for board issues

See merge request gitlab-org/gitlab!24631

app/controllers/boards/issues_controller.rb

@@ -86,8 +86,12 @@ module Boards
       head(:forbidden) unless can?(current_user, :admin_issue, board)
     end
 
+    def serializer_options(issues)
+      {}
+    end
+
     def render_issues(issues, metadata)
-      data = { issues: serialize_as_json(issues) }
+      data = { issues: serialize_as_json(issues, opts: serializer_options(issues)) }
       data.merge!(metadata)
 
       render json: data
@@ -133,8 +137,10 @@ module Boards
       IssueSerializer.new(current_user: current_user)
     end
 
-    def serialize_as_json(resource)
-      serializer.represent(resource, serializer: 'board', include_full_project_path: board.group_board?)
+    def serialize_as_json(resource, opts: {})
+      opts.merge!(include_full_project_path: board.group_board?, serializer: 'board')
+
+      serializer.represent(resource, opts)
     end
 
     def whitelist_query_limiting

changelogs/unreleased/fix-blocked-status.yml

+---
+title: Show blocked status for all blocked issues on issue boards
+merge_request: 24631
+author:
+type: fixed

ee/app/controllers/ee/boards/issues_controller.rb

@@ -5,9 +5,9 @@ module EE
     module IssuesController
       extend ::Gitlab::Utils::Override
 
-      override :associations_to_preload
-      def associations_to_preload
-        super << { blocked_by_issues: { project: :project_feature } }
+      override :serializer_options
+      def serializer_options(issues)
+        super.merge(blocked_issue_ids: ::IssueLink.blocked_issue_ids(issues.map(&:id)))
       end
     end
   end

ee/app/models/ee/issue.rb

@@ -43,9 +43,6 @@ module EE
       has_many :vulnerability_links, class_name: 'Vulnerabilities::IssueLink', inverse_of: :issue
       has_many :related_vulnerabilities, through: :vulnerability_links, source: :vulnerability
 
-      has_many :blocked_by_issue_links, -> { where(link_type: IssueLink::TYPE_BLOCKS) }, class_name: 'IssueLink', foreign_key: :target_id
-      has_many :blocked_by_issues, through: :blocked_by_issue_links, source: :source
-
       validates :weight, allow_nil: true, numericality: { greater_than_or_equal_to: 0 }
 
       after_create :update_generic_alert_title, if: :generic_alert_with_default_title?

ee/app/models/issue_link.rb

 # frozen_string_literal: true
 
 class IssueLink < ApplicationRecord
+  include FromUnion
+
   belongs_to :source, class_name: 'Issue'
   belongs_to :target, class_name: 'Issue'
 
@@ -29,6 +31,13 @@ class IssueLink < ApplicationRecord
     end
   end
 
+  def self.blocked_issue_ids(issue_ids)
+    from_union([
+      IssueLink.select('target_id as issue_id').where(link_type: IssueLink::TYPE_BLOCKS).where(target_id: issue_ids),
+      IssueLink.select('source_id as issue_id').where(link_type: IssueLink::TYPE_IS_BLOCKED_BY).where(source_id: issue_ids)
+    ]).pluck(:issue_id)
+  end
+
   private
 
   def check_self_relation

ee/app/serializers/ee/issue_board_entity.rb

@@ -6,8 +6,8 @@ module EE
 
     prepended do
       expose :weight, if: ->(issue, _) { issue.supports_weight? }
-      expose :blocked do |issue|
-        issue.blocked_by_issues.any? { |blocked_by_issue| can?(request.current_user, :read_issue, blocked_by_issue) }
+      expose :blocked do |issue, options|
+        options[:blocked_issue_ids].present? && options[:blocked_issue_ids].include?(issue.id)
       end
     end
   end

ee/spec/controllers/boards/issues_controller_spec.rb

@@ -50,6 +50,23 @@ describe Boards::IssuesController do
           expect(development.issues.map(&:relative_position)).not_to include(nil)
         end
 
+        it 'returns blocked status for each issue' do
+          issue1 = create(:labeled_issue, project: project_1, labels: [development], relative_position: 1)
+          issue2 = create(:labeled_issue, project: project_1, labels: [development], relative_position: 2)
+          issue3 = create(:labeled_issue, project: project_1, labels: [development], relative_position: 3)
+          create(:issue_link, source: issue1, target: issue2, link_type: IssueLink::TYPE_IS_BLOCKED_BY)
+          create(:issue_link, source: issue2, target: issue3, link_type: IssueLink::TYPE_BLOCKS)
+
+          list_issues user: user, board: board, list: list2
+
+          expect(response).to match_response_schema('entities/issue_boards')
+          issues = json_response['issues']
+          expect(issues.length).to eq 3
+          expect(issues[0]['blocked']).to be_truthy
+          expect(issues[1]['blocked']).to be_falsey
+          expect(issues[2]['blocked']).to be_truthy
+        end
+
         context 'with search param' do
           context 'when board_search_optimization is enabled' do
             before do

ee/spec/models/issue_link_spec.rb

@@ -51,6 +51,17 @@ describe IssueLink do
     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)
+      link2 = create(:issue_link, link_type: described_class::TYPE_IS_BLOCKED_BY)
+      link3 = create(:issue_link, link_type: described_class::TYPE_RELATES_TO)
+
+      expect(described_class.blocked_issue_ids([link1.target_id, link2.source_id, link3.source_id]))
+        .to match_array([link1.target_id, link2.source_id])
+    end
+  end
+
   describe '.inverse_link_type' do
     it 'returns reverse type of link' do
       expect(described_class.inverse_link_type('relates_to')).to eq 'relates_to'

ee/spec/models/issue_spec.rb

@@ -127,8 +127,6 @@ describe Issue do
     it { is_expected.to have_many(:related_vulnerabilities).through(:vulnerability_links).source(:vulnerability) }
     it { is_expected.to belong_to(:promoted_to_epic).class_name('Epic') }
     it { is_expected.to have_many(:resource_weight_events) }
-    it { is_expected.to have_many(:blocked_by_issue_links) }
-    it { is_expected.to have_many(:blocked_by_issues).through(:blocked_by_issue_links).source(:source) }
 
     describe 'versions.most_recent' do
       it 'returns the most recent version' do

ee/spec/serializers/ee/issue_board_entity_spec.rb

@@ -7,8 +7,9 @@ describe IssueBoardEntity do
   let_it_be(:issue) { create(:issue, project: project) }
   let_it_be(:user) { create(:user) }
   let(:request) { double('request', current_user: user) }
+  let(:blocked_ids) { [] }
 
-  subject { described_class.new(issue.reload, request: request).as_json }
+  subject { described_class.new(issue.reload, request: request, blocked_issue_ids: blocked_ids).as_json }
 
   describe '#weight' do
     it 'has `weight` attribute' do
@@ -27,45 +28,24 @@ describe IssueBoardEntity do
   end
 
   describe '#blocked' do
-    it 'is not blocked by default' do
+    it 'the issue is not blocked by default' do
       expect(subject[:blocked]).to be_falsey
     end
 
-    context 'when the issue is referenced by other issue' do
-      let_it_be(:project2) { create(:project) }
-      let_it_be(:related_issue) { create(:issue, project: project2) }
+    context 'when blocked_issue_ids contains the issue id' do
+      let(:blocked_ids) { [issue.id] }
 
-      context 'when the issue is blocking' do
-        let_it_be(:issue_link) { create(:issue_link, source: related_issue, target: issue, link_type: IssueLink::TYPE_BLOCKS) }
-
-        context 'when the referencing issue is not visible to the user' do
-          it 'is not blocked' do
-            expect(subject[:blocked]).to be_falsey
-          end
-        end
-
-        context 'when the referencing issue is visible to the user' do
-          before do
-            project2.add_developer(user)
-          end
-
-          it 'is blocked' do
+      it 'the issue is blocked' do
         expect(subject[:blocked]).to be_truthy
       end
     end
-      end
-
-      context 'when the issue is not blocking' do
-        let_it_be(:issue_link) { create(:issue_link, source: related_issue, target: issue, link_type: IssueLink::TYPE_RELATES_TO) }
 
-        before do
-          project2.add_developer(user)
-        end
+    context 'when blocked_issue_ids is not set' do
+      let(:blocked_ids) { nil }
 
-        it 'is not blocked' do
+      it 'the issue is not blocked' do
         expect(subject[:blocked]).to be_falsey
       end
     end
   end
-  end
 end

spec/lib/gitlab/import_export/all_models.yml

@@ -36,8 +36,6 @@ issues:
 - vulnerability_links
 - related_vulnerabilities
 - user_mentions
-- blocked_by_issue_links
-- blocked_by_issues
 events:
 - author
 - project