Commit 45afffdb authored by Jan Provaznik's avatar Jan Provaznik

Fix `blocked` status for board issues

Take into account also issues which reference another issue
with link type `blocked_by`.

Also issue visibility is now not taken into account - `blocked` status
is displayed to all users no matter if they can access blocking issue
or not.
parent b1fe7c90
......@@ -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
......
---
title: Show blocked status for all blocked issues on issue boards
merge_request: 24631
author:
type: fixed
......@@ -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
......
......@@ -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?
......
# 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
......
......@@ -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
......
......@@ -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
......
......@@ -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'
......
......@@ -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
......
......@@ -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,44 +28,23 @@ 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
expect(subject[:blocked]).to be_truthy
end
end
it 'the issue is blocked' do
expect(subject[:blocked]).to be_truthy
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
expect(subject[:blocked]).to be_falsey
end
it 'the issue is not blocked' do
expect(subject[:blocked]).to be_falsey
end
end
end
......
......@@ -36,8 +36,6 @@ issues:
- vulnerability_links
- related_vulnerabilities
- user_mentions
- blocked_by_issue_links
- blocked_by_issues
events:
- author
- project
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment