Commit 73109af8 authored by Florie Guibert's avatar Florie Guibert

Epic tree move child with drag and drop

- Move epic or issue to epic with children
- Move epic or issue to epic with no children
parent a3acb758
......@@ -3361,7 +3361,7 @@ input EpicTreeNodeFieldsInputType {
"""
The id of the epic_issue or issue that the actual epic or issue is switched with
"""
adjacentReferenceId: ID!
adjacentReferenceId: ID
"""
The id of the epic_issue or epic that is being moved
......
......@@ -9668,13 +9668,9 @@
"name": "adjacentReferenceId",
"description": "The id of the epic_issue or issue that the actual epic or issue is switched with",
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "ID",
"ofType": null
}
"kind": "SCALAR",
"name": "ID",
"ofType": null
},
"defaultValue": null
},
......
......@@ -112,9 +112,9 @@ export default {
/>
</div>
<tree-root
v-if="itemExpanded"
v-if="item.type === $options.ChildType.Epic"
:parent-item="item"
:children="children[itemReference]"
:children="children[itemReference] || []"
class="sub-tree-root"
/>
</li>
......
......@@ -34,7 +34,7 @@ export default {
},
},
methods: {
...mapActions(['fetchNextPageItems', 'reorderItem']),
...mapActions(['fetchNextPageItems', 'reorderItem', 'moveItem']),
handleShowMoreClick() {
this.fetchInProgress = true;
this.fetchNextPageItems({
......
......@@ -12,10 +12,11 @@ export default {
const options = {
...defaultSortableConfig,
fallbackOnBody: false,
group: this.parentItem.reference,
group: 'sortable-container',
tag: 'ul',
'ghost-class': 'tree-item-drag-active',
'data-parent-reference': this.parentItem.reference,
'data-parent-id': this.parentItem.id,
value: this.children,
// This filters out/ignores all the chevron buttons (used for
// expanding and collapsing epic tree items) so the drag action
......@@ -104,22 +105,33 @@ export default {
*
* @param {object} event Object representing drag end event.
*/
handleDragOnEnd({ oldIndex, newIndex }) {
handleDragOnEnd(params) {
const { oldIndex, newIndex, from, to } = params;
document.body.classList.remove('is-dragging');
// If both old and new index of target are same,
// nothing was moved, we do an early return.
if (oldIndex === newIndex) return;
const targetItem = this.children[oldIndex];
this.reorderItem({
treeReorderMutation: this.getTreeReorderMutation({ oldIndex, newIndex, targetItem }),
parentItem: this.parentItem,
targetItem,
oldIndex,
newIndex,
});
if (from === to) {
// If both old and new index of target are same,
// nothing was moved, we do an early return.
if (oldIndex === newIndex) return;
this.reorderItem({
treeReorderMutation: this.getTreeReorderMutation({ oldIndex, newIndex, targetItem }),
parentItem: this.parentItem,
targetItem,
oldIndex,
newIndex,
});
} else {
this.moveItem({
oldParentItem: this.parentItem,
newParentItem: to.dataset,
targetItem,
oldIndex,
newIndex,
});
}
},
},
};
......@@ -12,7 +12,7 @@ import httpStatusCodes from '~/lib/utils/http_status';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { processQueryResponse, formatChildItem, gqClient } from '../utils/epic_utils';
import { ChildType, ChildState } from '../constants';
import { ChildType, ChildState, idProp, relativePositions } from '../constants';
import epicChildren from '../queries/epicChildren.query.graphql';
import epicChildReorder from '../queries/epicChildReorder.mutation.graphql';
......@@ -454,6 +454,84 @@ export const reorderItem = (
});
};
export const receiveMoveItemFailure = ({ commit }, data) => {
commit(types.MOVE_ITEM_FAILURE, data);
flash(s__('Epics|Something went wrong while moving item.'));
};
export const moveItem = (
{ dispatch, commit, state },
{ oldParentItem, newParentItem, targetItem, oldIndex, newIndex },
) => {
let adjacentItem;
let adjacentReferenceId;
let relativePosition = relativePositions.After;
let isFirstChild = false;
const newParentChildren = state.children[newParentItem.parentReference];
if (newParentChildren !== undefined) {
adjacentItem = newParentChildren[newIndex];
if (adjacentItem === undefined) {
adjacentItem = newParentChildren[newParentChildren.length - 1];
relativePosition = relativePositions.Before;
}
adjacentReferenceId = adjacentItem[idProp[adjacentItem.type]];
} else {
isFirstChild = true;
relativePosition = relativePositions.Before;
}
commit(types.MOVE_ITEM, {
oldParentItem,
newParentItem,
targetItem,
oldIndex,
newIndex,
isFirstChild,
});
return gqClient
.mutate({
mutation: epicChildReorder,
variables: {
epicTreeReorderInput: {
baseEpicId: oldParentItem.id,
moved: {
id: targetItem[idProp[targetItem.type]],
adjacentReferenceId,
relativePosition,
newParentId: newParentItem.parentId,
},
},
},
})
.then(({ data }) => {
// Mutation was unsuccessful;
// revert to original order and show flash error
if (data.epicTreeReorder.errors.length) {
dispatch('receiveMoveItemFailure', {
oldParentItem,
newParentItem,
targetItem,
newIndex,
oldIndex,
});
}
})
.catch(() => {
// Mutation was unsuccessful;
// revert to original order and show flash error
dispatch('receiveMoveItemFailure', {
oldParentItem,
newParentItem,
targetItem,
newIndex,
oldIndex,
});
});
};
export const receiveCreateIssueSuccess = ({ commit }) =>
commit(types.RECEIVE_CREATE_ITEM_SUCCESS, { insertAt: 0, items: [] });
export const receiveCreateIssueFailure = ({ commit }) => {
......
......@@ -41,6 +41,9 @@ export const RECEIVE_CREATE_ITEM_FAILURE = 'RECEIVE_CREATE_ITEM_FAILURE';
export const REORDER_ITEM = 'REORDER_ITEM';
export const MOVE_ITEM = 'MOVE_ITEM';
export const MOVE_ITEM_FAILURE = 'MOVE_ITEM_FAILURE';
export const SET_PROJECTS = 'SET_PROJECTS';
export const REQUEST_PROJECTS = 'REQUEST_PROJECTS';
export const RECIEVE_PROJECTS_SUCCESS = 'RECIEVE_PROJECTS_SUCCESS';
......
......@@ -214,6 +214,31 @@ export default {
state.children[parentItem.reference].splice(newIndex, 0, targetItem);
},
[types.MOVE_ITEM](
state,
{ oldParentItem, newParentItem, targetItem, oldIndex, newIndex, isFirstChild },
) {
// Remove from old position in previous parent
state.children[oldParentItem.reference].splice(oldIndex, 1);
// Insert at new position in new parent
if (isFirstChild) {
Vue.set(state.children, newParentItem.parentReference, [targetItem]);
} else {
state.children[newParentItem.parentReference].splice(newIndex, 0, targetItem);
}
},
[types.MOVE_ITEM_FAILURE](
state,
{ oldParentItem, newParentItem, targetItem, oldIndex, newIndex },
) {
// Remove from new position in new parent
state.children[newParentItem.parentReference].splice(newIndex, 1);
// Insert at old position in old parent
state.children[oldParentItem.reference].splice(oldIndex, 0, targetItem);
},
[types.REQUEST_PROJECTS](state) {
state.projectsFetchInProgress = true;
},
......
......@@ -14,7 +14,7 @@ module Types
argument :adjacent_reference_id,
GraphQL::ID_TYPE,
required: true,
required: false,
description: 'The id of the epic_issue or issue that the actual epic or issue is switched with'
argument :relative_position,
......
......@@ -42,7 +42,12 @@ module Epics
end
def move!
moving_object.move_between(before_object, after_object)
if adjacent_reference
moving_object.move_between(before_object, after_object)
else
moving_object.move_to_start
end
moving_object.save!(touch: false)
end
......@@ -59,16 +64,20 @@ module Epics
end
def validate_objects
return 'Relative position is not valid.' unless valid_relative_position?
if adjacent_reference
return 'Relative position is not valid.' unless valid_relative_position?
unless supported_type?(moving_object) && supported_type?(adjacent_reference)
return 'Only epics and epic_issues are supported.'
unless supported_type?(moving_object) && supported_type?(adjacent_reference)
return 'Only epics and epic_issues are supported.'
end
end
return 'You don\'t have permissions to move the objects.' unless authorized?
if different_epic_parent?
return "The sibling object's parent must match the #{new_parent ? "new" : "current"} parent epic."
if adjacent_reference
if different_epic_parent?
return "The sibling object's parent must match the #{new_parent ? "new" : "current"} parent epic."
end
end
end
......@@ -90,7 +99,10 @@ module Epics
def authorized?
return false unless can?(current_user, :admin_epic, base_epic.group)
return false unless can?(current_user, :admin_epic, adjacent_reference_group)
if adjacent_reference
return false unless can?(current_user, :admin_epic, adjacent_reference_group)
end
if new_parent
return false unless can?(current_user, :admin_epic, new_parent.group)
......@@ -116,6 +128,8 @@ module Epics
end
def adjacent_reference
return unless params[:adjacent_reference_id]
@adjacent_reference ||= find_object(params[:adjacent_reference_id])&.sync
end
......
---
title: Epic tree move child with drag and drop
merge_request: 28629
author:
type: added
......@@ -114,10 +114,11 @@ describe('RelatedItemsTree', () => {
fallbackClass: 'is-dragging',
fallbackOnBody: false,
ghostClass: 'is-ghost',
group: mockParentItem.reference,
group: 'sortable-container',
tag: 'ul',
'ghost-class': 'tree-item-drag-active',
'data-parent-reference': mockParentItem.reference,
'data-parent-id': mockParentItem.id,
value: wrapper.vm.children,
filter: `.${treeItemChevronBtnClassName}`,
}),
......@@ -270,34 +271,63 @@ describe('RelatedItemsTree', () => {
expect(document.body.classList.contains('is-dragging')).toBe(false);
});
it('does not call `reorderItem` action when newIndex is same as oldIndex', () => {
jest.spyOn(wrapper.vm, 'reorderItem').mockImplementation(() => {});
describe('origin parent is destination parent', () => {
it('does not call `reorderItem` action when newIndex is same as oldIndex', () => {
jest.spyOn(wrapper.vm, 'reorderItem').mockImplementation(() => {});
wrapper.vm.handleDragOnEnd({
oldIndex: 0,
newIndex: 0,
});
wrapper.vm.handleDragOnEnd({
oldIndex: 0,
newIndex: 0,
from: wrapper.element,
to: wrapper.element,
});
expect(wrapper.vm.reorderItem).not.toHaveBeenCalled();
});
expect(wrapper.vm.reorderItem).not.toHaveBeenCalled();
});
it('calls `reorderItem` action when newIndex is different from oldIndex', () => {
jest.spyOn(wrapper.vm, 'reorderItem').mockImplementation(() => {});
it('calls `reorderItem` action when newIndex is different from oldIndex', () => {
jest.spyOn(wrapper.vm, 'reorderItem').mockImplementation(() => {});
wrapper.vm.handleDragOnEnd({
oldIndex: 1,
newIndex: 0,
wrapper.vm.handleDragOnEnd({
oldIndex: 1,
newIndex: 0,
from: wrapper.element,
to: wrapper.element,
});
expect(wrapper.vm.reorderItem).toHaveBeenCalledWith(
expect.objectContaining({
treeReorderMutation: expect.any(Object),
parentItem: wrapper.vm.parentItem,
targetItem: wrapper.vm.children[1],
oldIndex: 1,
newIndex: 0,
}),
);
});
});
expect(wrapper.vm.reorderItem).toHaveBeenCalledWith(
expect.objectContaining({
treeReorderMutation: expect.any(Object),
parentItem: wrapper.vm.parentItem,
targetItem: wrapper.vm.children[1],
describe('origin parent is different than destination parent', () => {
it('calls `moveItem`', () => {
jest.spyOn(wrapper.vm, 'moveItem').mockImplementation(() => {});
wrapper.vm.handleDragOnEnd({
oldIndex: 1,
newIndex: 0,
}),
);
from: wrapper.element,
to: wrapper.find('li:first-child .sub-tree-root'),
});
expect(wrapper.vm.moveItem).toHaveBeenCalledWith(
expect.objectContaining({
oldParentItem: wrapper.vm.parentItem,
newParentItem: wrapper.find('li:first-child .sub-tree-root').dataset,
targetItem: wrapper.vm.children[1],
oldIndex: 1,
newIndex: 0,
}),
);
});
});
});
});
......
......@@ -33,6 +33,30 @@ export const mockParentItem = {
},
};
export const mockParentItem2 = {
id: 'gid://gitlab/Epic/43',
iid: 2,
fullPath: 'gitlab-org',
title: 'Some sample epic 2',
reference: 'gitlab-org&2',
parentReference: 'gitlab-org&2',
userPermissions: {
adminEpic: true,
createEpic: true,
},
descendantCounts: {
openedEpics: 1,
closedEpics: 1,
openedIssues: 1,
closedIssues: 1,
},
healthStatus: {
issuesOnTrack: 1,
issuesAtRisk: 0,
issuesNeedingAttention: 1,
},
};
export const mockEpic1 = {
id: 'gid://gitlab/Epic/4',
iid: '4',
......
......@@ -19,6 +19,7 @@ import { TEST_HOST } from 'spec/test_constants';
import {
mockInitialConfig,
mockParentItem,
mockParentItem2,
mockQueryResponse,
mockEpicTreeReorderInput,
mockReorderMutationResponse,
......@@ -1265,6 +1266,199 @@ describe('RelatedItemTree', () => {
});
});
describe('receiveMoveItemFailure', () => {
it('should revert moved item back to its original position ion its original parent via MOVE_ITEM_FAILURE mutation', () => {
testAction(
actions.receiveMoveItemFailure,
{},
{},
[{ type: types.MOVE_ITEM_FAILURE, payload: {} }],
[],
);
});
it('should show flash error with message "Something went wrong while ordering item."', () => {
const message = 'Something went wrong while moving item.';
actions.receiveMoveItemFailure(
{
commit: () => {},
},
{
message,
},
);
expect(createFlash).toHaveBeenCalledWith(message);
});
});
describe('moveItem', () => {
beforeAll(() => {
state.children[mockParentItem2.parentReference] = [];
});
it('should perform MOVE_ITEM mutation with isFirstChild to true if parent has no children before request and do nothing on request success', () => {
jest.spyOn(epicUtils.gqClient, 'mutate').mockReturnValue(
Promise.resolve({
data: mockReorderMutationResponse,
}),
);
testAction(
actions.moveItem,
{
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
},
state,
[
{
type: types.MOVE_ITEM,
payload: {
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
isFirstChild: true,
},
},
],
[],
);
});
it('should perform MOVE_ITEM mutation with isFirstChild to false if parent has children before request and do nothing on request success', () => {
jest.spyOn(epicUtils.gqClient, 'mutate').mockReturnValue(
Promise.resolve({
data: mockReorderMutationResponse,
}),
);
state.children[mockParentItem2.parentReference] = [{ id: '33' }];
testAction(
actions.moveItem,
{
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
},
state,
[
{
type: types.MOVE_ITEM,
payload: {
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
isFirstChild: false,
},
},
],
[],
);
});
it('should perform MOVE_ITEM mutation before request and dispatch `receiveReorderItemFailure` when request response has errors on request success', () => {
jest.spyOn(epicUtils.gqClient, 'mutate').mockReturnValue(
Promise.resolve({
data: {
epicTreeReorder: {
...mockReorderMutationResponse.epicTreeReorder,
errors: [{ foo: 'bar' }],
},
},
}),
);
testAction(
actions.moveItem,
{
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
},
state,
[
{
type: types.MOVE_ITEM,
payload: {
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
isFirstChild: true,
},
},
],
[
{
type: 'receiveMoveItemFailure',
payload: {
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
},
},
],
);
});
it('should perform MOVE_ITEM mutation before request and dispatch `receiveReorderItemFailure` on request failure', () => {
jest.spyOn(epicUtils.gqClient, 'mutate').mockReturnValue(Promise.reject());
testAction(
actions.moveItem,
{
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
},
state,
[
{
type: types.MOVE_ITEM,
payload: {
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
isFirstChild: true,
},
},
],
[
{
type: 'receiveMoveItemFailure',
payload: {
oldParentItem: mockParentItem,
newParentItem: mockParentItem2,
targetItem: mockItems[1],
newIndex: 1,
oldIndex: 0,
},
},
],
);
});
});
describe('receiveCreateIssueSuccess', () => {
it('should set `state.itemCreateInProgress` & `state.itemsFetchResultEmpty` to false', () => {
testAction(
......
......@@ -545,7 +545,7 @@ describe('RelatedItemsTree', () => {
});
describe(types.REORDER_ITEM, () => {
it('should reorder an item within children of provided parent based on provided indices', () => {
it('should reorder an item within children of provided parent based on provided indexes', () => {
state.parentItem = { reference: '&1' };
state.children[state.parentItem.reference] = ['foo', 'bar'];
......@@ -564,6 +564,90 @@ describe('RelatedItemsTree', () => {
});
});
describe(types.MOVE_ITEM, () => {
it('should move an item from one parent to another with children based on provided indexes', () => {
const newParentItem = {
parentReference: '&2',
};
state.parentItem = { reference: '&1' };
state.children[state.parentItem.reference] = ['foo', 'bar'];
state.children[newParentItem.parentReference] = ['baz'];
mutations[types.MOVE_ITEM](state, {
oldParentItem: {
reference: '&1',
},
newParentItem,
targetItem: 'bar',
oldIndex: 1,
newIndex: 0,
isFirstChild: false,
});
expect(state.children[state.parentItem.reference]).toEqual(
expect.arrayContaining(['foo']),
);
expect(state.children[newParentItem.parentReference]).toEqual(
expect.arrayContaining(['bar', 'baz']),
);
});
it('should move an item from one parent to another without children based on provided indexes', () => {
const newParentItem = {
parentReference: '&2',
};
state.parentItem = { reference: '&1' };
state.children[state.parentItem.reference] = ['foo', 'bar'];
mutations[types.MOVE_ITEM](state, {
oldParentItem: {
reference: '&1',
},
newParentItem,
targetItem: 'bar',
oldIndex: 1,
newIndex: 0,
isFirstChild: true,
});
expect(state.children[state.parentItem.reference]).toEqual(
expect.arrayContaining(['foo']),
);
expect(state.children[newParentItem.parentReference]).toEqual(
expect.arrayContaining(['bar']),
);
});
});
describe(types.MOVE_ITEM_FAILURE, () => {
it('should move an item from one parent to another with children based on provided indexes', () => {
const newParentItem = {
parentReference: '&2',
};
state.parentItem = { reference: '&1' };
state.children[state.parentItem.reference] = ['foo'];
state.children[newParentItem.parentReference] = ['bar', 'baz'];
mutations[types.MOVE_ITEM_FAILURE](state, {
oldParentItem: {
reference: '&1',
},
newParentItem,
targetItem: 'bar',
oldIndex: 1,
newIndex: 0,
isFirstChild: false,
});
expect(state.children[state.parentItem.reference]).toEqual(
expect.arrayContaining(['foo', 'bar']),
);
expect(state.children[newParentItem.parentReference]).toEqual(
expect.arrayContaining(['baz']),
);
});
});
describe(types.REQUEST_PROJECTS, () => {
it('should set `projectsFetchInProgress` to true within state', () => {
mutations[types.REQUEST_PROJECTS](state);
......
......@@ -93,9 +93,18 @@ describe Epics::TreeReorderService do
context 'when no object to switch is provided' do
let(:adjacent_reference_id) { nil }
let(:new_parent_id) { GitlabSchema.id_from_object(epic) }
before do
tree_object_2.update(epic: epic1)
end
# it 'raises an error' do
# expect { subject }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
# end
it 'raises an error' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
it 'updates the parent' do
expect { subject }.to change { tree_object_2.reload.epic }.from(epic1).to(epic)
end
end
......
......@@ -8276,6 +8276,9 @@ msgstr ""
msgid "Epics|Something went wrong while fetching group epics."
msgstr ""
msgid "Epics|Something went wrong while moving item."
msgstr ""
msgid "Epics|Something went wrong while ordering item."
msgstr ""
......
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