Added option to view diffs file-by-file

This adds a user preference to view diffs file-by-file instead
of viewing all the diff files in one big list.
This helps with performance on large merge request,
but it can also be handy sometimes to view merge requests
file-by-file instead of in a full list of files.

The option to enable this feature is inside of user prefences.
For now this is also behind a feature flag so we can roll
this out safely.

https://gitlab.com/gitlab-org/gitlab/-/issues/222790

app/assets/javascripts/diffs/components/app.vue

 <script>
 import { mapState, mapGetters, mapActions } from 'vuex';
-import { GlLoadingIcon } from '@gitlab/ui';
+import { GlLoadingIcon, GlButtonGroup, GlButton } from '@gitlab/ui';
 import Mousetrap from 'mousetrap';
 import { __ } from '~/locale';
 import createFlash from '~/flash';
@@ -36,6 +36,8 @@ export default {
     TreeList,
     GlLoadingIcon,
     PanelResizer,
+    GlButtonGroup,
+    GlButton,
   },
   mixins: [glFeatureFlagsMixin()],
   props: {
@@ -94,6 +96,11 @@ export default {
       required: false,
       default: false,
     },
+    viewDiffsFileByFile: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     const treeWidth =
@@ -120,9 +127,18 @@ export default {
       emailPatchPath: state => state.diffs.emailPatchPath,
       retrievingBatches: state => state.diffs.retrievingBatches,
     }),
-    ...mapState('diffs', ['showTreeList', 'isLoading', 'startVersion']),
+    ...mapState('diffs', ['showTreeList', 'isLoading', 'startVersion', 'currentDiffFileId']),
     ...mapGetters('diffs', ['isParallelView', 'currentDiffIndex']),
     ...mapGetters(['isNotesFetched', 'getNoteableData']),
+    diffs() {
+      if (!this.viewDiffsFileByFile) {
+        return this.diffFiles;
+      }
+
+      return this.diffFiles.filter((file, i) => {
+        return file.file_hash === this.currentDiffFileId || (i === 0 && !this.currentDiffFileId);
+      });
+    },
     canCurrentUserFork() {
       return this.currentUser.can_fork === true && this.currentUser.can_create_merge_request;
     },
@@ -183,16 +199,22 @@ export default {
       dismissEndpoint: this.dismissEndpoint,
       showSuggestPopover: this.showSuggestPopover,
       useSingleDiffStyle: this.glFeatures.singleMrDiffView,
+      viewDiffsFileByFile: this.viewDiffsFileByFile,
     });
 
     if (this.shouldShow) {
       this.fetchData();
     }
 
-    const id = window && window.location && window.location.hash;
+    const id = window?.location?.hash;
 
