Epic tree move child with drag and drop

- Update docs
- Feedback
- Expand parent when dragging element over

doc/api/graphql/reference/gitlab_schema.graphql

@@ -3376,7 +3376,7 @@ input EpicTreeNodeFieldsInputType {
   """
   The type of the switch, after or before allowed
   """
-  relativePosition: MoveType!
+  relativePosition: MoveType
 }
 
 """

doc/api/graphql/reference/gitlab_schema.json

@@ -9678,13 +9678,9 @@
               "name": "relativePosition",
               "description": "The type of the switch, after or before allowed",
               "type": {
-                "kind": "NON_NULL",
-                "name": null,
-                "ofType": {
-                  "kind": "ENUM",
-                  "name": "MoveType",
-                  "ofType": null
-                }
+                "kind": "ENUM",
+                "name": "MoveType",
+                "ofType": null
               },
               "defaultValue": null
             },

doc/user/group/epics/index.md

@@ -215,6 +215,22 @@ To reorder child epics assigned to an epic:
 1. Go to the **Epics and Issues** tab.
 1. Drag and drop epics into the desired order.
 
+## Moving issues and child epics between epics
+
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/33039) in GitLab 13.0.
+
+New issues and child epics are added to the top of their respective lists in the **Epics and Issues** tab. You can move issues and child epics from one epic to another. Issues and child epics cannot be intermingled.
+
+To move an issue from an epic to another:
+
+1. Go to the **Epics and Issues** tab.
+1. Drag and drop issues into the desired parent epic.
+
+To move child epics assigned from an epic to another:
+
+1. Go to the **Epics and Issues** tab.
+1. Drag and drop epics into the desired parent epic.
+
 ## Updating epics
 
 ### Using bulk editing

ee/app/assets/javascripts/related_items_tree/components/tree_item.vue

@@ -48,9 +48,7 @@ export default {
       return this.childrenFlags[this.itemReference].itemExpanded ? __('Collapse') : __('Expand');
     },
     childrenFetchInProgress() {
-      return (
-        this.hasChildren && !this.childrenFlags[this.itemReference].itemChildrenFetchInProgress
-      );
+      return this.hasChildren && this.childrenFlags[this.itemReference].itemChildrenFetchInProgress;
     },
     itemExpanded() {
       return this.hasChildren && this.childrenFlags[this.itemReference].itemExpanded;
@@ -62,6 +60,9 @@ export default {
         !this.childrenFlags[this.itemReference].itemChildrenFetchInProgress
       );
     },
+    showEpicDropzone() {
+      return !this.hasChildren && this.item.type === ChildType.Epic;
+    },
   },
   methods: {
     ...mapActions(['toggleItem']),
@@ -87,7 +88,7 @@ export default {
   >
     <div class="list-item-body d-flex align-items-center">
       <gl-deprecated-button
-        v-if="childrenFetchInProgress"
+        v-if="!childrenFetchInProgress && hasChildren"
         v-gl-tooltip.hover
         :title="chevronTooltip"
         :class="chevronType"
@@ -97,11 +98,7 @@ export default {
       >
         <icon :name="chevronType" />
       </gl-deprecated-button>
-      <gl-loading-icon
-        v-if="childrenFlags[itemReference].itemChildrenFetchInProgress"
-        class="loading-icon"
-        size="sm"
-      />
+      <gl-loading-icon v-if="childrenFetchInProgress" class="loading-icon" size="sm" />
       <tree-item-body
         class="tree-item-row"
         :parent-item="parentItem"
@@ -112,7 +109,7 @@ export default {
       />
     </div>
     <tree-root
-      v-if="item.type === $options.ChildType.Epic"
+      v-if="itemExpanded || showEpicDropzone"
       :parent-item="item"
       :children="children[itemReference] || []"
       class="sub-tree-root"

ee/app/assets/javascripts/related_items_tree/components/tree_root.vue

@@ -4,6 +4,8 @@ import { GlDeprecatedButton, GlLoadingIcon } from '@gitlab/ui';
 
 import TreeDragAndDropMixin from '../mixins/tree_dd_mixin';
 
+import { ChildType } from '../constants';
+
 export default {
   components: {
     GlDeprecatedButton,
@@ -34,7 +36,7 @@ export default {
     },
   },
   methods: {
-    ...mapActions(['fetchNextPageItems', 'reorderItem', 'moveItem']),
+    ...mapActions(['fetchNextPageItems', 'reorderItem', 'moveItem', 'toggleItem']),
     handleShowMoreClick() {
       this.fetchInProgress = true;
       this.fetchNextPageItems({
@@ -47,6 +49,14 @@ export default {
           this.fetchInProgress = false;
         });
     },
+    onMove(e) {
+      const item = e.relatedContext.element;
+      if (item && item.type && item.type === ChildType.Epic)
+        this.toggleItem({
+          parentItem: item,
+          isDragging: true,
+        });
+    },
   },
 };
 </script>
@@ -56,6 +66,7 @@ export default {
     :is="treeRootWrapper"
     v-bind="treeRootOptions"
     class="list-unstyled related-items-list tree-root"
+    :move="onMove"
     @start="handleDragOnStart"
     @end="handleDragOnEnd"
   >

ee/app/assets/javascripts/related_items_tree/store/actions.js

@@ -197,7 +197,7 @@ export const fetchNextPageItems = ({ dispatch, state }, { parentItem, isSubItem
     });
 };
 
-export const toggleItem = ({ state, dispatch }, { parentItem }) => {
+export const toggleItem = ({ state, dispatch }, { parentItem, isDragging = false }) => {
   if (!state.childrenFlags[parentItem.reference].itemExpanded) {
     if (!state.children[parentItem.reference]) {
       dispatch('fetchItems', {
@@ -209,7 +209,7 @@ export const toggleItem = ({ state, dispatch }, { parentItem }) => {
         parentItem,
       });
     }
-  } else {
+  } else if (!isDragging) {
     dispatch('collapseItem', {
       parentItem,
     });
@@ -470,7 +470,7 @@ export const moveItem = (
   let isFirstChild = false;
   const newParentChildren = state.children[newParentItem.parentReference];
 
-  if (newParentChildren !== undefined) {
+  if (newParentChildren !== undefined && newParentChildren.length > 0) {
     adjacentItem = newParentChildren[newIndex];
     if (adjacentItem === undefined) {
       adjacentItem = newParentChildren[newParentChildren.length - 1];

ee/app/assets/javascripts/related_items_tree/store/mutations.js

@@ -220,10 +220,17 @@ export default {
   ) {
     // Remove from old position in previous parent
     state.children[oldParentItem.reference].splice(oldIndex, 1);
+    if (state.children[oldParentItem.reference].length === 0) {
+      state.childrenFlags[oldParentItem.reference].itemHasChildren = false;
+    }
 
     // Insert at new position in new parent
     if (isFirstChild) {
       Vue.set(state.children, newParentItem.parentReference, [targetItem]);
+      Vue.set(state.childrenFlags, newParentItem.parentReference, {
+        itemExpanded: true,
+        itemHasChildren: true,
+      });
     } else {
       state.children[newParentItem.parentReference].splice(newIndex, 0, targetItem);
     }

ee/app/graphql/types/epic_tree/epic_tree_node_input_type.rb

@@ -19,7 +19,7 @@ module Types
 
       argument :relative_position,
                MoveTypeEnum,
-               required: true,
+               required: false,
                description: 'The type of the switch, after or before allowed'
 
       argument :new_parent_id,

ee/app/services/epics/tree_reorder_service.rb

@@ -64,23 +64,26 @@ module Epics
     end
 
     def validate_objects
-      if adjacent_reference
-        return 'Relative position is not valid.' unless valid_relative_position?
+      return 'Only epics and epic_issues are supported.' unless supported_types?
+      return 'You don\'t have permissions to move the objects.' unless authorized?
 
-        unless supported_type?(moving_object) && supported_type?(adjacent_reference)
-          return 'Only epics and epic_issues are supported.'
-        end
-      end
+      validate_adjacent_reference if adjacent_reference
+    end
 
-      return 'You don\'t have permissions to move the objects.' unless authorized?
+    def validate_adjacent_reference
+      return 'Relative position is not valid.' unless valid_relative_position?
 
-      if adjacent_reference
-        if different_epic_parent?
-          return "The sibling object's parent must match the #{new_parent ? "new" : "current"} parent epic."
-        end
+      if different_epic_parent?
+        return "The sibling object's parent must match the #{new_parent ? "new" : "current"} parent epic."
       end
     end
 
+    def supported_types?
+      return false if adjacent_reference && !supported_type?(adjacent_reference)
+
+      supported_type?(moving_object)
+    end
+
     def valid_relative_position?
       %w(before after).include?(params[:relative_position])
     end

ee/spec/frontend/related_items_tree/components/tree_root_spec.js

@@ -376,6 +376,32 @@ describe('RelatedItemsTree', () => {
           );
         });
       });
+
+      describe('onMove', () => {
+        it('calls toggleItem action if move event finds epic element', () => {
+          jest.spyOn(wrapper.vm, 'toggleItem').mockImplementation(() => {});
+          const evt = {
+            relatedContext: {
+              element: mockParentItem,
+            },
+          };
+          wrapper.vm.onMove(evt);
+
+          expect(wrapper.vm.toggleItem).toHaveBeenCalled();
+        });
+
+        it(' does not call toggleItem action if move event does not find epic element', () => {
+          jest.spyOn(wrapper.vm, 'toggleItem').mockImplementation(() => {});
+          const evt = {
+            relatedContext: {
+              element: mockIssue2,
+            },
+          };
+          wrapper.vm.onMove(evt);
+
+          expect(wrapper.vm.toggleItem).not.toHaveBeenCalled();
+        });
+      });
     });
 
     describe('template', () => {

ee/spec/frontend/related_items_tree/mock_data.js

@@ -16,6 +16,7 @@ export const mockParentItem = {
   fullPath: 'gitlab-org',
   title: 'Some sample epic',
   reference: 'gitlab-org&1',
+  type: 'Epic',
   userPermissions: {
     adminEpic: true,
     createEpic: true,

ee/spec/frontend/related_items_tree/store/actions_spec.js

@@ -1267,7 +1267,7 @@ describe('RelatedItemTree', () => {
       });
 
       describe('receiveMoveItemFailure', () => {
-        it('should revert moved item back to its original position ion its original parent via MOVE_ITEM_FAILURE mutation', () => {
+        it('should revert moved item back to its original position on its original parent via MOVE_ITEM_FAILURE mutation', () => {
           testAction(
             actions.receiveMoveItemFailure,
             {},

ee/spec/frontend/related_items_tree/store/mutations_spec.js

@@ -617,6 +617,33 @@ describe('RelatedItemsTree', () => {
             expect.arrayContaining(['bar']),
           );
         });
+
+        it('should update itemHasChildren flags', () => {
+          const newParentItem = {
+            parentReference: '&2',
+          };
+          state.parentItem = { reference: '&1' };
+          state.children[state.parentItem.reference] = ['bar'];
+          state.childrenFlags[state.parentItem.reference] = { itemHasChildren: true };
+
+          mutations[types.MOVE_ITEM](state, {
+            oldParentItem: {
+              reference: '&1',
+            },
+            newParentItem,
+            targetItem: 'bar',
+            oldIndex: 0,
+            newIndex: 0,
+            isFirstChild: true,
+          });
+
+          expect(state.children[state.parentItem.reference].length).toEqual(0);
+          expect(state.childrenFlags[state.parentItem.reference].itemHasChildren).toEqual(false);
+          expect(state.children[newParentItem.parentReference]).toEqual(
+            expect.arrayContaining(['bar']),
+          );
+          expect(state.childrenFlags[newParentItem.parentReference].itemHasChildren).toEqual(true);
+        });
       });
 
       describe(types.MOVE_ITEM_FAILURE, () => {

ee/spec/services/epics/tree_reorder_service_spec.rb

@@ -99,10 +99,6 @@ describe Epics::TreeReorderService do
               tree_object_2.update(epic: epic1)
             end
 
-            # it 'raises an error' do
-            #   expect { subject }.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
-            # end
-
             it 'updates the parent' do
               expect { subject }.to change { tree_object_2.reload.epic }.from(epic1).to(epic)
             end