From 5a55bb9d969290812348833f0564b476b16d0e0a Mon Sep 17 00:00:00 2001
From: Coung Ngo <cngo@gitlab.com>
Date: Tue, 23 Mar 2021 11:55:10 +0000
Subject: [PATCH 1/2] Add bulk edit to issues list page refactor

This commit adds bulk edit to the Vue issues list page,
which is under the feature flag `vue_issues_list` defaulted to off.
---
 .../issuable_bulk_update_actions.js           |  4 +--
 .../issuable_bulk_update_sidebar.js           | 22 ++++++-------
 .../components/issuable_item.vue              |  6 +++-
 .../components/issuable_list_root.vue         |  2 ++
 .../components/issues_list_app.vue            | 18 ++++++++++
 app/assets/javascripts/labels_select.js       |  8 +++--
 app/views/projects/issues/index.html.haml     |  2 ++
 .../components/issuable_list_root_spec.js     | 15 ++++++++-
 .../components/issues_list_app_spec.js        | 33 +++++++++++++++++++
 9 files changed, 92 insertions(+), 18 deletions(-)

diff --git a/app/assets/javascripts/issuable_bulk_update_actions.js b/app/assets/javascripts/issuable_bulk_update_actions.js
index f507f072253d50..366a9a8a883183 100644
--- a/app/assets/javascripts/issuable_bulk_update_actions.js
+++ b/app/assets/javascripts/issuable_bulk_update_actions.js
@@ -87,7 +87,7 @@ export default {
   // From issuable's initial bulk selection
   getOriginalCommonIds() {
     const labelIds = [];
-    this.getElement('.selected-issuable:checked').each((i, el) => {
+    this.getElement('.issuable-list input[type="checkbox"]:checked').each((i, el) => {
       labelIds.push(this.getElement(`#${this.prefixId}${el.dataset.id}`).data('labels'));
     });
     return intersection.apply(this, labelIds);
@@ -100,7 +100,7 @@ export default {
     let issuableLabels = [];
 
     // Collect unique label IDs for all checked issues
-    this.getElement('.selected-issuable:checked').each((i, el) => {
+    this.getElement('.issuable-list input[type="checkbox"]:checked').each((i, el) => {
       issuableLabels = this.getElement(`#${this.prefixId}${el.dataset.id}`).data('labels');
       issuableLabels.forEach((labelId) => {
         // Store unique IDs
diff --git a/app/assets/javascripts/issuable_bulk_update_sidebar.js b/app/assets/javascripts/issuable_bulk_update_sidebar.js
index ef98db5151a66c..1f707bc955f1de 100644
--- a/app/assets/javascripts/issuable_bulk_update_sidebar.js
+++ b/app/assets/javascripts/issuable_bulk_update_sidebar.js
@@ -34,7 +34,7 @@ export default class IssuableBulkUpdateSidebar {
     this.$otherFilters = $('.issues-other-filters');
     this.$checkAllContainer = $('.check-all-holder');
     this.$issueChecks = $('.issue-check');
-    this.$issuesList = $('.selected-issuable');
+    this.$issuesList = $('.issuable-list input[type="checkbox"]');
     this.$issuableIdsInput = $('#update_issuable_ids');
   }
 
@@ -46,16 +46,14 @@ export default class IssuableBulkUpdateSidebar {
     this.$bulkEditSubmitBtn.on('click', () => this.prepForSubmit());
     this.$checkAllContainer.on('click', () => this.updateFormState());
 
-    if (this.vueIssuablesListFeature) {
-      issueableEventHub.$on('issuables:updateBulkEdit', () => {
-        // Danger! Strong coupling ahead!
-        // The bulk update sidebar and its dropdowns look for .selected-issuable checkboxes, and get data on which issue
-        // is selected by inspecting the DOM. Ideally, we would pass the selected issuable IDs and their properties
-        // explicitly, but this component is used in too many places right now to refactor straight away.
+    issueableEventHub.$on('issuables:updateBulkEdit', () => {
+      // Danger! Strong coupling ahead!
+      // The bulk update sidebar and its dropdowns look for checkboxes, and get data on which issue
+      // is selected by inspecting the DOM. Ideally, we would pass the selected issuable IDs and their properties
+      // explicitly, but this component is used in too many places right now to refactor straight away.
 
-        this.updateFormState();
-      });
-    }
+      this.updateFormState();
+    });
   }
 
   initDropdowns() {
@@ -96,7 +94,7 @@ export default class IssuableBulkUpdateSidebar {
   }
 
   updateFormState() {
-    const noCheckedIssues = !$('.selected-issuable:checked').length;
+    const noCheckedIssues = !$('.issuable-list input[type="checkbox"]:checked').length;
 
     this.toggleSubmitButtonDisabled(noCheckedIssues);
     this.updateSelectedIssuableIds();
@@ -166,7 +164,7 @@ export default class IssuableBulkUpdateSidebar {
   }
 
   static getCheckedIssueIds() {
-    const $checkedIssues = $('.selected-issuable:checked');
+    const $checkedIssues = $('.issuable-list input[type="checkbox"]:checked');
 
     if ($checkedIssues.length > 0) {
       return $.map($checkedIssues, (value) => $(value).data('id'));
diff --git a/app/assets/javascripts/issuable_list/components/issuable_item.vue b/app/assets/javascripts/issuable_list/components/issuable_item.vue
index 92c527c79ffc3d..880ad686f17566 100644
--- a/app/assets/javascripts/issuable_list/components/issuable_item.vue
+++ b/app/assets/javascripts/issuable_list/components/issuable_item.vue
@@ -65,6 +65,9 @@ export default {
     labels() {
       return this.issuable.labels?.nodes || this.issuable.labels || [];
     },
+    labelIdsString() {
+      return JSON.stringify(this.labels.map((label) => label.id));
+    },
     assignees() {
       return this.issuable.assignees || [];
     },
@@ -149,12 +152,13 @@ export default {
 </script>
 
 <template>
-  <li class="issue gl-px-5!">
+  <li :id="`issue_${issuable.id}`" class="issue gl-px-5!" :data-labels="labelIdsString">
     <div class="issuable-info-container">
       <div v-if="showCheckbox" class="issue-check">
         <gl-form-checkbox
           class="gl-mr-0"
           :checked="checked"
+          :data-id="issuable.id"
           @input="$emit('checked-input', $event)"
         />
       </div>
diff --git a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue
index 1b54ba766ff3f1..6b95c3a578e7d1 100644
--- a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue
+++ b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue
@@ -218,11 +218,13 @@ export default {
     },
     handleIssuableCheckedInput(issuable, value) {
       this.checkedIssuables[this.issuableId(issuable)].checked = value;
+      this.$emit('update-legacy-bulk-edit');
     },
     handleAllIssuablesCheckedInput(value) {
       Object.keys(this.checkedIssuables).forEach((issuableId) => {
         this.checkedIssuables[issuableId].checked = value;
       });
+      this.$emit('update-legacy-bulk-edit');
     },
     handleVueDraggableUpdate({ newIndex, oldIndex }) {
       this.$emit('reorder', { newIndex, oldIndex });
diff --git a/app/assets/javascripts/issues_list/components/issues_list_app.vue b/app/assets/javascripts/issues_list/components/issues_list_app.vue
index e4bb3ecabd01ea..d7af388c8931bb 100644
--- a/app/assets/javascripts/issues_list/components/issues_list_app.vue
+++ b/app/assets/javascripts/issues_list/components/issues_list_app.vue
@@ -14,6 +14,7 @@ import {
 import axios from '~/lib/utils/axios_utils';
 import { convertObjectPropsToCamelCase, getParameterByName } from '~/lib/utils/common_utils';
 import { __ } from '~/locale';
+import eventHub from '../eventhub';
 import IssueCardTimeInfo from './issue_card_time_info.vue';
 
 export default {
@@ -56,6 +57,7 @@ export default {
       filters: sortParams[sortKey] || {},
       isLoading: false,
       issues: [],
+      showBulkEditSidebar: false,
       sortKey: sortKey || CREATED_DESC,
       totalIssues: 0,
     };
@@ -73,8 +75,15 @@ export default {
     },
   },
   mounted() {
+    eventHub.$on('issuables:toggleBulkEdit', (showBulkEditSidebar) => {
+      this.showBulkEditSidebar = showBulkEditSidebar;
+    });
     this.fetchIssues();
   },
+  beforeDestroy() {
+    // eslint-disable-next-line @gitlab/no-global-event-off
+    eventHub.$off('issuables:toggleBulkEdit');
+  },
   methods: {
     fetchIssues(pageToFetch) {
       this.isLoading = true;
@@ -101,6 +110,13 @@ export default {
           this.isLoading = false;
         });
     },
+    handleUpdateLegacyBulkEdit() {
+      // If "select all" checkbox was checked, wait for all checkboxes
+      // to be checked before updating IssuableBulkUpdateSidebar class
+      this.$nextTick(() => {
+        eventHub.$emit('issuables:updateBulkEdit');
+      });
+    },
     handlePageChange(page) {
       this.fetchIssues(page);
     },
@@ -159,6 +175,7 @@ export default {
     current-tab=""
     :issuables-loading="isLoading"
     :is-manual-ordering="isManualOrdering"
+    :show-bulk-edit-sidebar="showBulkEditSidebar"
     :show-pagination-controls="true"
     :total-items="totalIssues"
     :current-page="currentPage"
@@ -168,6 +185,7 @@ export default {
     @page-change="handlePageChange"
     @reorder="handleReorder"
     @sort="handleSort"
+    @update-legacy-bulk-edit="handleUpdateLegacyBulkEdit"
   >
     <template #timeframe="{ issuable = {} }">
       <issue-card-time-info :issue="issuable" />
diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js
index ed08a024b59231..2503648e6f5324 100644
--- a/app/assets/javascripts/labels_select.js
+++ b/app/assets/javascripts/labels_select.js
@@ -526,11 +526,15 @@ export default class LabelsSelect {
   }
 
   bindEvents() {
-    return $('body').on('change', '.selected-issuable', this.onSelectCheckboxIssue);
+    return $('body').on(
+      'change',
+      '.issuable-list input[type="checkbox"]',
+      this.onSelectCheckboxIssue,
+    );
   }
   // eslint-disable-next-line class-methods-use-this
   onSelectCheckboxIssue() {
-    if ($('.selected-issuable:checked').length) {
+    if ($('.issuable-list input[type="checkbox"]:checked').length) {
       return;
     }
     return $('.issues-bulk-update .labels-filter .dropdown-toggle-text').text(__('Label'));
diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml
index 9c54b117f4c0cc..3b35bd7544a43a 100644
--- a/app/views/projects/issues/index.html.haml
+++ b/app/views/projects/issues/index.html.haml
@@ -26,6 +26,8 @@
       has_issuable_health_status_feature: Gitlab.ee? && @project.feature_available?(:issuable_health_status).to_s,
       has_issue_weights_feature: Gitlab.ee? && @project.feature_available?(:issue_weights).to_s,
       issues_path: project_issues_path(@project) } }
+    - if @can_bulk_update
+      = render 'shared/issuable/bulk_update_sidebar', type: :issues
   - else
     = render 'shared/issuable/search_bar', type: :issues
 
diff --git a/spec/frontend/issuable_list/components/issuable_list_root_spec.js b/spec/frontend/issuable_list/components/issuable_list_root_spec.js
index 53f5d7874e11e9..38d6d6d86bc035 100644
--- a/spec/frontend/issuable_list/components/issuable_list_root_spec.js
+++ b/spec/frontend/issuable_list/components/issuable_list_root_spec.js
@@ -35,6 +35,7 @@ describe('IssuableListRoot', () => {
 
   const findFilteredSearchBar = () => wrapper.findComponent(FilteredSearchBar);
   const findGlPagination = () => wrapper.findComponent(GlPagination);
+  const findIssuableItem = () => wrapper.findComponent(IssuableItem);
   const findIssuableTabs = () => wrapper.findComponent(IssuableTabs);
   const findVueDraggable = () => wrapper.findComponent(VueDraggable);
 
@@ -351,6 +352,18 @@ describe('IssuableListRoot', () => {
       });
     });
 
+    it('emits `update-legacy-bulk-edit` when filtered-search-bar checkbox is checked', () => {
+      findFilteredSearchBar().vm.$emit('checked-input');
+
+      expect(wrapper.emitted('update-legacy-bulk-edit')).toEqual([[]]);
+    });
+
+    it('emits `update-legacy-bulk-edit` when issuable-item checkbox is checked', () => {
+      findIssuableItem().vm.$emit('checked-input');
+
+      expect(wrapper.emitted('update-legacy-bulk-edit')).toEqual([[]]);
+    });
+
     it('gl-pagination component emits `page-change` event on `input` event', async () => {
       wrapper.setProps({
         showPaginationControls: true,
@@ -379,7 +392,7 @@ describe('IssuableListRoot', () => {
       });
 
       it('IssuableItem has grab cursor', () => {
-        expect(wrapper.findComponent(IssuableItem).classes()).toContain('gl-cursor-grab');
+        expect(findIssuableItem().classes()).toContain('gl-cursor-grab');
       });
 
       it('emits a "reorder" event when user updates the issue order', () => {
diff --git a/spec/frontend/issues_list/components/issues_list_app_spec.js b/spec/frontend/issues_list/components/issues_list_app_spec.js
index 0a445973c14a10..bc649858cd49a5 100644
--- a/spec/frontend/issues_list/components/issues_list_app_spec.js
+++ b/spec/frontend/issues_list/components/issues_list_app_spec.js
@@ -13,6 +13,7 @@ import {
   sortOptions,
   sortParams,
 } from '~/issues_list/constants';
+import eventHub from '~/issues_list/eventhub';
 import axios from '~/lib/utils/axios_utils';
 import { setUrlParams } from '~/lib/utils/url_utility';
 
@@ -101,6 +102,23 @@ describe('IssuesListApp component', () => {
     });
   });
 
+  describe('bulk edit', () => {
+    describe.each([true, false])(
+      'when "issuables:toggleBulkEdit" event is received with payload %s',
+      (isBulkEdit) => {
+        beforeEach(() => {
+          wrapper = mountComponent();
+
+          eventHub.$emit('issuables:toggleBulkEdit', isBulkEdit);
+        });
+
+        it(`${isBulkEdit ? 'enables' : 'disables'} bulk edit`, async () => {
+          expect(findIssuableList().props('showBulkEditSidebar')).toBe(isBulkEdit);
+        });
+      },
+    );
+  });
+
   describe('when "page-change" event is emitted by IssuableList', () => {
     const data = [{ id: 10, title: 'title', state }];
     const page = 2;
@@ -210,4 +228,19 @@ describe('IssuesListApp component', () => {
       },
     );
   });
+
+  describe('when "update-legacy-bulk-edit" event is emitted by IssuableList', () => {
+    beforeEach(() => {
+      wrapper = mountComponent();
+      jest.spyOn(eventHub, '$emit');
+    });
+
+    it('emits an "issuables:updateBulkEdit" event to the legacy bulk edit class', async () => {
+      findIssuableList().vm.$emit('update-legacy-bulk-edit');
+
+      await waitForPromises();
+
+      expect(eventHub.$emit).toHaveBeenCalledWith('issuables:updateBulkEdit');
+    });
+  });
 });
-- 
GitLab


From 69fc3250b43f4f67236d4709e05e9e650b6ed6b8 Mon Sep 17 00:00:00 2001
From: Coung Ngo <cngo@gitlab.com>
Date: Wed, 24 Mar 2021 18:28:03 +0000
Subject: [PATCH 2/2] Address reviewer comments

---
 app/assets/javascripts/issuable_index.js                  | 2 +-
 .../issuable_list/components/issuable_item.vue            | 2 +-
 .../javascripts/pages/projects/issues/index/index.js      | 7 ++++++-
 app/controllers/projects/issues_controller.rb             | 1 +
 .../issues_list/components/issues_list_app_spec.js        | 8 ++++----
 5 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/assets/javascripts/issuable_index.js b/app/assets/javascripts/issuable_index.js
index 4856f9781ce2bc..cdeee68b762e55 100644
--- a/app/assets/javascripts/issuable_index.js
+++ b/app/assets/javascripts/issuable_index.js
@@ -1,7 +1,7 @@
 import issuableInitBulkUpdateSidebar from './issuable_init_bulk_update_sidebar';
 
 export default class IssuableIndex {
-  constructor(pagePrefix) {
+  constructor(pagePrefix = 'issuable_') {
     issuableInitBulkUpdateSidebar.init(pagePrefix);
   }
 }
diff --git a/app/assets/javascripts/issuable_list/components/issuable_item.vue b/app/assets/javascripts/issuable_list/components/issuable_item.vue
index 880ad686f17566..5d497369f5a5f5 100644
--- a/app/assets/javascripts/issuable_list/components/issuable_item.vue
+++ b/app/assets/javascripts/issuable_list/components/issuable_item.vue
@@ -152,7 +152,7 @@ export default {
 </script>
 
 <template>
-  <li :id="`issue_${issuable.id}`" class="issue gl-px-5!" :data-labels="labelIdsString">
+  <li :id="`issuable_${issuable.id}`" class="issue gl-px-5!" :data-labels="labelIdsString">
     <div class="issuable-info-container">
       <div v-if="showCheckbox" class="issue-check">
         <gl-form-checkbox
diff --git a/app/assets/javascripts/pages/projects/issues/index/index.js b/app/assets/javascripts/pages/projects/issues/index/index.js
index 366f8dc61bcb0e..85489ae86873e6 100644
--- a/app/assets/javascripts/pages/projects/issues/index/index.js
+++ b/app/assets/javascripts/pages/projects/issues/index/index.js
@@ -20,7 +20,12 @@ initFilteredSearch({
   useDefaultState: true,
 });
 
-new IssuableIndex(ISSUABLE_INDEX.ISSUE);
+if (gon.features?.vueIssuesList) {
+  new IssuableIndex();
+} else {
+  new IssuableIndex(ISSUABLE_INDEX.ISSUE);
+}
+
 new ShortcutsNavigation();
 new UsersSelect();
 
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index e8151a7c173539..e5771d791ade9d 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -45,6 +45,7 @@ class Projects::IssuesController < Projects::ApplicationController
     push_frontend_feature_flag(:vue_issuables_list, project)
     push_frontend_feature_flag(:usage_data_design_action, project, default_enabled: true)
     push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml)
+    push_frontend_feature_flag(:vue_issues_list, project)
   end
 
   before_action only: :show do
diff --git a/spec/frontend/issues_list/components/issues_list_app_spec.js b/spec/frontend/issues_list/components/issues_list_app_spec.js
index bc649858cd49a5..20644f80bf6fa5 100644
--- a/spec/frontend/issues_list/components/issues_list_app_spec.js
+++ b/spec/frontend/issues_list/components/issues_list_app_spec.js
@@ -104,7 +104,7 @@ describe('IssuesListApp component', () => {
 
   describe('bulk edit', () => {
     describe.each([true, false])(
-      'when "issuables:toggleBulkEdit" event is received with payload %s',
+      'when "issuables:toggleBulkEdit" event is received with payload `%s`',
       (isBulkEdit) => {
         beforeEach(() => {
           wrapper = mountComponent();
@@ -112,7 +112,7 @@ describe('IssuesListApp component', () => {
           eventHub.$emit('issuables:toggleBulkEdit', isBulkEdit);
         });
 
-        it(`${isBulkEdit ? 'enables' : 'disables'} bulk edit`, async () => {
+        it(`${isBulkEdit ? 'enables' : 'disables'} bulk edit`, () => {
           expect(findIssuableList().props('showBulkEditSidebar')).toBe(isBulkEdit);
         });
       },
@@ -137,7 +137,7 @@ describe('IssuesListApp component', () => {
       await waitForPromises();
     });
 
-    it('fetches issues with expected params', async () => {
+    it('fetches issues with expected params', () => {
       expect(axiosMock.history.get[1].params).toEqual({
         page,
         per_page: PAGE_SIZE,
@@ -210,7 +210,7 @@ describe('IssuesListApp component', () => {
 
   describe('when "sort" event is emitted by IssuableList', () => {
     it.each(Object.keys(sortParams))(
-      'fetches issues with correct params for "sort" payload %s',
+      'fetches issues with correct params for "sort" payload `%s`',
       async (sortKey) => {
         wrapper = mountComponent();
 
-- 
GitLab