From b4aaced71a65faffd49ffa2c705fb574ed532701 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 21 Aug 2017 17:27:06 +0200
Subject: Fix display of push events for removed refs

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
---
 app/helpers/events_helper.rb                       |  1 +
 app/models/event.rb                                |  2 +-
 app/views/events/_event_push.atom.haml             | 13 ++++---
 app/views/events/event/_push.html.haml             |  4 --
 .../unreleased/fix-push-events-branch-removals.yml |  5 +++
 spec/helpers/events_helper_spec.rb                 |  4 ++
 spec/models/event_spec.rb                          | 44 ++++++++++++++++++++++
 7 files changed, 62 insertions(+), 11 deletions(-)
 create mode 100644 changelogs/unreleased/fix-push-events-branch-removals.yml

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)
 
-- 
cgit v1.2.1