Commit 9c2ee39f authored by Alex Kalderimis's avatar Alex Kalderimis Committed by Mayra Cabrera

Add mutation to move designs

This adds DesignManagementMove, as well as a new design collection type.
parent b274beb3
......@@ -77,6 +77,8 @@ class GitlabSchema < GraphQL::Schema
# will be called.
# * All other classes will use `GlobalID#find`
def find_by_gid(gid)
return unless gid
if gid.model_class < ApplicationRecord
Gitlab::Graphql::Loaders::BatchModelLoader.new(gid.model_class, gid.model_id).find
elsif gid.model_class.respond_to?(:lazy_find)
......
# frozen_string_literal: true
module Mutations
module DesignManagement
class Move < ::Mutations::BaseMutation
graphql_name "DesignManagementMove"
DesignID = ::Types::GlobalIDType[::DesignManagement::Design]
argument :id, DesignID, required: true, as: :current_design,
description: "ID of the design to move"
argument :previous, DesignID, required: false, as: :previous_design,
description: "ID of the immediately preceding design"
argument :next, DesignID, required: false, as: :next_design,
description: "ID of the immediately following design"
field :design_collection, Types::DesignManagement::DesignCollectionType,
null: true,
description: "The current state of the collection"
def ready(*)
raise ::Gitlab::Graphql::Errors::ResourceNotAvailable unless ::Feature.enabled?(:reorder_designs)
end
def resolve(**args)
service = ::DesignManagement::MoveDesignsService.new(current_user, parameters(args))
{ design_collection: service.collection, errors: service.execute.errors }
end
private
def parameters(**args)
args.transform_values { |id| GitlabSchema.find_by_gid(id) }.transform_values(&:sync).tap do |hash|
hash.each { |k, design| not_found(args[k]) unless current_user.can?(:read_design, design) }
end
end
def not_found(gid)
raise Gitlab::Graphql::Errors::ResourceNotAvailable, "Resource not available: #{gid}"
end
end
end
end
......@@ -57,6 +57,7 @@ module Types
mount_mutation Mutations::JiraImport::ImportUsers
mount_mutation Mutations::DesignManagement::Upload, calls_gitaly: true
mount_mutation Mutations::DesignManagement::Delete, calls_gitaly: true
mount_mutation Mutations::DesignManagement::Move
mount_mutation Mutations::ContainerExpirationPolicies::Update
end
end
......
......@@ -217,6 +217,17 @@ module DesignManagement
project
end
def immediately_before?(next_design)
return false if next_design.relative_position <= relative_position
interloper = self.class.on_issue(issue).where(
"relative_position <@ int4range(?, ?, '()')",
*[self, next_design].map(&:relative_position)
)
!interloper.exists?
end
private
def head_version
......
......@@ -10,6 +10,10 @@ module DesignManagement
@issue = issue
end
def ==(other)
other.is_a?(self.class) && issue == other.issue
end
def find_or_create_design!(filename:)
designs.find { |design| design.filename == filename } ||
designs.safe_find_or_create_by!(project: project, filename: filename) do |design|
......
# frozen_string_literal: true
module DesignManagement
class MoveDesignsService < DesignService
# @param user [User] The current user
# @param [Hash] params
# @option params [DesignManagement::Design] :current_design
# @option params [DesignManagement::Design] :previous_design (nil)
# @option params [DesignManagement::Design] :next_design (nil)
def initialize(user, params)
super(nil, user, params.merge(issue: nil))
end
def execute
return error(:no_focus) unless current_design.present?
return error(:cannot_move) unless ::Feature.enabled?(:reorder_designs, project)
return error(:cannot_move) unless current_user.can?(:move_design, current_design)
return error(:no_neighbors) unless neighbors.present?
return error(:not_distinct) unless all_distinct?
return error(:not_adjacent) if any_in_gap?
return error(:not_same_issue) unless all_same_issue?
current_design.move_between(previous_design, next_design)
success
end
def error(message)
ServiceResponse.error(message: message)
end
def success
ServiceResponse.success
end
private
delegate :issue, :project, to: :current_design
def neighbors
[previous_design, next_design].compact
end
def all_distinct?
ids.uniq.size == ids.size
end
def any_in_gap?
return false unless previous_design && next_design
!previous_design.immediately_before?(next_design)
end
def all_same_issue?
issue.designs.id_in(ids).count == ids.size
end
def ids
@ids ||= [current_design, *neighbors].map(&:id)
end
def current_design
params[:current_design]
end
def previous_design
params[:previous_design]
end
def next_design
params[:next_design]
end
end
end
---
title: Add GraphQL mutation to re-order designs
merge_request: 37603
author:
type: added
......@@ -3201,6 +3201,56 @@ type DesignManagementDeletePayload {
version: DesignVersion
}
"""
Identifier of DesignManagement::Design
"""
scalar DesignManagementDesignID
"""
Autogenerated input type of DesignManagementMove
"""
input DesignManagementMoveInput {
"""
A unique identifier for the client performing the mutation.
"""
clientMutationId: String
"""
ID of the design to move
"""
id: DesignManagementDesignID!
"""
ID of the immediately following design
"""
next: DesignManagementDesignID
"""
ID of the immediately preceding design
"""
previous: DesignManagementDesignID
}
"""
Autogenerated return type of DesignManagementMove
"""
type DesignManagementMovePayload {
"""
A unique identifier for the client performing the mutation.
"""
clientMutationId: String
"""
The current state of the collection
"""
designCollection: DesignCollection
"""
Errors encountered during execution of the mutation.
"""
errors: [String!]!
}
"""
Autogenerated input type of DesignManagementUpload
"""
......@@ -8863,6 +8913,7 @@ type Mutation {
dastSiteProfileDelete(input: DastSiteProfileDeleteInput!): DastSiteProfileDeletePayload
deleteAnnotation(input: DeleteAnnotationInput!): DeleteAnnotationPayload
designManagementDelete(input: DesignManagementDeleteInput!): DesignManagementDeletePayload
designManagementMove(input: DesignManagementMoveInput!): DesignManagementMovePayload
designManagementUpload(input: DesignManagementUploadInput!): DesignManagementUploadPayload
destroyNote(input: DestroyNoteInput!): DestroyNotePayload
destroySnippet(input: DestroySnippetInput!): DestroySnippetPayload
......
......@@ -8799,6 +8799,138 @@
"enumValues": null,
"possibleTypes": null
},
{
"kind": "SCALAR",
"name": "DesignManagementDesignID",
"description": "Identifier of DesignManagement::Design",
"fields": null,
"inputFields": null,
"interfaces": null,
"enumValues": null,
"possibleTypes": null
},
{
"kind": "INPUT_OBJECT",
"name": "DesignManagementMoveInput",
"description": "Autogenerated input type of DesignManagementMove",
"fields": null,
"inputFields": [
{
"name": "id",
"description": "ID of the design to move",
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "DesignManagementDesignID",
"ofType": null
}
},
"defaultValue": null
},
{
"name": "previous",
"description": "ID of the immediately preceding design",
"type": {
"kind": "SCALAR",
"name": "DesignManagementDesignID",
"ofType": null
},
"defaultValue": null
},
{
"name": "next",
"description": "ID of the immediately following design",
"type": {
"kind": "SCALAR",
"name": "DesignManagementDesignID",
"ofType": null
},
"defaultValue": null
},
{
"name": "clientMutationId",
"description": "A unique identifier for the client performing the mutation.",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
}
],
"interfaces": null,
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "DesignManagementMovePayload",
"description": "Autogenerated return type of DesignManagementMove",
"fields": [
{
"name": "clientMutationId",
"description": "A unique identifier for the client performing the mutation.",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "designCollection",
"description": "The current state of the collection",
"args": [
],
"type": {
"kind": "OBJECT",
"name": "DesignCollection",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "errors",
"description": "Errors encountered during execution of the mutation.",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
}
}
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [
],
"enumValues": null,
"possibleTypes": null
},
{
"kind": "INPUT_OBJECT",
"name": "DesignManagementUploadInput",
......@@ -25535,6 +25667,33 @@
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "designManagementMove",
"description": null,
"args": [
{
"name": "input",
"description": null,
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "INPUT_OBJECT",
"name": "DesignManagementMoveInput",
"ofType": null
}
},
"defaultValue": null
}
],
"type": {
"kind": "OBJECT",
"name": "DesignManagementMovePayload",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "designManagementUpload",
"description": null,
......@@ -547,6 +547,16 @@ Autogenerated return type of DesignManagementDelete
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
| `version` | DesignVersion | The new version in which the designs are deleted |
## DesignManagementMovePayload
Autogenerated return type of DesignManagementMove
| Name | Type | Description |
| --- | ---- | ---------- |
| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
| `designCollection` | DesignCollection | The current state of the collection |
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
## DesignManagementUploadPayload
Autogenerated return type of DesignManagementUpload
......
......@@ -22,6 +22,10 @@ FactoryBot.define do
imported { true }
end
trait :with_relative_position do
sequence(:relative_position) { |n| n * 1000 }
end
create_versions = ->(design, evaluator, commit_version) do
unless evaluator.versions_count == 0
project = design.project
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Mutations::DesignManagement::Move do
include DesignManagementTestHelpers
let_it_be(:issue) { create(:issue) }
let_it_be(:designs) { create_list(:design, 3, issue: issue) }
let_it_be(:developer) { create(:user, developer_projects: [issue.project]) }
let(:user) { developer }
let(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
let(:current_design) { designs.first }
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
before do
enable_design_management
end
describe "#resolve" do
subject(:resolve) do
args = {
current_design: current_design.to_global_id,
previous_design: previous_design&.to_global_id,
next_design: next_design&.to_global_id
}.compact
mutation.resolve(args)
end
shared_examples "resource not available" do
it "raises an error" do
expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
end
context 'when the feature is not available' do
before do
enable_design_management(false)
end
it_behaves_like 'resource not available'
end
%i[current_design previous_design next_design].each do |binding|
context "When #{binding} cannot be found" do
let(binding) { build_stubbed(:design) }
it_behaves_like 'resource not available'
end
end
context 'the service runs' do
before do
expect_next_instance_of(::DesignManagement::MoveDesignsService) do |service|
expect(service).to receive(:execute).and_return(service_result)
end
end
context 'raising an error' do
let(:service_result) { ServiceResponse.error(message: 'bang!') }
it 'reports the service-level error' do
expect(resolve).to include(errors: ['bang!'], design_collection: eq(issue.design_collection))
end
end
context 'successfully' do
let(:service_result) { ServiceResponse.success }
it 'reports the service-level error' do
expect(resolve).to include(errors: be_empty, design_collection: eq(issue.design_collection))
end
end
end
end
end
......@@ -603,4 +603,25 @@ RSpec.describe DesignManagement::Design do
end
end
end
describe '#immediately_before' do
let_it_be(:design) { create(:design, issue: issue, relative_position: 100) }
let_it_be(:next_design) { create(:design, issue: issue, relative_position: 200) }
it 'is true when there is no element positioned between this item and the next' do
expect(design.immediately_before?(next_design)).to be true
end
it 'is false when there is an element positioned between this item and the next' do
create(:design, issue: issue, relative_position: 150)
expect(design.immediately_before?(next_design)).to be false
end
it 'is false when the next design is to the left of this design' do
further_left = create(:design, issue: issue, relative_position: 50)
expect(design.immediately_before?(further_left)).to be false
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe DesignManagement::MoveDesignsService do
include DesignManagementTestHelpers
let_it_be(:issue) { create(:issue) }
let_it_be(:developer) { create(:user, developer_projects: [issue.project]) }
let_it_be(:designs) { create_list(:design, 3, :with_relative_position, issue: issue) }
let(:project) { issue.project }
let(:service) { described_class.new(current_user, params) }
let(:params) do
{
current_design: current_design,
previous_design: previous_design,
next_design: next_design
}
end
let(:current_user) { developer }
let(:current_design) { nil }
let(:previous_design) { nil }
let(:next_design) { nil }
before do
enable_design_management
end
describe '#execute' do
subject { service.execute }
context 'the feature is unavailable' do
let(:current_design) { designs.first }
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
before do
stub_feature_flags(reorder_designs: false)
end
it 'raises cannot_move' do
expect(subject).to be_error.and(have_attributes(message: :cannot_move))
end
context 'but it is available on the current project' do
before do
stub_feature_flags(reorder_designs: issue.project)
end
it 'is successful' do
expect(subject).to be_success
end
end
end
context 'the user cannot move designs' do
let(:current_design) { designs.first }
let(:current_user) { build_stubbed(:user) }
it 'raises cannot_move' do
expect(subject).to be_error.and(have_attributes(message: :cannot_move))
end
end
context 'the designs are not distinct' do
let(:current_design) { designs.first }
let(:previous_design) { designs.first }
it 'raises not_distinct' do
expect(subject).to be_error.and(have_attributes(message: :not_distinct))
end
end
context 'the designs are not on the same issue' do
let(:current_design) { designs.first }
let(:previous_design) { create(:design) }
it 'raises not_same_issue' do
expect(subject).to be_error.and(have_attributes(message: :not_same_issue))
end
end
context 'no focus is passed' do
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
it 'raises no_focus' do
expect(subject).to be_error.and(have_attributes(message: :no_focus))
end
end
context 'no neighbours are passed' do
let(:current_design) { designs.first }
it 'raises no_neighbors' do
expect(subject).to be_error.and(have_attributes(message: :no_neighbors))
end
end
context 'the designs are not adjacent' do
let(:current_design) { designs.first }
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
it 'raises not_adjacent' do
create(:design, issue: issue, relative_position: next_design.relative_position - 1)
expect(subject).to be_error.and(have_attributes(message: :not_adjacent))
end
end
context 'moving a design with neighbours' do
let(:current_design) { designs.first }
let(:previous_design) { designs.second }
let(:next_design) { designs.third }
it 'calls move_between and is successful' do
expect(current_design).to receive(:move_between).with(previous_design, next_design)
expect(subject).to be_success
end
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