Merge branch 'diff-file-moved-view' into 'master'

Fixed renamed and mode changed diff viewers not rendering on merge requests

Closes #52607

See merge request gitlab-org/gitlab-ce!23600

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

@@ -90,6 +90,8 @@ export default {
         :old-sha="diffFile.diff_refs.base_sha"
         :file-hash="diffFile.file_hash"
         :project-path="projectPath"
+        :a-mode="diffFile.a_mode"
+        :b-mode="diffFile.b_mode"
       >
         <image-diff-overlay
           slot="image-overlay"

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

@@ -52,7 +52,9 @@ export default {
         (!this.file.highlighted_diff_lines &&
           !this.isLoadingCollapsedDiff &&
           !this.file.too_large &&
-          this.file.text)
+          this.file.text &&
+          !this.file.renamed_file &&
+          !this.file.mode_changed)
       );
     },
     showLoadingIcon() {
@@ -143,9 +145,8 @@ export default {
       <a
         :href="file.fork_path"
         class="js-fork-suggestion-button btn btn-grouped btn-inverted btn-success"
+        >Fork</a
       >
-        Fork
-      </a>
       <button
         class="js-cancel-fork-suggestion-button btn btn-grouped"
         type="button"
@@ -163,9 +164,9 @@ export default {
     <gl-loading-icon v-if="showLoadingIcon" class="diff-content loading" />
     <div v-else-if="showExpandMessage" class="nothing-here-block diff-collapsed">
       {{ __('This diff is collapsed.') }}
-      <a class="click-to-expand js-click-to-expand" href="#" @click.prevent="handleToggle">
-        {{ __('Click to expand it.') }}
-      </a>
+      <a class="click-to-expand js-click-to-expand" href="#" @click.prevent="handleToggle">{{
+        __('Click to expand it.')
+      }}</a>
     </div>
     <div v-if="file.too_large" class="nothing-here-block diff-collapsed js-too-large-diff">
       {{ __('This source diff could not be displayed because it is too large.') }}

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

@@ -324,5 +324,9 @@ export const generateTreeList = files =>
 
 export const getDiffMode = diffFile => {
   const diffModeKey = Object.keys(diffModes).find(key => diffFile[`${key}_file`]);
-  return diffModes[diffModeKey] || diffModes.replaced;
+  return (
+    diffModes[diffModeKey] ||
+    (diffFile.mode_changed && diffModes.mode_changed) ||
+    diffModes.replaced
+  );
 };

app/assets/javascripts/ide/constants.js

@@ -26,6 +26,7 @@ export const diffModes = {
   new: 'new',
   deleted: 'deleted',
   renamed: 'renamed',
+  mode_changed: 'mode_changed',
 };
 
 export const rightSidebarViews = {

app/assets/javascripts/vue_shared/components/diff_viewer/diff_viewer.vue

 <script>
+import { diffModes } from '~/ide/constants';
 import { viewerInformationForPath } from '../content_viewer/lib/viewer_utils';
 import ImageDiffViewer from './viewers/image_diff_viewer.vue';
 import DownloadDiffViewer from './viewers/download_diff_viewer.vue';
+import RenamedFile from './viewers/renamed.vue';
+import ModeChanged from './viewers/mode_changed.vue';
 
 export default {
   props: {
@@ -30,9 +33,25 @@ export default {
       required: false,
       default: '',
     },
+    aMode: {
+      type: String,
+      required: false,
+      default: null,
+    },
+    bMode: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   computed: {
     viewer() {
+      if (this.diffMode === diffModes.renamed) {
+        return RenamedFile;
+      } else if (this.diffMode === diffModes.mode_changed) {
+        return ModeChanged;
+      }
+
       if (!this.newPath) return null;
 
       const previewInfo = viewerInformationForPath(this.newPath);
@@ -67,8 +86,10 @@ export default {
       :new-path="fullNewPath"
       :old-path="fullOldPath"
       :project-path="projectPath"
+      :a-mode="aMode"
+      :b-mode="bMode"
     >
-      <slot slot="image-overlay" name="image-overlay"> </slot>
+      <slot slot="image-overlay" name="image-overlay"></slot>
     </component>
     <slot></slot>
   </div>

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/mode_changed.vue

+<script>
+import { sprintf, __ } from '~/locale';
+
+export default {
+  props: {
+    aMode: {
+      type: String,
+      required: false,
+      default: null,
+    },
+    bMode: {
+      type: String,
+      required: false,
+      default: null,
+    },
+  },
+  computed: {
+    outputText() {
+      return sprintf(__('File mode changed from %{a_mode} to %{b_mode}'), {
+        a_mode: this.aMode,
+        b_mode: this.bMode,
+      });
+    },
+  },
+};
+</script>
+
+<template>
+  <div class="nothing-here-block">{{ outputText }}</div>
+</template>

app/assets/javascripts/vue_shared/components/diff_viewer/viewers/renamed.vue

+<template>
+  <div class="nothing-here-block">{{ __('File moved') }}</div>
+</template>

app/serializers/diff_file_entity.rb

@@ -46,6 +46,7 @@ class DiffFileEntity < Grape::Entity
 
   expose :deleted_file?, as: :deleted_file
   expose :renamed_file?, as: :renamed_file
+  expose :mode_changed?, as: :mode_changed
   expose :old_path
   expose :new_path
   expose :mode_changed?, as: :mode_changed

spec/features/merge_request/user_expands_diff_spec.rb

@@ -2,16 +2,19 @@ require 'spec_helper'
 
 describe 'User expands diff', :js do
   let(:project) { create(:project, :public, :repository) }
-  let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
+  let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) }
 
   before do
+    allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes)
+    allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(10.kilobytes)
+
     visit(diffs_project_merge_request_path(project, merge_request))
 
     wait_for_requests
   end
 
   it 'allows user to expand diff' do
-    page.within find('[id="19763941ab80e8c09871c0a425f0560d9053bcb3"]') do
+    page.within find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do
       click_link 'Click to expand it.'
 
       wait_for_requests

spec/javascripts/diffs/components/diff_file_spec.js

@@ -74,6 +74,32 @@ describe('DiffFile', () => {
         });
       });
 
+      it('should be collapsed for renamed files', done => {
+        vm.file.renderIt = true;
+        vm.file.collapsed = false;
+        vm.file.highlighted_diff_lines = null;
+        vm.file.renamed_file = true;
+
+        vm.$nextTick(() => {
+          expect(vm.$el.innerText).not.toContain('This diff is collapsed');
+
+          done();
+        });
+      });
+
+      it('should be collapsed for mode changed files', done => {
+        vm.file.renderIt = true;
+        vm.file.collapsed = false;
+        vm.file.highlighted_diff_lines = null;
+        vm.file.mode_changed = true;
+
+        vm.$nextTick(() => {
+          expect(vm.$el.innerText).not.toContain('This diff is collapsed');
+
+          done();
+        });
+      });
+
       it('should have loading icon while loading a collapsed diffs', done => {
         vm.file.collapsed = true;
         vm.isLoadingCollapsedDiff = true;

spec/javascripts/diffs/store/utils_spec.js

@@ -559,4 +559,26 @@ describe('DiffsStoreUtils', () => {
       ]);
     });
   });
+
+  describe('getDiffMode', () => {
+    it('returns mode when matched in file', () => {
+      expect(
+        utils.getDiffMode({
+          renamed_file: true,
+        }),
+      ).toBe('renamed');
+    });
+
+    it('returns mode_changed if key has no match', () => {
+      expect(
+        utils.getDiffMode({
+          mode_changed: true,
+        }),
+      ).toBe('mode_changed');
+    });
+
+    it('defaults to replaced', () => {
+      expect(utils.getDiffMode({})).toBe('replaced');
+    });
+  });
 });

