diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index f92d9976..67f96178 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -75,7 +75,7 @@ class MergeRequestsController < ProjectResourceController 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.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 1bc34284..12a6dd2d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -24,6 +24,8 @@ require Rails.root.join("lib/static_model") 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 - UNCHECKED = 1 - CAN_BE_MERGED = 2 - CANNOT_BE_MERGED = 3 + event :mark_as_mergeable do + transition unchecked: :can_be_merged + end + + event :mark_as_unmergeable do + transition unchecked: :cannot_be_merged + end + + state :unchecked + + 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 @@ class MergeRequest < ActiveRecord::Base 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 @@ class MergeRequest < ActiveRecord::Base 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 @@ class MergeRequest < ActiveRecord::Base 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 28f56416..6da1b0b1 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -321,7 +321,7 @@ class Project < ActiveRecord::Base 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 ae2cfe92..08b80172 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.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 00000000..d78bd0ae --- /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 00000000..b310b35e --- /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 00000000..9083183b --- /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 00000000..3f8f38dc --- /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 f837e6ed..74d5f9a3 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 @@ ActiveRecord::Schema.define(:version => 20130218141554) do 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 @@ ActiveRecord::Schema.define(:version => 20130218141554) do 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 @@ ActiveRecord::Schema.define(:version => 20130218141554) do 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 @@ ActiveRecord::Schema.define(:version => 20130218141554) do 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 e61bf44c..dbae019e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -32,6 +32,12 @@ describe MergeRequest do 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