-    if (id) {
-      this.setHighlightedRow(id.slice(1));
+    if (id && id.indexOf('#note') !== 0) {
+      this.setHighlightedRow(
+        id
+          .split('diff-content')
+          .pop()
+          .slice(1),
+      );
     }
   },
   created() {
@@ -236,6 +258,7 @@ export default {
       'cacheTreeListWidth',
       'scrollToFile',
       'toggleShowTreeList',
+      'navigateToDiffFileIndex',
     ]),
     refetchDiffData() {
       this.fetchData(false);
@@ -422,12 +445,31 @@ export default {
           <div v-if="isBatchLoading" class="loading"><gl-loading-icon size="lg" /></div>
           <template v-else-if="renderDiffFiles">
             <diff-file
-              v-for="file in diffFiles"
+              v-for="file in diffs"
               :key="file.newPath"
               :file="file"
               :help-page-path="helpPagePath"
               :can-current-user-fork="canCurrentUserFork"
+              :view-diffs-file-by-file="viewDiffsFileByFile"
             />
+            <div v-if="viewDiffsFileByFile" class="d-flex gl-justify-content-center">
+              <gl-button-group>
+                <gl-button
+                  :disabled="currentDiffIndex === 0"
+                  data-testid="singleFilePrevious"
+                  @click="navigateToDiffFileIndex(currentDiffIndex - 1)"
+                >
+                  {{ __('Prev') }}
+                </gl-button>
+                <gl-button
+                  :disabled="currentDiffIndex === diffFiles.length - 1"
+                  data-testid="singleFileNext"
+                  @click="navigateToDiffFileIndex(currentDiffIndex + 1)"
+                >
+                  {{ __('Next') }}
+                </gl-button>
+              </gl-button-group>
+            </div>
           </template>
           <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" />
         </div>

app/assets/javascripts/diffs/components/diff_file.vue

@@ -30,6 +30,10 @@ export default {
       required: false,
       default: '',
     },
+    viewDiffsFileByFile: {
+      type: Boolean,
+      required: true,
+    },
   },
   data() {
     return {
@@ -154,6 +158,7 @@ export default {
       :collapsible="true"
       :expanded="!isCollapsed"
       :add-merge-request-buttons="true"
+      :view-diffs-file-by-file="viewDiffsFileByFile"
       class="js-file-title file-title"
       @toggleFile="handleToggle"
       @showForkMessage="showForkMessage"

app/assets/javascripts/diffs/components/diff_file_header.vue

@@ -54,6 +54,11 @@ export default {
       type: Boolean,
       required: true,
     },
+    viewDiffsFileByFile: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   computed: {
     ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']),
@@ -166,7 +171,13 @@ export default {
         class="diff-toggle-caret gl-mr-2"
         @click.stop="handleToggleFile"
       />
-      <a v-once ref="titleWrapper" class="gl-mr-2" :href="titleLink" @click="handleFileNameClick">
+      <a
+        ref="titleWrapper"
+        :v-once="!viewDiffsFileByFile"
+        class="gl-mr-2"
+        :href="titleLink"
+        @click="handleFileNameClick"
+      >
         <file-icon :file-name="filePath" :size="18" aria-hidden="true" css-classes="gl-mr-2" />
         <span v-if="isFileRenamed">
           <strong

app/assets/javascripts/diffs/components/diff_file_row.vue

@@ -23,6 +23,11 @@ export default {
       type: Boolean,
       required: true,
     },
+    currentDiffFileId: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   computed: {
     showFileRowStats() {
@@ -33,7 +38,13 @@ export default {
 </script>
 
 <template>
-  <file-row :file="file" v-bind="$attrs" v-on="$listeners">
+  <file-row
+    :file="file"
+    v-bind="$attrs"
+    :class="{ 'is-active': currentDiffFileId === file.fileHash }"
+    class="diff-file-row"
+    v-on="$listeners"
+  >
     <file-row-stats v-if="showFileRowStats" :file="file" class="mr-1" />
     <changed-file-icon :file="file" :size="16" :show-tooltip="true" />
   </file-row>

app/assets/javascripts/diffs/components/tree_list.vue

@@ -26,7 +26,7 @@ export default {
     };
   },
   computed: {
-    ...mapState('diffs', ['tree', 'renderTreeList']),
+    ...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId']),
     ...mapGetters('diffs', ['allBlobs']),
     filteredTreeList() {
       const search = this.search.toLowerCase().trim();
@@ -96,6 +96,7 @@ export default {
           :level="0"
           :hide-file-stats="hideFileStats"
           :file-row-component="$options.DiffFileRow"
+          :current-diff-file-id="currentDiffFileId"
           @toggleTreeOpen="toggleTreeOpen"
           @clickFile="scrollToFile"
         />

app/assets/javascripts/diffs/index.js

@@ -78,6 +78,7 @@ export default function initDiffsApp(store) {
         dismissEndpoint: dataset.dismissEndpoint,
         showSuggestPopover: parseBoolean(dataset.showSuggestPopover),
         showWhitespaceDefault: parseBoolean(dataset.showWhitespaceDefault),
+        viewDiffsFileByFile: parseBoolean(dataset.fileByFileDefault),
       };
     },
     computed: {
@@ -115,6 +116,7 @@ export default function initDiffsApp(store) {
           isFluidLayout: this.isFluidLayout,
           dismissEndpoint: this.dismissEndpoint,
           showSuggestPopover: this.showSuggestPopover,
+          viewDiffsFileByFile: this.viewDiffsFileByFile,
         },
       });
     },

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

@@ -105,7 +105,9 @@ export const fetchDiffFiles = ({ state, commit }) => {
     .catch(() => worker.terminate());
 };
 
-export const fetchDiffFilesBatch = ({ commit, state }) => {
+export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
+  const id = window?.location?.hash;
+  const isNoteLink = id.indexOf('#note') === 0;
   const urlParams = {
     per_page: DIFFS_PER_PAGE,
     w: state.showWhitespace ? '0' : '1',
@@ -125,8 +127,26 @@ export const fetchDiffFilesBatch = ({ commit, state }) => {
         commit(types.SET_DIFF_DATA_BATCH, { diff_files });
         commit(types.SET_BATCH_LOADING, false);
 
+        if (!isNoteLink && !state.currentDiffFileId) {
+          commit(types.UPDATE_CURRENT_DIFF_FILE_ID, diff_files[0].file_hash);
+        }
+
+        if (isNoteLink) {
+          dispatch('setCurrentDiffFileIdFromNote', id.split('_').pop());
+        }
+
         if (!pagination.next_page) {
           commit(types.SET_RETRIEVING_BATCHES, false);
+
+          // We need to check that the currentDiffFileId points to a file that exists
+          if (
+            state.currentDiffFileId &&
+            !state.diffFiles.some(f => f.file_hash === state.currentDiffFileId) &&
+            !isNoteLink
+          ) {
+            commit(types.UPDATE_CURRENT_DIFF_FILE_ID, state.diffFiles[0].file_hash);
+          }
+
           if (gon.features?.codeNavigation) {
             // eslint-disable-next-line promise/catch-or-return,promise/no-nesting
             import('~/code_navigation').then(m =>
@@ -450,6 +470,8 @@ export const toggleTreeOpen = ({ commit }, path) => {
 };
 
 export const scrollToFile = ({ state, commit }, path) => {
+  if (!state.treeEntries[path]) return;
+
   const { fileHash } = state.treeEntries[path];
   document.location.hash = fileHash;
 
@@ -713,5 +735,19 @@ export function moveToNeighboringCommit({ dispatch, state }, { direction }) {
   }
 }
 
+export const setCurrentDiffFileIdFromNote = ({ commit, rootGetters }, noteId) => {
+  const fileHash = rootGetters.getDiscussion(rootGetters.notesById[noteId].discussion_id).diff_file
+    .file_hash;
+
+  commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
+};
+
+export const navigateToDiffFileIndex = ({ commit, state }, index) => {
+  const fileHash = state.diffFiles[index].file_hash;
+  document.location.hash = fileHash;
+
+  commit(types.UPDATE_CURRENT_DIFF_FILE_ID, fileHash);
+};
+
 // prevent babel-plugin-rewire from generating an invalid default during karma tests
 export default () => {};

app/assets/javascripts/notes/mixins/discussion_navigation.js

@@ -78,8 +78,16 @@ function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId)
   const isDiffView = window.mrTabs.currentAction === 'diffs';
   const targetId = fn(discussionId, isDiffView);
   const discussion = self.getDiscussion(targetId);
+  const discussionFilePath = discussion.diff_file?.file_path;
+
+  if (discussionFilePath) {
+    self.scrollToFile(discussionFilePath);
+  }
+
+  self.$nextTick(() => {
     jumpToDiscussion(self, discussion);
     self.setCurrentDiscussionId(targetId);
+  });
 }
 
 export default {
@@ -95,6 +103,7 @@ export default {
   },
   methods: {
     ...mapActions(['expandDiscussion', 'setCurrentDiscussionId']),
+    ...mapActions('diffs', ['scrollToFile']),
 
     jumpToNextDiscussion() {
       handleDiscussionJump(this, this.nextUnresolvedDiscussionId);

app/assets/stylesheets/pages/merge_requests.scss

@@ -1036,3 +1036,7 @@ $mr-widget-min-height: 69px;
     }
   }
 }
+
+.diff-file-row.is-active {
+  background-color: $gray-50;
+}

app/controllers/profiles/preferences_controller.rb

@@ -48,6 +48,7 @@ class Profiles::PreferencesController < Profiles::ApplicationController
       :time_display_relative,
       :time_format_in_24h,
       :show_whitespace_in_diffs,
+      :view_diffs_file_by_file,
       :tab_width,
       :sourcegraph_enabled,
       :render_whitespace_in_code

app/controllers/projects/merge_requests_controller.rb

@@ -84,6 +84,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
         @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar')
         @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json
         @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs
+        @file_by_file_default = Feature.enabled?(:view_diffs_file_by_file) && current_user&.view_diffs_file_by_file
         @coverage_path = coverage_reports_project_merge_request_path(@project, @merge_request, format: :json) if @merge_request.has_coverage_reports?
 
         set_pipeline_variables

app/models/user.rb

@@ -271,6 +271,7 @@ class User < ApplicationRecord
             :time_display_relative, :time_display_relative=,
             :time_format_in_24h, :time_format_in_24h=,
             :show_whitespace_in_diffs, :show_whitespace_in_diffs=,
+            :view_diffs_file_by_file, :view_diffs_file_by_file=,
             :tab_width, :tab_width=,
             :sourcegraph_enabled, :sourcegraph_enabled=,
             :setup_for_company, :setup_for_company=,

app/views/profiles/preferences/show.html.haml

@@ -70,6 +70,13 @@
       = f.check_box :show_whitespace_in_diffs, class: 'form-check-input'
       = f.label :show_whitespace_in_diffs, class: 'form-check-label' do
         = s_('Preferences|Show whitespace changes in diffs')
+    - if Feature.enabled?(:view_diffs_file_by_file)
+      .form-group.form-check
+        = f.check_box :view_diffs_file_by_file, class: 'form-check-input'
+        = f.label :view_diffs_file_by_file, class: 'form-check-label' do
+          = s_("Preferences|Show one file at a time on merge request's Changes tab")
+        .form-text.text-muted
+          = s_("Preferences|Instead of all the files changed, show only one file at a time. To switch between files, use the file browser.")
     .form-group
       = f.label :tab_width, s_('Preferences|Tab width'), class: 'label-bold'
       = f.number_field :tab_width,

app/views/projects/merge_requests/show.html.haml

@@ -88,7 +88,8 @@
         is_fluid_layout: fluid_layout.to_s,
         dismiss_endpoint: user_callouts_path,
         show_suggest_popover: show_suggest_popover?.to_s,
-        show_whitespace_default: @show_whitespace_default.to_s }
+        show_whitespace_default: @show_whitespace_default.to_s,
+        file_by_file_default: @file_by_file_default.to_s }
 
     .mr-loading-status
       .loading.hide

changelogs/unreleased/ph-222790-diffSingleFileView.yml

+---
+title: Allow diffs to be viewed file-by-file
+merge_request: 35223
+author: rinslow
+type: added

db/migrate/20200623141217_add_view_diffs_file_by_file_to_user_preferences.rb

+# frozen_string_literal: true
+
+class AddViewDiffsFileByFileToUserPreferences < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    with_lock_retries do
+      add_column :user_preferences, :view_diffs_file_by_file, :boolean, default: false, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :user_preferences, :view_diffs_file_by_file, :boolean
+    end
+  end
+end

db/structure.sql

@@ -15717,7 +15717,8 @@ CREATE TABLE public.user_preferences (
     render_whitespace_in_code boolean,
     tab_width smallint,
     feature_filter_type bigint,
-    experience_level smallint
+    experience_level smallint,
+    view_diffs_file_by_file boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE public.user_preferences_id_seq
@@ -23704,6 +23705,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200623000320
 20200623090030
 20200623121135
+20200623141217
 20200623141544
 20200623170000
 20200623185440

locale/gitlab.pot

@@ -17277,6 +17277,9 @@ msgstr ""
 msgid "Preferences|For example: 30 mins ago."
 msgstr ""
 
+msgid "Preferences|Instead of all the files changed, show only one file at a time. To switch between files, use the file browser."
+msgstr ""
+
 msgid "Preferences|Integrations"
 msgstr ""
 
@@ -17295,6 +17298,9 @@ msgstr ""
 msgid "Preferences|Render whitespace characters in the Web IDE"
 msgstr ""
 
+msgid "Preferences|Show one file at a time on merge request's Changes tab"
+msgstr ""
+
 msgid "Preferences|Show whitespace changes in diffs"
 msgstr ""
 

spec/features/merge_request/user_views_diffs_file_by_file_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User views diffs file-by-file', :js do
+  let(:merge_request) do
+    create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
+  end
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user, view_diffs_file_by_file: true) }
+
+  before do
+    project.add_developer(user)
+
+    sign_in(user)
+
+    visit(diffs_project_merge_request_path(project, merge_request))
+
+    wait_for_requests
+  end
+
+  it 'shows diffs file-by-file' do
+    page.within('#diffs') do
+      expect(page).to have_selector('.file-holder', count: 1)
+      expect(page).to have_selector('.diff-file .file-title', text: '.DS_Store')
+
+      click_button 'Next'
+
+      expect(page).to have_selector('.file-holder', count: 1)
+      expect(page).to have_selector('.diff-file .file-title', text: '.gitignore')
+    end
+  end
+end

spec/frontend/diffs/components/app_spec.js

@@ -56,6 +56,7 @@ describe('diffs/components/app', () => {
         changesEmptyStateIllustration: '',
         dismissEndpoint: '',
         showSuggestPopover: true,
+        viewDiffsFileByFile: false,
         ...props,
       },
       provide,
@@ -829,4 +830,58 @@ describe('diffs/components/app', () => {
       expect(toggleShowTreeList).not.toHaveBeenCalled();
     });
   });
+
+  describe('file-by-file', () => {
+    it('renders a single diff', () => {
+      createComponent({ viewDiffsFileByFile: true }, ({ state }) => {
+        state.diffs.diffFiles.push({ file_hash: '123' });
+        state.diffs.diffFiles.push({ file_hash: '312' });
+      });
+
+      expect(wrapper.findAll(DiffFile).length).toBe(1);
+    });
+
+    describe('pagination', () => {
+      it('sets previous button as disabled', () => {
+        createComponent({ viewDiffsFileByFile: true }, ({ state }) => {
+          state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' });
+        });
+
+        expect(wrapper.find('[data-testid="singleFilePrevious"]').props('disabled')).toBe(true);
+        expect(wrapper.find('[data-testid="singleFileNext"]').props('disabled')).toBe(false);
+      });
+
+      it('sets next button as disabled', () => {
+        createComponent({ viewDiffsFileByFile: true }, ({ state }) => {
+          state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' });
+          state.diffs.currentDiffFileId = '312';
+        });
+
+        expect(wrapper.find('[data-testid="singleFilePrevious"]').props('disabled')).toBe(false);
+        expect(wrapper.find('[data-testid="singleFileNext"]').props('disabled')).toBe(true);
+      });
+
+      it.each`
+        currentDiffFileId | button                  | index
+        ${'123'}          | ${'singleFileNext'}     | ${1}
+        ${'312'}          | ${'singleFilePrevious'} | ${0}
+      `(
+        'it calls navigateToDiffFileIndex with $index when $button is clicked',
+        ({ currentDiffFileId, button, index }) => {
+          createComponent({ viewDiffsFileByFile: true }, ({ state }) => {
+            state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' });
+            state.diffs.currentDiffFileId = currentDiffFileId;
+          });
+
+          jest.spyOn(wrapper.vm, 'navigateToDiffFileIndex');
+
+          wrapper.find(`[data-testid="${button}"]`).vm.$emit('click');
+
+          return wrapper.vm.$nextTick().then(() => {
+            expect(wrapper.vm.navigateToDiffFileIndex).toHaveBeenCalledWith(index);
+          });
+        },
+      );
+    });
+  });
 });

spec/frontend/diffs/components/diff_file_header_spec.js

@@ -87,6 +87,7 @@ describe('DiffFileHeader component', () => {
       propsData: {
         diffFile,
         canCurrentUserFork: false,
+        viewDiffsFileByFile: false,
         ...props,
       },
       localVue,

spec/frontend/diffs/components/diff_file_row_spec.js

@@ -73,4 +73,15 @@ describe('Diff File Row component', () => {
       expect(wrapper.find(FileRowStats).exists()).toEqual(value);
     });
   });
+
+  it('adds is-active class when currentDiffFileId matches file_hash', () => {
+    createComponent({
+      level: 0,
+      currentDiffFileId: '123',
+      file: { fileHash: '123' },
+      hideFileStats: false,
+    });
+
+    expect(wrapper.classes('is-active')).toBe(true);
+  });
 });

spec/frontend/diffs/components/diff_file_spec.js

@@ -15,6 +15,7 @@ describe('DiffFile', () => {
     vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
       file: JSON.parse(JSON.stringify(diffFileMockDataReadable)),
       canCurrentUserFork: false,
+      viewDiffsFileByFile: false,
     }).$mount();
     trackingSpy = mockTracking('_category_', vm.$el, jest.spyOn);
   });
@@ -113,6 +114,7 @@ describe('DiffFile', () => {
         vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
           file: JSON.parse(JSON.stringify(diffFileMockDataUnreadable)),
           canCurrentUserFork: false,
+          viewDiffsFileByFile: false,
         }).$mount();
 
         vm.renderIt = false;
@@ -235,6 +237,7 @@ describe('DiffFile', () => {
       vm = createComponentWithStore(Vue.extend(DiffFileComponent), createStore(), {
         file: JSON.parse(JSON.stringify(diffFileMockDataUnreadable)),
         canCurrentUserFork: false,
+        viewDiffsFileByFile: false,
       }).$mount();
 
       jest.spyOn(vm, 'handleLoadCollapsedDiff').mockImplementation(() => {});

spec/frontend/diffs/store/actions_spec.js

@@ -46,6 +46,8 @@ import {
   setSuggestPopoverDismissed,
   changeCurrentCommit,
   moveToNeighboringCommit,
+  setCurrentDiffFileIdFromNote,
+  navigateToDiffFileIndex,
 } from '~/diffs/store/actions';
 import eventHub from '~/notes/event_hub';
 import * as types from '~/diffs/store/mutation_types';
@@ -189,8 +191,8 @@ describe('DiffsStoreActions', () => {
 
     it('should fetch batch diff files', done => {
       const endpointBatch = '/fetch/diffs_batch';
-      const res1 = { diff_files: [], pagination: { next_page: 2 } };
-      const res2 = { diff_files: [], pagination: {} };
+      const res1 = { diff_files: [{ file_hash: 'test' }], pagination: { next_page: 2 } };
+      const res2 = { diff_files: [{ file_hash: 'test2' }], pagination: {} };
       mock
         .onGet(
           mergeUrlParams(
@@ -226,8 +228,10 @@ describe('DiffsStoreActions', () => {
           { type: types.SET_RETRIEVING_BATCHES, payload: true },
           { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } },
           { type: types.SET_BATCH_LOADING, payload: false },
-          { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: [] } },
+          { type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'test' },
+          { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res2.diff_files } },
           { type: types.SET_BATCH_LOADING, payload: false },
+          { type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'test2' },
           { type: types.SET_RETRIEVING_BATCHES, payload: false },
         ],
         [],
@@ -311,6 +315,7 @@ describe('DiffsStoreActions', () => {
             showWhitespace: false,
             diffViewType: 'inline',
             useSingleDiffStyle: false,
+            currentDiffFileId: null,
           },
           [
             { type: types.SET_LOADING, payload: true },
@@ -347,8 +352,8 @@ describe('DiffsStoreActions', () => {
 
       it('should fetch batch diff files', done => {
         const endpointBatch = '/fetch/diffs_batch';
-        const res1 = { diff_files: [], pagination: { next_page: 2 } };
-        const res2 = { diff_files: [], pagination: {} };
+        const res1 = { diff_files: [{ file_hash: 'test' }], pagination: { next_page: 2 } };
+        const res2 = { diff_files: [{ file_hash: 'test2' }], pagination: {} };
         mock
           .onGet(mergeUrlParams({ per_page: DIFFS_PER_PAGE, w: '1', page: 1 }, endpointBatch))
           .reply(200, res1)
@@ -358,14 +363,16 @@ describe('DiffsStoreActions', () => {
         testAction(
           fetchDiffFilesBatch,
           {},
-          { endpointBatch, useSingleDiffStyle: false },
+          { endpointBatch, useSingleDiffStyle: false, currentDiffFileId: null },
           [
             { type: types.SET_BATCH_LOADING, payload: true },
             { type: types.SET_RETRIEVING_BATCHES, payload: true },
             { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res1.diff_files } },
             { type: types.SET_BATCH_LOADING, payload: false },
-            { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: [] } },
+            { type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'test' },
+            { type: types.SET_DIFF_DATA_BATCH, payload: { diff_files: res2.diff_files } },
             { type: types.SET_BATCH_LOADING, payload: false },
+            { type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: 'test2' },
             { type: types.SET_RETRIEVING_BATCHES, payload: false },
           ],
           [],
@@ -1565,4 +1572,31 @@ describe('DiffsStoreActions', () => {
       },
     );
   });
+
+  describe('setCurrentDiffFileIdFromNote', () => {
+    it('commits UPDATE_CURRENT_DIFF_FILE_ID', () => {
+      const commit = jest.fn();
+      const rootGetters = {
+        getDiscussion: () => ({ diff_file: { file_hash: '123' } }),
+        notesById: { '1': { discussion_id: '2' } },
+      };
+
+      setCurrentDiffFileIdFromNote({ commit, rootGetters }, '1');
+
+      expect(commit).toHaveBeenCalledWith(types.UPDATE_CURRENT_DIFF_FILE_ID, '123');
+    });
+  });
+
+  describe('navigateToDiffFileIndex', () => {
+    it('commits UPDATE_CURRENT_DIFF_FILE_ID', done => {
+      testAction(
+        navigateToDiffFileIndex,
+        0,
+        { diffFiles: [{ file_hash: '123' }] },
+        [{ type: types.UPDATE_CURRENT_DIFF_FILE_ID, payload: '123' }],
+        [],
+        done,
+      );
+    });
+  });
 });

spec/frontend/notes/mixins/discussion_navigation_spec.js

@@ -91,6 +91,8 @@ describe('Discussion navigation mixin', () => {
         beforeEach(() => {
           window.mrTabs.currentAction = 'show';
           wrapper.vm[fn](...args);
+
+          return wrapper.vm.$nextTick();
         });
 
         it('sets current discussion', () => {
@@ -112,6 +114,8 @@ describe('Discussion navigation mixin', () => {
         beforeEach(() => {
           window.mrTabs.currentAction = 'diffs';
           wrapper.vm[fn](...args);
+
+          return wrapper.vm.$nextTick();
         });
 
         it('sets current discussion', () => {
@@ -137,6 +141,8 @@ describe('Discussion navigation mixin', () => {
         beforeEach(() => {
           window.mrTabs.currentAction = 'other';
           wrapper.vm[fn](...args);
+
+          return wrapper.vm.$nextTick();
         });
 
         it('sets current discussion', () => {