Merge branch '13226-version-dropdown-goes-wrong-if-versions-are-not-monotonic' into 'master'

Resolve "Version dropdown goes wrong if versions are not monotonic"

See merge request gitlab-org/gitlab!20515

ee/app/assets/javascripts/design_management/components/upload/design_version_dropdown.vue

@@ -2,6 +2,7 @@
 import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
 import { __, sprintf } from '~/locale';
 import allVersionsMixin from '../../mixins/all_versions';
+import { findVersionId } from '../../utils/design_management_utils';
 
 export default {
   components: {
@@ -10,35 +11,40 @@ export default {
   },
   mixins: [allVersionsMixin],
   computed: {
+    queryVersion() {
+      return this.$route.query.version;
+    },
+    currentVersionIdx() {
+      if (!this.queryVersion) return 0;
+
+      const idx = this.allVersions.findIndex(
+        version => this.findVersionId(version.node.id) === this.queryVersion,
+      );
+
+      // if the currentVersionId isn't a valid version (i.e. not in allVersions)
+      // then return the latest version (index 0)
+      return idx !== -1 ? idx : 0;
+    },
+    currentVersionId() {
+      if (this.queryVersion) return this.queryVersion;
+
+      const currentVersion = this.allVersions[this.currentVersionIdx];
+      return this.findVersionId(currentVersion.node.id);
+    },
     dropdownText() {
-      if (
-        !this.$route.query.version ||
-        Number(this.$route.query.version) === this.allVersions.length
-      ) {
+      if (this.isLatestVersion) {
         return __('Showing Latest Version');
       }
-      const versionNumber = this.getCurrentVersionNumber();
+      // allVersions is sorted in reverse chronological order (latest first)
+      const currentVersionNumber = this.allVersions.length - this.currentVersionIdx;
+
       return sprintf(__('Showing Version #%{versionNumber}'), {
-        versionNumber,
+        versionNumber: currentVersionNumber,
       });
     },
-    currentVersion() {
-      return this.$route.query.version || this.getLatestVersionId();
-    },
   },
   methods: {
-    getVersionId(versionId) {
-      return versionId.match('::Version\/(.+$)')[1]; // eslint-disable-line no-useless-escape
-    },
-    getLatestVersionId() {
-      return this.getVersionId(this.allVersions[0].node.id);
-    },
-    getCurrentVersionNumber() {
-      const versionIndex = this.allVersions.findIndex(
-        version => this.getVersionId(version.node.id) === this.$route.query.version,
-      );
-      return this.allVersions.length - versionIndex;
-    },
+    findVersionId,
   },
 };
 </script>
@@ -48,20 +54,20 @@ export default {
     <gl-dropdown-item v-for="(version, index) in allVersions" :key="version.node.id">
       <router-link
         class="d-flex js-version-link"
-        :to="{ path: $route.path, query: { version: getVersionId(version.node.id) } }"
+        :to="{ path: $route.path, query: { version: findVersionId(version.node.id) } }"
       >
         <div class="flex-grow-1 ml-2">
           <div>
             <strong
               >{{ __('Version') }} {{ allVersions.length - index }}
-              <span v-if="getVersionId(version.node.id) === getLatestVersionId()"
+              <span v-if="findVersionId(version.node.id) === latestVersionId"
                 >({{ __('latest') }})</span
               >
             </strong>
           </div>
         </div>
         <i
-          v-if="getVersionId(version.node.id) === currentVersion"
+          v-if="findVersionId(version.node.id) === currentVersionId"
           class="fa fa-check pull-right"
         ></i>
       </router-link>

ee/app/assets/javascripts/design_management/mixins/all_versions.js

@@ -37,10 +37,17 @@ export default {
         ? `gid://gitlab/DesignManagement::Version/${this.$route.query.version}`
         : null;
     },
+    latestVersionId() {
+      const latestVersion = this.allVersions[0];
+      return latestVersion && findVersionId(latestVersion.node.id);
+    },
     isLatestVersion() {
       if (this.allVersions.length > 0) {
-        const versionId = findVersionId(this.allVersions[0].node.id);
-        return !this.$route.query.version || this.$route.query.version === versionId;
+        return (
+          !this.$route.query.version ||
+          !this.latestVersionId ||
+          this.$route.query.version === this.latestVersionId
+        );
       }
       return true;
     },

ee/app/assets/javascripts/design_management/utils/design_management_utils.js

@@ -30,6 +30,6 @@ export const extractDiscussions = discussions =>
 export const extractCurrentDiscussion = (discussions, id) =>
   discussions.edges.find(({ node }) => node.id === id);
 
-export const findVersionId = id => id.match('::Version/(.+$)')[1];
+export const findVersionId = id => (id.match('::Version/(.+$)') || [])[1];
 
 export const extractDesign = data => data.project.issue.designCollection.designs.edges[0].node;

ee/changelogs/unreleased/13226-version-dropdown-goes-wrong-if-versions-are-not-monotonic.yml

+---
+title: Resolve Version dropdown goes wrong if versions are not monotonic
+merge_request: 20515
+author: Tom Quirk
+type: fixed

ee/spec/frontend/design_management/components/upload/__snapshots__/design_version_dropdown_spec.js.snap

@@ -9,10 +9,8 @@ exports[`Design management design version dropdown component renders design vers
   variant="link"
 >
   <gldropdownitem-stub>
-    <routerlink-stub
+    <router-link-stub
       class="d-flex js-version-link"
-      event="click"
-      tag="a"
       to="[object Object]"
     >
       <div
@@ -32,13 +30,11 @@ exports[`Design management design version dropdown component renders design vers
       <i
         class="fa fa-check pull-right"
       />
-    </routerlink-stub>
+    </router-link-stub>
   </gldropdownitem-stub>
   <gldropdownitem-stub>
-    <routerlink-stub
+    <router-link-stub
       class="d-flex js-version-link"
-      event="click"
-      tag="a"
       to="[object Object]"
     >
       <div
@@ -54,7 +50,7 @@ exports[`Design management design version dropdown component renders design vers
       </div>
        
       <!---->
-    </routerlink-stub>
+    </router-link-stub>
   </gldropdownitem-stub>
 </gldropdown-stub>
 `;
@@ -68,10 +64,8 @@ exports[`Design management design version dropdown component renders design vers
   variant="link"
 >
   <gldropdownitem-stub>
-    <routerlink-stub
+    <router-link-stub
       class="d-flex js-version-link"
-      event="click"
-      tag="a"
       to="[object Object]"
     >
       <div
@@ -91,13 +85,11 @@ exports[`Design management design version dropdown component renders design vers
       <i
         class="fa fa-check pull-right"
       />
-    </routerlink-stub>
+    </router-link-stub>
   </gldropdownitem-stub>
   <gldropdownitem-stub>
-    <routerlink-stub
+    <router-link-stub
       class="d-flex js-version-link"
-      event="click"
-      tag="a"
       to="[object Object]"
     >
       <div
@@ -113,7 +105,7 @@ exports[`Design management design version dropdown component renders design vers
       </div>
        
       <!---->
-    </routerlink-stub>
+    </router-link-stub>
   </gldropdownitem-stub>
 </gldropdown-stub>
 `;

ee/spec/frontend/design_management/components/upload/design_version_dropdown_spec.js

 import { createLocalVue, shallowMount } from '@vue/test-utils';
-import VueRouter from 'vue-router';
 import DesignVersionDropdown from 'ee/design_management/components/upload/design_version_dropdown.vue';
 import { GlDropdown, GlDropdownItem } from '@gitlab/ui';
 import mockAllVersions from './mock_data/all_versions';
 
-const VERSION_ID = 3;
+const LATEST_VERSION_ID = 3;
+const PREVIOUS_VERSION_ID = 2;
+
 const localVue = createLocalVue();
-localVue.use(VueRouter);
-const router = new VueRouter();
+
+const designRouteFactory = versionId => ({
+  path: `/designs?version=${versionId}`,
+  query: {
+    version: `${versionId}`,
+  },
+});
+
+const MOCK_ROUTE = {
+  path: '/designs',
+  query: {},
+};
 
 describe('Design management design version dropdown component', () => {
   let wrapper;
 
-  function createComponent() {
+  function createComponent({ maxVersions = -1, $route = MOCK_ROUTE } = {}) {
     wrapper = shallowMount(DesignVersionDropdown, {
       propsData: {
         projectPath: '',
         issueIid: '',
       },
       localVue,
-      router,
+      mocks: {
+        $route,
+      },
+      stubs: ['router-link'],
     });
 
     wrapper.setData({
-      allVersions: mockAllVersions,
+      allVersions: maxVersions > -1 ? mockAllVersions.slice(0, maxVersions) : mockAllVersions,
     });
   }
 
@@ -54,14 +68,26 @@ describe('Design management design version dropdown component', () => {
   });
 
   describe('versions list', () => {
-    it('pushes version id when a version is clicked', () => {
+    it('displays latest version text by default', () => {
       createComponent();
-      wrapper.vm.$router.push(`/designs?version=${VERSION_ID}`);
-      const CurrentVersionNumber = wrapper.vm.getCurrentVersionNumber();
 
-      expect(wrapper.find(GlDropdown).attributes('text')).toBe(
-        `Showing Version #${CurrentVersionNumber}`,
-      );
+      expect(wrapper.find(GlDropdown).attributes('text')).toBe('Showing Latest Version');
+    });
+
+    it('displays latest version text when only 1 version is present', () => {
+      createComponent({ maxVersions: 1 });
+
+      expect(wrapper.find(GlDropdown).attributes('text')).toBe('Showing Latest Version');
+    });
+
+    it('displays version text when the current version is not the latest', () => {
+      createComponent({ $route: designRouteFactory(PREVIOUS_VERSION_ID) });
+      expect(wrapper.find(GlDropdown).attributes('text')).toBe(`Showing Version #1`);
+    });
+
+    it('displays latest version text when the current version is the latest', () => {
+      createComponent({ $route: designRouteFactory(LATEST_VERSION_ID) });
+      expect(wrapper.find(GlDropdown).attributes('text')).toBe('Showing Latest Version');
     });
 
     it('should have the same length as apollo query', () => {

ee/spec/frontend/design_management/utils/design_management_utils_spec.js

 import {
   extractCurrentDiscussion,
   extractDiscussions,
+  findVersionId,
 } from 'ee/design_management/utils/design_management_utils';
 
 describe('extractCurrentDiscussion', () => {
@@ -52,3 +53,18 @@ describe('extractDiscussions', () => {
     ]);
   });
 });
+
+describe('version parser', () => {
+  it('correctly extracts version ID from a valid version string', () => {
+    const testVersionId = '123';
+    const testVersionString = `gid://gitlab/DesignManagement::Version/${testVersionId}`;
+
+    expect(findVersionId(testVersionString)).toEqual(testVersionId);
+  });
+
+  it('fails to extract version ID from an invalid version string', () => {
+    const testInvalidVersionString = `gid://gitlab/DesignManagement::Version`;
+
+    expect(findVersionId(testInvalidVersionString)).toBeUndefined();
+  });
+});