Add latest changes from gitlab-org/gitlab@master

17 files changed, 601 insertions, 18 deletions

diff --git a/spec/config/object_store_settings_spec.rb b/spec/config/object_store_settings_spec.rb
index 0689dc130d9..14995e2934e 100644
--- a/spec/config/object_store_settings_spec.rb
+++ b/spec/config/object_store_settings_spec.rb
@@ -25,7 +25,6 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do
           'artifacts' => { 'enabled' => true },
           'external_diffs' => { 'enabled' => false },
           'pages' => { 'enabled' => true },
-          'ci_secure_files' => { 'enabled' => true },
           'object_store' => {
             'enabled' => true,
             'connection' => connection,
@@ -101,14 +100,6 @@ RSpec.describe ObjectStoreSettings, feature_category: :shared do
         expect(settings.external_diffs['enabled']).to be false
         expect(settings.external_diffs['object_store']).to be_nil
         expect(settings.external_diffs).to eq(settings['external_diffs'])
-
-        expect(settings.ci_secure_files['enabled']).to be true
-        expect(settings.ci_secure_files['object_store']['enabled']).to be true
-        expect(settings.ci_secure_files['object_store']['connection'].to_hash).to eq(connection)
-        expect(settings.ci_secure_files['object_store']['remote_directory']).to eq('ci_secure_files')
-        expect(settings.ci_secure_files['object_store']['bucket_prefix']).to eq(nil)
-        expect(settings.ci_secure_files['object_store']['consolidated_settings']).to be true
-        expect(settings.ci_secure_files).to eq(settings['ci_secure_files'])
       end
 
       it 'supports bucket prefixes' do
diff --git a/spec/features/merge_request/user_creates_discussion_on_diff_file_spec.rb b/spec/features/merge_request/user_creates_discussion_on_diff_file_spec.rb
new file mode 100644
index 00000000000..bb41ea6f6ed
--- /dev/null
+++ b/spec/features/merge_request/user_creates_discussion_on_diff_file_spec.rb
@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'User creates discussion on diff file', :js, feature_category: :code_review_workflow do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :public, :repository) }
+  let_it_be(:merge_request) do
+    create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
+  end
+
+  before do
+    project.add_maintainer(user)
+    sign_in(user)
+
+    visit(diffs_project_merge_request_path(project, merge_request))
+  end
+
+  it 'creates discussion on diff file' do
+    first('.diff-file [data-testid="comment-files-button"]').click
+
+    send_keys "Test comment"
+
+    click_button "Add comment now"
+
+    expect(first('.diff-file')).to have_selector('.note-text', text: 'Test comment')
+  end
+end
diff --git a/spec/features/projects/settings/secure_files_spec.rb b/spec/features/projects/settings/secure_files_spec.rb
index 8ac64287fcb..7ff1a5f3568 100644
--- a/spec/features/projects/settings/secure_files_spec.rb
+++ b/spec/features/projects/settings/secure_files_spec.rb
@@ -12,6 +12,17 @@ RSpec.describe 'Secure Files', :js, feature_category: :groups_and_projects do
     sign_in(user)
   end
 
+  context 'when disabled at the instance level' do
+    before do
+      stub_config(ci_secure_files: { enabled: false })
+    end
+
+    it 'does not show the secure files settings' do
+      visit project_settings_ci_cd_path(project)
+      expect(page).not_to have_content('Secure Files')
+    end
+  end
+
   context 'authenticated user with admin permissions' do
     it 'shows the secure files settings' do
       visit project_settings_ci_cd_path(project)
