From 3a0ab2a9f351622f0298ab2040e03374cb21bff5 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Wed, 24 Jun 2020 10:20:03 +0100
Subject: [PATCH] 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
---
 .../javascripts/diffs/components/app.vue      | 54 ++++++++++++++++--
 .../diffs/components/diff_file.vue            |  5 ++
 .../diffs/components/diff_file_header.vue     | 13 ++++-
 .../diffs/components/diff_file_row.vue        | 13 ++++-
 .../diffs/components/tree_list.vue            |  3 +-
 app/assets/javascripts/diffs/index.js         |  2 +
 app/assets/javascripts/diffs/store/actions.js | 38 ++++++++++++-
 .../notes/mixins/discussion_navigation.js     | 13 ++++-
 .../stylesheets/pages/merge_requests.scss     |  4 ++
 .../profiles/preferences_controller.rb        |  1 +
 .../projects/merge_requests_controller.rb     |  1 +
 app/models/user.rb                            |  1 +
 app/views/profiles/preferences/show.html.haml |  7 +++
 .../projects/merge_requests/show.html.haml    |  3 +-
 .../ph-222790-diffSingleFileView.yml          |  5 ++
 ..._diffs_file_by_file_to_user_preferences.rb | 21 +++++++
 db/structure.sql                              |  4 +-
 locale/gitlab.pot                             |  6 ++
 .../user_views_diffs_file_by_file_spec.rb     | 33 +++++++++++
 spec/frontend/diffs/components/app_spec.js    | 55 +++++++++++++++++++
 .../diffs/components/diff_file_header_spec.js |  1 +
 .../diffs/components/diff_file_row_spec.js    | 11 ++++
 .../diffs/components/diff_file_spec.js        |  3 +
 spec/frontend/diffs/store/actions_spec.js     | 48 +++++++++++++---
 .../mixins/discussion_navigation_spec.js      |  6 ++
 25 files changed, 330 insertions(+), 21 deletions(-)
 create mode 100644 changelogs/unreleased/ph-222790-diffSingleFileView.yml
 create mode 100644 db/migrate/20200623141217_add_view_diffs_file_by_file_to_user_preferences.rb
 create mode 100644 spec/features/merge_request/user_views_diffs_file_by_file_spec.rb

diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue
index 406b0098d99d32..1e524882d5fcec 100644
--- a/app/assets/javascripts/diffs/components/app.vue
+++ b/app/assets/javascripts/diffs/components/app.vue
@@ -1,6 +1,6 @@
 <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>
diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index 54852b113ae20a..00d36c0b97836e 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/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"
diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue
index 5dd80528726e39..5727fbaaf6813e 100644
--- a/app/assets/javascripts/diffs/components/diff_file_header.vue
+++ b/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
diff --git a/app/assets/javascripts/diffs/components/diff_file_row.vue b/app/assets/javascripts/diffs/components/diff_file_row.vue
index c8ba8d6040e630..43b669625f4e6d 100644
--- a/app/assets/javascripts/diffs/components/diff_file_row.vue
+++ b/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>
diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue
index 52611f3c82a4ca..38fbd8e61d4257 100644
--- a/app/assets/javascripts/diffs/components/tree_list.vue
+++ b/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"
         />
diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js
index 028f04095a9018..76ff67ab861961 100644
--- a/app/assets/javascripts/diffs/index.js
+++ b/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,
         },
       });
     },
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 6f903bd09f0afd..7d0103b44c885c 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/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 () => {};
diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js
index 9281149d9d3f30..889883a23d0721 100644
--- a/app/assets/javascripts/notes/mixins/discussion_navigation.js
+++ b/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);
-  jumpToDiscussion(self, discussion);
-  self.setCurrentDiscussionId(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);
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index 61232b0027547b..5cf2d847405335 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/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;
+}
diff --git a/app/controllers/profiles/preferences_controller.rb b/app/controllers/profiles/preferences_controller.rb
index 1477d79c911eb7..8653fe3b6ede31 100644
--- a/app/controllers/profiles/preferences_controller.rb
+++ b/app/controllers/profiles/preferences_controller.rb
@@ -48,6 +48,7 @@ def preferences_param_names
       :time_display_relative,
       :time_format_in_24h,
       :show_whitespace_in_diffs,
