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.
This commit is contained in:
Drew Blessing 2017-12-04 13:00:57 -06:00
parent 4b99bee096
commit 38e9e934f4
5 changed files with 47 additions and 6 deletions

View file

@ -46,10 +46,11 @@ class PushEvent < Event
# Returns PushEvent instances for which no merge requests have been created. # Returns PushEvent instances for which no merge requests have been created.
def self.without_existing_merge_requests def self.without_existing_merge_requests
existing_mrs = MergeRequest.except(:order) existing_mrs = MergeRequest.except(:order, :where)
.select(1) .select(1)
.where('merge_requests.source_project_id = events.project_id') .where('merge_requests.source_project_id = events.project_id')
.where('merge_requests.source_branch = push_event_payloads.ref') .where('merge_requests.source_branch = push_event_payloads.ref')
.where(state: :opened)
# For reasons unknown the use of #eager_load will result in the # For reasons unknown the use of #eager_load will result in the
# "push_event_payload" association not being set. Because of this we're # "push_event_payload" association not being set. Because of this we're

View file

@ -0,0 +1,5 @@
---
title: Last push widget will show banner for new pushes to previously merged branch
merge_request:
author:
type: changed

View file

@ -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

View file

@ -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", ["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", ["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_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", ["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_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 add_index "merge_requests", ["target_project_id", "iid"], name: "index_merge_requests_on_target_project_id_and_iid", unique: true, using: :btree

View file

@ -63,12 +63,14 @@ describe PushEvent do
let(:event2) { create(:push_event, project: project) } let(:event2) { create(:push_event, project: project) }
let(:event3) { create(:push_event, project: project) } let(:event3) { create(:push_event, project: project) }
let(:event4) { create(:push_event, project: project) } let(:event4) { create(:push_event, project: project) }
let(:event5) { create(:push_event, project: project) }
before do before do
create(:push_event_payload, event: event1, ref: 'foo', action: :created) 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: event2, ref: 'bar', action: :created)
create(:push_event_payload, event: event3, ref: 'baz', action: :removed) create(:push_event_payload, event: event3, ref: 'qux', action: :created)
create(:push_event_payload, event: event4, ref: 'baz', ref_type: :tag) 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') project.repository.create_branch('bar', 'master')
@ -78,6 +80,16 @@ describe PushEvent do
target_project: project, target_project: project,
source_branch: 'bar' source_branch: 'bar'
) )
project.repository.create_branch('qux', 'master')
create(
:merge_request,
:closed,
source_project: project,
target_project: project,
source_branch: 'qux'
)
end end
let(:relation) { described_class.without_existing_merge_requests } let(:relation) { described_class.without_existing_merge_requests }
@ -86,16 +98,20 @@ describe PushEvent do
expect(relation).to include(event1) expect(relation).to include(event1)
end 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) expect(relation).not_to include(event2)
end 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 it 'does not include events for removed refs' do
expect(relation).not_to include(event3) expect(relation).not_to include(event4)
end end
it 'does not include events for pushing to tags' do it 'does not include events for pushing to tags' do
expect(relation).not_to include(event4) expect(relation).not_to include(event5)
end end
end end