diff --git a/app/graphql/mutations/work_items/reorder.rb b/app/graphql/mutations/work_items/reorder.rb new file mode 100644 index 0000000000000000000000000000000000000000..4de05fe70cf0e95c5911aac643fab1956607597e --- /dev/null +++ b/app/graphql/mutations/work_items/reorder.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Mutations + module WorkItems + class Reorder < BaseMutation + graphql_name 'workItemsReorder' + description 'Reorders a project level work item.' + + argument :id, + ::Types::GlobalIDType[::WorkItem], + required: true, + description: 'Global ID of the work item to be reordered.' + + argument :move_before_id, + ::Types::GlobalIDType[::WorkItem], + required: false, + description: 'Global ID of a project’s work item that should be placed before the work item.', + prepare: ->(id, _ctx) { GitlabSchema.parse_gid(id)&.model_id } + + argument :move_after_id, + ::Types::GlobalIDType[::WorkItem], + required: false, + description: 'Global ID of a project’s work item that should be placed after the work item.', + prepare: ->(id, _ctx) { GitlabSchema.parse_gid(id)&.model_id } + + field :work_item, + ::Types::WorkItemType, + null: true, + description: 'Work item after mutation.' + + authorize :update_work_item + + def ready?(**args) + return super if args.slice(:move_after_id, :move_before_id).compact.present? + + raise Gitlab::Graphql::Errors::ArgumentError, + 'At least one of move_before_id and move_after_id are required' + end + + def resolve(**args) + work_item = authorized_find!(id: args[:id]) + + ::WorkItems::ReorderService.new( + current_user: current_user, + params: args.slice(:move_before_id, :move_after_id) + ).execute(work_item).payload + end + end + end +end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 35a9b9ff6a06ca7dea159ba06a82ee76b148567b..bc5d314f73384973ad5056ec1addaa64230ea70c 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -251,6 +251,7 @@ class MutationType < BaseObject mount_mutation Mutations::WorkItems::Hierarchy::AddChildrenItems, experiment: { milestone: '18.2' } mount_mutation Mutations::WorkItems::BulkUpdate, experiment: { milestone: '17.4' } mount_mutation Mutations::WorkItems::BulkMove, experiment: { milestone: '18.2' } + mount_mutation Mutations::WorkItems::Reorder, experiment: { milestone: '18.3' } mount_mutation Mutations::WorkItems::UserPreference::Update, experiment: { milestone: '17.10' } mount_mutation Mutations::Users::SavedReplies::Create mount_mutation Mutations::Users::SavedReplies::Update diff --git a/app/services/work_items/reorder_service.rb b/app/services/work_items/reorder_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..0a7d92247aadc819698e38fbaf7ca372f2ec9a71 --- /dev/null +++ b/app/services/work_items/reorder_service.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module WorkItems + class ReorderService + include Gitlab::Utils::StrongMemoize + + def initialize(current_user:, params:) + @current_user = current_user + @params = params + end + + def execute(work_item) + return unauthorized_error(work_item) unless current_user.can?(:update_work_item, work_item) + return non_project_work_item_error(work_item) if work_item.project.blank? + return missing_arguments_error(work_item) unless move_between_ids + + ::WorkItems::UpdateService.new( + container: work_item.project, + current_user: current_user, + params: { move_between_ids: move_between_ids } + ).execute(work_item) + + success(work_item) + rescue ActiveRecord::RecordNotFound + error(work_item, message: "Work item not found") + end + + private + + attr_reader :current_user, :params + + def success(work_item) + ServiceResponse.success(payload: { + work_item: work_item, + errors: [] + }) + end + + def non_project_work_item_error(work_item) + error( + work_item, + message: 'Only project work items are supported at this moment' + ) + end + + def unauthorized_error(work_item) + error( + work_item, + message: "You don't have permissions to update this work item" + ) + end + + def missing_arguments_error(work_item) + error(work_item, message: 'At least one of move_before_id or move_after_id is required') + end + + def error(work_item, message: nil) + ServiceResponse.new(status: :error, payload: { + work_item: work_item, + errors: Array.wrap(message) + }) + end + + def move_between_ids + params + .values_at(:move_before_id, :move_after_id) + .map(&:to_i) + .map { |id| id > 0 ? id : nil } + .then { |ids| ids.any? ? ids : nil } + end + strong_memoize_attr :move_between_ids + end +end diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 757644740e064f3bf491549299080bc1193168a8..7a208516e83e510fb6007b12af1c466414a8b4a0 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -13767,6 +13767,34 @@ Input type: `workItemsHierarchyReorderInput` | `parentWorkItem` | [`WorkItem`](#workitem) | Work item's parent after mutation. | | `workItem` | [`WorkItem`](#workitem) | Work item after mutation. | +### `Mutation.workItemsReorder` + +{{< details >}} +**Introduced** in GitLab 18.3. +**Status**: Experiment. +{{< /details >}} + +Reorders a project level work item. + +Input type: `workItemsReorderInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `id` | [`WorkItemID!`](#workitemid) | Global ID of the work item to be reordered. | +| `moveAfterId` | [`WorkItemID`](#workitemid) | Global ID of a project’s work item that should be placed after the work item. | +| `moveBeforeId` | [`WorkItemID`](#workitemid) | Global ID of a project’s work item that should be placed before the work item. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during the mutation. | +| `workItem` | [`WorkItem`](#workitem) | Work item after mutation. | + ### `Mutation.workspaceCreate` Input type: `WorkspaceCreateInput` diff --git a/ee/spec/services/ee/work_items/reorder_service_spec.rb b/ee/spec/services/ee/work_items/reorder_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..069b2b89ef224f91e417c4ddca873157dabc0370 --- /dev/null +++ b/ee/spec/services/ee/work_items/reorder_service_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::ReorderService, feature_category: :team_planning do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create_default(:user, developer_of: group) } + let_it_be(:project) { create(:project, namespace: group) } + + describe '#execute' do + let_it_be(:item1) { create(:work_item, :issue, project: project, relative_position: 10) } + let_it_be(:item2) { create(:work_item, :issue, project: project, relative_position: 20) } + let_it_be(:non_project_work_item) do + create(:work_item, :group_level, namespace: group, relative_position: 10) + end + + let(:work_item) { non_project_work_item } + let(:params) { {} } + + subject(:service_result) do + described_class + .new(current_user: user, params: params) + .execute(work_item) + end + + context 'with epics enabled' do + before do + stub_licensed_features(epics: true) + end + + context 'when ordering work items in a group' do + context 'with a non-project level work item' do + let(:params) { { move_after_id: item1.id, move_before_id: item2.id } } + + it 'does not update the position' do + expect { service_result } + .not_to change { non_project_work_item.relative_position } + + expect(service_result[:errors]) + .to eq(["Only project work items are supported at this moment"]) + end + end + end + end + end +end diff --git a/spec/graphql/mutations/work_items/reorder_spec.rb b/spec/graphql/mutations/work_items/reorder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..88b3055a627098b3fba114c913ea9d001f6c0b0a --- /dev/null +++ b/spec/graphql/mutations/work_items/reorder_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Mutations::WorkItems::Reorder, feature_category: :team_planning do + include GraphqlHelpers + using RSpec::Parameterized::TableSyntax + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user, developer_of: project) } + let_it_be(:item1, reload: true) { create(:work_item, :issue, project: project, relative_position: 10) } + let_it_be(:item2, reload: true) { create(:work_item, :issue, project: project, relative_position: 20) } + + subject(:mutation) { described_class.new(object: nil, context: query_context, field: nil) } + + describe '#ready?' do + context 'with invalid arguments' do + it 'raises an error' do + expect { mutation.ready? } + .to raise_error('At least one of move_before_id and move_after_id are required') + end + end + + context 'with valid arguments' do + where(:move_before, :move_after, :expected_result) do + nil | ref(:item2) | true + ref(:item2) | nil | true + end + + with_them do + specify do + ready_result = mutation.ready?( + move_before_id: move_before&.id, + move_after_id: move_after&.id + ) + + expect(ready_result).to be(expected_result) + end + end + end + end + + describe '#resolve' do + context 'when it fails to move' do + specify do + result = mutation.resolve( + id: item2.to_gid, + move_before_id: non_existing_record_id + ) + + expect(result[:errors]).to eq(["Work item not found"]) + expect(result[:work_item]&.relative_position).to be(item2.relative_position) + end + end + + context 'when moving it before other item' do + specify do + result = mutation.resolve( + id: item1.to_gid, + move_before_id: item2.id + ) + + expect(result[:errors]).to be_empty + expect(result[:work_item].relative_position).to be > item2.relative_position + end + end + + context 'when moving it after other item' do + specify do + result = mutation.resolve( + id: item2.to_gid, + move_after_id: item1.id + ) + + expect(result[:errors]).to be_empty + expect(result[:work_item].relative_position).to be < item1.relative_position + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/work_items/reorder_spec.rb b/spec/requests/api/graphql/mutations/work_items/reorder_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..3b510d7b0ea9528355686c5a5381f855d2c6f50d --- /dev/null +++ b/spec/requests/api/graphql/mutations/work_items/reorder_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Reorder work items', feature_category: :team_planning do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user, developer_of: project) } + let_it_be(:item1, reload: true) { create(:work_item, :issue, project: project, relative_position: 10) } + let_it_be(:item2, reload: true) { create(:work_item, :issue, project: project, relative_position: 20) } + let_it_be(:item3, reload: true) { create(:work_item, :issue, project: project, relative_position: 20) } + + let(:input) do + { + id: item1.to_gid.to_s + } + end + + let(:mutation) { graphql_mutation(:work_items_reorder, input) } + let(:mutation_response) { graphql_mutation_response(:work_items_reorder) } + + context 'when missing arguments' do + let(:input) do + { + id: item1.to_gid.to_s + } + end + + it 'returns an error' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .not_to change { item1.relative_position } + + expect_graphql_errors_to_include( + 'At least one of move_before_id and move_after_id are required' + ) + end + end + + context 'when all arguments are given' do + context 'when moving it before other item' do + let(:input) do + { + id: item1.to_gid.to_s, + move_before_id: item2.to_gid.to_s + } + end + + specify do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { item1.reload.relative_position }.to be > item2.relative_position + + expect(graphql_errors).to be_blank + expect(mutation_response['errors']).to be_blank + end + end + + context 'when moving it after other item' do + let(:input) do + { + id: item2.to_gid.to_s, + move_after_id: item1.to_gid.to_s + } + end + + specify do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { item2.reload.relative_position }.to be < item1.relative_position + + expect(graphql_errors).to be_blank + expect(mutation_response['errors']).to be_blank + end + end + end +end diff --git a/spec/services/work_items/reorder_service_spec.rb b/spec/services/work_items/reorder_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..6fdcae3eba63d9a3e85950a94e692760d3760284 --- /dev/null +++ b/spec/services/work_items/reorder_service_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe WorkItems::ReorderService, feature_category: :team_planning do + let_it_be(:user) { create_default(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project, reload: true) { create(:project, namespace: group) } + + describe '#execute' do + let_it_be(:item1, reload: true) { create(:work_item, :issue, project: project, relative_position: 10) } + let_it_be(:item2, reload: true) { create(:work_item, :issue, project: project, relative_position: 20) } + let_it_be(:item3, reload: true) { create(:work_item, :issue, project: project, relative_position: 30) } + + let(:work_item) { item1 } + let(:params) { {} } + + subject(:service_result) do + described_class + .new(current_user: user, params: params) + .execute(work_item) + end + + context 'when ordering work items in a project' do + before do + project.add_developer(user) + end + + it_behaves_like 'reorder service' + end + + context 'when ordering work items in a group' do + before_all do + group.add_developer(user) + end + + it_behaves_like 'reorder service' + + context 'when ordering work items from other namespaces' do + let_it_be(:project2) { create(:project) } + let(:work_item) { create(:work_item, :issue, project: project2) } + let(:params) { { move_after_id: item2.id, move_before_id: item3.id } } + + it 'does not reorder' do + expect { service_result } + .not_to change { work_item.relative_position } + + expect(service_result[:errors]) + .to eq(["You don't have permissions to update this work item"]) + end + end + end + end +end diff --git a/spec/support/shared_examples/reorder_service_shared_examples.rb b/spec/support/shared_examples/reorder_service_shared_examples.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9a15c3af2fc3330792122655cf1614ebbc249a8 --- /dev/null +++ b/spec/support/shared_examples/reorder_service_shared_examples.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'reorder service' do + context 'when reordering items' do + context 'when no params are passed' do + let(:params) { {} } + + it 'does not reorder' do + expect { service_result } + .not_to change { work_item.relative_position } + + expect(service_result[:errors]) + .to eq(["At least one of move_before_id or move_after_id is required"]) + end + end + + context 'with both invalid params' do + let(:params) { { move_after_id: nil, move_before_id: non_existing_record_id } } + + it 'does not reorder' do + expect { service_result } + .not_to change { work_item.relative_position } + + expect(service_result[:errors]).to eq(["Work item not found"]) + end + end + + context 'with valid params' do + let(:params) { { move_after_id: item2.id, move_before_id: item3.id } } + + it 'sorts items' do + expect { service_result } + .to change { work_item.relative_position }.to be_between( + item2.relative_position, + item3.relative_position + ) + + expect(service_result[:errors]).to be_empty + end + end + + context 'when only move_before_id is given' do + let(:params) { { move_before_id: item3.id } } + + it 'sorts items if only given one neighbour, on the left' do + expect { service_result } + .to change { work_item.relative_position }.to be > item3.relative_position + + expect(service_result[:errors]).to be_empty + end + end + + context 'when only move_after_id is given' do + let(:work_item) { item3 } + let(:params) { { move_after_id: item1.id } } + + it 'sorts items if only given one neighbour, on the right' do + expect { service_result } + .to change { work_item.relative_position }.to be < item1.relative_position + + expect(service_result[:errors]).to be_empty + end + end + end +end