Merge branch '25233-allow-dragging-for-project-reporters' into 'master'

Allow project reporters to drag issue cards in group board

See merge request gitlab-org/gitlab!68126

app/assets/javascripts/boards/components/board_content.vue

@@ -6,10 +6,12 @@ import { mapState, mapGetters, mapActions } from 'vuex';
 import BoardAddNewColumn from 'ee_else_ce/boards/components/board_add_new_column.vue';
 import defaultSortableConfig from '~/sortable/sortable_config';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
+import { DraggableItemTypes } from '../constants';
 import BoardColumn from './board_column.vue';
 import BoardColumnDeprecated from './board_column_deprecated.vue';
 
 export default {
+  draggableItemTypes: DraggableItemTypes,
   components: {
     BoardAddNewColumn,
     BoardColumn,
@@ -99,6 +101,7 @@ export default {
         :key="index"
         ref="board"
         :list="list"
+        :data-draggable-item-type="$options.draggableItemTypes.list"
         :disabled="disabled"
       />
 

app/assets/javascripts/boards/components/board_list.vue

@@ -6,12 +6,13 @@ import { sortableStart, sortableEnd } from '~/boards/mixins/sortable_default_opt
 import { sprintf, __ } from '~/locale';
 import defaultSortableConfig from '~/sortable/sortable_config';
 import Tracking from '~/tracking';
-import { toggleFormEventPrefix } from '../constants';
+import { toggleFormEventPrefix, DraggableItemTypes } from '../constants';
 import eventHub from '../eventhub';
 import BoardCard from './board_card.vue';
 import BoardNewIssue from './board_new_issue.vue';
 
 export default {
+  draggableItemTypes: DraggableItemTypes,
   name: 'BoardList',
   i18n: {
     loading: __('Loading'),
@@ -27,11 +28,6 @@ export default {
     GlIntersectionObserver,
   },
   mixins: [Tracking.mixin()],
-  inject: {
-    canAdminList: {
-      default: false,
-    },
-  },
   props: {
     disabled: {
       type: Boolean,
@@ -89,8 +85,8 @@ export default {
       return !this.isEpicBoard && this.list.listType !== 'closed' && this.showIssueForm;
     },
     listRef() {
-      // When  list is draggable, the reference to the list needs to be accessed differently
-      return this.canAdminList ? this.$refs.list.$el : this.$refs.list;
+      // When list is draggable, the reference to the list needs to be accessed differently
+      return this.canMoveIssue ? this.$refs.list.$el : this.$refs.list;
     },
     showingAllItems() {
       return this.boardItems.length === this.listItemsCount;
@@ -100,8 +96,11 @@ export default {
         ? this.$options.i18n.showingAllEpics
         : this.$options.i18n.showingAllIssues;
     },
+    canMoveIssue() {
+      return !this.disabled;
+    },
     treeRootWrapper() {
-      return this.canAdminList && !this.listsFlags[this.list.id]?.addItemToListInProgress
+      return this.canMoveIssue && !this.listsFlags[this.list.id]?.addItemToListInProgress
         ? Draggable
         : 'ul';
     },
@@ -116,7 +115,7 @@ export default {
         value: this.boardItems,
       };
 
-      return this.canAdminList ? options : {};
+      return this.canMoveIssue ? options : {};
     },
   },
   watch: {
@@ -172,15 +171,33 @@ export default {
         this.loadNextPage();
       }
     },
-    handleDragOnStart() {
+    handleDragOnStart({
+      item: {
+        dataset: { draggableItemType },
+      },
+    }) {
+      if (draggableItemType !== DraggableItemTypes.card) {
+        return;
+      }
+
       sortableStart();
       this.track('drag_card', { label: 'board' });
     },
-    handleDragOnEnd(params) {
+    handleDragOnEnd({
+      newIndex: originalNewIndex,
+      oldIndex,
+      from,
+      to,
+      item: {
+        dataset: { draggableItemType, itemId, itemIid, itemPath },
+      },
+    }) {
+      if (draggableItemType !== DraggableItemTypes.card) {
+        return;
+      }
+
       sortableEnd();
-      const { oldIndex, from, to, item } = params;
-      let { newIndex } = params;
-      const { itemId, itemIid, itemPath } = item.dataset;
+      let newIndex = originalNewIndex;
       let { children } = to;
       let moveBeforeId;
       let moveAfterId;
@@ -267,6 +284,7 @@ export default {
         :index="index"
         :list="list"
         :item="item"
+        :data-draggable-item-type="$options.draggableItemTypes.card"
         :disabled="disabled"
       />
       <gl-intersection-observer @appear="onReachingListBottom">

app/assets/javascripts/boards/constants.js

@@ -114,6 +114,11 @@ export const FilterFields = {
   ],
 };
 
+export const DraggableItemTypes = {
+  card: 'card',
+  list: 'list',
+};
+
 export default {
   BoardType,
   ListType,

app/assets/javascripts/boards/stores/actions.js

@@ -14,6 +14,7 @@ import {
   issuableTypes,
   FilterFields,
   ListTypeTitles,
+  DraggableItemTypes,
 } from 'ee_else_ce/boards/constants';
 import createBoardListMutation from 'ee_else_ce/boards/graphql/board_list_create.mutation.graphql';
 import issueMoveListMutation from 'ee_else_ce/boards/graphql/issue_move_list.mutation.graphql';
@@ -267,12 +268,16 @@ export default {
     { state: { boardLists }, commit, dispatch },
     {
       item: {
-        dataset: { listId: movedListId },
+        dataset: { listId: movedListId, draggableItemType },
       },
       newIndex,
       to: { children },
     },
   ) => {
+    if (draggableItemType !== DraggableItemTypes.list) {
+      return;
+    }
+
     const displacedListId = children[newIndex].dataset.listId;
     if (movedListId === displacedListId) {
       return;

spec/features/boards/boards_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe 'Project issue boards', :js do
   include DragTo
   include MobileHelpers
+  include BoardHelpers
 
   let_it_be(:group)   { create(:group, :nested) }
   let_it_be(:project) { create(:project, :public, namespace: group) }
@@ -546,23 +547,6 @@ RSpec.describe 'Project issue boards', :js do
     end
   end
 
-  def drag(selector: '.board-list', list_from_index: 0, from_index: 0, to_index: 0, list_to_index: 0, perform_drop: true)
-    # ensure there is enough horizontal space for four boards
-    inspect_requests(inject_headers: { 'X-GITLAB-DISABLE-SQL-QUERY-LIMIT' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/323426' }) do
-      resize_window(2000, 800)
-
-      drag_to(selector: selector,
-              scrollable: '#board-app',
-              list_from_index: list_from_index,
-              from_index: from_index,
-              to_index: to_index,
-              list_to_index: list_to_index,
-              perform_drop: perform_drop)
-    end
-
-    wait_for_requests
-  end
-
   def wait_for_board_cards(board_number, expected_cards)
     page.within(find(".board:nth-child(#{board_number})")) do
       expect(page.find('.board-header')).to have_content(expected_cards.to_s)

spec/features/groups/board_spec.rb

@@ -3,16 +3,21 @@
 require 'spec_helper'
 
 RSpec.describe 'Group Boards' do
-  let(:group) { create(:group) }
-  let!(:project) { create(:project_empty_repo, group: group) }
-  let(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user }
+  include DragTo
+  include MobileHelpers
+  include BoardHelpers
 
-  before do
-    sign_in(user)
-  end
+  let_it_be(:group) { create(:group) }
+  let_it_be(:user) { create(:user) }
+
+  context 'Creates an issue', :js do
+    let_it_be(:project) { create(:project_empty_repo, group: group) }
 
-  context 'Creates a an issue', :js do
     before do
+      group.add_maintainer(user)
+
+      sign_in(user)
+
       visit group_boards_path(group)
     end
 
@@ -39,4 +44,58 @@ RSpec.describe 'Group Boards' do
       end
     end
   end
+
+  context "when user is a Reporter in one of the group's projects", :js do
+    let_it_be(:board) { create(:board, group: group) }
+
+    let_it_be(:backlog_list) { create(:backlog_list, board: board) }
+    let_it_be(:group_label1) { create(:group_label, title: "bug", group: group) }
+    let_it_be(:group_label2) { create(:group_label, title: "dev", group: group) }
+    let_it_be(:list1) { create(:list, board: board, label: group_label1, position: 0) }
+    let_it_be(:list2) { create(:list, board: board, label: group_label2, position: 1) }
+
+    let_it_be(:project1) { create(:project_empty_repo, :private, group: group) }
+    let_it_be(:project2) { create(:project_empty_repo, :private, group: group) }
+    let_it_be(:issue1) { create(:issue, title: 'issue1', project: project1, labels: [group_label2]) }
+    let_it_be(:issue2) { create(:issue, title: 'issue2', project: project2) }
+
+    before do
+      project1.add_guest(user)
+      project2.add_reporter(user)
+
+      sign_in(user)
+
+      inspect_requests(inject_headers: { 'X-GITLAB-DISABLE-SQL-QUERY-LIMIT' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/323426' }) do
+        visit group_boards_path(group)
+      end
+    end
+
+    it 'allows user to move issue of project where they are a Reporter' do
+      expect(find('.board:nth-child(1)')).to have_content(issue2.title)
+
+      drag(list_from_index: 0, from_index: 0, list_to_index: 1)
+
+      expect(find('.board:nth-child(2)')).to have_content(issue2.title)
+      expect(issue2.reload.labels).to contain_exactly(group_label1)
+    end
+
+    it 'does not allow user to move issue of project where they are a Guest' do
+      expect(find('.board:nth-child(3)')).to have_content(issue1.title)
+
+      drag(list_from_index: 2, from_index: 0, list_to_index: 1)
+
+      expect(find('.board:nth-child(3)')).to have_content(issue1.title)
+      expect(issue1.reload.labels).to contain_exactly(group_label2)
+      expect(issue2.reload.labels).to eq([])
+    end
+
+    it 'does not allow user to re-position lists' do
+      drag(list_from_index: 1, list_to_index: 2, selector: '.board-header')
+
+      expect(find('.board:nth-child(2) [data-testid="board-list-header"]')).to have_content(group_label1.title)
+      expect(find('.board:nth-child(3) [data-testid="board-list-header"]')).to have_content(group_label2.title)
+      expect(list1.reload.position).to eq(0)
+      expect(list2.reload.position).to eq(1)
+    end
+  end
 end

spec/frontend/boards/board_list_spec.js

+import Draggable from 'vuedraggable';
+import { DraggableItemTypes } from 'ee_else_ce/boards/constants';
 import { useFakeRequestAnimationFrame } from 'helpers/fake_request_animation_frame';
 import createComponent from 'jest/boards/board_list_helper';
 import BoardCard from '~/boards/components/board_card.vue';
@@ -10,6 +12,23 @@ describe('Board list component', () => {
 
   const findByTestId = (testId) => wrapper.find(`[data-testid="${testId}"]`);
   const findIssueCountLoadingIcon = () => wrapper.find('[data-testid="count-loading-icon"]');
+  const findDraggable = () => wrapper.findComponent(Draggable);
+
+  const startDrag = (
+    params = {
+      item: {
+        dataset: {
+          draggableItemType: DraggableItemTypes.card,
+        },
+      },
+    },
+  ) => {
+    findByTestId('tree-root-wrapper').vm.$emit('start', params);
+  };
+
+  const endDrag = (params) => {
+    findByTestId('tree-root-wrapper').vm.$emit('end', params);
+  };
 
   useFakeRequestAnimationFrame();
 
@@ -155,40 +174,89 @@ describe('Board list component', () => {
   });
 
   describe('drag & drop issue', () => {
-    beforeEach(() => {
-      wrapper = createComponent();
-    });
+    describe('when dragging is allowed', () => {
+      beforeEach(() => {
+        wrapper = createComponent({
+          componentProps: {
+            disabled: false,
+          },
+        });
+      });
 
-    describe('handleDragOnStart', () => {
-      it('adds a class `is-dragging` to document body', () => {
-        expect(document.body.classList.contains('is-dragging')).toBe(false);
+      it('Draggable is used', () => {
+        expect(findDraggable().exists()).toBe(true);
+      });
+
+      describe('handleDragOnStart', () => {
+        it('adds a class `is-dragging` to document body', () => {
+          expect(document.body.classList.contains('is-dragging')).toBe(false);
 
-        findByTestId('tree-root-wrapper').vm.$emit('start');
+          startDrag();
 
-        expect(document.body.classList.contains('is-dragging')).toBe(true);
+          expect(document.body.classList.contains('is-dragging')).toBe(true);
+        });
       });
-    });
 
-    describe('handleDragOnEnd', () => {
-      it('removes class `is-dragging` from document body', () => {
-        jest.spyOn(wrapper.vm, 'moveItem').mockImplementation(() => {});
-        document.body.classList.add('is-dragging');
+      describe('handleDragOnEnd', () => {
+        beforeEach(() => {
+          jest.spyOn(wrapper.vm, 'moveItem').mockImplementation(() => {});
+
+          startDrag();
+        });
+
+        it('removes class `is-dragging` from document body', () => {
+          document.body.classList.add('is-dragging');
+
+          endDrag({
+            oldIndex: 1,
+            newIndex: 0,
+            item: {
+              dataset: {
+                draggableItemType: DraggableItemTypes.card,
+                itemId: mockIssues[0].id,
+                itemIid: mockIssues[0].iid,
+                itemPath: mockIssues[0].referencePath,
+              },
+            },
+            to: { children: [], dataset: { listId: 'gid://gitlab/List/1' } },
+            from: { dataset: { listId: 'gid://gitlab/List/2' } },
+          });
 
-        findByTestId('tree-root-wrapper').vm.$emit('end', {
-          oldIndex: 1,
-          newIndex: 0,
-          item: {
-            dataset: {
-              itemId: mockIssues[0].id,
-              itemIid: mockIssues[0].iid,
-              itemPath: mockIssues[0].referencePath,
+          expect(document.body.classList.contains('is-dragging')).toBe(false);
+        });
+
+        it(`should not handle the event if the dragged item is not a "${DraggableItemTypes.card}"`, () => {
+          endDrag({
+            oldIndex: 1,
+            newIndex: 0,
+            item: {
+              dataset: {
+                draggableItemType: DraggableItemTypes.list,
+                itemId: mockIssues[0].id,
+                itemIid: mockIssues[0].iid,
+                itemPath: mockIssues[0].referencePath,
+              },
             },
+            to: { children: [], dataset: { listId: 'gid://gitlab/List/1' } },
+            from: { dataset: { listId: 'gid://gitlab/List/2' } },
+          });
+
+          expect(document.body.classList.contains('is-dragging')).toBe(true);
+        });
+      });
+    });
+
+    describe('when dragging is not allowed', () => {
+      beforeEach(() => {
+        wrapper = createComponent({
+          componentProps: {
+            disabled: true,
           },
-          to: { children: [], dataset: { listId: 'gid://gitlab/List/1' } },
-          from: { dataset: { listId: 'gid://gitlab/List/2' } },
         });
+      });
 
-        expect(document.body.classList.contains('is-dragging')).toBe(false);
+      it('Draggable is not used', () => {
+        expect(findDraggable().exists()).toBe(false);
       });
     });
   });

spec/frontend/boards/stores/actions_spec.js

@@ -9,6 +9,7 @@ import {
   issuableTypes,
   BoardType,
   listsQuery,
+  DraggableItemTypes,
 } from 'ee_else_ce/boards/constants';
 import issueMoveListMutation from 'ee_else_ce/boards/graphql/issue_move_list.mutation.graphql';
 import testAction from 'helpers/vuex_action_helper';
@@ -525,6 +526,21 @@ describe('moveList', () => {
   const movableListsOrder = ['gid://3', 'gid://4', 'gid://5'];
   const allListsOrder = [backlogListId, ...movableListsOrder, closedListId];
 
+  it(`should not handle the event if the dragged item is not a "${DraggableItemTypes.list}"`, () => {
+    return testAction({
+      action: actions.moveList,
+      payload: {
+        item: { dataset: { listId: '', draggableItemType: DraggableItemTypes.card } },
+        to: {
+          children: [],
+        },
+      },
+      state: {},
+      expectedMutations: [],
+      expectedActions: [],
+    });
+  });
+
   describe.each`
     draggableFrom | draggableTo | boardLists     | boardListsOrder      | expectedMovableListsOrder
     ${0}          | ${2}        | ${boardLists1} | ${movableListsOrder} | ${['gid://4', 'gid://5', 'gid://3']}
@@ -544,7 +560,12 @@ describe('moveList', () => {
       const displacedListId = boardListsOrder[draggableTo];
       const buildDraggablePayload = () => {
         return {
-          item: { dataset: { listId: boardListsOrder[draggableFrom] } },
+          item: {
+            dataset: {
+              listId: boardListsOrder[draggableFrom],
+              draggableItemType: DraggableItemTypes.list,
+            },
+          },
           newIndex: draggableTo,
           to: {
             children: boardListsOrder.map((listId) => ({ dataset: { listId } })),
@@ -584,7 +605,7 @@ describe('moveList', () => {
       return testAction({
         action: actions.moveList,
         payload: {
-          item: { dataset: { listId } },
+          item: { dataset: { listId, draggbaleItemType: DraggableItemTypes.list } },
           newIndex: 0,
           to: {
             children: [{ dataset: { listId } }],

spec/support/helpers/board_helpers.rb

@@ -23,4 +23,21 @@ module BoardHelpers
       wait_for_requests
     end
   end
+
+  def drag(selector: '.board-list', list_from_index: 0, from_index: 0, to_index: 0, list_to_index: 0, perform_drop: true)
+    inspect_requests(inject_headers: { 'X-GITLAB-DISABLE-SQL-QUERY-LIMIT' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/323426' }) do
+      # ensure there is enough horizontal space for four board lists
+      resize_window(2000, 800)
+
+      drag_to(selector: selector,
+              scrollable: '#board-app',
+              list_from_index: list_from_index,
+              from_index: from_index,
+              to_index: to_index,
+              list_to_index: list_to_index,
+              perform_drop: perform_drop)
+    end
+
+    wait_for_requests
+  end
 end