Fix blame view line groupings

Closes #2305

app/controllers/projects/blame_controller.rb

@@ -8,6 +8,28 @@ class Projects::BlameController < Projects::ApplicationController
 
   def show
     @blob = @repository.blob_at(@commit.id, @path)
-    @blame = Gitlab::Git::Blame.new(@repository, @commit.id, @path)
+    @blame = group_blame_lines
+  end
+
+  def group_blame_lines
+    blame = Gitlab::Git::Blame.new(@repository, @commit.id, @path)
+
+    prev_sha = nil
+    groups = []
+    current_group = nil
+
+    blame.each do |commit, line|
+      if prev_sha && prev_sha == commit.sha
+        current_group[:lines] << line
+      else
+        groups << current_group if current_group.present?
+        current_group = { commit: commit, lines: [line] }
+      end
+
+      prev_sha = commit.sha
+    end
+
+    groups << current_group if current_group.present?
+    groups
   end
 end

app/views/projects/blame/show.html.haml

@@ -13,12 +13,11 @@
     .file-content.blame.highlight
       %table
         - current_line = 1
-        - @blame.each do |raw_commit, line|
+        - @blame.each do |blame_group|
           %tr
             %td.blame-commit
               .commit
-                - unless @prev_commit && @prev_commit.sha == raw_commit.sha
-                  - commit = Commit.new(raw_commit, @project)
+                - commit = Commit.new(blame_group[:commit], @project)
                 .commit-row-title
                   %strong
                     = link_to_gfm truncate(commit.title, length: 35), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "cdark"
@@ -29,14 +28,17 @@
                   = commit_author_link(commit, avatar: false)
                   authored
                   #{time_ago_with_tooltip(commit.committed_date)}
-                - @prev_commit = raw_commit
             %td.lines.blame-numbers
               %pre
-                = current_line
-                - current_line += 1
+                - line_count = blame_group[:lines].count
+                - (current_line...(current_line + line_count)).each do |i|
+                  = i
+                  \
+                - current_line += line_count
             %td.lines
               %pre{class: 'code highlight white'}
                 %code
+                  - blame_group[:lines].each do |line|
                     :erb
                       <%= highlight(@blob.name, line, nowrap: true, continue: true).html_safe %>
 

spec/controllers/blame_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::BlameController do
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+
+  before do
+    sign_in(user)
+
+    project.team << [user, :master]
+    controller.instance_variable_set(:@project, project)
+  end
+
+  describe "GET show" do
+    render_views
+
+    before do
+      get(:show,
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          id: id)
+    end
+
+    context "valid file" do
+      let(:id) { 'master/files/ruby/popen.rb' }
+      it { is_expected.to respond_with(:success) }
+
+      it 'groups blames properly' do
+        blame = assigns(:blame)
+        # Sanity check a few items
+        expect(blame.count).to eq(18)
+        expect(blame[0][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
+        expect(blame[0][:lines]).to eq(["require 'fileutils'", "require 'open3'", ""])
+
+        expect(blame[1][:commit].sha).to eq('874797c3a73b60d2187ed6e2fcabd289ff75171e')
+        expect(blame[1][:lines]).to eq(["module Popen", "  extend self"])
+
+        expect(blame[-1][:commit].sha).to eq('913c66a37b4a45b9769037c55c2d238bd0942d2e')
+        expect(blame[-1][:lines]).to eq(["  end", "end"])
+      end
+    end
+  end
+end