Toggle file tree bolding based on file "Viewed" status

Before, when scrolling to a file in the MR Review OR when
clicking the file title in the file tree view, the file would
be marked as "viewed" (and thus unbolded).

Now, the only action that can signify that a file is viewed
(and unbold the entry in the tree) is clicking the "Viewed"
checkbox on the file header in the Diffs view.

Changelog: changed

app/assets/javascripts/diffs/store/actions.js

@@ -85,6 +85,12 @@ export const setBaseConfig = ({ commit }, options) => {
     viewDiffsFileByFile,
     mrReviews,
   });
+
+  Array.from(new Set(Object.values(mrReviews).flat())).forEach((id) => {
+    const viewedId = id.replace(/^hash:/, '');
+
+    commit(types.SET_DIFF_FILE_VIEWED, { id: viewedId, seen: true });
+  });
 };
 
 export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
@@ -850,6 +856,8 @@ export function reviewFile({ commit, state }, { file, reviewed = true }) {
   const reviews = markFileReview(state.mrReviews, file, reviewed);
 
   setReviewsForMergeRequest(mrPath, reviews);
+
+  commit(types.SET_DIFF_FILE_VIEWED, { id: file.file_hash, seen: reviewed });
   commit(types.SET_MR_FILE_REVIEWS, reviews);
 }
 

app/assets/javascripts/diffs/store/mutations.js

