Merge branch 'Undev-state-machine'

Gemfile

@@ -70,6 +70,9 @@ gem "github-markup", "~> 0.7.4", require: 'github/markup'
 # Servers
 gem "unicorn", "~> 4.4.0"
+# State machine
+gem "state_machine"
+
 # Issue tags
 gem "acts-as-taggable-on", "2.3.3"

Gemfile.lock

@@ -425,6 +425,7 @@ GEM
       rack (~> 1.0)
       tilt (~> 1.1, != 1.3.0)
     stamp (0.3.0)
+    state_machine (1.1.2)
     temple (0.5.5)
     test_after_commit (0.0.1)
     therubyracer (0.10.2)
@@ -536,6 +537,7 @@ DEPENDENCIES
   slim
   spinach-rails
   stamp
+  state_machine
   test_after_commit
   therubyracer
   thin

app/assets/javascripts/merge_requests.js.coffee

@@ -27,7 +27,7 @@ class MergeRequest
     this.$el.find(selector)
   initMergeWidget: ->
-    this.showState( @opts.current_state )
+    this.showState( @opts.current_status )
     if this.$('.automerge_widget').length and @opts.check_enable
       $.get @opts.url_to_automerge_check, (data) =>

app/contexts/merge_requests_load_context.rb

@@ -14,7 +14,7 @@ class MergeRequestsLoadContext < BaseContext
                      end
     merge_requests = merge_requests.page(params[:page]).per(20)
-    merge_requests = merge_requests.includes(:author, :project).order("closed, created_at desc")
+    merge_requests = merge_requests.includes(:author, :project).order("state, created_at desc")
     # Filter by specific assignee_id (or lack thereof)?
     if params[:assignee_id].present?

app/controllers/merge_requests_controller.rb

@@ -73,14 +73,14 @@ class MergeRequestsController < ProjectResourceController
     if @merge_request.unchecked?
       @merge_request.check_if_can_be_merged
     end
-    render json: {state: @merge_request.human_state}
+    render json: {merge_status: @merge_request.human_merge_status}
   rescue Gitlab::SatelliteNotExistError
-    render json: {state: :no_satellite}
+    render json: {merge_status: :no_satellite}
   end
   def automerge
     return access_denied! unless can?(current_user, :accept_mr, @project)
-    if @merge_request.open? && @merge_request.can_be_merged?
+    if @merge_request.opened? && @merge_request.can_be_merged?
       @merge_request.should_remove_source_branch = params[:should_remove_source_branch]
       @merge_request.automerge!(current_user)
       @status = true

app/controllers/milestones_controller.rb

@@ -12,7 +12,7 @@ class MilestonesController < ProjectResourceController
   def index
     @milestones = case params[:f]
-                  when 'all'; @project.milestones.order("closed, due_date DESC")
+                  when 'all'; @project.milestones.order("state, due_date DESC")
                   when 'closed'; @project.milestones.closed.order("due_date DESC")
                   else @project.milestones.active.order("due_date ASC")
                   end

app/helpers/issues_helper.rb

@@ -6,7 +6,7 @@ module IssuesHelper
   def issue_css_classes issue
     classes = "issue"
-    classes << " closed" if issue.closed
+    classes << " closed" if issue.closed?
     classes << " today" if issue.today?
     classes
   end

app/helpers/merge_requests_helper.rb

@@ -12,7 +12,7 @@ module MergeRequestsHelper
   def mr_css_classes mr
     classes = "merge_request"
-    classes << " closed" if mr.closed
+    classes << " closed" if mr.closed?
     classes << " merged" if mr.merged?
     classes
   end

app/models/concerns/issuable.rb

@@ -17,10 +17,9 @@ module Issuable
     validates :project, presence: true
     validates :author, presence: true
     validates :title, presence: true, length: { within: 0..255 }
-    validates :closed, inclusion: { in: [true, false] }
-    scope :opened, -> { where(closed: false) }
-    scope :closed, -> { where(closed: true) }
+    scope :opened, -> { with_state(:opened) }
+    scope :closed, -> { with_state(:closed) }
     scope :of_group, ->(group) { where(project_id: group.project_ids) }
     scope :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) }
     scope :assigned, ->(u) { where(assignee_id: u.id)}
@@ -62,14 +61,6 @@ module Issuable
     assignee_id_changed?
   end
-  def is_being_closed?
-    closed_changed? && closed
-  end
-
-  def is_being_reopened?
-    closed_changed? && !closed
-  end
-
   #
   # Votes
   #

