Merge branch 'astoicescu/removeDuplicateDashboardItemFromDashboardsDropdown' into 'master'

Remove Duplicate Dashboard item from dashboards dropdown

See merge request gitlab-org/gitlab!38053

app/assets/javascripts/monitoring/components/dashboard_header.vue

@@ -232,7 +232,6 @@ export default {
         class="flex-grow-1"
         toggle-class="dropdown-menu-toggle"
         :default-branch="defaultBranch"
-        :modal-id="$options.modalIds.duplicateDashboard"
         @selectDashboard="selectDashboard"
       />
     </div>
@@ -429,6 +428,7 @@ export default {
 
           <template v-if="isOutOfTheBoxDashboard">
             <gl-new-dropdown-divider />
+
             <gl-new-dropdown-item
               ref="duplicateDashboardItem"
               v-gl-modal="$options.modalIds.duplicateDashboard"
@@ -436,6 +436,12 @@ export default {
             >
               {{ s__('Metrics|Duplicate current dashboard') }}
             </gl-new-dropdown-item>
+
+            <duplicate-dashboard-modal
+              :default-branch="defaultBranch"
+              :modal-id="$options.modalIds.duplicateDashboard"
+              @dashboardDuplicated="selectDashboard"
+            />
           </template>
         </gl-new-dropdown>
       </div>
@@ -450,10 +456,5 @@ export default {
         />
       </div>
     </div>
-    <duplicate-dashboard-modal
-      :default-branch="defaultBranch"
-      :modal-id="$options.modalIds.duplicateDashboard"
-      @dashboardDuplicated="selectDashboard"
-    />
   </div>
 </template>

app/assets/javascripts/monitoring/components/dashboards_dropdown.vue

 <script>
-import { mapState, mapActions, mapGetters } from 'vuex';
+import { mapState, mapGetters } from 'vuex';
 import {
   GlIcon,
   GlDeprecatedDropdown,
@@ -31,10 +31,6 @@ export default {
       type: String,
       required: true,
     },
-    modalId: {
-      type: String,
-      required: true,
-    },
   },
   data() {
     return {
@@ -44,9 +40,6 @@ export default {
   computed: {
     ...mapState('monitoringDashboard', ['allDashboards']),
     ...mapGetters('monitoringDashboard', ['selectedDashboard']),
-    isOutOfTheBoxDashboard() {
-      return this.selectedDashboard?.out_of_the_box_dashboard;
-    },
     selectedDashboardText() {
       return this.selectedDashboard?.display_name;
     },
@@ -70,7 +63,6 @@ export default {
     },
   },
   methods: {
-    ...mapActions('monitoringDashboard', ['duplicateSystemDashboard']),
     dashboardDisplayName(dashboard) {
       return dashboard.display_name || dashboard.path || '';
     },
@@ -134,18 +126,6 @@ export default {
       >
         {{ __('No matching results') }}
       </div>
-
-      <!-- 
-           This Duplicate Dashboard item will be removed from the dashboards dropdown 
-           in https://gitlab.com/gitlab-org/gitlab/-/issues/223223
-      -->
-      <template v-if="isOutOfTheBoxDashboard">
-        <gl-deprecated-dropdown-divider />
-
-        <gl-deprecated-dropdown-item v-gl-modal="modalId" data-testid="duplicateDashboardItem">
-          {{ s__('Metrics|Duplicate dashboard') }}
-        </gl-deprecated-dropdown-item>
-      </template>
     </div>
   </gl-deprecated-dropdown>
 </template>

changelogs/unreleased/astoicescu-removeDuplicateDashboardItemFromDashboardsDropdown.yml

+---
+title: Remove Duplicate Dashboard item from dashboards dropdown
+merge_request: 38053
+author:
+type: changed

doc/operations/metrics/dashboards/index.md

@@ -83,7 +83,7 @@ The resulting `.yml` file can be customized and adapted to your project.
 You can decide to save the dashboard `.yml` file in the project's **default** branch or in a
 new branch.
 
-1. Click **Duplicate dashboard** in the dashboard dropdown or in the actions menu.
+1. Click **Duplicate dashboard** in the actions menu.
 
    NOTE: **Note:**
    You can duplicate only GitLab-defined dashboards.

spec/frontend/monitoring/components/__snapshots__/dashboard_template_spec.js.snap

@@ -20,7 +20,6 @@ exports[`Dashboard template matches the default snapshot 1`] = `
         data-qa-selector="dashboards_filter_dropdown"
         defaultbranch="master"
         id="monitor-dashboards-dropdown"
-        modalid="duplicateDashboard"
         toggle-class="dropdown-menu-toggle"
       />
     </div>
@@ -134,11 +133,6 @@ exports[`Dashboard template matches the default snapshot 1`] = `
        
       <!---->
     </div>
-     
-    <duplicate-dashboard-modal-stub
-      defaultbranch="master"
-      modalid="duplicateDashboard"
-    />
   </div>
    
   <empty-state-stub

spec/frontend/monitoring/components/dashboard_header_spec.js

@@ -208,14 +208,10 @@ describe('Dashboard header', () => {
   describe('when a dashboard has been duplicated in the duplicate dashboard modal', () => {
     beforeEach(() => {
       store.state.monitoringDashboard.projectPath = 'root/sandbox';
+
+      setupAllDashboards(store, dashboardGitResponse[0].path);
     });
-    /**
-     * The duplicate dashboard modal gets called both by a menu item from the
-     * dashboards dropdown and by an item from the actions menu.
-     *
-     * This spec is context agnostic, so it addresses all cases where the
-     * duplicate dashboard modal gets called.
-     */
+
     it('redirects to the newly created dashboard', () => {
       delete window.location;
       window.location = new URL('https://localhost');
@@ -252,7 +248,7 @@ describe('Dashboard header', () => {
       expect(findActionsMenu().exists()).toBe(false);
     });
 
-    it('contains a modal', () => {
+    it('contains the create dashboard modal', () => {
       store.state.monitoringDashboard.projectPath = mockProjectPath;
 
       return wrapper.vm.$nextTick().then(() => {
@@ -270,13 +266,14 @@ describe('Dashboard header', () => {
     describe.each(duplicableCases)(
       'when the selected dashboard can be duplicated',
       dashboardPath => {
-        it('contains a "Create New" menu item and a "Duplicate Dashboard" menu item', () => {
+        it('contains menu items for "Create New", "Duplicate Dashboard" and a modal for duplicating dashboards', () => {
           store.state.monitoringDashboard.projectPath = mockProjectPath;
           setupAllDashboards(store, dashboardPath);
 
           return wrapper.vm.$nextTick().then(() => {
             expect(findCreateDashboardMenuItem().exists()).toBe(true);
             expect(findCreateDashboardDuplicateItem().exists()).toBe(true);
+            expect(findDuplicateDashboardModal().exists()).toBe(true);
           });
         });
       },
@@ -290,13 +287,14 @@ describe('Dashboard header', () => {
     describe.each(nonDuplicableCases)(
       'when the selected dashboard cannot be duplicated',
       dashboardPath => {
-        it('contains a "Create New" menu item and no "Duplicate Dashboard" menu item', () => {
+        it('contains a "Create New" menu item, but no "Duplicate Dashboard" menu item and modal', () => {
           store.state.monitoringDashboard.projectPath = mockProjectPath;
           setupAllDashboards(store, dashboardPath);
 
           return wrapper.vm.$nextTick().then(() => {
             expect(findCreateDashboardMenuItem().exists()).toBe(true);
             expect(findCreateDashboardDuplicateItem().exists()).toBe(false);
+            expect(findDuplicateDashboardModal().exists()).toBe(false);
           });
         });
       },

spec/frontend/monitoring/components/dashboards_dropdown_spec.js

@@ -3,10 +3,9 @@ import { GlDeprecatedDropdownItem, GlIcon } from '@gitlab/ui';
 
 import DashboardsDropdown from '~/monitoring/components/dashboards_dropdown.vue';
 
-import { dashboardGitResponse, selfMonitoringDashboardGitResponse } from '../mock_data';
+import { dashboardGitResponse } from '../mock_data';
 
 const defaultBranch = 'master';
-const modalId = 'duplicateDashboardModalId';
 const starredDashboards = dashboardGitResponse.filter(({ starred }) => starred);
 const notStarredDashboards = dashboardGitResponse.filter(({ starred }) => !starred);
 
@@ -17,9 +16,6 @@ describe('DashboardsDropdown', () => {
 
   function createComponent(props, opts = {}) {
     const storeOpts = {
-      methods: {
-        duplicateSystemDashboard: jest.fn(),
-      },
       computed: {
         allDashboards: () => mockDashboards,
         selectedDashboard: () => mockSelectedDashboard,
@@ -30,7 +26,6 @@ describe('DashboardsDropdown', () => {
       propsData: {
         ...props,
         defaultBranch,
-        modalId,
       },
       sync: false,
       ...storeOpts,
@@ -150,87 +145,6 @@ describe('DashboardsDropdown', () => {
     });
   });
 
-  const duplicableCases = [
-    dashboardGitResponse[0],
-    dashboardGitResponse[2],
-    selfMonitoringDashboardGitResponse[0],
-  ];
-
-  describe.each(duplicableCases)('when the selected dashboard can be duplicated', dashboard => {
-    let duplicateDashboardAction;
-    let modalDirective;
-
-    beforeEach(() => {
-      mockSelectedDashboard = dashboard;
-      modalDirective = jest.fn();
-      duplicateDashboardAction = jest.fn().mockResolvedValue();
-
-      wrapper = createComponent(
-        {},
-        {
-          directives: {
-            GlModal: modalDirective,
-          },
-          methods: {
-            // Mock vuex actions
-            duplicateSystemDashboard: duplicateDashboardAction,
-          },
-        },
-      );
-    });
-
-    it('displays a dropdown item for each dashboard', () => {
-      expect(findItems().length).toEqual(dashboardGitResponse.length + 1);
-    });
-
-    it('displays one "duplicate dashboard" dropdown item with a directive attached', () => {
-      const item = wrapper.findAll('[data-testid="duplicateDashboardItem"]');
-
-      expect(item.length).toBe(1);
-    });
-
-    it('"duplicate dashboard" dropdown item directive works', () => {
-      const item = wrapper.find('[data-testid="duplicateDashboardItem"]');
-
-      item.trigger('click');
-
-      return wrapper.vm.$nextTick().then(() => {
-        expect(modalDirective).toHaveBeenCalled();
-      });
-    });
-
-    it('id is correct, as the value of modal directive binding matches modal id', () => {
-      expect(modalDirective).toHaveBeenCalledTimes(1);
-
-      // Binding's second argument contains the modal id
-      expect(modalDirective.mock.calls[0][1]).toEqual(
-        expect.objectContaining({
-          value: modalId,
-        }),
-      );
-    });
-  });
-
-  const nonDuplicableCases = [dashboardGitResponse[1], selfMonitoringDashboardGitResponse[1]];
-
-  describe.each(nonDuplicableCases)(
-    'when the selected dashboard can not be duplicated',
-    dashboard => {
-      beforeEach(() => {
-        mockSelectedDashboard = dashboard;
-
-        wrapper = createComponent();
-      });
-
-      it('displays a dropdown list item for each dashboard, but no list item for "duplicate dashboard"', () => {
-        const item = wrapper.findAll('[data-testid="duplicateDashboardItem"]');
-
-        expect(findItems()).toHaveLength(dashboardGitResponse.length);
-        expect(item.length).toBe(0);
-      });
-    },
-  );
-
   describe('when a dashboard gets selected by the user', () => {
     beforeEach(() => {
       wrapper = createComponent();