Commit e2d94e0719cb4eed4757f4cef946b1c29ef971f0
1 parent
1b7b17d1
Exists in
master
and in
4 other branches
State machine added for merge_status field
Showing
3 changed files
with
35 additions
and
36 deletions
Show diff stats
app/models/merge_request.rb
| @@ -53,16 +53,32 @@ class MergeRequest < ActiveRecord::Base | @@ -53,16 +53,32 @@ class MergeRequest < ActiveRecord::Base | ||
| 53 | 53 | ||
| 54 | BROKEN_DIFF = "--broken-diff" | 54 | BROKEN_DIFF = "--broken-diff" |
| 55 | 55 | ||
| 56 | - UNCHECKED = 1 | ||
| 57 | - CAN_BE_MERGED = 2 | ||
| 58 | - CANNOT_BE_MERGED = 3 | 56 | + state_machine :merge_status, initial: :unchecked do |
| 57 | + event :mark_as_unchecked do | ||
| 58 | + transition [:can_be_merged, :cannot_be_merged] => :unchecked | ||
| 59 | + end | ||
| 60 | + | ||
| 61 | + event :mark_as_mergeable do | ||
| 62 | + transition unchecked: :can_be_merged | ||
| 63 | + end | ||
| 64 | + | ||
| 65 | + event :mark_as_unmergeable do | ||
| 66 | + transition unchecked: :cannot_be_merged | ||
| 67 | + end | ||
| 68 | + | ||
| 69 | + state :unchecked | ||
| 70 | + | ||
| 71 | + state :can_be_merged | ||
| 72 | + | ||
| 73 | + state :cannot_be_merged | ||
| 74 | + end | ||
| 59 | 75 | ||
| 60 | serialize :st_commits | 76 | serialize :st_commits |
| 61 | serialize :st_diffs | 77 | serialize :st_diffs |
| 62 | 78 | ||
| 63 | validates :source_branch, presence: true | 79 | validates :source_branch, presence: true |
| 64 | validates :target_branch, presence: true | 80 | validates :target_branch, presence: true |
| 65 | - validate :validate_branches | 81 | + validate :validate_branches |
| 66 | 82 | ||
| 67 | scope :merged, -> { with_state(:merged) } | 83 | scope :merged, -> { with_state(:merged) } |
| 68 | 84 | ||
| @@ -84,13 +100,9 @@ class MergeRequest < ActiveRecord::Base | @@ -84,13 +100,9 @@ class MergeRequest < ActiveRecord::Base | ||
| 84 | end | 100 | end |
| 85 | end | 101 | end |
| 86 | 102 | ||
| 103 | + # DEPRECATED: Please use human_merge_status_name instead | ||
| 87 | def human_merge_status | 104 | def human_merge_status |
| 88 | - merge_statuses = { | ||
| 89 | - CAN_BE_MERGED => "can_be_merged", | ||
| 90 | - CANNOT_BE_MERGED => "cannot_be_merged", | ||
| 91 | - UNCHECKED => "unchecked" | ||
| 92 | - } | ||
| 93 | - merge_statuses[self.merge_status] | 105 | + human_merge_status_name |
| 94 | end | 106 | end |
| 95 | 107 | ||
| 96 | def validate_branches | 108 | def validate_branches |
| @@ -104,26 +116,12 @@ class MergeRequest < ActiveRecord::Base | @@ -104,26 +116,12 @@ class MergeRequest < ActiveRecord::Base | ||
| 104 | self.reloaded_diffs | 116 | self.reloaded_diffs |
| 105 | end | 117 | end |
| 106 | 118 | ||
| 107 | - def unchecked? | ||
| 108 | - merge_status == UNCHECKED | ||
| 109 | - end | ||
| 110 | - | ||
| 111 | - def mark_as_unchecked | ||
| 112 | - self.merge_status = UNCHECKED | ||
| 113 | - self.save | ||
| 114 | - end | ||
| 115 | - | ||
| 116 | - def can_be_merged? | ||
| 117 | - merge_status == CAN_BE_MERGED | ||
| 118 | - end | ||
| 119 | - | ||
| 120 | def check_if_can_be_merged | 119 | def check_if_can_be_merged |
| 121 | - self.merge_status = if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? | ||
| 122 | - CAN_BE_MERGED | ||
| 123 | - else | ||
| 124 | - CANNOT_BE_MERGED | ||
| 125 | - end | ||
| 126 | - self.save | 120 | + if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? |
| 121 | + mark_as_mergeable | ||
| 122 | + else | ||
| 123 | + mark_as_unmergeable | ||
| 124 | + end | ||
| 127 | end | 125 | end |
| 128 | 126 | ||
| 129 | def diffs | 127 | def diffs |
| @@ -178,11 +176,6 @@ class MergeRequest < ActiveRecord::Base | @@ -178,11 +176,6 @@ class MergeRequest < ActiveRecord::Base | ||
| 178 | commits.any? && opened? | 176 | commits.any? && opened? |
| 179 | end | 177 | end |
| 180 | 178 | ||
| 181 | - def mark_as_unmergable | ||
| 182 | - self.merge_status = CANNOT_BE_MERGED | ||
| 183 | - self.save | ||
| 184 | - end | ||
| 185 | - | ||
| 186 | def reloaded_commits | 179 | def reloaded_commits |
| 187 | if opened? && unmerged_commits.any? | 180 | if opened? && unmerged_commits.any? |
| 188 | self.st_commits = unmerged_commits | 181 | self.st_commits = unmerged_commits |
| @@ -217,7 +210,7 @@ class MergeRequest < ActiveRecord::Base | @@ -217,7 +210,7 @@ class MergeRequest < ActiveRecord::Base | ||
| 217 | true | 210 | true |
| 218 | end | 211 | end |
| 219 | rescue | 212 | rescue |
| 220 | - self.mark_as_unmergable | 213 | + mark_as_unmergeable |
| 221 | false | 214 | false |
| 222 | end | 215 | end |
| 223 | 216 |
app/views/merge_requests/_show.html.haml
| @@ -29,7 +29,7 @@ | @@ -29,7 +29,7 @@ | ||
| 29 | $(function(){ | 29 | $(function(){ |
| 30 | merge_request = new MergeRequest({ | 30 | merge_request = new MergeRequest({ |
| 31 | url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", | 31 | url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", |
| 32 | - check_enable: #{@merge_request.merge_status == MergeRequest::UNCHECKED ? "true" : "false"}, | 32 | + check_enable: #{@merge_request.unchecked? ? "true" : "false"}, |
| 33 | url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}", | 33 | url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}", |
| 34 | ci_enable: #{@project.gitlab_ci? ? "true" : "false"}, | 34 | ci_enable: #{@project.gitlab_ci? ? "true" : "false"}, |
| 35 | current_status: "#{@merge_request.human_merge_status}", | 35 | current_status: "#{@merge_request.human_merge_status}", |
spec/models/merge_request_spec.rb
| @@ -32,6 +32,12 @@ describe MergeRequest do | @@ -32,6 +32,12 @@ describe MergeRequest do | ||
| 32 | it { should_not allow_mass_assignment_of(:project_id) } | 32 | it { should_not allow_mass_assignment_of(:project_id) } |
| 33 | end | 33 | end |
| 34 | 34 | ||
| 35 | + describe "Respond to" do | ||
| 36 | + it { should respond_to(:unchecked?) } | ||
| 37 | + it { should respond_to(:can_be_merged?) } | ||
| 38 | + it { should respond_to(:cannot_be_merged?) } | ||
| 39 | + end | ||
| 40 | + | ||
| 35 | describe 'modules' do | 41 | describe 'modules' do |
| 36 | it { should include_module(Issuable) } | 42 | it { should include_module(Issuable) } |
| 37 | end | 43 | end |