app/models/issue.rb

@@ -9,7 +9,7 @@
 #  project_id   :integer
 #  created_at   :datetime         not null
 #  updated_at   :datetime         not null
-#  closed       :boolean          default(FALSE), not null
+#  state        :string           default(FALSE), not null
 #  position     :integer          default(0)
 #  branch_name  :string(255)
 #  description  :text
@@ -19,8 +19,9 @@
 class Issue < ActiveRecord::Base
   include Issuable
-  attr_accessible :title, :assignee_id, :closed, :position, :description,
-                  :milestone_id, :label_list, :author_id_of_changes
+  attr_accessible :title, :assignee_id, :position, :description,
+                  :milestone_id, :label_list, :author_id_of_changes,
+                  :state_event
   acts_as_taggable_on :labels
@@ -33,4 +34,20 @@ class Issue < ActiveRecord::Base
       opened.assigned(user)
     end
   end
+
+  state_machine :state, initial: :opened do
+    event :close do
+      transition [:reopened, :opened] => :closed
+    end
+
+    event :reopen do
+      transition closed: :reopened
+    end
+
+    state :opened
+
+    state :reopened
+
+    state :closed
+  end
 end

app/models/merge_request.rb

@@ -9,15 +9,14 @@
 #  author_id     :integer
 #  assignee_id   :integer
 #  title         :string(255)
-#  closed        :boolean          default(FALSE), not null
+#  state         :string(255)      not null
 #  created_at    :datetime         not null
 #  updated_at    :datetime         not null
 #  st_commits    :text(2147483647)
 #  st_diffs      :text(2147483647)
-#  merged        :boolean          default(FALSE), not null
-#  state         :integer          default(1), not null
-#  milestone_id  :integer
+#  merge_status  :integer          default(1), not null
 #
+#  milestone_id  :integer
 require Rails.root.join("app/models/commit")
 require Rails.root.join("lib/static_model")
@@ -25,11 +24,33 @@ require Rails.root.join("lib/static_model")
 class MergeRequest < ActiveRecord::Base
   include Issuable
-  attr_accessible :title, :assignee_id, :closed, :target_branch, :source_branch, :milestone_id,
-                  :author_id_of_changes
+  attr_accessible :title, :assignee_id, :target_branch, :source_branch, :milestone_id,
+                  :author_id_of_changes, :state_event
   attr_accessor :should_remove_source_branch
+  state_machine :state, initial: :opened do
+    event :close do
+      transition [:reopened, :opened] => :closed
+    end
+
+    event :merge do
+      transition [:reopened, :opened] => :merged
+    end
+
+    event :reopen do
+      transition closed: :reopened
+    end
+
+    state :opened
+
+    state :reopened
+
+    state :closed
+
+    state :merged
+  end
+
   BROKEN_DIFF = "--broken-diff"
   UNCHECKED = 1
@@ -43,8 +64,13 @@ class MergeRequest < ActiveRecord::Base
   validates :target_branch, presence: true
   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
@@ -58,13 +84,13 @@ class MergeRequest < ActiveRecord::Base
     end
   end