+      :view_diffs_file_by_file,
       :tab_width,
       :sourcegraph_enabled,
       :render_whitespace_in_code
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 1aa6002e9c5fd5..55937a27ba601d 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -84,6 +84,7 @@ def show
         @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
diff --git a/app/models/user.rb b/app/models/user.rb
index 45ff69dfb6923b..36b9ed358ff5f4 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -271,6 +271,7 @@ def update_tracked_fields!(request)
             :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=,
diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml
index a90e43fcd865c3..659b30666036b6 100644
--- a/app/views/profiles/preferences/show.html.haml
+++ b/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,
diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml
index ba97bb81c56b69..6df5a3d59575a7 100644
--- a/app/views/projects/merge_requests/show.html.haml
+++ b/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
diff --git a/changelogs/unreleased/ph-222790-diffSingleFileView.yml b/changelogs/unreleased/ph-222790-diffSingleFileView.yml
new file mode 100644
index 00000000000000..96167c59294063
--- /dev/null
+++ b/changelogs/unreleased/ph-222790-diffSingleFileView.yml
@@ -0,0 +1,5 @@
+---
+title: Allow diffs to be viewed file-by-file
+merge_request: 35223
+author: rinslow
+type: added
diff --git a/db/migrate/20200623141217_add_view_diffs_file_by_file_to_user_preferences.rb b/db/migrate/20200623141217_add_view_diffs_file_by_file_to_user_preferences.rb
new file mode 100644
index 00000000000000..9ea38bd4ab4441
--- /dev/null
+++ b/db/migrate/20200623141217_add_view_diffs_file_by_file_to_user_preferences.rb
@@ -0,0 +1,21 @@
+# 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
diff --git a/db/structure.sql b/db/structure.sql
index 8ad6b7be5a43ec..f961838f8cd172 100644
--- a/db/structure.sql
+++ b/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
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index f5ae293e608ddb..37fefc0d52e801 100644
--- a/locale/gitlab.pot
+++ b/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 ""
 
diff --git a/spec/features/merge_request/user_views_diffs_file_by_file_spec.rb b/spec/features/merge_request/user_views_diffs_file_by_file_spec.rb
new file mode 100644
index 00000000000000..c254a142349ba8
--- /dev/null
+++ b/spec/features/merge_request/user_views_diffs_file_by_file_spec.rb
@@ -0,0 +1,33 @@
+# 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
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js
index 57e3a93c6f4472..b7f03f35dfb280 100644
--- a/spec/frontend/diffs/components/app_spec.js
+++ b/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);
+          });
+        },
+      );
+    });
+  });
 });
diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js
index e0b7e0bc0f3f1a..671dced080c5bb 100644
--- a/spec/frontend/diffs/components/diff_file_header_spec.js
+++ b/spec/frontend/diffs/components/diff_file_header_spec.js
@@ -87,6 +87,7 @@ describe('DiffFileHeader component', () => {
       propsData: {
         diffFile,
         canCurrentUserFork: false,
+        viewDiffsFileByFile: false,
         ...props,
       },
       localVue,
diff --git a/spec/frontend/diffs/components/diff_file_row_spec.js b/spec/frontend/diffs/components/diff_file_row_spec.js
index 856622b89cbf7e..afdd4bfb335cb5 100644
--- a/spec/frontend/diffs/components/diff_file_row_spec.js
+++ b/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);
+  });
 });
diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index 71e975f2409af1..7e154d76f456d5 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/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(() => {});
diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js
index ee55669cd9b807..2b069e1ebc9f9e 100644
--- a/spec/frontend/diffs/store/actions_spec.js
+++ b/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,
+      );
+    });
+  });
 });
diff --git a/spec/frontend/notes/mixins/discussion_navigation_spec.js b/spec/frontend/notes/mixins/discussion_navigation_spec.js
index ae30a36fc81bbb..ecff95b6fe062d 100644
--- a/spec/frontend/notes/mixins/discussion_navigation_spec.js
+++ b/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', () => {
-- 
GitLab