Commit d75bb7e0 authored by charlie ablett's avatar charlie ablett

Destroy epic board list

parent 958ffa99
# frozen_string_literal: true
module Boards
module Lists
# This class is used by issue and epic board lists
# for destroying a single list
class BaseDestroyService < Boards::BaseService
def execute(list)
unless list.destroyable?
return ServiceResponse.error(message: "Open and closed lists on a board cannot be destroyed.")
end
list.with_lock do
decrement_higher_lists(list)
list.destroy!
end
ServiceResponse.success
rescue StandardError => e
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
ServiceResponse.error(message: "List destroy failed.")
end
private
# rubocop: disable CodeReuse/ActiveRecord
def decrement_higher_lists(list)
list.board.lists.movable.where('position > ?', list.position)
.update_all('position = position - 1')
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
end
......@@ -2,36 +2,8 @@
module Boards
module Lists
class DestroyService < Boards::BaseService
def execute(list)
unless list.destroyable?
return ServiceResponse.error(message: "The list cannot be destroyed. Only label lists can be destroyed.")
end
@board = list.board
list.with_lock do
decrement_higher_lists(list)
remove_list(list)
end
ServiceResponse.success
end
private
attr_reader :board
# rubocop: disable CodeReuse/ActiveRecord
def decrement_higher_lists(list)
board.lists.movable.where('position > ?', list.position)
.update_all('position = position - 1')
end
# rubocop: enable CodeReuse/ActiveRecord
def remove_list(list)
list.destroy!
end
# overridden in EE for board lists and also for epic board lists.
class DestroyService < Boards::Lists::BaseDestroyService
end
end
end
......@@ -2006,6 +2006,27 @@ Input type: `EpicBoardListCreateInput`
| <a id="mutationepicboardlistcreateerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationepicboardlistcreatelist"></a>`list` | [`EpicList`](#epiclist) | Epic list in the epic board. |
### `Mutation.epicBoardListDestroy`
Destroys an epic board list.
Input type: `EpicBoardListDestroyInput`
#### Arguments
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationepicboardlistdestroyclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationepicboardlistdestroylistid"></a>`listId` | [`BoardsEpicListID!`](#boardsepiclistid) | Global ID of the epic board list to destroy. |
#### Fields
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationepicboardlistdestroyclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationepicboardlistdestroyerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationepicboardlistdestroylist"></a>`list` | [`EpicList`](#epiclist) | The epic board list. `null` if the board was destroyed successfully. |
### `Mutation.epicBoardUpdate`
Input type: `EpicBoardUpdateInput`
......
......@@ -44,6 +44,7 @@ module EE
mount_mutation ::Mutations::Boards::EpicBoards::EpicMoveList
mount_mutation ::Mutations::Boards::EpicBoards::Update
mount_mutation ::Mutations::Boards::EpicLists::Create
mount_mutation ::Mutations::Boards::EpicLists::Destroy
mount_mutation ::Mutations::Boards::EpicLists::Update
mount_mutation ::Mutations::Boards::Lists::UpdateLimitMetrics
mount_mutation ::Mutations::InstanceSecurityDashboard::AddProject
......
# frozen_string_literal: true
module Mutations
module Boards
module EpicLists
class Destroy < ::Mutations::BaseMutation
graphql_name 'EpicBoardListDestroy'
description 'Destroys an epic board list.'
argument :list_id, ::Types::GlobalIDType[::Boards::EpicList],
required: true,
loads: Types::Boards::EpicListType,
description: 'Global ID of the epic board list to destroy.'
field :list,
Types::Boards::EpicListType,
null: true,
description: 'The epic board list. `null` if the board was destroyed successfully.'
authorize :admin_epic_board_list
def resolve(list:)
raise_resource_not_available_error! unless can_admin_list?(list)
# authorisation is handled by the service in order to return consistent responses
# and so the service is essentially the authorisation SSOT.
response = ::Boards::EpicLists::DestroyService.new(list.board.resource_parent, current_user).execute(list)
list_result = response.success? ? nil : list
mutation_response(list_result, response.errors)
end
def mutation_response(list_object, errors)
{
list: list_object,
errors: errors
}
end
private
def can_admin_list?(list)
Ability.allowed?(current_user, :admin_epic_board_list, list)
end
end
end
end
end
......@@ -16,6 +16,7 @@ module Boards
scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) }
alias_method :preferences, :epic_list_user_preferences
alias_method :board, :epic_board
def preferences_for(user)
return preferences.build unless user
......@@ -29,9 +30,5 @@ module Boards
end
end
end
def board
epic_board
end
end
end
# frozen_string_literal: true
module Boards
class EpicListPolicy < ::BasePolicy
delegate :board
end
end
# frozen_string_literal: true
module Boards
module EpicLists
class DestroyService < ::Boards::Lists::BaseDestroyService
extend ::Gitlab::Utils::Override
override :execute
def execute(list)
unless list.board.group.licensed_feature_available?(:epics)
return ServiceResponse.error(message: 'Epics feature is not available.')
end
unless Feature.enabled?(:epic_boards, list.board.group)
return ServiceResponse.error(message: 'Epic boards feature is not enabled.')
end
unless can?(current_user, :admin_epic_board_list, list)
return ServiceResponse.error(message: 'The epic board list that you are attempting to destroy does not '\
'exist or you don\'t have permission to perform this action')
end
super
end
end
end
end
---
title: Epic board lists can now be destroyed via GraphQL
merge_request: 58719
author:
type: added
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Destroy an epic board list' do
include GraphqlHelpers
let_it_be(:current_user, reload: true) { create(:user) }
let_it_be(:group) { create(:group, :private) }
let_it_be(:board) { create(:epic_board, group: group) }
let_it_be(:list) { create(:epic_list, epic_board: board) }
let(:variables) do
{
list_id: global_id_of(list)
}
end
let(:mutation) do
graphql_mutation(:epic_board_list_destroy, variables)
end
let(:mutation_response) { graphql_mutation_response(:epic_board_list_destroy) }
before do
stub_licensed_features(epics: true)
end
it_behaves_like 'board lists destroy request' do
let(:klass) { Boards::EpicList }
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Boards::EpicLists::DestroyService do
let_it_be(:group) { create(:group) }
let_it_be_with_reload(:board) { create(:epic_board, group: group) }
let_it_be(:label) { create(:group_label, group: group, name: 'in-progress') }
let_it_be(:closed_list) { board.lists.create!(list_type: :closed) }
let!(:list) { create(:epic_list, epic_board: board) }
let(:user) { create(:user) }
let(:parent) { group }
let(:list_type) { :epic_list }
let(:params) do
{ epic_board: board }
end
before do
stub_licensed_features(epics: true)
stub_feature_flags(epic_boards: true)
end
context 'when user does not have permission' do
it 'returns an error' do
response = described_class.new(parent, nil).execute(list)
expect(response).to be_error
expect(response.errors).to include('The epic board list that you are attempting to destroy does not '\
'exist or you don\'t have permission to perform this action')
end
end
context 'when user has permission' do
before do
group.add_maintainer(user)
end
it_behaves_like 'lists destroy service'
context 'when epic feature is unavailable' do
before do
stub_licensed_features(epics: false)
end
it 'returns an error' do
response = described_class.new(parent, nil).execute(list)
expect(response).to be_error
expect(response.errors).to include("Epics feature is not available.")
end
end
context 'when epic_boards feature flag is disabled' do
before do
stub_feature_flags(epic_boards: false)
end
it 'returns an error' do
response = described_class.new(parent, nil).execute(list)
expect(response).to be_error
expect(response.errors).to include("Epic boards feature is not enabled.")
end
end
end
end
......@@ -6,72 +6,22 @@ RSpec.describe Mutations::Boards::Lists::Destroy do
include GraphqlHelpers
let_it_be(:current_user, reload: true) { create(:user) }
let_it_be(:project, reload: true) { create(:project) }
let_it_be(:board) { create(:board, project: project) }
it_behaves_like 'board lists destroy request' do
let_it_be(:group, reload: true) { create(:group) }
let_it_be(:board) { create(:board, group: group) }
let_it_be(:list) { create(:list, board: board) }
let(:mutation) do
variables = {
let(:variables) do
{
list_id: GitlabSchema.id_from_object(list).to_s
}
graphql_mutation(:destroy_board_list, variables)
end
subject { post_graphql_mutation(mutation, current_user: current_user) }
def mutation_response
graphql_mutation_response(:destroy_board_list)
end
context 'when the user does not have permission' do
it_behaves_like 'a mutation that returns a top-level access error'
it 'does not destroy the list' do
expect { subject }.not_to change { List.count }
end
end
context 'when the user has permission' do
before do
project.add_maintainer(current_user)
end
context 'when given id is not for a list' do
let_it_be(:list) { build_stubbed(:issue, project: project) }
it 'returns an error' do
subject
expect(graphql_errors.first['message']).to include('does not represent an instance of List')
end
end
context 'when everything is ok' do
it 'destroys the list' do
expect { subject }.to change { List.count }.from(2).to(1)
end
it 'returns an empty list' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response).to have_key('list')
expect(mutation_response['list']).to be_nil
end
end
context 'when the list is not destroyable' do
let_it_be(:list) { create(:list, board: board, list_type: :backlog) }
it 'does not destroy the list' do
expect { subject }.not_to change { List.count }.from(3)
let(:mutation) do
graphql_mutation(:destroy_board_list, variables)
end
it 'returns an error and not nil list' do
subject
expect(mutation_response['errors']).not_to be_empty
expect(mutation_response['list']).not_to be_nil
end
end
let(:mutation_response) { graphql_mutation_response(:destroy_board_list) }
let(:klass) { List }
end
end
......@@ -3,11 +3,20 @@
require 'spec_helper'
RSpec.describe Boards::Lists::DestroyService do
let_it_be(:user) { create(:user) }
let(:list_type) { :list }
describe '#execute' do
context 'when board parent is a project' do
let(:project) { create(:project) }
let(:board) { create(:board, project: project) }
let(:user) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:board) { create(:board, project: project) }
let_it_be(:list) { create(:list, board: board) }
let_it_be(:closed_list) { board.lists.closed.first }
let(:params) do
{ board: board }
end
let(:parent) { project }
......@@ -15,9 +24,14 @@ RSpec.describe Boards::Lists::DestroyService do
end
context 'when board parent is a group' do
let(:group) { create(:group) }
let(:board) { create(:board, group: group) }
let(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:board) { create(:board, group: group) }
let_it_be(:list) { create(:list, board: board) }
let_it_be(:closed_list) { board.lists.closed.first }
let(:params) do
{ board: board }
end
let(:parent) { group }
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.shared_examples 'board lists destroy request' do
include GraphqlHelpers
subject { post_graphql_mutation(mutation, current_user: current_user) }
shared_examples 'does not destroy the list and returns an error' do
it 'does not destroy the list' do
expect { subject }.not_to change { klass.count }
end
it 'returns an error and not nil list' do
subject
expect(mutation_response['errors']).not_to be_empty
expect(mutation_response['list']).not_to be_nil
end
end
context 'when the user does not have permission' do
it 'does not destroy the list' do
expect { subject }.not_to change { klass.count }
end
it 'returns an error' do
subject
expect(graphql_errors.first['message']).to include("The resource that you are attempting to access does not exist or you don't have permission to perform this action")
end
end
context 'when the user has permission' do
before do
group.add_maintainer(current_user)
end
context 'when given id is not for a list' do
# could be any non-list thing
let_it_be(:list) { group }
it 'returns an error' do
subject
expect(graphql_errors.first['message']).to include('does not represent an instance of')
end
end
context 'when list does not exist' do
let(:variables) do
{
list_id: "gid://gitlab/#{klass}/#{non_existing_record_id}"
}
end
it 'returns a top level error' do
subject
expect(graphql_errors.first['message']).to include('No object found for')
end
end
context 'when everything is ok' do
it 'destroys the list' do
expect { subject }.to change { klass.count }.by(-1)
end
it 'returns an empty list' do
post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response).to have_key('list')
expect(mutation_response['list']).to be_nil
expect(mutation_response['errors']).to be_empty
end
end
context 'when the list is not destroyable' do
before do
list.update!(list_type: :backlog)
end
it_behaves_like 'does not destroy the list and returns an error'
end
end
end
......@@ -3,30 +3,27 @@
RSpec.shared_examples 'lists destroy service' do
context 'when list type is label' do
it 'removes list from board' do
list = create(:list, board: board)
service = described_class.new(parent, user)
expect { service.execute(list) }.to change(board.lists, :count).by(-1)
end
it 'decrements position of higher lists' do
development = create(:list, board: board, position: 0)
review = create(:list, board: board, position: 1)
staging = create(:list, board: board, position: 2)
closed = board.lists.closed.first
development = create(list_type, params.merge(position: 0))
review = create(list_type, params.merge(position: 1))
staging = create(list_type, params.merge(position: 2))
described_class.new(parent, user).execute(development)
expect(review.reload.position).to eq 0
expect(staging.reload.position).to eq 1
expect(closed.reload.position).to be_nil
expect(closed_list.reload.position).to be_nil
end
end
it 'does not remove list from board when list type is closed' do
list = board.lists.closed.first
service = described_class.new(parent, user)
expect { service.execute(list) }.not_to change(board.lists, :count)
expect { service.execute(closed_list) }.not_to change(board.lists, :count)
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