diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb
index f92d9976b43982092b52fd40dfd9d4036608c746..b9bc99ec70604471ae348b23bad4660c5f44f275 100644
--- a/app/controllers/merge_requests_controller.rb
+++ b/app/controllers/merge_requests_controller.rb
@@ -75,7 +75,7 @@ def automerge_check
if @merge_request.unchecked?
@merge_request.check_if_can_be_merged
end
- render json: {merge_status: @merge_request.human_merge_status}
+ render json: {merge_status: @merge_request.human_merge_status_name}
rescue Gitlab::SatelliteNotExistError
render json: {merge_status: :no_satellite}
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 1bc3428476ecd337047b46493193ad547db767ea..12a6dd2d7b239e153f35bdcc43c93f1bff431c85 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -24,6 +24,8 @@
class MergeRequest < ActiveRecord::Base
include Issuable
+ BROKEN_DIFF = "--broken-diff"
+
attr_accessible :title, :assignee_id, :target_branch, :source_branch, :milestone_id,
:author_id_of_changes, :state_event
@@ -51,52 +53,42 @@ class MergeRequest < ActiveRecord::Base
state :merged
end
- BROKEN_DIFF = "--broken-diff"
+ state_machine :merge_status, initial: :unchecked do
+ event :mark_as_unchecked do
+ transition [:can_be_merged, :cannot_be_merged] => :unchecked
+ end
+
+ event :mark_as_mergeable do
+ transition unchecked: :can_be_merged
+ end
+
+ event :mark_as_unmergeable do
+ transition unchecked: :cannot_be_merged
+ end
+
+ state :unchecked
- UNCHECKED = 1
- CAN_BE_MERGED = 2
- CANNOT_BE_MERGED = 3
+ state :can_be_merged
+
+ state :cannot_be_merged
+ end
serialize :st_commits
serialize :st_diffs
validates :source_branch, presence: true
validates :target_branch, presence: true
- validate :validate_branches
+ validate :validate_branches
scope :merged, -> { with_state(:merged) }
+ scope :by_branch, ->(branch_name) { where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name) }
+ scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) }
+ scope :by_milestone, ->(milestone) { where(milestone_id: milestone) }
# Closed scope for merge request should return
# both merged and closed mr's
scope :closed, -> { with_states(:closed, :merged) }
- class << self
- def find_all_by_branch(branch_name)
- where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name)
- end
-
- def cared(user)
- where('assignee_id = :user OR author_id = :user', user: user.id)
- end
-
- def find_all_by_branch(branch_name)
- where("source_branch LIKE :branch OR target_branch LIKE :branch", branch: branch_name)
- end
-
- def find_all_by_milestone(milestone)
- where("milestone_id = :milestone_id", milestone_id: milestone)
- end
- end
-
- def human_merge_status
- merge_statuses = {
- CAN_BE_MERGED => "can_be_merged",
- CANNOT_BE_MERGED => "cannot_be_merged",
- UNCHECKED => "unchecked"
- }
- merge_statuses[self.merge_status]
- end
-
def validate_branches
if target_branch == source_branch
errors.add :base, "You can not use same branch for source and target branches"
@@ -108,26 +100,12 @@ def reload_code
self.reloaded_diffs
end
- def unchecked?
- merge_status == UNCHECKED
- end
-
- def mark_as_unchecked
- self.merge_status = UNCHECKED
- self.save
- end
-
- def can_be_merged?
- merge_status == CAN_BE_MERGED
- end
-
def check_if_can_be_merged
- self.merge_status = if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
- CAN_BE_MERGED
- else
- CANNOT_BE_MERGED
- end
- self.save
+ if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
+ mark_as_mergeable
+ else
+ mark_as_unmergeable
+ end
end
def diffs
@@ -182,11 +160,6 @@ def probably_merged?
commits.any? && opened?
end
- def mark_as_unmergable
- self.merge_status = CANNOT_BE_MERGED
- self.save
- end
-
def reloaded_commits
if opened? && unmerged_commits.any?
self.st_commits = unmerged_commits
@@ -221,7 +194,7 @@ def automerge!(current_user)
true
end
rescue
- self.mark_as_unmergable
+ mark_as_unmergeable
false
end
diff --git a/app/models/project.rb b/app/models/project.rb
index 28f564164e35f4b67cf3e5c475f681ccc53b93e6..6da1b0b18ea68a2d765ad791bcc9a052754e0d55 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -321,7 +321,7 @@ def update_merge_requests(oldrev, newrev, ref, user)
c_ids = self.repository.commits_between(oldrev, newrev).map(&:id)
# Update code for merge requests
- mrs = self.merge_requests.opened.find_all_by_branch(branch_name).all
+ mrs = self.merge_requests.opened.by_branch(branch_name).all
mrs.each { |merge_request| merge_request.reload_code; merge_request.mark_as_unchecked }
# Close merge requests
diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml
index ae2cfe924ec9e5e80ee6b9dca859ab176f54c425..6b6100f5d477d96a6d0c70e1507d66f12e1d8918 100644
--- a/app/views/merge_requests/_show.html.haml
+++ b/app/views/merge_requests/_show.html.haml
@@ -29,10 +29,10 @@
$(function(){
merge_request = new MergeRequest({
url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}",
- check_enable: #{@merge_request.merge_status == MergeRequest::UNCHECKED ? "true" : "false"},
+ check_enable: #{@merge_request.unchecked? ? "true" : "false"},
url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}",
ci_enable: #{@project.gitlab_ci? ? "true" : "false"},
- current_status: "#{@merge_request.human_merge_status}",
+ current_status: "#{@merge_request.human_merge_status_name}",
action: "#{controller.action_name}"
});
});
diff --git a/db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb b/db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d78bd0ae923c2627496096e2de2fe775b845d74c
--- /dev/null
+++ b/db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb
@@ -0,0 +1,5 @@
+class AddNewMergeStatusToMergeRequest < ActiveRecord::Migration
+ def change
+ add_column :merge_requests, :new_merge_status, :string
+ end
+end
diff --git a/db/migrate/20130220125544_convert_merge_status_in_merge_request.rb b/db/migrate/20130220125544_convert_merge_status_in_merge_request.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b310b35e37314de1350f61771057cc541cd0ae22
--- /dev/null
+++ b/db/migrate/20130220125544_convert_merge_status_in_merge_request.rb
@@ -0,0 +1,17 @@
+class ConvertMergeStatusInMergeRequest < ActiveRecord::Migration
+ def up
+ MergeRequest.transaction do
+ MergeRequest.where(merge_status: 1).update_all("new_merge_status = 'unchecked'")
+ MergeRequest.where(merge_status: 2).update_all("new_merge_status = 'can_be_merged'")
+ MergeRequest.where(merge_status: 3).update_all("new_merge_status = 'cannot_be_merged'")
+ end
+ end
+
+ def down
+ MergeRequest.transaction do
+ MergeRequest.where(new_merge_status: :unchecked).update_all("merge_status = 1")
+ MergeRequest.where(new_merge_status: :can_be_merged).update_all("merge_status = 2")
+ MergeRequest.where(new_merge_status: :cannot_be_merged).update_all("merge_status = 3")
+ end
+ end
+end
diff --git a/db/migrate/20130220125545_remove_merge_status_from_merge_request.rb b/db/migrate/20130220125545_remove_merge_status_from_merge_request.rb
new file mode 100644
index 0000000000000000000000000000000000000000..9083183beb0dfb818c5eedcfc540888762acf8c4
--- /dev/null
+++ b/db/migrate/20130220125545_remove_merge_status_from_merge_request.rb
@@ -0,0 +1,9 @@
+class RemoveMergeStatusFromMergeRequest < ActiveRecord::Migration
+ def up
+ remove_column :merge_requests, :merge_status
+ end
+
+ def down
+ add_column :merge_requests, :merge_status, :integer
+ end
+end
diff --git a/db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb b/db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb
new file mode 100644
index 0000000000000000000000000000000000000000..3f8f38dc979af8fe0556bd78b9befb5124e9766e
--- /dev/null
+++ b/db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb
@@ -0,0 +1,5 @@
+class RenameNewMergeStatusToMergeStatusInMilestone < ActiveRecord::Migration
+ def change
+ rename_column :merge_requests, :new_merge_status, :merge_status
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f837e6edf98daa2cf1987b1c5977cc9ab515b016..74d5f9a360c30be1497b631d9fa6859303268c8c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.
-ActiveRecord::Schema.define(:version => 20130218141554) do
+ActiveRecord::Schema.define(:version => 20130220133245) do
create_table "events", :force => true do |t|
t.string "target_type"
@@ -68,19 +68,19 @@
add_index "keys", ["user_id"], :name => "index_keys_on_user_id"
create_table "merge_requests", :force => true do |t|
- t.string "target_branch", :null => false
- t.string "source_branch", :null => false
- t.integer "project_id", :null => false
+ t.string "target_branch", :null => false
+ t.string "source_branch", :null => false
+ t.integer "project_id", :null => false
t.integer "author_id"
t.integer "assignee_id"
t.string "title"
- t.datetime "created_at", :null => false
- t.datetime "updated_at", :null => false
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
t.text "st_commits", :limit => 2147483647
t.text "st_diffs", :limit => 2147483647
- t.integer "merge_status", :default => 1, :null => false
t.integer "milestone_id"
t.string "state"
+ t.string "merge_status"
end
add_index "merge_requests", ["assignee_id"], :name => "index_merge_requests_on_assignee_id"
@@ -106,11 +106,11 @@
add_index "milestones", ["project_id"], :name => "index_milestones_on_project_id"
create_table "namespaces", :force => true do |t|
- t.string "name", :null => false
- t.string "path", :null => false
- t.integer "owner_id", :null => false
- t.datetime "created_at", :null => false
- t.datetime "updated_at", :null => false
+ t.string "name", :null => false
+ t.string "path", :null => false
+ t.integer "owner_id", :null => false
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
t.string "type"
end
@@ -142,16 +142,16 @@
t.string "name"
t.string "path"
t.text "description"
- t.datetime "created_at", :null => false
- t.datetime "updated_at", :null => false
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
t.integer "creator_id"
t.string "default_branch"
- t.boolean "issues_enabled", :default => true, :null => false
- t.boolean "wall_enabled", :default => true, :null => false
- t.boolean "merge_requests_enabled", :default => true, :null => false
- t.boolean "wiki_enabled", :default => true, :null => false
+ t.boolean "issues_enabled", :default => true, :null => false
+ t.boolean "wall_enabled", :default => true, :null => false
+ t.boolean "merge_requests_enabled", :default => true, :null => false
+ t.boolean "wiki_enabled", :default => true, :null => false
t.integer "namespace_id"
- t.boolean "public", :default => false, :null => false
+ t.boolean "public", :default => false, :null => false
end
add_index "projects", ["creator_id"], :name => "index_projects_on_owner_id"
@@ -230,8 +230,8 @@
t.string "name"
t.string "path"
t.integer "owner_id"
- t.datetime "created_at", :null => false
- t.datetime "updated_at", :null => false
+ t.datetime "created_at", :null => false
+ t.datetime "updated_at", :null => false
end
create_table "users", :force => true do |t|
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index e61bf44ce53e8e512af12f3650ea81be68c362b1..dbae019e7d8895c3c561f8592aba35f0d04b61b6 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -32,6 +32,12 @@
it { should_not allow_mass_assignment_of(:project_id) }
end
+ describe "Respond to" do
+ it { should respond_to(:unchecked?) }
+ it { should respond_to(:can_be_merged?) }
+ it { should respond_to(:cannot_be_merged?) }
+ end
+
describe 'modules' do
it { should include_module(Issuable) }
end