spec/javascripts/vue_shared/components/diff_viewer/diff_viewer_spec.js

@@ -68,4 +68,30 @@ describe('DiffViewer', () => {
       done();
     });
   });
+
+  it('renders renamed component', () => {
+    createComponent({
+      diffMode: 'renamed',
+      newPath: 'test.abc',
+      newSha: 'ABC',
+      oldPath: 'testold.abc',
+      oldSha: 'DEF',
+    });
+
+    expect(vm.$el.textContent).toContain('File moved');
+  });
+
+  it('renders mode changed component', () => {
+    createComponent({
+      diffMode: 'mode_changed',
+      newPath: 'test.abc',
+      newSha: 'ABC',
+      oldPath: 'testold.abc',
+      oldSha: 'DEF',
+      aMode: '123',
+      bMode: '321',
+    });
+
+    expect(vm.$el.textContent).toContain('File mode changed from 123 to 321');
+  });
 });

spec/javascripts/vue_shared/components/diff_viewer/viewers/mode_changed_spec.js

+import { shallowMount } from '@vue/test-utils';
+import ModeChanged from '~/vue_shared/components/diff_viewer/viewers/mode_changed.vue';
+
+describe('Diff viewer mode changed component', () => {
+  let vm;
+
+  beforeEach(() => {
+    vm = shallowMount(ModeChanged, {
+      propsData: {
+        aMode: '123',
+        bMode: '321',
+      },
+    });
+  });
+
+  afterEach(() => {
+    vm.destroy();
+  });
+
+  it('renders aMode & bMode', () => {
+    expect(vm.text()).toContain('File mode changed from 123 to 321');
+  });
+});