From 8ba27b7b463e6691c75528748590a1cd9b9651c9 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:13:18 +0400 Subject: [PATCH 1/6] Migrations for convertion merge_status added --- ...204_add_new_merge_status_to_merge_request.rb | 5 +++++ ...544_convert_merge_status_in_merge_request.rb | 17 +++++++++++++++++ ...45_remove_merge_status_from_merge_request.rb | 9 +++++++++ ...merge_status_to_merge_status_in_milestone.rb | 5 +++++ 4 files changed, 36 insertions(+) create mode 100644 db/migrate/20130220124204_add_new_merge_status_to_merge_request.rb create mode 100644 db/migrate/20130220125544_convert_merge_status_in_merge_request.rb create mode 100644 db/migrate/20130220125545_remove_merge_status_from_merge_request.rb create mode 100644 db/migrate/20130220133245_rename_new_merge_status_to_merge_status_in_milestone.rb 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 From 1b7b17d12a9f176dd305b9ec95bd50ba3f56f520 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:13:42 +0400 Subject: [PATCH 2/6] Database schema updated --- db/schema.rb | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index f837e6ed..04ed7984 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,12 +106,13 @@ 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" + t.string "description", :default => "", :null => false end add_index "namespaces", ["name"], :name => "index_namespaces_on_name" @@ -142,16 +143,18 @@ 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 + t.string "issues_tracker", :default => "gitlab", :null => false + t.string "issues_tracker_id" end add_index "projects", ["creator_id"], :name => "index_projects_on_owner_id" @@ -230,8 +233,9 @@ 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 + t.string "description", :default => "", :null => false end create_table "users", :force => true do |t| From e2d94e0719cb4eed4757f4cef946b1c29ef971f0 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:15:01 +0400 Subject: [PATCH 3/6] State machine added for merge_status field --- app/models/merge_request.rb | 63 +++++++++++------------- app/views/merge_requests/_show.html.haml | 2 +- spec/models/merge_request_spec.rb | 6 +++ 3 files changed, 35 insertions(+), 36 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 06aa9f3c..05ebf44c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -53,16 +53,32 @@ class MergeRequest < ActiveRecord::Base BROKEN_DIFF = "--broken-diff" - UNCHECKED = 1 - CAN_BE_MERGED = 2 - CANNOT_BE_MERGED = 3 + 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 + + 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) } @@ -84,13 +100,9 @@ class MergeRequest < ActiveRecord::Base end end + # DEPRECATED: Please use human_merge_status_name instead 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] + human_merge_status_name end def validate_branches @@ -104,26 +116,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 @@ -178,11 +176,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 @@ -217,7 +210,7 @@ class MergeRequest < ActiveRecord::Base true end rescue - self.mark_as_unmergable + mark_as_unmergeable false end diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index ae2cfe92..b1e282a8 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -29,7 +29,7 @@ $(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}", 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 From 52e0df5c23b94cc8f2929a97bc2211fd51bc1de4 Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:37:20 +0400 Subject: [PATCH 4/6] class.self methods moved to scopes --- app/models/merge_request.rb | 21 +++------------------ app/models/project.rb | 2 +- 2 files changed, 4 insertions(+), 19 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 05ebf44c..e8e15aec 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -81,24 +81,9 @@ class MergeRequest < ActiveRecord::Base validate :validate_branches scope :merged, -> { with_state(: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 + 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_id", milestone_id: milestone) } # DEPRECATED: Please use human_merge_status_name instead def human_merge_status diff --git a/app/models/project.rb b/app/models/project.rb index b7d688bf..2120f53b 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -424,7 +424,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 From 6d68923edceb23ae6a5281e022a351a0d9c0d59b Mon Sep 17 00:00:00 2001 From: Andrew8xx8 Date: Wed, 20 Feb 2013 17:45:29 +0400 Subject: [PATCH 5/6] human_merge_status replaced by human_merge_status_name --- app/controllers/merge_requests_controller.rb | 2 +- app/models/merge_request.rb | 5 ----- app/views/merge_requests/_show.html.haml | 2 +- 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index bf665272..3ebef869 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -73,7 +73,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.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 e8e15aec..fad6f9c4 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -85,11 +85,6 @@ class MergeRequest < ActiveRecord::Base scope :cared, ->(user) { where('assignee_id = :user OR author_id = :user', user: user.id) } scope :by_milestone, ->(milestone) { where("milestone_id = :milestone_id", milestone_id: milestone) } - # DEPRECATED: Please use human_merge_status_name instead - def human_merge_status - human_merge_status_name - end - def validate_branches if target_branch == source_branch errors.add :base, "You can not use same branch for source and target branches" diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index b1e282a8..6b6100f5 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -32,7 +32,7 @@ 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}" }); }); From 5ac5510fd68fa060986733c07421d8411731ee24 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 26 Feb 2013 10:47:27 +0200 Subject: [PATCH 6/6] Fix automerge detection client-side since #3056 --- app/controllers/merge_requests_controller.rb | 2 +- app/views/merge_requests/_show.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index b9bc99ec..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_name} + render json: {merge_status: @merge_request.merge_status_name} rescue Gitlab::SatelliteNotExistError render json: {merge_status: :no_satellite} end diff --git a/app/views/merge_requests/_show.html.haml b/app/views/merge_requests/_show.html.haml index 6b6100f5..08b80172 100644 --- a/app/views/merge_requests/_show.html.haml +++ b/app/views/merge_requests/_show.html.haml @@ -32,7 +32,7 @@ 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_name}", + current_status: "#{@merge_request.merge_status_name}", action: "#{controller.action_name}" }); });