From 1ab8525fb5b719a27be56637fa2ed617b92607ce Mon Sep 17 00:00:00 2001
From: Rajat Jain <rjain@gitlab.com>
Date: Mon, 25 Apr 2022 14:22:09 +0530
Subject: [PATCH] Add epic board list resolver
- Add top level query to fetch an epic list
Update graphql docs
- Modify list service to take board and list
Modify Boards::BaseItemsListService to use list
from params if present
Modify Boards::Epics::ListService to use board
rom params if present
Add specs
Changelog: added
EE: true
---
.../boards/base_items_list_service.rb | 7 +-
doc/api/graphql/reference/index.md | 11 +++
ee/app/graphql/ee/types/query_type.rb | 4 +
.../boards/board_list_epics_resolver.rb | 8 +-
.../resolvers/boards/epic_list_resolver.rb | 32 +++++++
ee/app/graphql/types/boards/epic_list_type.rb | 14 +++-
ee/app/services/boards/epics/list_service.rb | 2 +-
.../boards/epic_list_resolver_spec.rb | 45 ++++++++++
ee/spec/graphql/types/query_type_spec.rb | 13 ++-
.../graphql/boards/epic_list_query_spec.rb | 84 +++++++++++++++++++
.../boards/epics/list_service_spec.rb | 14 +++-
.../items_list_service_shared_examples.rb | 40 ++++++---
12 files changed, 250 insertions(+), 24 deletions(-)
create mode 100644 ee/app/graphql/resolvers/boards/epic_list_resolver.rb
create mode 100644 ee/spec/graphql/resolvers/boards/epic_list_resolver_spec.rb
create mode 100644 ee/spec/requests/api/graphql/boards/epic_list_query_spec.rb
diff --git a/app/services/boards/base_items_list_service.rb b/app/services/boards/base_items_list_service.rb
index 01fad14d036e..2a9cbb83cc41 100644
--- a/app/services/boards/base_items_list_service.rb
+++ b/app/services/boards/base_items_list_service.rb
@@ -78,12 +78,15 @@ def filter(items)
end
def list
- return unless params.key?(:id)
+ return unless params.key?(:id) || params.key?(:list)
strong_memoize(:list) do
id = params[:id]
+ list = params[:list]
- if board.lists.loaded?
+ if list.present?
+ list
+ elsif board.lists.loaded?
board.lists.find { |l| l.id == id }
else
board.lists.find(id)
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index d07b84cbe69c..8306249eb373 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -145,6 +145,17 @@ Returns [`String!`](#string).
| ---- | ---- | ----------- |
| <a id="queryechotext"></a>`text` | [`String!`](#string) | Text to echo back. |
+### `Query.epicBoardList`
+
+Returns [`EpicList`](#epiclist).
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="queryepicboardlistepicfilters"></a>`epicFilters` | [`EpicFilters`](#epicfilters) | Filters applied when getting epic metadata in the epic board list. |
+| <a id="queryepicboardlistid"></a>`id` | [`BoardsEpicListID!`](#boardsepiclistid) | Global ID of the list. |
+
### `Query.geoNode`
Find a Geo node.
diff --git a/ee/app/graphql/ee/types/query_type.rb b/ee/app/graphql/ee/types/query_type.rb
index 1658bd517e41..cfd4a1de1840 100644
--- a/ee/app/graphql/ee/types/query_type.rb
+++ b/ee/app/graphql/ee/types/query_type.rb
@@ -75,6 +75,10 @@ module QueryType
required: false,
description: 'Global ID of the Namespace for the monthly CI/CD minutes usage.'
end
+
+ field :epic_board_list, ::Types::Boards::EpicListType,
+ null: true,
+ resolver: ::Resolvers::Boards::EpicListResolver
end
def vulnerability(id:)
diff --git a/ee/app/graphql/resolvers/boards/board_list_epics_resolver.rb b/ee/app/graphql/resolvers/boards/board_list_epics_resolver.rb
index c62f8ac21bbc..123dfe502e7c 100644
--- a/ee/app/graphql/resolvers/boards/board_list_epics_resolver.rb
+++ b/ee/app/graphql/resolvers/boards/board_list_epics_resolver.rb
@@ -12,9 +12,13 @@ class BoardListEpicsResolver < BaseResolver
description: 'Filters applied when selecting epics in the board list.'
def resolve(filters: {}, **args)
- filter_params = { board_id: list.epic_board.id, id: list.id }.merge(filters)
+ filter_params = { board: list.epic_board, list: list }.merge(filters)
- service = ::Boards::Epics::ListService.new(list.epic_board.group, context[:current_user], filter_params)
+ service = ::Boards::Epics::ListService.new(
+ list.epic_board.group,
+ context[:current_user],
+ filter_params
+ )
offset_pagination(service.execute)
end
diff --git a/ee/app/graphql/resolvers/boards/epic_list_resolver.rb b/ee/app/graphql/resolvers/boards/epic_list_resolver.rb
new file mode 100644
index 000000000000..f6c8421169e8
--- /dev/null
+++ b/ee/app/graphql/resolvers/boards/epic_list_resolver.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Resolvers
+ module Boards
+ class EpicListResolver < BaseResolver.single
+ include Gitlab::Graphql::Authorize::AuthorizeResource
+ include ::BoardItemFilterable
+
+ type Types::Boards::EpicListType, null: true
+
+ argument :id, ::Types::GlobalIDType[::Boards::EpicList],
+ required: true,
+ loads: Types::Boards::EpicListType,
+ as: :list,
+ description: 'Global ID of the list.'
+
+ argument :epic_filters, ::Types::Boards::BoardEpicInputType,
+ required: false,
+ description: 'Filters applied when getting epic metadata in the epic board list.'
+
+ authorize :read_epic_board_list
+
+ def resolve(list:, epic_filters: {})
+ authorize! list
+
+ context.scoped_set!(:epic_filters, item_filters(epic_filters))
+
+ list
+ end
+ end
+ end
+end
diff --git a/ee/app/graphql/types/boards/epic_list_type.rb b/ee/app/graphql/types/boards/epic_list_type.rb
index 79e90190d348..beb401791a06 100644
--- a/ee/app/graphql/types/boards/epic_list_type.rb
+++ b/ee/app/graphql/types/boards/epic_list_type.rb
@@ -2,7 +2,6 @@
module Types
module Boards
- # rubocop: disable Graphql/AuthorizeTypes
class EpicListType < BaseObject
graphql_name 'EpicList'
description 'Represents an epic board list'
@@ -10,6 +9,7 @@ class EpicListType < BaseObject
include Gitlab::Utils::StrongMemoize
accepts ::Boards::EpicList
+ authorize :read_epic_board_list
alias_method :list, :object
@@ -64,9 +64,17 @@ def list_service
end
def params
- (context[:epic_filters] || {}).merge(board_id: list.epic_board_id, id: list.id)
+ (context[:epic_filters] || {}).merge(board: list.epic_board, list: list)
+ end
+
+ def title
+ BatchLoader::GraphQL.for(object).batch do |lists, callback|
+ ActiveRecord::Associations::Preloader.new.preload(lists, :label) # rubocop: disable CodeReuse/ActiveRecord
+
+ # all list titles are preloaded at this point
+ lists.each { |list| callback.call(list, list.title) }
+ end
end
end
- # rubocop: enable Graphql/AuthorizeTypes
end
end
diff --git a/ee/app/services/boards/epics/list_service.rb b/ee/app/services/boards/epics/list_service.rb
index ceba8181d273..57fedbead33f 100644
--- a/ee/app/services/boards/epics/list_service.rb
+++ b/ee/app/services/boards/epics/list_service.rb
@@ -47,7 +47,7 @@ def exclude_positioned?
end
def board
- @board ||= parent.epic_boards.find(params[:board_id])
+ @board ||= params[:board].presence || parent.epic_boards.find(params[:board_id])
end
def order(items)
diff --git a/ee/spec/graphql/resolvers/boards/epic_list_resolver_spec.rb b/ee/spec/graphql/resolvers/boards/epic_list_resolver_spec.rb
new file mode 100644
index 000000000000..a0e6e9bf05f8
--- /dev/null
+++ b/ee/spec/graphql/resolvers/boards/epic_list_resolver_spec.rb
@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Resolvers::Boards::EpicListResolver do
+ include GraphqlHelpers
+ include Gitlab::Graphql::Laziness
+
+ let_it_be(:guest) { create(:user) }
+ let_it_be(:unauth_user) { create(:user) }
+ let_it_be(:group) { create(:group, :private) }
+ let_it_be(:group_label) { create(:group_label, group: group, name: 'Development') }
+ let_it_be(:board) { create(:epic_board, group: group) }
+ let_it_be(:label_list) { create(:epic_list, epic_board: board, label: group_label) }
+
+ describe '#resolve' do
+ subject { resolve_epic_board_list(args: { id: global_id_of(label_list) }, current_user: current_user) }
+
+ before do
+ stub_licensed_features(epics: true)
+ end
+
+ context 'with unauthorized user' do
+ let(:current_user) { unauth_user }
+
+ it 'raises unauthorized error' do
+ expect { subject }.to raise_error(GraphQL::UnauthorizedError)
+ end
+ end
+
+ context 'when authorized' do
+ let(:current_user) { guest }
+
+ before do
+ group.add_guest(guest)
+ end
+
+ it { is_expected.to eq label_list }
+ end
+ end
+
+ def resolve_epic_board_list(args: {}, current_user: user)
+ force(resolve(described_class, obj: nil, args: args, ctx: { current_user: current_user }))
+ end
+end
diff --git a/ee/spec/graphql/types/query_type_spec.rb b/ee/spec/graphql/types/query_type_spec.rb
index deeccfe8caed..e60017bd4b43 100644
--- a/ee/spec/graphql/types/query_type_spec.rb
+++ b/ee/spec/graphql/types/query_type_spec.rb
@@ -14,7 +14,18 @@
:subscription_future_entries,
:vulnerabilities,
:vulnerabilities_count_by_day,
- :vulnerability
+ :vulnerability,
+ :epic_board_list
).at_least
end
+
+ describe 'epicBoardList field' do
+ subject { described_class.fields['epicBoardList'] }
+
+ it 'finds an epic board list by its gid' do
+ is_expected.to have_graphql_arguments(:id, :epic_filters)
+ is_expected.to have_graphql_type(Types::Boards::EpicListType)
+ is_expected.to have_graphql_resolver(Resolvers::Boards::EpicListResolver)
+ end
+ end
end
diff --git a/ee/spec/requests/api/graphql/boards/epic_list_query_spec.rb b/ee/spec/requests/api/graphql/boards/epic_list_query_spec.rb
new file mode 100644
index 000000000000..962e0ec05517
--- /dev/null
+++ b/ee/spec/requests/api/graphql/boards/epic_list_query_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Querying an Epic board list' do
+ include GraphqlHelpers
+
+ let_it_be(:current_user) { create(:user) }
+ let_it_be(:group) { create(:group, :private) }
+ let_it_be(:board) { create(:epic_board, group: group) }
+ let_it_be(:label) { create(:group_label, group: group, name: 'Development') }
+ let_it_be(:list) { create(:epic_list, epic_board: board, label: label) }
+ let_it_be(:epic1) { create(:epic, group: group, labels: [label], title: 'Epic1') }
+ let_it_be(:epic2) { create(:epic, group: group, labels: [label], title: 'Epic2') }
+ let_it_be(:epic3) { create(:epic, group: group, labels: [label], title: 'Epic3') }
+
+ let(:filters) { {} }
+ let(:query) do
+ graphql_query_for(
+ :epic_board_list,
+ { id: list.to_global_id.to_s, epicFilters: filters }, %w[id]
+ )
+ end
+
+ subject { graphql_data['epicBoardList'] }
+
+ before do
+ stub_licensed_features(epics: true)
+ post_graphql(query, current_user: current_user)
+ end
+
+ context 'when the user has access to the epic list' do
+ before_all do
+ group.add_guest(current_user)
+ end
+
+ it_behaves_like 'a working graphql query'
+
+ it { is_expected.to include({ 'id' => list.to_global_id.to_s }) }
+ end
+
+ context 'when the user does not have access to the list' do
+ it { is_expected.to be_nil }
+ end
+
+ context 'when ID argument is missing' do
+ let(:query) do
+ graphql_query_for('epicBoardList', {}, 'title')
+ end
+
+ it 'raises an exception' do
+ expect(graphql_errors).to include(a_hash_including('message' =>
+ "Field 'epicBoardList' is missing required arguments: id"))
+ end
+ end
+
+ context 'when list ID is not found' do
+ let(:query) do
+ graphql_query_for('boardList', { id: "gid://gitlab/List/#{non_existing_record_id}" }, 'title')
+ end
+
+ it { is_expected.to be_nil }
+ end
+
+ it 'does not have an N+1 when querying title' do
+ a, b = create_list(:epic_list, 2, epic_board: board)
+ ctx = { current_user: current_user }
+ group.add_guest(current_user)
+
+ baseline = graphql_query_for(:epic_board_list, { id: global_id_of(a) }, 'title')
+ query = <<~GQL
+ query {
+ a: #{query_graphql_field(:epic_board_list, { id: global_id_of(a) }, 'title')}
+ b: #{query_graphql_field(:epic_board_list, { id: global_id_of(b) }, 'title')}
+ }
+ GQL
+
+ control = ActiveRecord::QueryRecorder.new do
+ run_with_clean_state(baseline, context: ctx)
+ end
+
+ expect { run_with_clean_state(query, context: ctx) }.not_to exceed_query_limit(control)
+ end
+end
diff --git a/ee/spec/services/boards/epics/list_service_spec.rb b/ee/spec/services/boards/epics/list_service_spec.rb
index 83daf2d0ba99..81c4c5ed5a6b 100644
--- a/ee/spec/services/boards/epics/list_service_spec.rb
+++ b/ee/spec/services/boards/epics/list_service_spec.rb
@@ -58,6 +58,12 @@
.new(group, user, { board_id: board.id, id: list1.id, from_id: list1_epic2.id })
.execute
end
+
+ it 'returns opened items when using board param only' do
+ epics = described_class.new(group, user, { board: board }).execute
+
+ expect(epics).to match_array([backlog_epic1])
+ end
end
describe '#metadata' do
@@ -74,7 +80,7 @@
context 'with all fields included in the required_fields' do
let(:fields) { [:total_weight, :epics_count] }
- it 'containes correct data including weight' do
+ it 'contains correct data including weight' do
expect(subject).to eq({ total_weight: 7, epics_count: 3 })
end
end
@@ -82,15 +88,15 @@
context 'with total_weight not included in the required_fields' do
let(:fields) { [:epics_count] }
- it 'containes correct data without weight' do
+ it 'contains correct data without weight' do
expect(subject).to eq({ epics_count: 3 })
end
end
- context 'with epics_countr not included in the required_fields' do
+ context 'with epics_count not included in the required_fields' do
let(:fields) { [:total_weight] }
- it 'containes correct data without weight' do
+ it 'contains correct data without weight' do
expect(subject).to eq({ total_weight: 7 })
end
end
diff --git a/spec/support/shared_examples/services/boards/items_list_service_shared_examples.rb b/spec/support/shared_examples/services/boards/items_list_service_shared_examples.rb
index 9a3a0cc9cc8f..ed05a150f8b0 100644
--- a/spec/support/shared_examples/services/boards/items_list_service_shared_examples.rb
+++ b/spec/support/shared_examples/services/boards/items_list_service_shared_examples.rb
@@ -3,17 +3,17 @@
RSpec.shared_examples 'items list service' do
it 'avoids N+1' do
params = { board_id: board.id }
- control = ActiveRecord::QueryRecorder.new { described_class.new(parent, user, params).execute }
+ control = ActiveRecord::QueryRecorder.new { list_service(params).execute }
new_list
- expect { described_class.new(parent, user, params).execute }.not_to exceed_query_limit(control)
+ expect { list_service(params).execute }.not_to exceed_query_limit(control)
end
- it 'returns opened items when list_id is missing' do
+ it 'returns opened items when list_id and list are missing' do
params = { board_id: board.id }
- items = described_class.new(parent, user, params).execute
+ items = list_service(params).execute
expect(items).to match_array(backlog_items)
end
@@ -21,7 +21,7 @@
it 'returns opened items when listing items from Backlog' do
params = { board_id: board.id, id: backlog.id }
- items = described_class.new(parent, user, params).execute
+ items = list_service(params).execute
expect(items).to match_array(backlog_items)
end
@@ -29,7 +29,7 @@
it 'returns opened items that have label list applied when listing items from a label list' do
params = { board_id: board.id, id: list1.id }
- items = described_class.new(parent, user, params).execute
+ items = list_service(params).execute
expect(items).to match_array(list1_items)
end
@@ -37,20 +37,24 @@
it 'returns closed items when listing items from Closed sorted by closed_at in descending order' do
params = { board_id: board.id, id: closed.id }
- items = described_class.new(parent, user, params).execute
+ items = list_service(params).execute
expect(items).to eq(closed_items)
end
it 'raises an error if the list does not belong to the board' do
list = create(list_factory) # rubocop:disable Rails/SaveBang
- service = described_class.new(parent, user, board_id: board.id, id: list.id)
+ params = { board_id: board.id, id: list.id }
+
+ service = list_service(params)
expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound)
end
- it 'raises an error if list id is invalid' do
- service = described_class.new(parent, user, board_id: board.id, id: nil)
+ it 'raises an error if list and list id are invalid or missing' do
+ params = { board_id: board.id, id: nil, list: nil }
+
+ service = list_service(params)
expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound)
end
@@ -58,8 +62,22 @@
it 'returns items from all lists if :all_list is used' do
params = { board_id: board.id, all_lists: true }
- items = described_class.new(parent, user, params).execute
+ items = list_service(params).execute
expect(items).to match_array(all_items)
end
+
+ it 'returns opened items that have label list applied when using list param' do
+ params = { board_id: board.id, list: list1 }
+
+ items = list_service(params).execute
+
+ expect(items).to match_array(list1_items)
+ end
+
+ def list_service(params)
+ args = [parent, user].push(params)
+
+ described_class.new(*args)
+ end
end
--
GitLab