Commit 48f741eed9f3676a8d0d089d7336fcffd31ebaa7
1 parent
d1d13856
Exists in
spb-stable
and in
3 other branches
MergeRequests services
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Showing
6 changed files
with
90 additions
and
31 deletions
Show diff stats
app/models/merge_request.rb
| @@ -50,17 +50,26 @@ class MergeRequest < ActiveRecord::Base | @@ -50,17 +50,26 @@ class MergeRequest < ActiveRecord::Base | ||
| 50 | end | 50 | end |
| 51 | 51 | ||
| 52 | event :merge do | 52 | event :merge do |
| 53 | - transition [:reopened, :opened] => :merged | 53 | + transition [:reopened, :opened, :locked] => :merged |
| 54 | end | 54 | end |
| 55 | 55 | ||
| 56 | event :reopen do | 56 | event :reopen do |
| 57 | transition closed: :reopened | 57 | transition closed: :reopened |
| 58 | end | 58 | end |
| 59 | 59 | ||
| 60 | + event :lock do | ||
| 61 | + transition [:reopened, :opened] => :locked | ||
| 62 | + end | ||
| 63 | + | ||
| 64 | + event :unlock do | ||
| 65 | + transition locked: :reopened | ||
| 66 | + end | ||
| 67 | + | ||
| 60 | state :opened | 68 | state :opened |
| 61 | state :reopened | 69 | state :reopened |
| 62 | state :closed | 70 | state :closed |
| 63 | state :merged | 71 | state :merged |
| 72 | + state :locked | ||
| 64 | end | 73 | end |
| 65 | 74 | ||
| 66 | state_machine :merge_status, initial: :unchecked do | 75 | state_machine :merge_status, initial: :unchecked do |
| @@ -136,19 +145,8 @@ class MergeRequest < ActiveRecord::Base | @@ -136,19 +145,8 @@ class MergeRequest < ActiveRecord::Base | ||
| 136 | self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last | 145 | self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last |
| 137 | end | 146 | end |
| 138 | 147 | ||
| 139 | - def merge!(user_id) | ||
| 140 | - self.author_id_of_changes = user_id | ||
| 141 | - self.merge | ||
| 142 | - end | ||
| 143 | - | ||
| 144 | def automerge!(current_user, commit_message = nil) | 148 | def automerge!(current_user, commit_message = nil) |
| 145 | - if Gitlab::Satellite::MergeAction.new(current_user, self).merge!(commit_message) | ||
| 146 | - self.merge!(current_user.id) | ||
| 147 | - true | ||
| 148 | - end | ||
| 149 | - rescue | ||
| 150 | - mark_as_unmergeable | ||
| 151 | - false | 149 | + MergeRequests::AutoMergeService.new.execute(self, current_user, commit_message) |
| 152 | end | 150 | end |
| 153 | 151 | ||
| 154 | def mr_and_commit_notes | 152 | def mr_and_commit_notes |
app/models/project.rb
| @@ -350,7 +350,7 @@ class Project < ActiveRecord::Base | @@ -350,7 +350,7 @@ class Project < ActiveRecord::Base | ||
| 350 | # Close merge requests | 350 | # Close merge requests |
| 351 | mrs = self.merge_requests.opened.where(target_branch: branch_name).to_a | 351 | mrs = self.merge_requests.opened.where(target_branch: branch_name).to_a |
| 352 | mrs = mrs.select(&:last_commit).select { |mr| c_ids.include?(mr.last_commit.id) } | 352 | mrs = mrs.select(&:last_commit).select { |mr| c_ids.include?(mr.last_commit.id) } |
| 353 | - mrs.each { |merge_request| merge_request.merge!(user.id) } | 353 | + mrs.each { |merge_request| MergeRequests::MergeService.new.execute(merge_request, user, nil) } |
| 354 | 354 | ||
| 355 | true | 355 | true |
| 356 | end | 356 | end |
app/observers/merge_request_observer.rb
| @@ -18,23 +18,6 @@ class MergeRequestObserver < ActivityObserver | @@ -18,23 +18,6 @@ class MergeRequestObserver < ActivityObserver | ||
| 18 | execute_hooks(merge_request) | 18 | execute_hooks(merge_request) |
| 19 | end | 19 | end |
| 20 | 20 | ||
| 21 | - def after_merge(merge_request, transition) | ||
| 22 | - notification.merge_mr(merge_request) | ||
| 23 | - # Since MR can be merged via sidekiq | ||
| 24 | - # to prevent event duplication do this check | ||
| 25 | - return true if merge_request.merge_event | ||
| 26 | - | ||
| 27 | - Event.create( | ||
| 28 | - project: merge_request.target_project, | ||
| 29 | - target_id: merge_request.id, | ||
| 30 | - target_type: merge_request.class.name, | ||
| 31 | - action: Event::MERGED, | ||
| 32 | - author_id: merge_request.author_id_of_changes | ||
| 33 | - ) | ||
| 34 | - | ||
| 35 | - execute_hooks(merge_request) | ||
| 36 | - end | ||
| 37 | - | ||
| 38 | def after_reopen(merge_request, transition) | 21 | def after_reopen(merge_request, transition) |
| 39 | create_event(merge_request, Event::REOPENED) | 22 | create_event(merge_request, Event::REOPENED) |
| 40 | create_note(merge_request) | 23 | create_note(merge_request) |
| @@ -0,0 +1,30 @@ | @@ -0,0 +1,30 @@ | ||
| 1 | +module MergeRequests | ||
| 2 | + # AutoMergeService class | ||
| 3 | + # | ||
| 4 | + # Do git merge in satellite and in case of success | ||
| 5 | + # mark merge request as merged and execute all hooks and notifications | ||
| 6 | + # Called when you do merge via GitLab UI | ||
| 7 | + class AutoMergeService < BaseMergeService | ||
| 8 | + def execute(merge_request, current_user, commit_message) | ||
| 9 | + merge_request.lock | ||
| 10 | + | ||
| 11 | + if Gitlab::Satellite::MergeAction.new(current_user, merge_request).merge!(commit_message) | ||
| 12 | + merge_request.author_id_of_changes = current_user.id | ||
| 13 | + merge_request.merge | ||
| 14 | + | ||
| 15 | + notification.merge_mr(merge_request) | ||
| 16 | + create_merge_event(merge_request) | ||
| 17 | + execute_project_hooks(merge_request) | ||
| 18 | + | ||
| 19 | + true | ||
| 20 | + else | ||
| 21 | + merge_request.unlock | ||
| 22 | + false | ||
| 23 | + end | ||
| 24 | + rescue | ||
| 25 | + merge_request.unlock if merge_request.locked? | ||
| 26 | + merge_request.mark_as_unmergeable | ||
| 27 | + false | ||
| 28 | + end | ||
| 29 | + end | ||
| 30 | +end |
| @@ -0,0 +1,26 @@ | @@ -0,0 +1,26 @@ | ||
| 1 | +module MergeRequests | ||
| 2 | + class BaseMergeService | ||
| 3 | + | ||
| 4 | + private | ||
| 5 | + | ||
| 6 | + def notification | ||
| 7 | + NotificationService.new | ||
| 8 | + end | ||
| 9 | + | ||
| 10 | + def create_merge_event(merge_request) | ||
| 11 | + Event.create( | ||
| 12 | + project: merge_request.target_project, | ||
| 13 | + target_id: merge_request.id, | ||
| 14 | + target_type: merge_request.class.name, | ||
| 15 | + action: Event::MERGED, | ||
| 16 | + author_id: merge_request.author_id_of_changes | ||
| 17 | + ) | ||
| 18 | + end | ||
| 19 | + | ||
| 20 | + def execute_project_hooks(merge_request) | ||
| 21 | + if merge_request.project | ||
| 22 | + merge_request.project.execute_hooks(merge_request.to_hook_data, :merge_request_hooks) | ||
| 23 | + end | ||
| 24 | + end | ||
| 25 | + end | ||
| 26 | +end |
| @@ -0,0 +1,22 @@ | @@ -0,0 +1,22 @@ | ||
| 1 | +module MergeRequests | ||
| 2 | + # MergeService class | ||
| 3 | + # | ||
| 4 | + # Mark existing merge request as merged | ||
| 5 | + # and execute all hooks and notifications | ||
| 6 | + # Called when you do merge via command line and push code | ||
| 7 | + # to target branch | ||
| 8 | + class MergeService < BaseMergeService | ||
| 9 | + def execute(merge_request, current_user, commit_message) | ||
| 10 | + merge_request.author_id_of_changes = current_user.id | ||
| 11 | + merge_request.merge | ||
| 12 | + | ||
| 13 | + notification.merge_mr(merge_request) | ||
| 14 | + create_merge_event(merge_request) | ||
| 15 | + execute_project_hooks(merge_request) | ||
| 16 | + | ||
| 17 | + true | ||
| 18 | + rescue | ||
| 19 | + false | ||
| 20 | + end | ||
| 21 | + end | ||
| 22 | +end |