Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-09-19

# Conflicts:
#	config/initializers/0_post_deployment_migrations.rb
#	locale/gitlab.pot

[ci skip]

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

@@ -22,7 +22,7 @@ export default {
       type: Object,
       required: true,
     },
-    position: {
+    linePosition: {
       type: String,
       required: false,
       default: '',
@@ -81,7 +81,7 @@ export default {
         noteTargetLine: this.noteTargetLine,
         diffViewType: this.diffViewType,
         diffFile: selectedDiffFile,
-        linePosition: this.position,
+        linePosition: this.linePosition,
       });
 
       this.saveNote(postData)

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

@@ -92,7 +92,7 @@ export default {
         :diff-file-hash="diffFileHash"
         :line="line.left"
         :note-target-line="line.left"
-        position="left"
+        line-position="left"
       />
     </td>
     <td class="notes_line new"></td>
@@ -111,7 +111,7 @@ export default {
         :diff-file-hash="diffFileHash"
         :line="line.right"
         :note-target-line="line.right"
-        position="right"
+        line-position="right"
       />
     </td>
   </tr>

app/assets/javascripts/diffs/constants.js

@@ -7,6 +7,7 @@ export const CONTEXT_LINE_TYPE = 'context';
 export const EMPTY_CELL_TYPE = 'empty-cell';
 export const COMMENT_FORM_TYPE = 'commentForm';
 export const DIFF_NOTE_TYPE = 'DiffNote';
+export const LEGACY_DIFF_NOTE_TYPE = 'LegacyDiffNote';
 export const NOTE_TYPE = 'Note';
 export const NEW_LINE_TYPE = 'new';
 export const OLD_LINE_TYPE = 'old';

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

@@ -90,16 +90,18 @@ export default {
     const firstDiscussion = discussions[0];
     const isDiffDiscussion = firstDiscussion.diff_discussion;
     const hasLineCode = firstDiscussion.line_code;
-    const isResolvable = firstDiscussion.resolvable;
     const diffPosition = diffPositionByLineCode[firstDiscussion.line_code];
 
     if (
       selectedFile &&
       isDiffDiscussion &&
       hasLineCode &&
-      isResolvable &&
       diffPosition &&
-      isDiscussionApplicableToLine(firstDiscussion, diffPosition)
+      isDiscussionApplicableToLine({
+        discussion: firstDiscussion,
+        diffPosition,
+        latestDiff: state.latestDiff,
+      })
     ) {
       const targetLine = selectedFile.parallelDiffLines.find(
         line =>

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

@@ -4,6 +4,7 @@ import {
   LINE_POSITION_LEFT,
   LINE_POSITION_RIGHT,
   TEXT_DIFF_POSITION_TYPE,
+  LEGACY_DIFF_NOTE_TYPE,
   DIFF_NOTE_TYPE,
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
@@ -60,7 +61,10 @@ export function getNoteFormData(params) {
       noteable_type: noteableType,
       noteable_id: noteableData.id,
       commit_id: '',
-      type: DIFF_NOTE_TYPE,
+      type:
+        diffFile.diffRefs.startSha && diffFile.diffRefs.headSha
+          ? DIFF_NOTE_TYPE
+          : LEGACY_DIFF_NOTE_TYPE,
       line_code: noteTargetLine.lineCode,
     },
   };
@@ -230,7 +234,16 @@ export function getDiffPositionByLineCode(diffFiles) {
         const { lineCode, oldLine, newLine } = line;
 
         if (lineCode) {
-          acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine };
+          acc[lineCode] = {
+            baseSha,
+            headSha,
+            startSha,
+            newPath,
+            oldPath,
+            oldLine,
+            newLine,
+            lineCode,
+          };
         }
       });
     }
@@ -241,9 +254,15 @@ export function getDiffPositionByLineCode(diffFiles) {
 
 // This method will check whether the discussion is still applicable
 // to the diff line in question regarding different versions of the MR
-export function isDiscussionApplicableToLine(discussion, diffPosition) {
-  const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter);
-  const refs = convertObjectPropsToCamelCase(discussion.position.formatter);
+export function isDiscussionApplicableToLine({ discussion, diffPosition, latestDiff }) {
+  const { lineCode, ...diffPositionCopy } = diffPosition;
 
-  return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition);
+  if (discussion.original_position && discussion.position) {
+    const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter);
+    const refs = convertObjectPropsToCamelCase(discussion.position.formatter);
+
+    return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy);
+  }
+
+  return latestDiff && discussion.active && lineCode === discussion.line_code;
 }

app/assets/javascripts/notes/stores/utils.js

@@ -27,7 +27,7 @@ export const getQuickActionText = note => {
 
 export const reduceDiscussionsToLineCodes = selectedDiscussions =>
   selectedDiscussions.reduce((acc, note) => {
-    if (note.diff_discussion && note.line_code && note.resolvable) {
+    if (note.diff_discussion && note.line_code) {
       // For context about line notes: there might be multiple notes with the same line code
       const items = acc[note.line_code] || [];
       items.push(note);

app/models/diff_note.rb

@@ -131,7 +131,7 @@ class DiffNote < Note
       # As an extra benefit, the returned `diff_file` already
       # has `highlighted_diff_lines` data set from Redis on
       # `Diff::FileCollection::MergeRequestDiff`.
-      noteable.diffs(paths: original_position.paths, expanded: true).diff_files.first
+      noteable.diffs(original_position.diff_options).diff_files.first
     else
       original_position.diff_file(self.project.repository)
     end

app/services/merge_requests/reload_diffs_service.rb

@@ -31,7 +31,7 @@ module MergeRequests
     def clear_cache(new_diff)
       # Executing the iteration we cache highlighted diffs for each diff file of
       # MergeRequestDiff.
-      new_diff.diffs_collection.write_cache
+      cacheable_collection(new_diff).write_cache
 
       # Remove cache for all diffs on this MR. Do not use the association on the
       # model, as that will interfere with other actions happening when
@@ -39,9 +39,15 @@ module MergeRequests
       MergeRequestDiff.where(merge_request: merge_request).each do |merge_request_diff|
         next if merge_request_diff == new_diff
 
-        merge_request_diff.diffs_collection.clear_cache
+        cacheable_collection(merge_request_diff).clear_cache
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord
+
+    def cacheable_collection(diff)
+      # There are scenarios where we don't need to request Diff Stats.
+      # Mainly when clearing / writing diff caches.
+      diff.diffs(include_stats: false)
+    end
   end
 end

app/workers/new_merge_request_worker.rb

@@ -10,7 +10,7 @@ class NewMergeRequestWorker
     EventCreateService.new.open_mr(issuable, user)
     NotificationService.new.new_merge_request(issuable, user)
 
-    issuable.diffs.write_cache
+    issuable.diffs(include_stats: false).write_cache
     issuable.create_cross_references!(user)
   end
 

changelogs/unreleased/force-post-migration-dir-schema-load.yml

+---
+title: Ensure the schema is loaded with post_migrations included
+merge_request: 21689
+author:
+type: changed

changelogs/unreleased/mr-legacy-diff-notes.yml

+---
+title: Correctly show legacy diff notes in the merge request changes tab
+merge_request: 21652
+author:
+type: fixed

changelogs/unreleased/osw-use-diff-stats-rpc-on-comparison-views.yml

+---
+title: Use stats RPC when comparing diffs
+merge_request: 21778
+author:
+type: fixed

config/initializers/0_post_deployment_migrations.rb

 # Post deployment migrations are included by default. This file must be loaded
 # before other initializers as Rails may otherwise memoize a list of migrations
 # excluding the post deployment migrations.
+<<<<<<< HEAD
 unless ENV['SKIP_POST_DEPLOYMENT_MIGRATIONS']
   Rails.application.config.paths['db'].each do |db_path|
     path = Rails.root.join(db_path, 'post_migrate').to_s
@@ -21,3 +22,6 @@ migrate_paths.each do |migrate_path|
   Rails.application.config.paths['db/migrate'] << ee_migrate_path.to_s
   ActiveRecord::Migrator.migrations_paths << ee_migrate_path.to_s
 end
+=======
+Gitlab::Database.add_post_migrate_path_to_rails
+>>>>>>> upstream/master

lib/gitlab/database.rb

@@ -261,5 +261,21 @@ module Gitlab
     end
 
     private_class_method :database_version
+
+    def self.add_post_migrate_path_to_rails(force: false)
+      return if ENV['SKIP_POST_DEPLOYMENT_MIGRATIONS'] && !force
+
+      Rails.application.config.paths['db'].each do |db_path|
+        path = Rails.root.join(db_path, 'post_migrate').to_s
+
+        unless Rails.application.config.paths['db/migrate'].include? path
+          Rails.application.config.paths['db/migrate'] << path
+
+          # Rails memoizes migrations at certain points where it won't read the above
+          # path just yet. As such we must also update the following list of paths.
+          ActiveRecord::Migrator.migrations_paths << path
+        end
+      end
+    end
   end
 end

lib/gitlab/diff/file.rb

@@ -20,8 +20,9 @@ module Gitlab
         DiffViewer::Image
       ].sort_by { |v| v.binary? ? 0 : 1 }.freeze
 
-      def initialize(diff, repository:, diff_refs: nil, fallback_diff_refs: nil)
+      def initialize(diff, repository:, diff_refs: nil, fallback_diff_refs: nil, stats: nil)
         @diff = diff
+        @stats = stats
         @repository = repository
         @diff_refs = diff_refs
         @fallback_diff_refs = fallback_diff_refs
@@ -165,11 +166,11 @@ module Gitlab
       end
 
       def added_lines
-        diff_lines.count(&:added?)
+        @stats&.additions || diff_lines.count(&:added?)
       end
 
       def removed_lines
-        diff_lines.count(&:removed?)
+        @stats&.deletions || diff_lines.count(&:removed?)
       end
 
       def file_identifier

lib/gitlab/diff/file_collection/base.rb

@@ -2,23 +2,27 @@ module Gitlab
   module Diff
     module FileCollection
       class Base
+        include Gitlab::Utils::StrongMemoize
+
         attr_reader :project, :diff_options, :diff_refs, :fallback_diff_refs, :diffable
 
         delegate :count, :size, :real_size, to: :diff_files
 
         def self.default_options
-          ::Commit.max_diff_options.merge(ignore_whitespace_change: false, expanded: false)
+          ::Commit.max_diff_options.merge(ignore_whitespace_change: false, expanded: false, include_stats: true)
         end
 
         def initialize(diffable, project:, diff_options: nil, diff_refs: nil, fallback_diff_refs: nil)
           diff_options = self.class.default_options.merge(diff_options || {})
 
           @diffable = diffable
+          @include_stats = diff_options.delete(:include_stats)
           @diffs = diffable.raw_diffs(diff_options)
           @project = project
           @diff_options = diff_options
           @diff_refs = diff_refs
           @fallback_diff_refs = fallback_diff_refs
+          @repository = project.repository
         end
 
         def diff_files
@@ -43,10 +47,27 @@ module Gitlab
 
         private
 
+        def diff_stats_collection
+          strong_memoize(:diff_stats) do
+            # There are scenarios where we don't need to request Diff Stats,
+            # when caching for instance.
+            next unless @include_stats
+            next unless diff_refs
+
+            @repository.diff_stats(diff_refs.base_sha, diff_refs.head_sha)
+          end
+        end
+
         def decorate_diff!(diff)
           return diff if diff.is_a?(File)
 
-          Gitlab::Diff::File.new(diff, repository: project.repository, diff_refs: diff_refs, fallback_diff_refs: fallback_diff_refs)
+          stats = diff_stats_collection&.find_by_path(diff.new_path)
+
+          Gitlab::Diff::File.new(diff,
+                                 repository: project.repository,
+                                 diff_refs: diff_refs,
+                                 fallback_diff_refs: fallback_diff_refs,
+                                 stats: stats)
         end
       end
     end

lib/gitlab/diff/position.rb

@@ -116,6 +116,10 @@ module Gitlab
         end
       end
 
+      def diff_options
+        { paths: paths, expanded: true, include_stats: false }
+      end
+
       def diff_line(repository)
         @diff_line ||= diff_file(repository)&.line_for_position(self)
       end
@@ -130,7 +134,7 @@ module Gitlab
         return unless diff_refs.complete?
         return unless comparison = diff_refs.compare_in(repository.project)
 
-        comparison.diffs(paths: paths, expanded: true).diff_files.first
+        comparison.diffs(diff_options).diff_files.first
       end
 
       def get_formatter_class(type)

lib/gitlab/git/diff_stats_collection.rb

@@ -3,6 +3,7 @@
 module Gitlab
   module Git
     class DiffStatsCollection
+      include Gitlab::Utils::StrongMemoize
       include Enumerable
 
       def initialize(diff_stats)
@@ -12,6 +13,18 @@ module Gitlab
       def each(&block)
         @collection.each(&block)
       end
+
+      def find_by_path(path)
+        indexed_by_path[path]
+      end
+
+      private
+
+      def indexed_by_path
+        strong_memoize(:indexed_by_path) do
+          index_by { |stats| stats.path }
+        end
+      end
     end
   end
 end

lib/gitlab/git/repository.rb

@@ -444,7 +444,7 @@ module Gitlab
         end
 
         Gitlab::Git::DiffStatsCollection.new(stats)
-      rescue CommandError
+      rescue CommandError, TypeError
         Gitlab::Git::DiffStatsCollection.new([])
       end
 

lib/tasks/gitlab/db.rake

@@ -51,6 +51,8 @@ namespace :gitlab do
       if ActiveRecord::Base.connection.tables.count > 1
         Rake::Task['db:migrate'].invoke
       else
+        # Add post-migrate paths to ensure we mark all migrations as up
+        Gitlab::Database.add_post_migrate_path_to_rails(force: true)
         Rake::Task['db:schema:load'].invoke
         Rake::Task['db:seed_fu'].invoke
       end

locale/gitlab.pot

@@ -771,6 +771,7 @@ msgstr ""
 msgid "Artifacts"
 msgstr ""
 
+<<<<<<< HEAD
 msgid "Ascending"
 msgstr ""
 
@@ -778,6 +779,9 @@ msgid "Ask your group maintainer to set up a group Runner."
 msgstr ""
 
 msgid "Assertion consumer service URL"
+=======
+msgid "Ask your group maintainer to set up a group Runner."
+>>>>>>> upstream/master
 msgstr ""
 
 msgid "Assign custom color like #FF0000"
@@ -6773,9 +6777,12 @@ msgstr ""
 msgid "Set up a specific Runner automatically"
 msgstr ""
 
+<<<<<<< HEAD
 msgid "Set up assertions/attributes/claims (email, first_name, last_name) and NameID according to %{docsLinkStart}the documentation %{icon}%{docsLinkEnd}"
 msgstr ""
 
+=======
+>>>>>>> upstream/master
 msgid "Set up your project to automatically push and/or pull changes to/from another repository. Branches, tags, and commits will be synced automatically."
 msgstr ""
 
@@ -8432,9 +8439,12 @@ msgid "You can resolve the merge conflict using either the Interactive mode, by
 msgstr ""
 
 msgid "You can set up jobs to only use Runners with specific tags. Separate tags with commas."
+<<<<<<< HEAD
 msgstr ""
 
 msgid "You cannot write to a read-only secondary GitLab Geo instance. Please use %{link_to_primary_node} instead."
+=======
+>>>>>>> upstream/master
 msgstr ""
 
 msgid "You cannot write to this read-only GitLab instance."

rubocop/cop/ruby_interpolation_in_translation.rb

@@ -6,7 +6,6 @@ module RuboCop
       MSG = "Don't use ruby interpolation \#{} inside translated strings, instead use \%{}"
 
       TRANSLATION_METHODS = ':_ :s_ :N_ :n_'
-      RUBY_INTERPOLATION_REGEX = /.*\#\{.*\}/
 
       def_node_matcher :translation_method?, <<~PATTERN
         (send nil? {#{TRANSLATION_METHODS}} $dstr ...)

spec/features/merge_request/user_posts_diff_notes_spec.rb

@@ -200,23 +200,20 @@ describe 'Merge request > User posts diff notes', :js do
     end
 
     context 'with a new line' do
-      # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-      xit 'allows commenting' do
-        should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]').find(:xpath, '..'))
+      it 'allows commenting' do
+        should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
       end
     end
 
     context 'with an old line' do
-      # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-      xit 'allows commenting' do
-        should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]').find(:xpath, '..'))
+      it 'allows commenting' do
+        should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
       end
     end
 
     context 'with an unchanged line' do
-      # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-      xit 'allows commenting' do
-        should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]').find(:xpath, '..'))
+      it 'allows commenting' do
+        should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
       end
     end
 

spec/javascripts/diffs/store/actions_spec.js

@@ -157,6 +157,7 @@ describe('DiffsStoreActions', () => {
                   newPath: 'file1',
                   oldLine: 5,
                   oldPath: 'file2',
+                  lineCode: 'ABC_1_1',
                 },
               },
             },

spec/javascripts/diffs/store/mutations_spec.js

@@ -162,6 +162,7 @@ describe('DiffsStoreMutations', () => {
       };
 
       const state = {
+        latestDiff: true,
         diffFiles: [
           {
             fileHash: 'ABC',
@@ -229,6 +230,76 @@ describe('DiffsStoreMutations', () => {
       expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2);
       expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2);
     });
+
+    it('should add legacy discussions to the given line', () => {
+      const diffPosition = {
+        baseSha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        headSha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
+        newLine: null,
+        newPath: '500-lines-4.txt',
+        oldLine: 5,
+        oldPath: '500-lines-4.txt',
+        startSha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        lineCode: 'ABC_1',
+      };
+
+      const state = {
+        latestDiff: true,
+        diffFiles: [
+          {
+            fileHash: 'ABC',
+            parallelDiffLines: [
+              {
+                left: {
+                  lineCode: 'ABC_1',
+                  discussions: [],
+                },
+                right: {
+                  lineCode: 'ABC_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlightedDiffLines: [
+              {
+                lineCode: 'ABC_1',
+                discussions: [],
+              },
+            ],
+          },
+        ],
+      };
+      const discussions = [
+        {
+          id: 1,
+          line_code: 'ABC_1',
+          diff_discussion: true,
+          active: true,
+        },
+        {
+          id: 2,
+          line_code: 'ABC_1',
+          diff_discussion: true,
+          active: true,
+        },
+      ];
+
+      const diffPositionByLineCode = {
+        ABC_1: diffPosition,
+      };
+
+      mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
+        fileHash: 'ABC',
+        discussions,
+        diffPositionByLineCode,
+      });
+
+      expect(state.diffFiles[0].parallelDiffLines[0].left.discussions.length).toEqual(2);
+      expect(state.diffFiles[0].parallelDiffLines[0].left.discussions[1].id).toEqual(2);
+
+      expect(state.diffFiles[0].highlightedDiffLines[0].discussions.length).toEqual(2);
+      expect(state.diffFiles[0].highlightedDiffLines[0].discussions[1].id).toEqual(2);
+    });
   });
 
   describe('REMOVE_LINE_DISCUSSIONS', () => {

spec/javascripts/diffs/store/utils_spec.js

@@ -3,6 +3,7 @@ import {
   LINE_POSITION_LEFT,
   LINE_POSITION_RIGHT,
   TEXT_DIFF_POSITION_TYPE,
+  LEGACY_DIFF_NOTE_TYPE,
   DIFF_NOTE_TYPE,
   NEW_LINE_TYPE,
   OLD_LINE_TYPE,
@@ -151,6 +152,64 @@ describe('DiffsStoreUtils', () => {
         data: postData,
       });
     });
+
+    it('should create legacy note form data', () => {
+      const diffFile = getDiffFileMock();
+      delete diffFile.diffRefs.startSha;
+      delete diffFile.diffRefs.headSha;
+
+      noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE;
+
+      const options = {
+        note: 'Hello world!',
+        noteableData: noteableDataMock,
+        noteableType: MERGE_REQUEST_NOTEABLE_TYPE,
+        diffFile,
+        noteTargetLine: {
+          lineCode: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3',
+          metaData: null,
+          newLine: 3,
+          oldLine: 1,
+        },
+        diffViewType: PARALLEL_DIFF_VIEW_TYPE,
+        linePosition: LINE_POSITION_LEFT,
+      };
+
+      const position = JSON.stringify({
+        base_sha: diffFile.diffRefs.baseSha,
+        start_sha: undefined,
+        head_sha: undefined,
+        old_path: diffFile.oldPath,
+        new_path: diffFile.newPath,
+        position_type: TEXT_DIFF_POSITION_TYPE,
+        old_line: options.noteTargetLine.oldLine,
+        new_line: options.noteTargetLine.newLine,
+      });
+
+      const postData = {
+        view: options.diffViewType,
+        line_type: options.linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE,
+        merge_request_diff_head_sha: undefined,
+        in_reply_to_discussion_id: '',
+        note_project_id: '',
+        target_type: options.noteableType,
+        target_id: options.noteableData.id,
+        note: {
+          noteable_type: options.noteableType,
+          noteable_id: options.noteableData.id,
+          commit_id: '',
+          type: LEGACY_DIFF_NOTE_TYPE,
+          line_code: options.noteTargetLine.lineCode,
+          note: options.note,
+          position,
+        },
+      };
+
+      expect(utils.getNoteFormData(options)).toEqual({
+        endpoint: options.noteableData.create_note_path,
+        data: postData,
+      });
+    });
   });
 
   describe('addLineReferences', () => {
@@ -291,13 +350,72 @@ describe('DiffsStoreUtils', () => {
 
     it('returns true when the discussion is up to date', () => {
       expect(
-        utils.isDiscussionApplicableToLine(discussions.upToDateDiscussion1, diffPosition),
+        utils.isDiscussionApplicableToLine({
+          discussion: discussions.upToDateDiscussion1,
+          diffPosition,
+          latestDiff: true,
+        }),
       ).toBe(true);
     });
 
     it('returns false when the discussion is not up to date', () => {
       expect(
-        utils.isDiscussionApplicableToLine(discussions.outDatedDiscussion1, diffPosition),
+        utils.isDiscussionApplicableToLine({
+          discussion: discussions.outDatedDiscussion1,
+          diffPosition,
+          latestDiff: true,
+        }),
+      ).toBe(false);
+    });
+
+    it('returns true when line codes match and discussion does not contain position and is not active', () => {
+      const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: false };
+      delete discussion.original_position;
+      delete discussion.position;
+
+      expect(
+        utils.isDiscussionApplicableToLine({
+          discussion,
+          diffPosition: {
+            ...diffPosition,
+            lineCode: 'ABC_1',
+          },
+          latestDiff: true,
+        }),
+      ).toBe(false);
+    });
+
+    it('returns true when line codes match and discussion does not contain position and is active', () => {
+      const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: true };
+      delete discussion.original_position;
+      delete discussion.position;
+
+      expect(
+        utils.isDiscussionApplicableToLine({
+          discussion,
+          diffPosition: {
+            ...diffPosition,
+            lineCode: 'ABC_1',
+          },
+          latestDiff: true,
+        }),
+      ).toBe(true);
+    });
+
+    it('returns false when not latest diff', () => {
+      const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: true };
+      delete discussion.original_position;
+      delete discussion.position;
+
+      expect(
+        utils.isDiscussionApplicableToLine({
+          discussion,
+          diffPosition: {
+            ...diffPosition,
+            lineCode: 'ABC_1',
+          },
+          latestDiff: false,
+        }),
       ).toBe(false);
     });
   });

spec/lib/gitlab/diff/file_collection/commit_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Diff::FileCollection::Commit do
+  let(:project) { create(:project, :repository) }
+
+  it_behaves_like 'diff statistics' do
+    let(:collection_default_args) do
+      { diff_options: {} }
+    end
+    let(:diffable) { project.commit }
+    let(:stub_path) { 'bar/branch-test.txt' }
+  end
+end

spec/lib/gitlab/diff/file_collection/compare_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Diff::FileCollection::Compare do
+  include RepoHelpers
+
+  let(:project) { create(:project, :repository) }
+  let(:commit)  { project.commit }
+  let(:start_commit) { sample_image_commit }
+  let(:head_commit) { sample_commit }
+  let(:raw_compare) do
+    Gitlab::Git::Compare.new(project.repository.raw_repository,
+                             start_commit.id,
+                             head_commit.id)
+  end
+
+  it_behaves_like 'diff statistics' do
+    let(:collection_default_args) do
+      {
+        project: diffable.project,
+        diff_options: {},
+        diff_refs: diffable.diff_refs
+      }
+    end
+    let(:diffable) { Compare.new(raw_compare, project) }
+    let(:stub_path) { '.gitignore' }
+  end
+end

spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb

@@ -29,6 +29,14 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
     expect(mr_diff.cache_key).not_to eq(key)
   end
 
+  it_behaves_like 'diff statistics' do
+    let(:collection_default_args) do
+      { diff_options: {} }
+    end
+    let(:diffable) { merge_request.merge_request_diff }
+    let(:stub_path) { '.gitignore' }
+  end
+
   shared_examples 'initializes a DiffCollection' do
     it 'returns a valid instance of a DiffCollection' do
       expect(diff_files).to be_a(Gitlab::Git::DiffCollection)

spec/lib/gitlab/diff/file_spec.rb

@@ -186,6 +186,70 @@ describe Gitlab::Diff::File do
     end
   end
 
+  context 'diff file stats' do
+    let(:diff_file) do
+      described_class.new(diff,
+                          diff_refs: commit.diff_refs,
+                          repository: project.repository,
+                          stats: stats)
+    end
+
+    let(:raw_diff) do
+      <<~EOS
+        --- a/files/ruby/popen.rb
+        +++ b/files/ruby/popen.rb
+        @@ -6,12 +6,18 @@ module Popen
+
+           def popen(cmd, path=nil)
+             unless cmd.is_a?(Array)
+        -      raise "System commands must be given as an array of strings"
+        +      raise RuntimeError, "System commands must be given as an array of strings"
+        +      # foobar
+             end
+      EOS
+    end
+
+    describe '#added_lines' do
+      context 'when stats argument given' do
+        let(:stats) { double(Gitaly::DiffStats, additions: 10, deletions: 15) }
+
+        it 'returns added lines from stats' do
+          expect(diff_file.added_lines).to eq(stats.additions)
+        end
+      end
+
+      context 'when stats argument not given' do
+        let(:stats) { nil }
+
+        it 'returns added lines by parsing raw diff' do
+          allow(diff_file).to receive(:raw_diff) { raw_diff }
+
+          expect(diff_file.added_lines).to eq(2)
+        end
+      end
+    end
+
+    describe '#removed_lines' do
+      context 'when stats argument given' do
+        let(:stats) { double(Gitaly::DiffStats, additions: 10, deletions: 15) }
+
+        it 'returns removed lines from stats' do
+          expect(diff_file.removed_lines).to eq(stats.deletions)
+        end
+      end
+
+      context 'when stats argument not given' do
+        let(:stats) { nil }
+
+        it 'returns removed lines by parsing raw diff' do
+          allow(diff_file).to receive(:raw_diff) { raw_diff }
+
+          expect(diff_file.removed_lines).to eq(1)
+        end
+      end
+    end
+  end
+
   describe '#simple_viewer' do
     context 'when the file is not diffable' do
       before do

spec/lib/gitlab/git/diff_stats_collection_spec.rb

+# frozen_string_literal: true
+
+require "spec_helper"
+
+describe Gitlab::Git::DiffStatsCollection do
+  let(:stats_a) do
+    double(Gitaly::DiffStats, additions: 10, deletions: 15, path: 'foo')
+  end
+
+  let(:stats_b) do
+    double(Gitaly::DiffStats, additions: 5, deletions: 1, path: 'bar')
+  end
+
+  let(:diff_stats) { [stats_a, stats_b] }
+  let(:collection) { described_class.new(diff_stats) }
+
+  describe '.find_by_path' do
+    it 'returns stats by path when found' do
+      expect(collection.find_by_path('foo')).to eq(stats_a)
+    end
+
+    it 'returns nil when stats is not found by path' do
+      expect(collection.find_by_path('no-file')).to be_nil
+    end
+  end
+end

spec/lib/gitlab/git/repository_spec.rb

@@ -1136,6 +1136,14 @@ describe Gitlab::Git::Repository, :seed_helper do
       expect(collection).to be_a(Enumerable)
       expect(collection.to_a).to be_empty
     end
+
+    it 'returns no Gitaly::DiffStats when there is a nil SHA' do
+      collection = repository.diff_stats(nil, 'master')
+
+      expect(collection).to be_a(Gitlab::Git::DiffStatsCollection)
+      expect(collection).to be_a(Enumerable)
+      expect(collection.to_a).to be_empty
+    end
   end
 
   describe "#ls_files" do

spec/support/shared_examples/diff_file_collections.rb

+# frozen_string_literal: true
+
+shared_examples 'diff statistics' do |test_include_stats_flag: true|
+  def stub_stats_find_by_path(path, stats_mock)
+    expect_next_instance_of(Gitlab::Git::DiffStatsCollection) do |collection|
+      allow(collection).to receive(:find_by_path).and_call_original
+      expect(collection).to receive(:find_by_path).with(path).and_return(stats_mock)
+    end
+  end
+
+  context 'when should request diff stats' do
+    it 'Repository#diff_stats is called' do
+      subject = described_class.new(diffable, collection_default_args)
+
+      expect(diffable.project.repository)
+        .to receive(:diff_stats)
+        .with(diffable.diff_refs.base_sha, diffable.diff_refs.head_sha)
+        .and_call_original
+
+      subject.diff_files
+    end
+
+    it 'Gitlab::Diff::File is initialized with diff stats' do
+      subject = described_class.new(diffable, collection_default_args)
+
+      stats_mock = double(Gitaly::DiffStats, path: '.gitignore', additions: 758, deletions: 120)
+      stub_stats_find_by_path(stub_path, stats_mock)
+
+      diff_file = subject.diff_files.find { |file| file.new_path == stub_path }
+
+      expect(diff_file.added_lines).to eq(stats_mock.additions)
+      expect(diff_file.removed_lines).to eq(stats_mock.deletions)
+    end
+  end
+
+  context 'when should not request diff stats' do
+    it 'Repository#diff_stats is not called' do
+      collection_default_args[:diff_options][:include_stats] = false
+
+      subject = described_class.new(diffable, collection_default_args)
+
+      expect(diffable.project.repository).not_to receive(:diff_stats)
+
+      subject.diff_files
+    end
+  end
+end

spec/tasks/gitlab/db_rake_spec.rb

@@ -61,6 +61,39 @@ describe 'gitlab:db namespace rake task' do
       expect(Rake::Task['db:migrate']).not_to receive(:invoke)
       expect { run_rake_task('gitlab:db:configure') }.to raise_error(RuntimeError, 'error')
     end
+
+    context 'SKIP_POST_DEPLOYMENT_MIGRATIONS environment variable set' do
+      let(:rails_paths) { { 'db' => ['db'], 'db/migrate' => ['db/migrate'] } }
+
+      before do
+        allow(ENV).to receive(:[]).and_call_original
+        allow(ENV).to receive(:[]).with('SKIP_POST_DEPLOYMENT_MIGRATIONS').and_return true
+
+        # Our environment has already been loaded, so we need to pretend like post_migrations were not
+        allow(Rails.application.config).to receive(:paths).and_return(rails_paths)
+        allow(ActiveRecord::Migrator).to receive(:migrations_paths).and_return(rails_paths['db/migrate'].dup)
+      end
+
+      it 'adds post deployment migrations before schema load if the schema is not already loaded' do
+        allow(ActiveRecord::Base.connection).to receive(:tables).and_return([])
+        expect(Gitlab::Database).to receive(:add_post_migrate_path_to_rails).and_call_original
+        expect(Rake::Task['db:schema:load']).to receive(:invoke)
+        expect(Rake::Task['db:seed_fu']).to receive(:invoke)
+        expect(Rake::Task['db:migrate']).not_to receive(:invoke)
+        expect { run_rake_task('gitlab:db:configure') }.not_to raise_error
+        expect(rails_paths['db/migrate'].include?(File.join(Rails.root, 'db', 'post_migrate'))).to be(true)
+      end
+
+      it 'ignores post deployment migrations  when schema has already been loaded' do
+        allow(ActiveRecord::Base.connection).to receive(:tables).and_return(%w[table1 table2])
+        expect(Rake::Task['db:migrate']).to receive(:invoke)
+        expect(Gitlab::Database).not_to receive(:add_post_migrate_path_to_rails)
+        expect(Rake::Task['db:schema:load']).not_to receive(:invoke)
+        expect(Rake::Task['db:seed_fu']).not_to receive(:invoke)
+        expect { run_rake_task('gitlab:db:configure') }.not_to raise_error
+        expect(rails_paths['db/migrate'].include?(File.join(Rails.root, 'db', 'post_migrate'))).to be(false)
+      end
+    end
   end
 
   def run_rake_task(task_name)