-  def human_state
-    states = {
+  def human_merge_status
+    merge_statuses = {
       CAN_BE_MERGED =>  "can_be_merged",
       CANNOT_BE_MERGED => "cannot_be_merged",
       UNCHECKED => "unchecked"
     }
-    states[self.state]
+    merge_statuses[self.merge_status]
   end
   def validate_branches
@@ -79,20 +105,20 @@ class MergeRequest < ActiveRecord::Base
   end
   def unchecked?
-    state == UNCHECKED
+    merge_status == UNCHECKED
   end
   def mark_as_unchecked
-    self.state = UNCHECKED
+    self.merge_status = UNCHECKED
     self.save
   end
   def can_be_merged?
-    state == CAN_BE_MERGED
+    merge_status == CAN_BE_MERGED
   end
   def check_if_can_be_merged
-    self.state = if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
+    self.merge_status = if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged?
                    CAN_BE_MERGED
                  else
                    CANNOT_BE_MERGED
@@ -105,7 +131,7 @@ class MergeRequest < ActiveRecord::Base
   end
   def reloaded_diffs
-    if open? && unmerged_diffs.any?
+    if opened? && unmerged_diffs.any?
       self.st_diffs = unmerged_diffs
       self.save
     end
@@ -135,10 +161,6 @@ class MergeRequest < ActiveRecord::Base
     commits.first
   end
-  def merged?
-    merged && merge_event
-  end
-
   def merge_event
     self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
   end
@@ -153,26 +175,16 @@ class MergeRequest < ActiveRecord::Base
   def probably_merged?
     unmerged_commits.empty? &&
-      commits.any? && open?
-  end
-
-  def open?
-    !closed
-  end
-
-  def mark_as_merged!
-    self.merged = true
-    self.closed = true
-    save
+      commits.any? && opened?
   end
   def mark_as_unmergable
-    self.state = CANNOT_BE_MERGED
+    self.merge_status = CANNOT_BE_MERGED
     self.save
   end
   def reloaded_commits
-    if open? && unmerged_commits.any?
+    if opened? && unmerged_commits.any?
       self.st_commits = unmerged_commits
       save
     end
@@ -188,7 +200,8 @@ class MergeRequest < ActiveRecord::Base
   end
   def merge!(user_id)
-    self.mark_as_merged!
+    self.merge
+
     Event.create(
       project: self.project,
       action: Event::MERGED,

app/models/milestone.rb

@@ -13,19 +13,32 @@
 #
 class Milestone < ActiveRecord::Base
-  attr_accessible :title, :description, :due_date, :closed, :author_id_of_changes
+  attr_accessible :title, :description, :due_date, :state_event, :author_id_of_changes
   attr_accessor :author_id_of_changes
   belongs_to :project
   has_many :issues
   has_many :merge_requests
-  scope :active, -> { where(closed: false) }
-  scope :closed, -> { where(closed: true) }
+  scope :active, -> { with_state(:active) }
+  scope :closed, -> { with_state(:closed) }
   validates :title, presence: true
   validates :project, presence: true
-  validates :closed, inclusion: { in: [true, false] }
+
+  state_machine :state, initial: :active do
+    event :close do
+      transition active: :closed
+    end
+
+    event :activate do
+      transition closed: :active
+    end
+
+    state :closed
+
+    state :active
+  end
   def expired?
     if due_date
@@ -68,17 +81,13 @@ class Milestone < ActiveRecord::Base
   end
   def can_be_closed?
-    open? && issues.opened.count.zero?
+    active? && issues.opened.count.zero?
   end
   def is_empty?
     total_items_count.zero?
   end
-  def open?
-    !closed
-  end
-
   def author_id
     author_id_of_changes
   end

app/models/project.rb

@@ -43,7 +43,7 @@ class Project < ActiveRecord::Base
   has_many :events,             dependent: :destroy
   has_many :merge_requests,     dependent: :destroy
-  has_many :issues,             dependent: :destroy, order: "closed, created_at DESC"
+  has_many :issues,             dependent: :destroy, order: "state, created_at DESC"
   has_many :milestones,         dependent: :destroy
   has_many :users_projects,     dependent: :destroy
   has_many :notes,              dependent: :destroy

app/observers/activity_observer.rb

@@ -20,15 +20,23 @@ class ActivityObserver < ActiveRecord::Observer
     end
   end
-  def after_save(record)
-    if record.changed.include?("closed") && record.author_id_of_changes
+  def after_close(record, transition)
       Event.create(
         project: record.project,
         target_id: record.id,
         target_type: record.class.name,
-        action: (record.closed ? Event::CLOSED : Event::REOPENED),
+        action: Event::CLOSED,
+        author_id: record.author_id_of_changes
+      )
+  end
+
+  def after_reopen(record, transition)
+      Event.create(
+        project: record.project,
+        target_id: record.id,
+        target_type: record.class.name,
+        action: Event::REOPENED,
         author_id: record.author_id_of_changes
       )
-    end
   end
 end

app/observers/issue_observer.rb

@@ -7,22 +7,31 @@ class IssueObserver < ActiveRecord::Observer
     end
   end
-  def after_update(issue)
+  def after_close(issue, transition)
     send_reassigned_email(issue) if issue.is_being_reassigned?
-    status = nil
-    status = 'closed' if issue.is_being_closed?
-    status = 'reopened' if issue.is_being_reopened?
-    if status
-      Note.create_status_change_note(issue, current_user, status)
-      [issue.author, issue.assignee].compact.each do |recipient|
-        Notify.delay.issue_status_changed_email(recipient.id, issue.id, status, current_user.id)
-      end
-    end
+    create_note(issue)
+  end
+
+  def after_reopen(issue, transition)
+    send_reassigned_email(issue) if issue.is_being_reassigned?
+
+    create_note(issue)
+  end
+
+  def after_update(issue)
+    send_reassigned_email(issue) if issue.is_being_reassigned?
   end
   protected
+  def create_note(issue)
+    Note.create_status_change_note(issue, current_user, issue.state)
+    [issue.author, issue.assignee].compact.each do |recipient|
+      Notify.delay.issue_status_changed_email(recipient.id, issue.id, issue.state, current_user.id)
+    end
+  end
+
   def send_reassigned_email(issue)
     recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id && id != current_user.id }

app/observers/merge_request_observer.rb

@@ -7,15 +7,20 @@ class MergeRequestObserver < ActiveRecord::Observer
     end
   end
-  def after_update(merge_request)
+  def after_close(merge_request, transition)
     send_reassigned_email(merge_request) if merge_request.is_being_reassigned?
-    status = nil
-    status = 'closed' if merge_request.is_being_closed?
-    status = 'reopened' if merge_request.is_being_reopened?
-    if status
-      Note.create_status_change_note(merge_request, current_user, status)
-    end
+    Note.create_status_change_note(merge_request, current_user, merge_request.state)
+  end
+
+  def after_reopen(merge_request, transition)
+    send_reassigned_email(merge_request) if merge_request.is_being_reassigned?
+
+    Note.create_status_change_note(merge_request, current_user, merge_request.state)
+  end
+
+  def after_update(merge_request)
+    send_reassigned_email(merge_request) if merge_request.is_being_reassigned?
   end
   protected

app/views/issues/_show.html.haml

@@ -8,10 +8,10 @@
         %i.icon-comment
         = issue.notes.count
     - if can? current_user, :modify_issue, issue
-      - if issue.closed
-        = link_to 'Reopen', project_issue_path(issue.project, issue, issue: {closed: false }, status_only: true), method: :put,  class: "btn btn-small grouped reopen_issue", remote: true
+      - if issue.closed?
+        = link_to 'Reopen', project_issue_path(issue.project, issue, issue: {state_event: :reopen }, status_only: true), method: :put,  class: "btn btn-small grouped reopen_issue", remote: true
       - else
-        = link_to 'Close', project_issue_path(issue.project, issue, issue: {closed: true }, status_only: true), method: :put, class: "btn btn-small grouped close_issue", remote: true
+        = link_to 'Close', project_issue_path(issue.project, issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn btn-small grouped close_issue", remote: true
       = link_to edit_project_issue_path(issue.project, issue), class: "btn btn-small edit-issue-link grouped" do
         %i.icon-edit
         Edit

app/views/issues/show.html.haml

@@ -7,10 +7,10 @@
   %span.pull-right
     - if can?(current_user, :admin_project, @project) || @issue.author == current_user
-      - if @issue.closed
-        = link_to 'Reopen', project_issue_path(@project, @issue, issue: {closed: false }, status_only: true), method: :put,  class: "btn grouped reopen_issue"
+      - if @issue.closed?
+        = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put,  class: "btn grouped reopen_issue"
       - else
-        = link_to 'Close', project_issue_path(@project, @issue, issue: {closed: true }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue"
+        = link_to 'Close', project_issue_path(@project, @issue, issue: {state_event: :close }, status_only: true), method: :put, class: "btn grouped close_issue", title: "Close Issue"
     - if can?(current_user, :admin_project, @project) || @issue.author == current_user
       = link_to edit_project_issue_path(@project, @issue), class: "btn grouped" do
         %i.icon-edit
@@ -27,7 +27,7 @@
 .ui-box.ui-box-show
   .ui-box-head
     %h4.box-title
-      - if @issue.closed
+      - if @issue.closed?
         .error.status_info Closed
       = gfm escape_once(@issue.title)

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.state == MergeRequest::UNCHECKED ? "true" : "false"},
+      check_enable: #{@merge_request.merge_status == MergeRequest::UNCHECKED ? "true" : "false"},
       url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}",
       ci_enable: #{@project.gitlab_ci? ? "true" : "false"},
-      current_state: "#{@merge_request.human_state}",
+      current_status: "#{@merge_request.human_merge_status}",
       action: "#{controller.action_name}"
     });
   });

app/views/merge_requests/show/_mr_accept.html.haml

@@ -3,7 +3,7 @@
     %strong Only masters can accept MR
-- if @merge_request.open? && @commits.any? && can?(current_user, :accept_mr, @project)
+- if @merge_request.opened? && @commits.any? && can?(current_user, :accept_mr, @project)
   .automerge_widget.can_be_merged{style: "display:none"}
     .alert.alert-success
       %span