diff --git a/spec/frontend/batch_comments/components/diff_file_drafts_spec.js b/spec/frontend/batch_comments/components/diff_file_drafts_spec.js
index f667ebc0fcb..014e28b7509 100644
--- a/spec/frontend/batch_comments/components/diff_file_drafts_spec.js
+++ b/spec/frontend/batch_comments/components/diff_file_drafts_spec.js
@@ -16,7 +16,10 @@ describe('Batch comments diff file drafts component', () => {
         batchComments: {
           namespaced: true,
           getters: {
-            draftsForFile: () => () => [{ id: 1 }, { id: 2 }],
+            draftsForFile: () => () => [
+              { id: 1, position: { position_type: 'file' } },
+              { id: 2, position: { position_type: 'file' } },
+            ],
           },
         },
       },
@@ -24,7 +27,7 @@ describe('Batch comments diff file drafts component', () => {
 
     vm = shallowMount(DiffFileDrafts, {
       store,
-      propsData: { fileHash: 'filehash' },
+      propsData: { fileHash: 'filehash', positionType: 'file' },
     });
   }
 
diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js
index fb271433762..39d9255aaf9 100644
--- a/spec/frontend/diffs/components/diff_content_spec.js
+++ b/spec/frontend/diffs/components/diff_content_spec.js
@@ -176,7 +176,12 @@ describe('DiffContent', () => {
       getCommentFormForDiffFileGetterMock.mockReturnValue(() => true);
       createComponent({
         props: {
-          diffFile: { ...imageDiffFile, discussions: [{ name: 'discussion-stub ' }] },
+          diffFile: {
+            ...imageDiffFile,
+            discussions: [
+              { name: 'discussion-stub', position: { position_type: IMAGE_DIFF_POSITION_TYPE } },
+            ],
+          },
         },
       });
 
@@ -186,7 +191,12 @@ describe('DiffContent', () => {
     it('emits saveDiffDiscussion when note-form emits `handleFormUpdate`', () => {
       const noteStub = {};
       getCommentFormForDiffFileGetterMock.mockReturnValue(() => true);
-      const currentDiffFile = { ...imageDiffFile, discussions: [{ name: 'discussion-stub ' }] };
+      const currentDiffFile = {
+        ...imageDiffFile,
+        discussions: [
+          { name: 'discussion-stub', position: { position_type: IMAGE_DIFF_POSITION_TYPE } },
+        ],
+      };
       createComponent({
         props: {
           diffFile: currentDiffFile,
diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js
index 900aa8d1469..3f75b086368 100644
--- a/spec/frontend/diffs/components/diff_file_header_spec.js
+++ b/spec/frontend/diffs/components/diff_file_header_spec.js
@@ -18,7 +18,10 @@ import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import testAction from '../../__helpers__/vuex_action_helper';
 import diffDiscussionsMockData from '../mock_data/diff_discussions';
 
-jest.mock('~/lib/utils/common_utils');
+jest.mock('~/lib/utils/common_utils', () => ({
+  scrollToElement: jest.fn(),
+  isLoggedIn: () => true,
+}));
 
 const diffFile = Object.freeze(
   Object.assign(diffDiscussionsMockData.diff_file, {
@@ -47,6 +50,9 @@ describe('DiffFileHeader component', () => {
   const diffHasDiscussionsResultMock = jest.fn();
   const defaultMockStoreConfig = {
     state: {},
+    getters: {
+      getNoteableData: () => ({ current_user: { can_create_note: true } }),
+    },
     modules: {
       diffs: {
         namespaced: true,
@@ -637,4 +643,23 @@ describe('DiffFileHeader component', () => {
       },
     );
   });
+
+  it.each`
+    commentOnFiles | exists   | existsText
+    ${false}       | ${false} | ${'does not'}
+    ${true}        | ${true}  | ${'does'}
+  `(
+    '$existsText render comment on files button when commentOnFiles is $commentOnFiles',
+    ({ commentOnFiles, exists }) => {
+      window.gon = { current_user_id: 1 };
+      createComponent({
+        props: {
+          addMergeRequestButtons: true,
+        },
+        options: { provide: { glFeatures: { commentOnFiles } } },
+      });
+
+      expect(wrapper.find('[data-testid="comment-files-button"]').exists()).toEqual(exists);
+    },
+  );
 });
diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js
index 389b192a515..d9c57ed1470 100644
--- a/spec/frontend/diffs/components/diff_file_spec.js
+++ b/spec/frontend/diffs/components/diff_file_spec.js
@@ -553,4 +553,69 @@ describe('DiffFile', () => {
       expect(wrapper.find('[data-testid="conflictsAlert"]').exists()).toBe(true);
     });
   });
+
+  describe('file discussions', () => {
+    it.each`
+      extraProps                                                           | exists   | existsText
+      ${{}}                                                                | ${false} | ${'does not'}
+      ${{ hasCommentForm: false }}                                         | ${false} | ${'does not'}
+      ${{ hasCommentForm: true }}                                          | ${true}  | ${'does'}
+      ${{ discussions: [{ id: 1, position: { position_type: 'file' } }] }} | ${true}  | ${'does'}
+      ${{ drafts: [{ id: 1 }] }}                                           | ${true}  | ${'does'}
+    `(
+      'discussions wrapper $existsText exist for file with $extraProps',
+      ({ extraProps, exists }) => {
+        const file = {
+          ...getReadableFile(),
+          ...extraProps,
+        };
+
+        ({ wrapper, store } = createComponent({
+          file,
+          options: { provide: { glFeatures: { commentOnFiles: true } } },
+        }));
+
+        expect(wrapper.find('[data-testid="file-discussions"]').exists()).toEqual(exists);
+      },
+    );
+
+    it.each`
+      hasCommentForm | exists   | existsText
+      ${false}       | ${false} | ${'does not'}
+      ${true}        | ${true}  | ${'does'}
+    `(
+      'comment form $existsText exist for hasCommentForm with $hasCommentForm',
+      ({ hasCommentForm, exists }) => {
+        const file = {
+          ...getReadableFile(),
+          hasCommentForm,
+        };
+
+        ({ wrapper, store } = createComponent({
+          file,
+          options: { provide: { glFeatures: { commentOnFiles: true } } },
+        }));
+
+        expect(wrapper.find('[data-testid="file-note-form"]').exists()).toEqual(exists);
+      },
+    );
+
+    it.each`
+      discussions                                         | exists   | existsText
+      ${[]}                                               | ${false} | ${'does not'}
+      ${[{ id: 1, position: { position_type: 'file' } }]} | ${true}  | ${'does'}
+    `('discussions $existsText exist for $discussions', ({ discussions, exists }) => {
+      const file = {
+        ...getReadableFile(),
+        discussions,
+      };
+
+      ({ wrapper, store } = createComponent({
+        file,
+        options: { provide: { glFeatures: { commentOnFiles: true } } },
+      }));
+
+      expect(wrapper.find('[data-testid="diff-file-discussions"]').exists()).toEqual(exists);
+    });
+  });
 });
diff --git a/spec/frontend/diffs/mock_data/diff_file.js b/spec/frontend/diffs/mock_data/diff_file.js
index e0e5778e0d5..eef68100378 100644
--- a/spec/frontend/diffs/mock_data/diff_file.js
+++ b/spec/frontend/diffs/mock_data/diff_file.js
@@ -334,5 +334,6 @@ export const getDiffFileMock = () => ({
     },
   ],
   discussions: [],
+  drafts: [],
   renderingLines: false,
 });
diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js
index f0061ad88d7..7534fe741e7 100644
--- a/spec/frontend/diffs/store/actions_spec.js
+++ b/spec/frontend/diffs/store/actions_spec.js
@@ -1889,4 +1889,28 @@ describe('DiffsStoreActions', () => {
       },
     );
   });
+
+  describe('toggleFileCommentForm', () => {
+    it('commits TOGGLE_FILE_COMMENT_FORM', () => {
+      return testAction(
+        diffActions.toggleFileCommentForm,
+        'path',
+        {},
+        [{ type: types.TOGGLE_FILE_COMMENT_FORM, payload: 'path' }],
+        [],
+      );
+    });
+  });
+
+  describe('addDraftToFile', () => {
+    it('commits ADD_DRAFT_TO_FILE', () => {
+      return testAction(
+        diffActions.addDraftToFile,
+        { filePath: 'path', draft: 'draft' },
+        {},
+        [{ type: types.ADD_DRAFT_TO_FILE, payload: { filePath: 'path', draft: 'draft' } }],
+        [],
+      );
+    });
+  });
 });
diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js
index ed7b6699e2c..8097f0976f6 100644
--- a/spec/frontend/diffs/store/getters_spec.js
+++ b/spec/frontend/diffs/store/getters_spec.js
@@ -188,6 +188,24 @@ describe('Diffs Module Getters', () => {
       expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(true);
     });
 
+    it('returns true when file discussion is expanded', () => {
+      const diffFile = {
+        discussions: [{ ...discussionMock, expanded: true }],
+        highlighted_diff_lines: [],
+      };
+
+      expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(true);
+    });
+
+    it('returns false when file discussion is expanded', () => {
+      const diffFile = {
+        discussions: [{ ...discussionMock, expanded: false }],
+        highlighted_diff_lines: [],
+      };
+
+      expect(getters.diffHasExpandedDiscussions(localState)(diffFile)).toEqual(false);
+    });
+
     it('returns false when there are no discussions', () => {
       const diffFile = {
         parallel_diff_lines: [],
@@ -231,6 +249,15 @@ describe('Diffs Module Getters', () => {
       expect(getters.diffHasDiscussions(localState)(diffFile)).toEqual(true);
     });
 
+    it('returns true when file has discussions', () => {
+      const diffFile = {
+        discussions: [discussionMock, discussionMock],
+        highlighted_diff_lines: [],
+      };
+
+      expect(getters.diffHasDiscussions(localState)(diffFile)).toEqual(true);
+    });
+
     it('returns false when getDiffFileDiscussions returns no discussions', () => {
       const diffFile = {
         parallel_diff_lines: [],
diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js
index ed8d7397bbc..b089cf22b14 100644
--- a/spec/frontend/diffs/store/mutations_spec.js
+++ b/spec/frontend/diffs/store/mutations_spec.js
@@ -269,6 +269,53 @@ describe('DiffsStoreMutations', () => {
       expect(state.diffFiles[0][INLINE_DIFF_LINES_KEY][0].discussions[0].id).toEqual(1);
     });
 
+    it('should add discussions to the given file', () => {
+      const diffPosition = {
+        base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
+        new_line: null,
+        new_path: '500-lines-4.txt',
+        old_line: 5,
+        old_path: '500-lines-4.txt',
+        start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        type: 'file',
+      };
+
+      const state = {
+        latestDiff: true,
+        diffFiles: [
+          {
+            file_hash: 'ABC',
+            [INLINE_DIFF_LINES_KEY]: [],
+            discussions: [],
+          },
+        ],
+      };
+      const discussion = {
+        id: 1,
+        line_code: 'ABC_1',
+        diff_discussion: true,
+        resolvable: true,
+        original_position: diffPosition,
+        position: diffPosition,
+        diff_file: {
+          file_hash: state.diffFiles[0].file_hash,
+        },
+      };
+
+      const diffPositionByLineCode = {
+        ABC_1: diffPosition,
+      };
+
+      mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
+        discussion,
+        diffPositionByLineCode,
+      });
+
+      expect(state.diffFiles[0].discussions.length).toEqual(1);
+      expect(state.diffFiles[0].discussions[0].id).toEqual(1);
+    });
+
     it('should not duplicate discussions on line', () => {
       const diffPosition = {
         base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
@@ -957,4 +1004,25 @@ describe('DiffsStoreMutations', () => {
       expect(state.mrReviews).toStrictEqual(newReviews);
     });
   });
+
+  describe('TOGGLE_FILE_COMMENT_FORM', () => {
+    it('toggles diff files hasCommentForm', () => {
+      const state = { diffFiles: [{ file_path: 'path', hasCommentForm: false }] };
+
+      mutations[types.TOGGLE_FILE_COMMENT_FORM](state, 'path');
+
+      expect(state.diffFiles[0].hasCommentForm).toEqual(true);
+    });
+  });
+
+  describe('ADD_DRAFT_TO_FILE', () => {
+    it('adds draft to diff file', () => {
+      const state = { diffFiles: [{ file_path: 'path', drafts: [] }] };
+
+      mutations[types.ADD_DRAFT_TO_FILE](state, { filePath: 'path', draft: 'test' });
+
+      expect(state.diffFiles[0].drafts.length).toEqual(1);
+      expect(state.diffFiles[0].drafts[0]).toEqual('test');
+    });
+  });
 });
diff --git a/spec/helpers/ci/secure_files_helper_spec.rb b/spec/helpers/ci/secure_files_helper_spec.rb
index 54307e670e1..049a09afd03 100644
--- a/spec/helpers/ci/secure_files_helper_spec.rb
+++ b/spec/helpers/ci/secure_files_helper_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Ci::SecureFilesHelper do
+RSpec.describe Ci::SecureFilesHelper, feature_category: :mobile_devops do
   let_it_be(:maintainer) { create(:user) }
   let_it_be(:developer) { create(:user) }
   let_it_be(:guest) { create(:user) }
@@ -19,6 +19,16 @@ RSpec.describe Ci::SecureFilesHelper do
   subject { helper.show_secure_files_setting(project, user) }
 
   describe '#show_secure_files_setting' do
+    context 'when disabled at the instance level' do
+      before do
+        stub_config(ci_secure_files: { enabled: false })
+      end
+
+      let(:user) { maintainer }
+
+      it { is_expected.to be false }
+    end
+
     context 'authenticated user with admin permissions' do
       let(:user) { maintainer }
 
diff --git a/spec/lib/gitlab/diff/formatters/file_formatter_spec.rb b/spec/lib/gitlab/diff/formatters/file_formatter_spec.rb
new file mode 100644
index 00000000000..32e5f17f7eb
--- /dev/null
+++ b/spec/lib/gitlab/diff/formatters/file_formatter_spec.rb
@@ -0,0 +1,44 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Diff::Formatters::FileFormatter, feature_category: :code_review_workflow do
+  let(:base_attrs) do
+    {
+      base_sha: 123,
+      start_sha: 456,
+      head_sha: 789,
+      old_path: nil,
+      new_path: nil,
+      position_type: 'file'
+    }
+  end
+
+  let(:attrs) { base_attrs.merge(old_path: 'path.rb', new_path: 'path.rb') }
+
+  it_behaves_like 'position formatter' do
+    # rubocop:disable Fips/SHA1 (This is used to match the existing class method)
+    let(:key) do
+      [123, 456, 789,
+        Digest::SHA1.hexdigest(formatter.old_path), Digest::SHA1.hexdigest(formatter.new_path),
+        'path.rb', 'path.rb']
+    end
+    # rubocop:enable Fips/SHA1
+  end
+
+  describe '#==' do
+    subject { described_class.new(attrs) }
+
+    it { is_expected.to eq(subject) }
+
+    [:old_path, :new_path].each do |attr|
+      context "with attribute:#{attr}" do
+        let(:other_formatter) do
+          described_class.new(attrs.merge(attr => 9))
+        end
+
+        it { is_expected.not_to eq(other_formatter) }
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/diff/position_tracer/file_strategy_spec.rb b/spec/lib/gitlab/diff/position_tracer/file_strategy_spec.rb
new file mode 100644
index 00000000000..0d03f7ce6ca
--- /dev/null
+++ b/spec/lib/gitlab/diff/position_tracer/file_strategy_spec.rb
@@ -0,0 +1,238 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Diff::PositionTracer::FileStrategy, feature_category: :code_review_workflow do
+  include PositionTracerHelpers
+
+  let_it_be(:project) { create(:project, :repository) }
+  let(:current_user) { project.first_owner }
+  let(:file_name) { 'test-file' }
+  let(:new_file_name) { "#{file_name}-new" }
+  let(:second_file_name) { "#{file_name}-2" }
+  let(:branch_name) { 'position-tracer-test' }
+  let(:old_position) { position(old_path: file_name, new_path: file_name, position_type: 'file') }
+
+  let(:tracer) do
+    Gitlab::Diff::PositionTracer.new(
+      project: project,
+      old_diff_refs: old_diff_refs,
+      new_diff_refs: new_diff_refs
+    )
+  end
+
+  let(:strategy) { described_class.new(tracer) }
+
+  let(:initial_commit) do
+    project.commit(create_branch(branch_name, 'master')[:branch]&.name || 'master')
+  end
+
+  subject { strategy.trace(old_position) }
+
+  describe '#trace' do
+    describe 'diff scenarios' do
+      let(:create_file_commit) do
+        initial_commit
+
+        create_file(
+          branch_name,
+          file_name,
+          Base64.encode64('content')
+        )
+      end
+
+      let(:update_file_commit) do
+        create_file_commit
+
+        update_file(
+          branch_name,
+          file_name,
+          Base64.encode64('updatedcontent')
+        )
+      end
+
+      let(:update_file_again_commit) do
+        update_file_commit
+
+        update_file(
+          branch_name,
+          file_name,
+          Base64.encode64('updatedcontentagain')
+        )
+      end
+
+      let(:delete_file_commit) do
+        create_file_commit
+        delete_file(branch_name, file_name)
+      end
+
+      let(:rename_file_commit) do
+        delete_file_commit
+
+        create_file(
+          branch_name,
+          new_file_name,
+          Base64.encode64('renamedcontent')
+        )
+      end
+
+      let(:create_second_file_commit) do
+        create_file_commit
+
+        create_file(
+          branch_name,
+          second_file_name,
+          Base64.encode64('morecontent')
+        )
+      end
+
+      let(:create_another_file_commit) do
+        create_file(
+          branch_name,
+          second_file_name,
+          Base64.encode64('morecontent')
+        )
+      end
+
+      let(:update_another_file_commit) do
+        update_file(
+          branch_name,
+          second_file_name,
+          Base64.encode64('updatedmorecontent')
+        )
+      end
+
+      context 'when the file was created in the old diff' do
+        context 'when the file is unchanged between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+          let(:new_diff_refs) { diff_refs(initial_commit, create_second_file_commit) }
+
+          it 'returns the new position' do
+            expect_new_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file was updated between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+          let(:new_diff_refs) { diff_refs(initial_commit, update_file_commit) }
+          let(:change_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file was renamed in between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+          let(:new_diff_refs) { diff_refs(initial_commit, rename_file_commit) }
+          let(:change_diff_refs) { diff_refs(create_file_commit, rename_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file was removed in between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+          let(:new_diff_refs) { diff_refs(initial_commit, delete_file_commit) }
+          let(:change_diff_refs) { diff_refs(create_file_commit, delete_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file is unchanged in the new diff' do
+          let(:old_diff_refs) { diff_refs(initial_commit, create_file_commit) }
+          let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) }
+          let(:change_diff_refs) { diff_refs(initial_commit, create_another_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+      end
+
+      context 'when the file was changed in the old diff' do
+        context 'when the file is unchanged in between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+          let(:new_diff_refs) { diff_refs(create_file_commit, create_second_file_commit) }
+
+          it 'returns the new position' do
+            expect_new_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file was updated in between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+          let(:new_diff_refs) { diff_refs(create_file_commit, update_file_again_commit) }
+          let(:change_diff_refs) { diff_refs(update_file_commit, update_file_again_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file was renamed in between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+          let(:new_diff_refs) { diff_refs(create_file_commit, rename_file_commit) }
+          let(:change_diff_refs) { diff_refs(update_file_commit, rename_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file was removed in between the old and the new diff' do
+          let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+          let(:new_diff_refs) { diff_refs(create_file_commit, delete_file_commit) }
+          let(:change_diff_refs) { diff_refs(update_file_commit, delete_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+
+        context 'when the file is unchanged in the new diff' do
+          let(:old_diff_refs) { diff_refs(create_file_commit, update_file_commit) }
+          let(:new_diff_refs) { diff_refs(create_another_file_commit, update_another_file_commit) }
+          let(:change_diff_refs) { diff_refs(create_file_commit, create_another_file_commit) }
+
+          it 'returns the position of the change' do
+            expect_change_position(
+              old_path: file_name,
+              new_path: file_name
+            )
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb
index 8831cee690d..4aa4f160fc9 100644
--- a/spec/lib/gitlab/diff/position_tracer_spec.rb
+++ b/spec/lib/gitlab/diff/position_tracer_spec.rb
@@ -18,8 +18,13 @@ RSpec.describe Gitlab::Diff::PositionTracer do
     let(:project) { double }
     let(:old_diff_refs) { diff_refs }
     let(:new_diff_refs) { diff_refs }
-    let(:position) { double(on_text?: on_text?, diff_refs: diff_refs, ignore_whitespace_change: false) }
+    let(:on_file?) { false }
+    let(:on_text?) { false }
     let(:tracer) { double }
+    let(:position) do
+      double(on_text?: on_text?, on_image?: false, on_file?: on_file?, diff_refs: diff_refs,
+        ignore_whitespace_change: false)
+    end
 
     context 'position is on text' do
       let(:on_text?) { true }
@@ -48,6 +53,20 @@ RSpec.describe Gitlab::Diff::PositionTracer do
         subject.trace(position)
       end
     end
+
+    context 'position on file' do
+      let(:on_file?) { true }
+
+      it 'calls ImageStrategy#trace' do
+        expect(Gitlab::Diff::PositionTracer::FileStrategy)
+          .to receive(:new)
+          .with(subject)
+          .and_return(tracer)
+        expect(tracer).to receive(:trace).with(position)
+
+        subject.trace(position)
+      end
+    end
   end
 
   describe 'diffs methods' do
diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb
index db12576154e..4e1afd66683 100644
--- a/spec/requests/api/ci/secure_files_spec.rb
+++ b/spec/requests/api/ci/secure_files_spec.rb
@@ -56,6 +56,26 @@ RSpec.describe API::Ci::SecureFiles, feature_category: :mobile_devops do
         end
       end
 
+      context 'when the feature is disabled at the instance level' do
+        before do
+          stub_config(ci_secure_files: { enabled: false })
+        end
+
+        it 'returns a 403 when attempting to upload a file' do
+          expect do
+            post api("/projects/#{project.id}/secure_files", maintainer), params: file_params
+          end.not_to change { project.secure_files.count }
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+
+        it 'returns a 403 when downloading a file' do
+          get api("/projects/#{project.id}/secure_files", developer)
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
       context 'when the flag is disabled' do
         it 'returns a 201 when uploading a file when the ci_secure_files_read_only feature flag is disabled' do
           expect do
diff --git a/spec/support/shared_examples/lib/gitlab/position_formatters_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/position_formatters_shared_examples.rb
index c9300aff3e6..1e03ddac42e 100644
--- a/spec/support/shared_examples/lib/gitlab/position_formatters_shared_examples.rb
+++ b/spec/support/shared_examples/lib/gitlab/position_formatters_shared_examples.rb
@@ -2,10 +2,9 @@
 
 RSpec.shared_examples "position formatter" do
   let(:formatter) { described_class.new(attrs) }
+  let(:key) { [123, 456, 789, Digest::SHA1.hexdigest(formatter.old_path), Digest::SHA1.hexdigest(formatter.new_path), 1, 2] }
 
   describe '#key' do
-    let(:key) { [123, 456, 789, Digest::SHA1.hexdigest(formatter.old_path), Digest::SHA1.hexdigest(formatter.new_path), 1, 2] }
-
     subject { formatter.key }
 
     it { is_expected.to eq(key) }