Fix display of push events for removed refsfix-push-events-branch-removals

This changes the style of push events that remove tags or branches so
they don't display the commit details. This prevents displaying commit
details such as:

    000000 . --broken encoding

Instead we now simply display the header such as:

    Administrator deleted branch example-branch

This is displayed in the same style as events for newly created
branches/tags.

This commit also ensures that if no commit message is present we simply
don't display anything, instead of "--broken encoding".

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/36685
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/36722

7 files changed, 62 insertions, 11 deletions

diff --git a/app/helpers/events_helper.rb b/app/helpers/events_helper.rb
index c6f98e7e782..b331693c789 100644
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@@ -181,6 +181,7 @@ module EventsHelper
   end
 
   def event_commit_title(message)
+    message ||= ''
     (message.split("\n").first || "").truncate(70)
   rescue
     "--broken encoding"
diff --git a/app/models/event.rb b/app/models/event.rb
index 15ee170ca75..996768a267b 100644
--- a/app/models/event.rb
+++ b/app/models/event.rb
@@ -406,7 +406,7 @@ class Event < ActiveRecord::Base
 
   def body?
     if push?
-      push_with_commits? || rm_ref?
+      push_with_commits?
     elsif note?
       true
     else
diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml
index bf655f9d21a..e3c5fd55f08 100644
--- a/app/views/events/_event_push.atom.haml
+++ b/app/views/events/_event_push.atom.haml
@@ -5,9 +5,10 @@
     %i
       at
       = event.created_at.to_s(:short)
-  %blockquote= markdown(escape_once(event.commit_title), pipeline: :atom, project: event.project, author: event.author)
-  - if event.commits_count > 1
-    %p
-      %i
-        \... and
-        = pluralize(event.commits_count - 1, "more commit")
+  - unless event.rm_ref?
+    %blockquote= markdown(escape_once(event.commit_title), pipeline: :atom, project: event.project, author: event.author)
+    - if event.commits_count > 1
+      %p
+        %i
+          \... and
+          = pluralize(event.commits_count - 1, "more commit")
diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml
index 973c652ad88..53ebdd6d2ff 100644
--- a/app/views/events/event/_push.html.haml
+++ b/app/views/events/event/_push.html.haml
@@ -41,7 +41,3 @@
         %li.commits-stat
           = link_to create_mr_path(project.default_branch, event.ref_name, project) do
             Create Merge Request
-- elsif event.rm_ref?
-  .event-body
-    %ul.well-list.event_commits
-      = render "events/commit", project: project, event: event
diff --git a/changelogs/unreleased/fix-push-events-branch-removals.yml b/changelogs/unreleased/fix-push-events-branch-removals.yml
new file mode 100644
index 00000000000..71f368db296
--- /dev/null
+++ b/changelogs/unreleased/fix-push-events-branch-removals.yml
@@ -0,0 +1,5 @@
+---
+title: Fix display of push events for removed refs
+merge_request:
+author:
+type: fixed
diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb
index 4b72dbb7964..d5536fcb22b 100644
--- a/spec/helpers/events_helper_spec.rb
+++ b/spec/helpers/events_helper_spec.rb
@@ -106,5 +106,9 @@ describe EventsHelper do
     it "handles empty strings" do
       expect(helper.event_commit_title("")).to eq("")
     end
+
+    it 'handles nil values' do
+      expect(helper.event_commit_title(nil)).to eq('')
+    end
   end
 end
diff --git a/spec/models/event_spec.rb b/spec/models/event_spec.rb
index ff3224dd298..f55c161c821 100644
--- a/spec/models/event_spec.rb
+++ b/spec/models/event_spec.rb
@@ -304,6 +304,50 @@ describe Event do
     end
   end
 
+  describe '#body?' do
+    let(:push_event) do
+      event = build(:push_event)
+
+      allow(event).to receive(:push?).and_return(true)
+
+      event
+    end
+
+    it 'returns true for a push event with commits' do
+      allow(push_event).to receive(:push_with_commits?).and_return(true)
+
+      expect(push_event).to be_body
+    end
+
+    it 'returns false for a push event without a valid commit range' do
+      allow(push_event).to receive(:push_with_commits?).and_return(false)
+
+      expect(push_event).not_to be_body
+    end
+
+    it 'returns true for a Note event' do
+      event = build(:event)
+
+      allow(event).to receive(:note?).and_return(true)
+
+      expect(event).to be_body
+    end
+
+    it 'returns true if the target responds to #title' do
+      event = build(:event)
+
+      allow(event).to receive(:target).and_return(double(:target, title: 'foo'))
+
+      expect(event).to be_body
+    end
+
+    it 'returns false for a regular event without a target' do
+      event = build(:event)
+
+      expect(event).not_to be_body
+    end
+  end
+
   def create_push_event(project, user)
     event = create(:push_event, project: project, author: user)