From 38e9e934f49115021614b66353bbac459b6b70bb Mon Sep 17 00:00:00 2001
From: Drew Blessing <drew@gitlab.com>
Date: Mon, 4 Dec 2017 13:00:57 -0600
Subject: [PATCH] Last push widget will show banner for new pushes to
 previously merged branch

Previously, the last push widget would only show when the branch never had
a merge request associated with it - even merged or closed ones. Now the
widget will disregard merge requests that are merged or closed.
---
 app/models/push_event.rb                      |  3 ++-
 ...es-not-appear-after-pushing-new-commit.yml |  5 ++++
 ...207185153_add_merge_request_state_index.rb | 18 +++++++++++++
 db/schema.rb                                  |  1 +
 spec/models/push_event_spec.rb                | 26 +++++++++++++++----
 5 files changed, 47 insertions(+), 6 deletions(-)
 create mode 100644 changelogs/unreleased/40818-last-push-widget-does-not-appear-after-pushing-new-commit.yml
 create mode 100644 db/migrate/20171207185153_add_merge_request_state_index.rb

diff --git a/app/models/push_event.rb b/app/models/push_event.rb
index 83ce901409..90c085c888 100644
--- a/app/models/push_event.rb
+++ b/app/models/push_event.rb
@@ -46,10 +46,11 @@ class PushEvent < Event
 
   # Returns PushEvent instances for which no merge requests have been created.
   def self.without_existing_merge_requests
-    existing_mrs = MergeRequest.except(:order)
+    existing_mrs = MergeRequest.except(:order, :where)
       .select(1)
       .where('merge_requests.source_project_id = events.project_id')
       .where('merge_requests.source_branch = push_event_payloads.ref')
+      .where(state: :opened)
 
     # For reasons unknown the use of #eager_load will result in the
     # "push_event_payload" association not being set. Because of this we're
diff --git a/changelogs/unreleased/40818-last-push-widget-does-not-appear-after-pushing-new-commit.yml b/changelogs/unreleased/40818-last-push-widget-does-not-appear-after-pushing-new-commit.yml
new file mode 100644
index 0000000000..c57caf31d1
--- /dev/null
+++ b/changelogs/unreleased/40818-last-push-widget-does-not-appear-after-pushing-new-commit.yml
@@ -0,0 +1,5 @@
+---
+title: Last push widget will show banner for new pushes to previously merged branch
+merge_request:
+author:
+type: changed
diff --git a/db/migrate/20171207185153_add_merge_request_state_index.rb b/db/migrate/20171207185153_add_merge_request_state_index.rb
new file mode 100644
index 0000000000..72f846c5c3
--- /dev/null
+++ b/db/migrate/20171207185153_add_merge_request_state_index.rb
@@ -0,0 +1,18 @@
+class AddMergeRequestStateIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :merge_requests, [:source_project_id, :source_branch],
+                         where: "state = 'opened'",
+                         name: 'index_merge_requests_on_source_project_and_branch_state_opened'
+  end
+
+  def down
+    remove_concurrent_index_by_name :merge_requests,
+                                    'index_merge_requests_on_source_project_and_branch_state_opened'
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f47accca21..a32d20b8f2 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1109,6 +1109,7 @@ ActiveRecord::Schema.define(version: 20180105212544) do
   add_index "merge_requests", ["merge_user_id"], name: "index_merge_requests_on_merge_user_id", where: "(merge_user_id IS NOT NULL)", using: :btree
   add_index "merge_requests", ["milestone_id"], name: "index_merge_requests_on_milestone_id", using: :btree
   add_index "merge_requests", ["source_branch"], name: "index_merge_requests_on_source_branch", using: :btree
+  add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_and_branch_state_opened", where: "((state)::text = 'opened'::text)", using: :btree
   add_index "merge_requests", ["source_project_id", "source_branch"], name: "index_merge_requests_on_source_project_id_and_source_branch", using: :btree
   add_index "merge_requests", ["target_branch"], name: "index_merge_requests_on_target_branch", using: :btree
   add_index "merge_requests", ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid", unique: true, using: :btree
diff --git a/spec/models/push_event_spec.rb b/spec/models/push_event_spec.rb
index ad3c3a406d..bfe7a30b96 100644
--- a/spec/models/push_event_spec.rb
+++ b/spec/models/push_event_spec.rb
@@ -63,12 +63,14 @@ describe PushEvent do
     let(:event2) { create(:push_event, project: project) }
     let(:event3) { create(:push_event, project: project) }
     let(:event4) { create(:push_event, project: project) }
+    let(:event5) { create(:push_event, project: project) }
 
     before do
       create(:push_event_payload, event: event1, ref: 'foo', action: :created)
       create(:push_event_payload, event: event2, ref: 'bar', action: :created)
-      create(:push_event_payload, event: event3, ref: 'baz', action: :removed)
-      create(:push_event_payload, event: event4, ref: 'baz', ref_type: :tag)
+      create(:push_event_payload, event: event3, ref: 'qux', action: :created)
+      create(:push_event_payload, event: event4, ref: 'baz', action: :removed)
+      create(:push_event_payload, event: event5, ref: 'baz', ref_type: :tag)
 
       project.repository.create_branch('bar', 'master')
 
@@ -78,6 +80,16 @@ describe PushEvent do
         target_project: project,
         source_branch: 'bar'
       )
+
+      project.repository.create_branch('qux', 'master')
+
+      create(
+        :merge_request,
+        :closed,
+        source_project: project,
+        target_project: project,
+        source_branch: 'qux'
+      )
     end
 
     let(:relation) { described_class.without_existing_merge_requests }
@@ -86,16 +98,20 @@ describe PushEvent do
       expect(relation).to include(event1)
     end
 
-    it 'does not include events that have a corresponding merge request' do
+    it 'does not include events that have a corresponding open merge request' do
       expect(relation).not_to include(event2)
     end
 
+    it 'includes events that has corresponding closed/merged merge requests' do
+      expect(relation).to include(event3)
+    end
+
     it 'does not include events for removed refs' do
-      expect(relation).not_to include(event3)
+      expect(relation).not_to include(event4)
     end
 
     it 'does not include events for pushing to tags' do
-      expect(relation).not_to include(event4)
+      expect(relation).not_to include(event5)
     end
   end
 
-- 
2.30.9