Merge branch 'ph/222790/diffSingleFileView' into 'master'

Added option to view diffs file-by-file

See merge request gitlab-org/gitlab!35223

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

@@ -15718,7 +15718,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
@@ -23705,6 +23706,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200623000320
 20200623090030
 20200623121135
+20200623141217
 20200623141544
 20200623170000
 20200623185440

locale/gitlab.pot

@@ -17325,6 +17325,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 ""
 
@@ -17343,6 +17346,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', () => {