@@ -256,7 +256,6 @@ export default {
   },
   [types.VIEW_DIFF_FILE](state, fileId) {
     state.currentDiffFileId = fileId;
-    Vue.set(state.viewedDiffFileIds, fileId, true);
   },
   [types.SET_DIFF_FILE_VIEWED](state, { id, seen }) {
     Vue.set(state.viewedDiffFileIds, id, seen);

app/assets/javascripts/diffs/utils/file_reviews.js

@@ -52,8 +52,10 @@ export function markFileReview(reviews, file, reviewed = true) {
 
     if (reviewed) {
       fileReviews.add(file.id);
+      fileReviews.add(`hash:${file.file_hash}`);
     } else {
       fileReviews.delete(file.id);
+      fileReviews.delete(`hash:${file.file_hash}`);
     }
 
     updatedReviews[file.file_identifier_hash] = Array.from(fileReviews);

spec/frontend/diffs/components/diff_file_header_spec.js

@@ -7,7 +7,7 @@ import { mockTracking, triggerEvent } from 'helpers/tracking_helper';
 import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
 import { DIFF_FILE_AUTOMATIC_COLLAPSE, DIFF_FILE_MANUAL_COLLAPSE } from '~/diffs/constants';
 import { reviewFile } from '~/diffs/store/actions';
-import { SET_MR_FILE_REVIEWS } from '~/diffs/store/mutation_types';
+import { SET_DIFF_FILE_VIEWED, SET_MR_FILE_REVIEWS } from '~/diffs/store/mutation_types';
 import { diffViewerModes } from '~/ide/constants';
 import { scrollToElement } from '~/lib/utils/common_utils';
 import { truncateSha } from '~/lib/utils/text_utility';
@@ -23,6 +23,7 @@ jest.mock('~/lib/utils/common_utils');
 const diffFile = Object.freeze(
   Object.assign(diffDiscussionsMockData.diff_file, {
     id: '123',
+    file_hash: 'xyz',
     file_identifier_hash: 'abc',
     edit_path: 'link:/to/edit/path',
     blob: {
@@ -553,7 +554,13 @@ describe('DiffFileHeader component', () => {
         reviewFile,
         { file, reviewed: true },
         {},
-        [{ type: SET_MR_FILE_REVIEWS, payload: { [file.file_identifier_hash]: [file.id] } }],
+        [
+          { type: SET_DIFF_FILE_VIEWED, payload: { id: file.file_hash, seen: true } },
+          {
+            type: SET_MR_FILE_REVIEWS,
+            payload: { [file.file_identifier_hash]: [file.id, `hash:${file.file_hash}`] },
+          },
+        ],
         [],
       );
     });

spec/frontend/diffs/store/actions_spec.js

@@ -99,6 +99,10 @@ describe('DiffsStoreActions', () => {
       const projectPath = '/root/project';
       const dismissEndpoint = '/-/user_callouts';
       const showSuggestPopover = false;
+      const mrReviews = {
+        a: ['z', 'hash:a'],
+        b: ['y', 'hash:a'],
+      };
 
       testAction(
         setBaseConfig,
@@ -110,6 +114,7 @@ describe('DiffsStoreActions', () => {
           projectPath,
           dismissEndpoint,
           showSuggestPopover,
+          mrReviews,
         },
         {
           endpoint: '',
@@ -131,8 +136,21 @@ describe('DiffsStoreActions', () => {
               projectPath,
               dismissEndpoint,
               showSuggestPopover,
+              mrReviews,
             },
           },
+          {
+            type: types.SET_DIFF_FILE_VIEWED,
+            payload: { id: 'z', seen: true },
+          },
+          {
+            type: types.SET_DIFF_FILE_VIEWED,
+            payload: { id: 'a', seen: true },
+          },
+          {
+            type: types.SET_DIFF_FILE_VIEWED,
+            payload: { id: 'y', seen: true },
+          },
         ],
         [],
         done,
@@ -1523,13 +1541,14 @@ describe('DiffsStoreActions', () => {
   describe('reviewFile', () => {
     const file = {
       id: '123',
+      file_hash: 'xyz',
       file_identifier_hash: 'abc',
       load_collapsed_diff_url: 'gitlab-org/gitlab-test/-/merge_requests/1/diffs',
     };
     it.each`
-      reviews             | diffFile | reviewed
-      ${{ abc: ['123'] }} | ${file}  | ${true}
-      ${{}}               | ${file}  | ${false}
+      reviews                         | diffFile | reviewed
+      ${{ abc: ['123', 'hash:xyz'] }} | ${file}  | ${true}
+      ${{}}                           | ${file}  | ${false}
     `(
       'sets reviews ($reviews) to localStorage and state for file $file if it is marked reviewed=$reviewed',
       ({ reviews, diffFile, reviewed }) => {

spec/frontend/diffs/utils/file_reviews_spec.js

@@ -11,14 +11,14 @@ import {
 
 function getDefaultReviews() {
   return {
-    abc: ['123', '098'],
+    abc: ['123', 'hash:xyz', '098', 'hash:uvw'],
   };
 }
 
 describe('File Review(s) utilities', () => {
   const mrPath = 'my/fake/mr/42';
   const storageKey = `${mrPath}-file-reviews`;
-  const file = { id: '123', file_identifier_hash: 'abc' };
+  const file = { id: '123', file_hash: 'xyz', file_identifier_hash: 'abc' };
   const storedValue = JSON.stringify(getDefaultReviews());
   let reviews;
 
@@ -44,14 +44,14 @@ describe('File Review(s) utilities', () => {
   });
 
   describe('reviewStatuses', () => {
-    const file1 = { id: '123', file_identifier_hash: 'abc' };
-    const file2 = { id: '098', file_identifier_hash: 'abc' };
+    const file1 = { id: '123', hash: 'xyz', file_identifier_hash: 'abc' };
+    const file2 = { id: '098', hash: 'uvw', file_identifier_hash: 'abc' };
 
     it.each`
       mrReviews                         | files             | fileReviews
       ${{}}                             | ${[file1, file2]} | ${{ 123: false, '098': false }}
-      ${{ abc: ['123'] }}               | ${[file1, file2]} | ${{ 123: true, '098': false }}
-      ${{ abc: ['098'] }}               | ${[file1, file2]} | ${{ 123: false, '098': true }}
+      ${{ abc: ['123', 'hash:xyz'] }}   | ${[file1, file2]} | ${{ 123: true, '098': false }}
+      ${{ abc: ['098', 'hash:uvw'] }}   | ${[file1, file2]} | ${{ 123: false, '098': true }}
       ${{ def: ['123'] }}               | ${[file1, file2]} | ${{ 123: false, '098': false }}
       ${{ abc: ['123'], def: ['098'] }} | ${[]}             | ${{}}
     `(
@@ -128,7 +128,7 @@ describe('File Review(s) utilities', () => {
 
   describe('markFileReview', () => {
     it("adds a review when there's nothing that already exists", () => {
-      expect(markFileReview(null, file)).toStrictEqual({ abc: ['123'] });
+      expect(markFileReview(null, file)).toStrictEqual({ abc: ['123', 'hash:xyz'] });
     });
 
     it("overwrites an existing review if it's for the same file (identifier hash)", () => {
@@ -136,15 +136,15 @@ describe('File Review(s) utilities', () => {
     });
 
     it('removes a review from the list when `reviewed` is `false`', () => {
-      expect(markFileReview(reviews, file, false)).toStrictEqual({ abc: ['098'] });
+      expect(markFileReview(reviews, file, false)).toStrictEqual({ abc: ['098', 'hash:uvw'] });
     });
 
     it('adds a new review if the file ID is new', () => {
-      const updatedFile = { ...file, id: '098' };
-      const allReviews = markFileReview({ abc: ['123'] }, updatedFile);
+      const updatedFile = { ...file, id: '098', file_hash: 'uvw' };
+      const allReviews = markFileReview({ abc: ['123', 'hash:xyz'] }, updatedFile);
 
       expect(allReviews).toStrictEqual(getDefaultReviews());
-      expect(allReviews.abc).toStrictEqual(['123', '098']);
+      expect(allReviews.abc).toStrictEqual(['123', 'hash:xyz', '098', 'hash:uvw']);
     });
 
     it.each`
@@ -158,7 +158,7 @@ describe('File Review(s) utilities', () => {
     it('removes the file key if there are no more reviews for it', () => {
       let updated = markFileReview(reviews, file, false);
 
-      updated = markFileReview(updated, { ...file, id: '098' }, false);
+      updated = markFileReview(updated, { ...file, id: '098', file_hash: 'uvw' }, false);
 
       expect(updated).toStrictEqual({});
     });