Commit 8c391ad3 authored by charlieablett's avatar charlieablett

Expose issue blocked in GraphQL

- Matches the REST API
- Add N+1 test
- Add LazyIssueLinkAggregator
- Add tests
- Add blocked collector
parent 8214da07
......@@ -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
"""
......
......@@ -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",
......@@ -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 |
......
......@@ -6,6 +6,7 @@ module EE
prepended do
lazy_resolve ::Gitlab::Graphql::Aggregations::Epics::LazyEpicAggregate, :epic_aggregate
lazy_resolve ::Gitlab::Graphql::Aggregations::Issues::LazyIssueLinkAggregate, :issue_link_aggregate
end
end
end
......@@ -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::LazyIssueLinkAggregate.new(ctx, obj.id)
}
field :health_status,
::Types::HealthStatusEnum,
null: true,
......
......@@ -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
......
---
title: Expose blocked field on GraphQL issue type
merge_request: 36428
author:
type: changed
# frozen_string_literal: true
module Gitlab
module Graphql
module Aggregations
module Issues
class LazyIssueLinkAggregate
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_issue_link_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 issue_link_aggregate
# Check if the record was already loaded:
# load from loaded_objects by issue
unless @lazy_state[:loaded_objects][@issue_id]
load_records_into_loaded_objects
end
loaded_objects[@issue_id].any?
end
private
def loaded_objects
@lazy_state[:loaded_objects]
end
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
pending_ids.each do |id|
# result is either [] or an array with a link aggregate object
@lazy_state[:loaded_objects][id] = blocked.select { |o| o.blocked_issue_id == id }
end
@lazy_state[:pending_ids].clear
end
end
end
end
end
end
......@@ -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
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Graphql::Aggregations::Issues::LazyIssueLinkAggregate 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 lazy state' do
described_class.new(query_ctx, issue_id)
expect(query_ctx[:lazy_issue_link_aggregate][:pending_ids]).to match [issue_id]
end
end
describe '#issue_link_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 => [] } }
end
it 'does not make the query again' do
expect(IssueLink).not_to receive(:blocked_issues_for_collection)
subject.issue_link_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.issue_link_aggregate
lazy_state = subject.instance_variable_get(:@lazy_state)
expect(lazy_state[:pending_ids]).to be_empty
end
end
end
end
......@@ -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
......@@ -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 LazyIssueLinkAggregate service' do
expect(::Gitlab::Graphql::Aggregations::Issues::LazyIssueLinkAggregate).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
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