Commit 7dea43b7 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'epic_board_update' into 'master'

Update epic boards in GraphQL API

See merge request gitlab-org/gitlab!55237
parents 01c2af91 a5a9f181
......@@ -2,9 +2,20 @@
module Boards
class UpdateService < Boards::BaseService
PERMITTED_PARAMS = %i(name hide_backlog_list hide_closed_list).freeze
def execute(board)
filter_params
board.update(params)
end
def filter_params
params.slice!(*permitted_params)
end
def permitted_params
PERMITTED_PARAMS
end
end
end
......
......@@ -1906,6 +1906,7 @@ Represents an epic board.
| `hideBacklogList` | Boolean | Whether or not backlog list is hidden. |
| `hideClosedList` | Boolean | Whether or not closed list is hidden. |
| `id` | BoardsEpicBoardID! | Global ID of the epic board. |
| `labels` | LabelConnection | Labels of the board. |
| `lists` | EpicListConnection | Epic board lists. |
| `name` | String | Name of the epic board. |
| `webPath` | String! | Web path of the epic board. |
......@@ -1931,6 +1932,16 @@ Autogenerated return type of EpicBoardListCreate.
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
| `list` | EpicList | Epic list in the epic board. |
### EpicBoardUpdatePayload
Autogenerated return type of EpicBoardUpdate.
| Field | Type | Description |
| ----- | ---- | ----------- |
| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
| `epicBoard` | EpicBoard | The updated epic board. |
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
### EpicDescendantCount
Counts of descendent epics.
......
......@@ -5,7 +5,7 @@ module EE
module Boards
module Create
extend ActiveSupport::Concern
include Mutations::Boards::ScopedBoardMutation
prepend ::Mutations::Boards::ScopedBoardMutation
prepended do
include Mutations::Boards::ScopedBoardArguments
......
......@@ -10,7 +10,6 @@ module EE
argument :assignee_id,
::Types::GlobalIDType[::User],
required: false,
loads: ::Types::UserType,
description: 'The ID of user to be assigned to the board.'
# Cannot pre-load ::Types::MilestoneType because we are also assigning values like:
......
# frozen_string_literal: true
module EE
module Mutations
module Boards
module ScopedBoardMutation
extend ::Gitlab::Utils::Override
override :resolve
def resolve(**args)
parsed_params = parse_arguments(args)
super(**parsed_params)
end
def ready?(**args)
if args.slice(*mutually_exclusive_args).size > 1
arg_str = mutually_exclusive_args.map { |x| x.to_s.camelize(:lower) }.join(' or ')
raise ::Gitlab::Graphql::Errors::ArgumentError, "one and only one of #{arg_str} is required"
end
super
end
private
def parse_arguments(args = {})
if args[:assignee_id]
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
args[:assignee_id] = ::Types::GlobalIDType[::User].coerce_isolated_input(args[:assignee_id])
args[:assignee_id] = args[:assignee_id].model_id
end
if args[:milestone_id]
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
args[:milestone_id] = ::Types::GlobalIDType[::Milestone].coerce_isolated_input(args[:milestone_id])
args[:milestone_id] = args[:milestone_id].model_id
end
args[:label_ids] &&= args[:label_ids].map do |label_id|
::GitlabSchema.parse_gid(label_id, expected_type: ::Label).model_id
end
# we need this because we also pass `gid://gitlab/Iteration/-4` or `gid://gitlab/Iteration/-4`
# as `iteration_id` when we scope board to `Iteration::Predefined::Current` or `Iteration::Predefined::None`
args[:iteration_id] = args[:iteration_id].model_id if args[:iteration_id]
args
end
def mutually_exclusive_args
[:labels, :label_ids]
end
end
end
end
end
......@@ -5,7 +5,7 @@ module EE
module Boards
module Update
extend ActiveSupport::Concern
include Mutations::Boards::ScopedBoardMutation
prepend ::Mutations::Boards::ScopedBoardMutation
prepended do
include Mutations::Boards::ScopedBoardArguments
......
......@@ -37,6 +37,7 @@ module EE
mount_mutation ::Mutations::Boards::Update
mount_mutation ::Mutations::Boards::UpdateEpicUserPreferences
mount_mutation ::Mutations::Boards::EpicBoards::Create
mount_mutation ::Mutations::Boards::EpicBoards::Update
mount_mutation ::Mutations::Boards::EpicLists::Create
mount_mutation ::Mutations::Boards::Lists::UpdateLimitMetrics
mount_mutation ::Mutations::InstanceSecurityDashboard::AddProject
......
# frozen_string_literal: true
module Mutations
module Boards
module EpicBoards
class Update < ::Mutations::BaseMutation
include Mutations::Boards::CommonMutationArguments
prepend Mutations::Boards::ScopedBoardMutation
graphql_name 'EpicBoardUpdate'
authorize :admin_epic_board
argument :id,
::Types::GlobalIDType[::Boards::EpicBoard],
required: true,
description: 'The epic board global ID.'
argument :labels, [GraphQL::STRING_TYPE],
required: false,
description: 'Labels to be added to the board.'
argument :label_ids, [::Types::GlobalIDType[::Label]],
required: false,
description: 'The IDs of labels to be added to the board.'
field :epic_board,
Types::Boards::EpicBoardType,
null: true,
description: 'The updated epic board.'
def resolve(**args)
board = authorized_find!(id: args[:id])
unless Feature.enabled?(:epic_boards, board.resource_parent)
raise Gitlab::Graphql::Errors::ResourceNotAvailable, 'epic_boards feature is disabled'
end
::Boards::EpicBoards::UpdateService.new(board.resource_parent, current_user, args).execute(board)
{
epic_board: board.reset,
errors: errors_on_object(board)
}
end
private
def find_object(id:)
GitlabSchema.find_by_gid(id)
end
end
end
end
end
# frozen_string_literal: true
module Mutations
module Boards
module ScopedBoardMutation
extend ActiveSupport::Concern
def resolve(**args)
parsed_params = parse_arguments(args)
super(**parsed_params)
end
def ready?(**args)
if args.slice(*mutually_exclusive_args).size > 1
arg_str = mutually_exclusive_args.map { |x| x.to_s.camelize(:lower) }.join(' or ')
raise ::Gitlab::Graphql::Errors::ArgumentError, "one and only one of #{arg_str} is required"
end
super
end
private
def parse_arguments(args = {})
if args[:assignee_id]
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
args[:assignee_id] = ::Types::GlobalIDType[::User].coerce_isolated_input(args[:assignee_id])
args[:assignee_id] = args[:assignee_id].model_id
end
if args[:milestone_id]
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
args[:milestone_id] = ::Types::GlobalIDType[::Milestone].coerce_isolated_input(args[:milestone_id])
args[:milestone_id] = args[:milestone_id].model_id
end
args[:label_ids] &&= args[:label_ids].map do |label_id|
::GitlabSchema.parse_gid(label_id, expected_type: ::Label).model_id
end
# we need this because we also pass `gid://gitlab/Iteration/-4` or `gid://gitlab/Iteration/-4`
# as `iteration_id` when we scope board to `Iteration::Predefined::Current` or `Iteration::Predefined::None`
args[:iteration_id] = args[:iteration_id].model_id if args[:iteration_id]
args
end
def mutually_exclusive_args
[:labels, :label_ids]
end
end
end
end
......@@ -23,6 +23,9 @@ module Types
field :hide_closed_list, type: GraphQL::BOOLEAN_TYPE, null: true,
description: 'Whether or not closed list is hidden.'
field :labels, ::Types::LabelType.connection_type, null: true,
description: 'Labels of the board.'
field :lists,
Types::Boards::EpicListType.connection_type,
null: true,
......
......@@ -6,6 +6,7 @@ module Boards
has_many :epic_board_labels, foreign_key: :epic_board_id, inverse_of: :epic_board
has_many :epic_board_positions, foreign_key: :epic_board_id, inverse_of: :epic_board
has_many :epic_lists, -> { ordered }, foreign_key: :epic_board_id, inverse_of: :epic_board
has_many :labels, through: :epic_board_labels
validates :name, length: { maximum: 255 }, presence: true
......@@ -59,14 +60,6 @@ module Boards
nil
end
def label_ids
[]
end
def labels
[]
end
def weight
nil
end
......
# frozen_string_literal: true
module Boards
module EpicBoards
class UpdateService < Boards::UpdateService
extend ::Gitlab::Utils::Override
override :permitted_params
def permitted_params
permitted = PERMITTED_PARAMS
if parent.feature_available?(:scoped_issue_board)
permitted += %i(labels label_ids)
end
permitted
end
end
end
end
......@@ -5,23 +5,25 @@ module EE
module UpdateService
extend ::Gitlab::Utils::Override
override :execute
def execute(board)
unless parent.feature_available?(:scoped_issue_board)
params.delete(:milestone_id)
params.delete(:iteration_id)
params.delete(:assignee_id)
params.delete(:label_ids)
params.delete(:labels)
params.delete(:weight)
end
override :filter_params
def filter_params
super
filter_assignee
filter_labels
filter_milestone
filter_iteration
end
super
override :permitted_params
def permitted_params
permitted = super
if parent.feature_available?(:scoped_issue_board)
permitted += %i(milestone_id iteration_id assignee_id weight labels label_ids)
end
permitted
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ::Mutations::Boards::EpicBoards::Update 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(:mutation) { described_class.new(object: nil, context: { current_user: current_user }, field: nil) }
subject { mutation.resolve(id: board.to_global_id) }
shared_examples 'epic board update error' do
it 'raises error' do
expect { subject }
.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'field tests' do
subject { described_class }
it { is_expected.to have_graphql_arguments(:id, :name, :hideBacklogList, :hideClosedList, :labels, :labelIds) }
it { is_expected.to have_graphql_fields(:epic_board).at_least }
end
context 'with epic feature enabled and epic_boards feature flag enabled' do
before do
stub_licensed_features(epics: true)
stub_feature_flags(epic_boards: true)
end
context 'when user does not have permission to update epic board' do
it_behaves_like 'epic board update error'
end
context 'when user has permission to update epic board' do
before do
group.add_reporter(current_user)
end
it 'updates the epic board' do
result = mutation.resolve(id: board.to_global_id, name: 'new name', hide_backlog_list: true)
expect(result[:errors]).to be_empty
expect(result[:epic_board].name).to eq('new name')
expect(result[:epic_board].hide_backlog_list).to be_truthy
end
end
context 'with epic_boards feature flag disabled' do
before do
stub_feature_flags(epic_boards: false)
end
it_behaves_like 'epic board update error'
end
end
describe '#ready?' do
it 'raises an error when both labels and label_ids arguments are passed' do
label = create(:group_label)
expect do
mutation.ready?(id: board.to_global_id, labels: ['foo'], label_ids: [label.to_global_id.to_s])
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of labels or labelIds is required/)
end
end
end
......@@ -8,7 +8,7 @@ RSpec.describe GitlabSchema.types['EpicBoard'] do
specify { expect(described_class).to require_graphql_authorizations(:read_epic_board) }
it 'has specific fields' do
expected_fields = %w[id name lists hideBacklogList hideClosedList web_url web_path]
expected_fields = %w[id name lists hideBacklogList hideClosedList web_url web_path labels]
expect(described_class).to include_graphql_fields(*expected_fields)
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Mutations::Boards::EpicBoards::Update do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be_with_reload(:board) { create(:epic_board, group: group, name: 'orig name') }
let(:name) { 'board name' }
let(:mutation) { graphql_mutation(:epic_board_update, params) }
let(:label) { create(:group_label, group: group) }
let(:params) { { id: board.to_global_id.to_s, name: 'foo', hide_backlog_list: true, labels: [label.name] } }
subject { post_graphql_mutation(mutation, current_user: current_user) }
def mutation_response
graphql_mutation_response(:epic_board_update)
end
before do
stub_licensed_features(epics: true, scoped_issue_board: true)
end
context 'when the user does not have permission' do
it_behaves_like 'a mutation that returns a top-level access error'
end
context 'when the user has permission' do
before do
group.add_developer(current_user)
end
it 'returns the updated board' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response).to have_key('epicBoard')
expect(mutation_response['epicBoard']['name']).to eq(params[:name])
expect(mutation_response['epicBoard']['hideBacklogList']).to eq(params[:hide_backlog_list])
expect(mutation_response['epicBoard']['labels']['count']).to eq(1)
end
context 'when update fails' do
let(:params) { { id: board.to_global_id.to_s, name: 'x' * 256 } }
it 'returns an error' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response).to have_key('epicBoard')
expect(mutation_response['epicBoard']['name']).to eq('orig name')
expect(mutation_response['errors'].first).to eq('Name is too long (maximum is 255 characters)')
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Boards::EpicBoards::UpdateService, services: true do
let_it_be(:parent_group) { create(:group) }
let_it_be_with_reload(:group) { create(:group, parent: parent_group) }
let_it_be_with_reload(:board) { create(:epic_board, group: group) }
let_it_be(:user) { create(:user) }
let(:parent_label) { create(:group_label, group: parent_group) }
let(:other_label) { create(:group_label) }
let(:label) { create(:group_label, group: group) }
let(:epic_boards_enabled) { false }
let(:all_params) do
{ label_ids: [label.id, other_label.id, parent_label.id],
hide_backlog_list: true, hide_closed_list: true }
end
let(:updated_scoped_params) do
{ labels: [label, parent_label],
hide_backlog_list: true, hide_closed_list: true }
end
let(:updated_without_scoped_params) do
{ labels: [], hide_backlog_list: true, hide_closed_list: true }
end
it_behaves_like 'board update service'
end
......@@ -4,57 +4,46 @@ require 'spec_helper'
RSpec.describe Boards::UpdateService, services: true do
describe '#execute' do
let(:project) { create(:project, group: group) }
let(:group) { create(:group) }
let!(:board) { create(:board, group: group, name: 'Backend') }
it "updates board's name" do
service = described_class.new(group, double, name: 'Engineering')
service.execute(board)
expect(board).to have_attributes(name: 'Engineering')
let_it_be(:parent_group) { create(:group) }
let_it_be_with_refind(:group) { create(:group, parent: parent_group) }
let_it_be_with_refind(:project) { create(:project, group: group) }
let_it_be_with_reload(:board) { create(:board, group: group, name: 'Backend') }
let_it_be(:assignee) { create(:user) }
let_it_be(:milestone) { create(:milestone, group: group) }
let_it_be(:iteration) { create(:iteration, group: group) }
let_it_be(:parent_label) { create(:group_label, group: parent_group) }
let_it_be(:other_label) { create(:group_label) }
let_it_be(:label) { create(:group_label, group: group) }
let_it_be(:user) { create(:user) }
let(:all_params) do
{ milestone_id: milestone.id, iteration_id: iteration.id,
assignee_id: assignee.id,
label_ids: [label.id, other_label.id, parent_label.id],
weight: 1, hide_backlog_list: true, hide_closed_list: true }
end
it 'returns true with valid params' do
service = described_class.new(group, double, name: 'Engineering')
expect(service.execute(board)).to eq true
let(:updated_scoped_params) do
{ milestone: milestone, assignee: assignee, labels: [label, parent_label],
weight: 1, hide_backlog_list: true, hide_closed_list: true }
end
it 'returns false with invalid params' do
service = described_class.new(group, double, name: nil)
expect(service.execute(board)).to eq false
let(:updated_without_scoped_params) do
{ milestone: nil, assignee: nil, labels: [], weight: nil,
hide_backlog_list: true, hide_closed_list: true }
end
it 'updates the configuration params when scoped issue board is enabled' do
stub_licensed_features(scoped_issue_board: true)
assignee = create(:user)
milestone = create(:milestone, group: group)
iteration = create(:iteration, group: group)
label = create(:group_label, group: board.group)
user = create(:user)
params = { milestone_id: milestone.id, iteration_id: iteration.id, assignee_id: assignee.id, label_ids: [label.id], hide_backlog_list: true, hide_closed_list: true }
service = described_class.new(group, user, params)
context 'with group board' do
let!(:board) { create(:board, group: group, name: 'Backend') }
service.execute(board)
expected_attributes = { milestone: milestone, assignee: assignee, labels: [label], hide_backlog_list: true, hide_closed_list: true }
expect(board.reload).to have_attributes(expected_attributes)
it_behaves_like 'board update service'
end
it 'filters unpermitted params when scoped issue board is not enabled' do
stub_licensed_features(scoped_issue_board: false)
unpermitted_params = { milestone_id: double, iteration_id: double, assignee_id: double, label_ids: double, weight: double }
permitted_params = { hide_backlog_list: true, hide_closed_list: true }
params = unpermitted_params.merge(permitted_params)
service = described_class.new(project, double, params)
service.execute(board)
context 'with project board' do
let!(:board) { create(:board, project: project, name: 'Backend') }
expected_attributes = { milestone: nil, iteration: nil, assignee: nil, labels: [], hide_backlog_list: true, hide_closed_list: true }
expect(board.reload).to have_attributes(expected_attributes)
it_behaves_like 'board update service'
end
it_behaves_like 'setting a milestone scope' do
......@@ -72,121 +61,5 @@ RSpec.describe Boards::UpdateService, services: true do
described_class.new(parent, nil, iteration_id: iteration.id).execute(board)
end
end
describe '#set_labels' do
def expect_label_assigned(user, board, params, expected_labels)
service = described_class.new(board.resource_parent, user, params)
service.execute(board)
expect(board.reload.labels.map(&:title)).to contain_exactly(*expected_labels)
end
let(:user) { create(:user) }
let(:role) { :guest }
let(:input_labels) { %w{group_label new_label} }
let(:labels_param) { { labels: input_labels.join(',') } }
let(:label_ids_param) { { label_ids: [group_label.id] } }
context 'group board labels' do
let!(:group_label) { create(:group_label, title: 'group_label', group: group) }
before do
group.add_user(user, role)
stub_licensed_features(scoped_issue_board: true)
end
it 'updates using only existing label' do
expect_label_assigned(user, board, labels_param, %w{group_label})
end
context 'user with admin_label ability' do
let(:role) { :reporter }
it 'finds and creates labels' do
expect_label_assigned(user, board, labels_param, input_labels)
end
context 'when scoped_issue_board disabled' do
before do
stub_licensed_features(scoped_issue_board: false)
end
it 'does not create labels' do
expect_label_assigned(user, board, labels_param, [])
expect_label_assigned(user, board, label_ids_param, [])
end
end
context 'nested group' do
let!(:child_group) { create(:group, parent: group)}
let!(:board) { create(:board, group: child_group, name: 'Child Backend') }
it "allows using ancestor group's label" do
expect_label_assigned(user, board, labels_param, input_labels)
end
end
end
end
context 'project board labels' do
let(:project) { create(:project, group: group) }
let!(:board) { create(:board, project: project, name: 'Backend') }
let!(:group_label) { create(:group_label, title: 'group_label', group: group) }
let!(:label) { create(:label, title: 'project_label', project: project) }
let(:input_labels) { %w{group_label project_label new_label} }
let(:labels_param) { { labels: input_labels.join(',') } }
let(:label_ids_param) { { label_ids: [group_label.id, label.id] } }
before do
project.add_user(user, role)
stub_licensed_features(scoped_issue_board: true)
end
context 'user with admin_label ability' do
let(:role) { :reporter }
it 'finds and creates labels' do
expect_label_assigned(user, board, labels_param, input_labels)
end
context 'when scoped_issue_board disabled' do
before do
stub_licensed_features(scoped_issue_board: false)
end
it 'does not create labels' do
expect_label_assigned(user, board, labels_param, [])
expect_label_assigned(user, board, label_ids_param, [])
end
end
end
it 'updates using only existing label' do
expect_label_assigned(user, board, labels_param, %w{group_label project_label})
end
context 'nested group' do
let!(:child_group) { create(:group, parent: group)}
let(:project) { create(:project, group: child_group) }
it "allows using ancestor group's label" do
expect_label_assigned(user, board, labels_param, %w{group_label project_label})
end
end
context 'when label_ids param is provided' do
it 'updates using only labels accessible by the project board' do
other_project_label = create(:label, title: 'other_project_label')
other_group_label = create(:group_label, title: 'other_group_label')
label_ids = [group_label.id, label.id, other_project_label.id, other_group_label.id]
described_class.new(board.resource_parent, user, label_ids: label_ids).execute(board)
expect(board.reload.labels).to contain_exactly(group_label, label)
end
end
end
end
end
end
# frozen_string_literal: true
RSpec.shared_examples 'board update service' do
subject(:service) { described_class.new(board.resource_parent, user, all_params) }
it 'updates the board with valid params' do
result = described_class.new(group, user, name: 'Engineering').execute(board)
expect(result).to eq(true)
expect(board.reload.name).to eq('Engineering')
end
it 'does not update the board with invalid params' do
orig_name = board.name
result = described_class.new(group, user, name: nil).execute(board)
expect(result).to eq(false)
expect(board.reload.name).to eq(orig_name)
end
context 'with scoped_issue_board available' do
before do
stub_licensed_features(scoped_issue_board: true)
end
context 'user is member of the board parent' do
before do
board.resource_parent.add_reporter(user)
end
it 'updates the configuration params when scoped issue board is enabled' do
service.execute(board)
labels = updated_scoped_params.delete(:labels)
expect(board.reload).to have_attributes(updated_scoped_params)
expect(board.labels).to match_array(labels)
end
end
context 'when labels param is used' do
let(:params) { { labels: [label.name, parent_label.name, 'new label'].join(',') } }
subject(:service) { described_class.new(board.resource_parent, user, params) }
context 'when user can create new labels' do
before do
board.resource_parent.add_reporter(user)
end
it 'adds labels to the board' do
service.execute(board)
expect(board.reload.labels.map(&:name)).to match_array([label.name, parent_label.name, 'new label'])
end
end
context 'when user can not create new labels' do
before do
board.resource_parent.add_guest(user)
end
it 'adds only existing labels to the board' do
service.execute(board)
expect(board.reload.labels.map(&:name)).to match_array([label.name, parent_label.name])
end
end
end
end
context 'without scoped_issue_board available' do
before do
stub_licensed_features(scoped_issue_board: false)
end
it 'filters unpermitted params when scoped issue board is not enabled' do
service.execute(board)
expect(board.reload).to have_attributes(updated_without_scoped_params)
end
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