Merge branch 'ps-fix-frequent-items-last-accessed-on' into 'master'

Fix frequent items timestamps not updated

See merge request gitlab-org/gitlab!64737

app/assets/javascripts/frequent_items/utils.js

@@ -35,13 +35,15 @@ export const getTopFrequentItems = (items) => {
 };
 
 export const updateExistingFrequentItem = (frequentItem, item) => {
-  const accessedOverHourAgo =
-    Math.abs(item.lastAccessedOn - frequentItem.lastAccessedOn) / HOUR_IN_MS > 1;
+  // `frequentItem` comes from localStorage and it's possible it doesn't have a `lastAccessedOn`
+  const neverAccessed = !frequentItem.lastAccessedOn;
+  const shouldUpdate =
+    neverAccessed || Math.abs(item.lastAccessedOn - frequentItem.lastAccessedOn) / HOUR_IN_MS > 1;
 
   return {
     ...item,
-    frequency: accessedOverHourAgo ? frequentItem.frequency + 1 : frequentItem.frequency,
-    lastAccessedOn: accessedOverHourAgo ? Date.now() : frequentItem.lastAccessedOn,
+    frequency: shouldUpdate ? frequentItem.frequency + 1 : frequentItem.frequency,
+    lastAccessedOn: shouldUpdate ? Date.now() : frequentItem.lastAccessedOn,
   };
 };
 

app/assets/javascripts/nav/components/top_nav_container_view.vue

@@ -20,6 +20,10 @@ export default {
       type: String,
       required: true,
     },
+    currentItem: {
+      type: Object,
+      required: true,
+    },
     containerClass: {
       type: String,
       required: false,
@@ -43,6 +47,12 @@ export default {
         { id: 'secondary', menuItems: this.linksSecondary },
       ].filter((x) => x.menuItems?.length);
     },
+    currentItemTimestamped() {
+      return {
+        ...this.currentItem,
+        lastAccessedOn: Date.now(),
+      };
+    },
   },
   mounted() {
     // For historic reasons, the frequent-items-app component requires this too start up.
@@ -62,7 +72,7 @@ export default {
     >
       <div class="frequent-items-dropdown-content gl-w-full! gl-pt-0!">
         <vuex-module-provider :vuex-module="frequentItemsVuexModule">
-          <frequent-items-app v-bind="$attrs" />
+          <frequent-items-app :current-item="currentItemTimestamped" v-bind="$attrs" />
         </vuex-module-provider>
       </div>
     </div>

spec/frontend/frequent_items/utils_spec.js

@@ -66,35 +66,36 @@ describe('Frequent Items utils spec', () => {
   });
 
   describe('updateExistingFrequentItem', () => {
-    let mockedProject;
-
-    beforeEach(() => {
-      mockedProject = {
-        ...mockProject,
-        frequency: 1,
-        lastAccessedOn: 1497979281815,
-      };
+    const LAST_ACCESSED = 1497979281815;
+    const WITHIN_AN_HOUR = LAST_ACCESSED + HOUR_IN_MS;
+    const OVER_AN_HOUR = WITHIN_AN_HOUR + 1;
+    const EXISTING_ITEM = Object.freeze({
+      ...mockProject,
+      frequency: 1,
+      lastAccessedOn: 1497979281815,
     });
 
-    it('updates item if accessed over an hour ago', () => {
-      const newTimestamp = Date.now() + HOUR_IN_MS + 1;
+    it.each`
+      desc                                           | existingProps                    | newProps                              | expected
+      ${'updates item if accessed over an hour ago'} | ${{}}                            | ${{ lastAccessedOn: OVER_AN_HOUR }}   | ${{ lastAccessedOn: Date.now(), frequency: 2 }}
+      ${'does not update is accessed with an hour'}  | ${{}}                            | ${{ lastAccessedOn: WITHIN_AN_HOUR }} | ${{ lastAccessedOn: EXISTING_ITEM.lastAccessedOn, frequency: 1 }}
+      ${'updates if lastAccessedOn not found'}       | ${{ lastAccessedOn: undefined }} | ${{ lastAccessedOn: WITHIN_AN_HOUR }} | ${{ lastAccessedOn: Date.now(), frequency: 2 }}
+    `('$desc', ({ existingProps, newProps, expected }) => {
       const newItem = {
-        ...mockedProject,
-        lastAccessedOn: newTimestamp,
+        ...EXISTING_ITEM,
+        ...newProps,
       };
-      const result = updateExistingFrequentItem(mockedProject, newItem);
-
-      expect(result.frequency).toBe(mockedProject.frequency + 1);
-    });
-
-    it('does not update item if accessed within the hour', () => {
-      const newItem = {
-        ...mockedProject,
-        lastAccessedOn: mockedProject.lastAccessedOn + HOUR_IN_MS,
+      const existingItem = {
+        ...EXISTING_ITEM,
+        ...existingProps,
       };
-      const result = updateExistingFrequentItem(mockedProject, newItem);
 
-      expect(result.frequency).toBe(mockedProject.frequency);
+      const result = updateExistingFrequentItem(existingItem, newItem);
+
+      expect(result).toEqual({
+        ...newItem,
+        ...expected,
+      });
     });
   });
 

spec/frontend/nav/components/responsive_app_spec.js

@@ -111,6 +111,7 @@ describe('~/nav/components/responsive_app.vue', () => {
     containerClass: 'gl-px-3',
     frequentItemsDropdownType: ResponsiveApp.FREQUENT_ITEMS_PROJECTS.namespace,
     frequentItemsVuexModule: ResponsiveApp.FREQUENT_ITEMS_PROJECTS.vuexModule,
+    currentItem: {},
     linksPrimary: TEST_NAV_DATA.views.projects.linksPrimary,
     linksSecondary: TEST_NAV_DATA.views.projects.linksSecondary,
   };
@@ -118,6 +119,7 @@ describe('~/nav/components/responsive_app.vue', () => {
     containerClass: 'gl-px-3',
     frequentItemsDropdownType: ResponsiveApp.FREQUENT_ITEMS_GROUPS.namespace,
     frequentItemsVuexModule: ResponsiveApp.FREQUENT_ITEMS_GROUPS.vuexModule,
+    currentItem: {},
     linksPrimary: TEST_NAV_DATA.views.groups.linksPrimary,
     linksSecondary: TEST_NAV_DATA.views.groups.linksSecondary,
   };

spec/frontend/nav/components/top_nav_container_view_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { merge } from 'lodash';
 import { nextTick } from 'vue';
 import FrequentItemsApp from '~/frequent_items/components/app.vue';
 import { FREQUENT_ITEMS_PROJECTS } from '~/frequent_items/constants';
@@ -82,7 +83,9 @@ describe('~/nav/components/top_nav_container_view.vue', () => {
     it('renders frequent items app', () => {
       expect(findFrequentItemsApp()).toEqual({
         vuexModule: DEFAULT_PROPS.frequentItemsVuexModule,
-        props: expect.objectContaining(TEST_OTHER_PROPS),
+        props: expect.objectContaining(
+          merge({ currentItem: { lastAccessedOn: Date.now() } }, TEST_OTHER_PROPS),
+        ),
         attributes: expect.objectContaining(EXTRA_ATTRS),
       });
     });