diff options
| -rw-r--r-- | app/controllers/merge_requests_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 57 | ||||
| -rw-r--r-- | app/views/merge_requests/show.html.erb | 2 | ||||
| -rw-r--r-- | test/functional/merge_requests_controller_test.rb | 4 | ||||
| -rw-r--r-- | test/unit/merge_request_test.rb | 28 |
5 files changed, 59 insertions, 34 deletions
diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index e60a394..286f47d 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -112,7 +112,7 @@ class MergeRequestsController < ApplicationController if state_changed flash[:notice] = I18n.t "merge_requests_controller.resolve_notice", :status => @merge_request.status_string else - flash[:error] = I18n.t "merge_requests_controller.resolve_disallowed", :status => MergeRequest.status_string(new_state) + flash[:error] = I18n.t "merge_requests_controller.resolve_disallowed", :status => "#{new_state}d" end redirect_to [@owner, @repository, @merge_request] end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a5170d9..83137bc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -38,6 +38,25 @@ class MergeRequest < ActiveRecord::Base STATUS_MERGED = 2 STATUS_REJECTED = 3 + state_machine :status, :initial => :pending do + state :pending, :value => ::MergeRequest::STATUS_PENDING_ACCEPTANCE_OF_TERMS + state :open, :value => ::MergeRequest::STATUS_OPEN + state :merged, :value => ::MergeRequest::STATUS_MERGED + state :rejected, :value => ::MergeRequest::STATUS_REJECTED + + event :open do + transition :pending => :open + end + + event :reject do + transition :open => :rejected + end + + event :merge do + transition :open => :merged + end + end + named_scope :open, :conditions => { :status => STATUS_OPEN } named_scope :closed, :conditions => ["status in (?)", [STATUS_MERGED, STATUS_REJECTED]] @@ -45,14 +64,17 @@ class MergeRequest < ActiveRecord::Base I18n.t("activerecord.models.merge_request") end - def self.statuses - { "Open" => STATUS_OPEN, "Merged" => STATUS_MERGED, "Rejected" => STATUS_REJECTED, 'Pending' => STATUS_PENDING_ACCEPTANCE_OF_TERMS } - end - def self.count_open count(:all, :conditions => {:status => STATUS_OPEN}) end + def self.statuses + @statuses ||= state_machines[:status].states.inject({}){ |result, state | + result[state.name.to_s.capitalize] = state.value + result + } + end + def status_string self.class.status_string(status) end @@ -61,20 +83,20 @@ class MergeRequest < ActiveRecord::Base statuses.invert[status_code.to_i].downcase end - def open? - status == STATUS_OPEN - end + # def open? + # status == STATUS_OPEN + # end - def merged? - status == STATUS_MERGED - end - - def rejected? - status == STATUS_REJECTED - end + # def merged? + # status == STATUS_MERGED + # end + # + # def rejected? + # status == STATUS_REJECTED + # end def pending_acceptance_of_terms? - status == STATUS_PENDING_ACCEPTANCE_OF_TERMS + pending? end def possible_next_states @@ -89,12 +111,13 @@ class MergeRequest < ActiveRecord::Base end def can_transition_to?(new_state) - return possible_next_states.include?(new_state.to_i) + send("can_#{new_state}?") end + def transition_to(status) if can_transition_to?(status) - self.status = status + send(status) yield return true end diff --git a/app/views/merge_requests/show.html.erb b/app/views/merge_requests/show.html.erb index 22291fc..a41ca27 100644 --- a/app/views/merge_requests/show.html.erb +++ b/app/views/merge_requests/show.html.erb @@ -31,7 +31,7 @@ @repository, @merge_request) do |f| -%> <p> <%= f.label :status -%><br /> - <%= f.select :status, MergeRequest.statuses.sort_by{|k,v| v } -%><br /> + <%= f.select :status, {'Open' => 'open', 'Merged' => 'merge', 'Rejected' => 'reject'} -%><br /> <%= f.label :reason -%><br /> <%= f.text_area :reason %><br /> diff --git a/test/functional/merge_requests_controller_test.rb b/test/functional/merge_requests_controller_test.rb index 5fbf43f..1bcbb7c 100644 --- a/test/functional/merge_requests_controller_test.rb +++ b/test/functional/merge_requests_controller_test.rb @@ -343,7 +343,7 @@ class MergeRequestsControllerTest < ActionController::TestCase :repository_id => @target_repository.to_param, :id => @merge_request, :merge_request => { - :status => MergeRequest::STATUS_MERGED, + :status => 'merge', }.merge(data) end @@ -381,7 +381,7 @@ class MergeRequestsControllerTest < ActionController::TestCase :repository_id => @target_repository.to_param, :id => @merge_request, :merge_request => { - :status => MergeRequest::STATUS_MERGED, + :status => 'merge', :reason => 'Not too good' } assert_equal "The merge request was marked as merged", flash[:notice] diff --git a/test/unit/merge_request_test.rb b/test/unit/merge_request_test.rb index 5ff4b32..de77562 100644 --- a/test/unit/merge_request_test.rb +++ b/test/unit/merge_request_test.rb @@ -187,35 +187,37 @@ class MergeRequestTest < ActiveSupport::TestCase setup {@merge_request = merge_requests(:moes_to_johans)} should 'allow transition to other states as long as it is not rejected or merged' do - @merge_request.status = MergeRequest::STATUS_OPEN - assert @merge_request.can_transition_to?(MergeRequest::STATUS_MERGED) - assert @merge_request.can_transition_to?(MergeRequest::STATUS_REJECTED) + @merge_request.open! + assert @merge_request.can_transition_to?('merge') + assert @merge_request.can_transition_to?('reject') end should 'not allow transition to other states when rejected' do - @merge_request.status = MergeRequest::STATUS_MERGED - assert !@merge_request.can_transition_to?(MergeRequest::STATUS_MERGED) - assert !@merge_request.can_transition_to?(MergeRequest::STATUS_REJECTED) + @merge_request.open! + @merge_request.reject! + assert !@merge_request.can_transition_to?('merge') + assert !@merge_request.can_transition_to?('reject') end should 'not allow transitions to other states when merged' do - @merge_request.status = MergeRequest::STATUS_REJECTED - assert !@merge_request.can_transition_to?(MergeRequest::STATUS_MERGED) - assert !@merge_request.can_transition_to?(MergeRequest::STATUS_REJECTED) + @merge_request.open! + @merge_request.reject! + assert !@merge_request.can_transition_to?('merge') + assert !@merge_request.can_transition_to?('reject') end should 'optionally take a block when performing a transition' do - @merge_request.status = MergeRequest::STATUS_OPEN + @merge_request.open! @merge_request.expects(:foo=).once - @merge_request.transition_to(MergeRequest::STATUS_PENDING) do + @merge_request.transition_to('pending') do @merge_request.foo = "Hello world" end end should 'optionally take a block when performing a transition' do - @merge_request.status = MergeRequest::STATUS_OPEN + @merge_request.open! @merge_request.expects(:foo=).once - status_changed = @merge_request.transition_to(MergeRequest::STATUS_MERGED) do + status_changed = @merge_request.transition_to('merge') do @merge_request.foo = "Hello world" end assert status_changed |