Commit 155703c6 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'state-machine' of https://github.com/Undev/gitlabhq into Undev-state-machine

Conflicts:
	app/models/issue.rb
	app/models/merge_request.rb
parents 52028dcd 6e333d50
...@@ -70,6 +70,9 @@ gem "github-markup", "~> 0.7.4", require: 'github/markup' ...@@ -70,6 +70,9 @@ gem "github-markup", "~> 0.7.4", require: 'github/markup'
# Servers # Servers
gem "unicorn", "~> 4.4.0" gem "unicorn", "~> 4.4.0"
# State machine
gem "state_machine"
# Issue tags # Issue tags
gem "acts-as-taggable-on", "2.3.3" gem "acts-as-taggable-on", "2.3.3"
......
...@@ -425,6 +425,7 @@ GEM ...@@ -425,6 +425,7 @@ GEM
rack (~> 1.0) rack (~> 1.0)
tilt (~> 1.1, != 1.3.0) tilt (~> 1.1, != 1.3.0)
stamp (0.3.0) stamp (0.3.0)
state_machine (1.1.2)
temple (0.5.5) temple (0.5.5)
test_after_commit (0.0.1) test_after_commit (0.0.1)
therubyracer (0.10.2) therubyracer (0.10.2)
...@@ -536,6 +537,7 @@ DEPENDENCIES ...@@ -536,6 +537,7 @@ DEPENDENCIES
slim slim
spinach-rails spinach-rails
stamp stamp
state_machine
test_after_commit test_after_commit
therubyracer therubyracer
thin thin
......
...@@ -27,7 +27,7 @@ class MergeRequest ...@@ -27,7 +27,7 @@ class MergeRequest
this.$el.find(selector) this.$el.find(selector)
initMergeWidget: -> initMergeWidget: ->
this.showState( @opts.current_state ) this.showState( @opts.current_status )
if this.$('.automerge_widget').length and @opts.check_enable if this.$('.automerge_widget').length and @opts.check_enable
$.get @opts.url_to_automerge_check, (data) => $.get @opts.url_to_automerge_check, (data) =>
......
...@@ -14,7 +14,7 @@ class MergeRequestsLoadContext < BaseContext ...@@ -14,7 +14,7 @@ class MergeRequestsLoadContext < BaseContext
end end
merge_requests = merge_requests.page(params[:page]).per(20) 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)? # Filter by specific assignee_id (or lack thereof)?
if params[:assignee_id].present? if params[:assignee_id].present?
......
...@@ -73,14 +73,14 @@ class MergeRequestsController < ProjectResourceController ...@@ -73,14 +73,14 @@ class MergeRequestsController < ProjectResourceController
if @merge_request.unchecked? if @merge_request.unchecked?
@merge_request.check_if_can_be_merged @merge_request.check_if_can_be_merged
end end
render json: {state: @merge_request.human_state} render json: {merge_status: @merge_request.human_merge_status}
rescue Gitlab::SatelliteNotExistError rescue Gitlab::SatelliteNotExistError
render json: {state: :no_satellite} render json: {merge_status: :no_satellite}
end end
def automerge def automerge
return access_denied! unless can?(current_user, :accept_mr, @project) 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.should_remove_source_branch = params[:should_remove_source_branch]
@merge_request.automerge!(current_user) @merge_request.automerge!(current_user)
@status = true @status = true
......
...@@ -12,7 +12,7 @@ class MilestonesController < ProjectResourceController ...@@ -12,7 +12,7 @@ class MilestonesController < ProjectResourceController
def index def index
@milestones = case params[:f] @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") when 'closed'; @project.milestones.closed.order("due_date DESC")
else @project.milestones.active.order("due_date ASC") else @project.milestones.active.order("due_date ASC")
end end
......
...@@ -6,7 +6,7 @@ module IssuesHelper ...@@ -6,7 +6,7 @@ module IssuesHelper
def issue_css_classes issue def issue_css_classes issue
classes = "issue" classes = "issue"
classes << " closed" if issue.closed classes << " closed" if issue.closed?
classes << " today" if issue.today? classes << " today" if issue.today?
classes classes
end end
......
...@@ -12,7 +12,7 @@ module MergeRequestsHelper ...@@ -12,7 +12,7 @@ module MergeRequestsHelper
def mr_css_classes mr def mr_css_classes mr
classes = "merge_request" classes = "merge_request"
classes << " closed" if mr.closed classes << " closed" if mr.closed?
classes << " merged" if mr.merged? classes << " merged" if mr.merged?
classes classes
end end
......
...@@ -17,10 +17,9 @@ module Issuable ...@@ -17,10 +17,9 @@ module Issuable
validates :project, presence: true validates :project, presence: true
validates :author, presence: true validates :author, presence: true
validates :title, presence: true, length: { within: 0..255 } validates :title, presence: true, length: { within: 0..255 }
validates :closed, inclusion: { in: [true, false] }
scope :opened, -> { where(closed: false) } scope :opened, -> { with_state(:opened) }
scope :closed, -> { where(closed: true) } scope :closed, -> { with_state(:closed) }
scope :of_group, ->(group) { where(project_id: group.project_ids) } 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 :of_user_team, ->(team) { where(project_id: team.project_ids, assignee_id: team.member_ids) }
scope :assigned, ->(u) { where(assignee_id: u.id)} scope :assigned, ->(u) { where(assignee_id: u.id)}
...@@ -62,14 +61,6 @@ module Issuable ...@@ -62,14 +61,6 @@ module Issuable
assignee_id_changed? assignee_id_changed?
end end
def is_being_closed?
closed_changed? && closed
end
def is_being_reopened?
closed_changed? && !closed
end
# #
# Votes # Votes
# #
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
# project_id :integer # project_id :integer
# created_at :datetime not null # created_at :datetime not null
# updated_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) # position :integer default(0)
# branch_name :string(255) # branch_name :string(255)
# description :text # description :text
...@@ -19,8 +19,9 @@ ...@@ -19,8 +19,9 @@
class Issue < ActiveRecord::Base class Issue < ActiveRecord::Base
include Issuable include Issuable
attr_accessible :title, :assignee_id, :closed, :position, :description, attr_accessible :title, :assignee_id, :position, :description,
:milestone_id, :label_list, :author_id_of_changes :milestone_id, :label_list, :author_id_of_changes,
:state_event
acts_as_taggable_on :labels acts_as_taggable_on :labels
...@@ -33,4 +34,20 @@ class Issue < ActiveRecord::Base ...@@ -33,4 +34,20 @@ class Issue < ActiveRecord::Base
opened.assigned(user) opened.assigned(user)
end end
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 end
...@@ -9,15 +9,14 @@ ...@@ -9,15 +9,14 @@
# author_id :integer # author_id :integer
# assignee_id :integer # assignee_id :integer
# title :string(255) # title :string(255)
# closed :boolean default(FALSE), not null # state :string(255) not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# st_commits :text(2147483647) # st_commits :text(2147483647)
# st_diffs :text(2147483647) # st_diffs :text(2147483647)
# merged :boolean default(FALSE), not null # merge_status :integer default(1), not null
# state :integer default(1), not null
# milestone_id :integer
# #
# milestone_id :integer
require Rails.root.join("app/models/commit") require Rails.root.join("app/models/commit")
require Rails.root.join("lib/static_model") require Rails.root.join("lib/static_model")
...@@ -25,11 +24,33 @@ require Rails.root.join("lib/static_model") ...@@ -25,11 +24,33 @@ require Rails.root.join("lib/static_model")
class MergeRequest < ActiveRecord::Base class MergeRequest < ActiveRecord::Base
include Issuable include Issuable
attr_accessible :title, :assignee_id, :closed, :target_branch, :source_branch, :milestone_id, attr_accessible :title, :assignee_id, :target_branch, :source_branch, :milestone_id,
:author_id_of_changes :author_id_of_changes, :state_event
attr_accessor :should_remove_source_branch 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" BROKEN_DIFF = "--broken-diff"
UNCHECKED = 1 UNCHECKED = 1
...@@ -43,8 +64,13 @@ class MergeRequest < ActiveRecord::Base ...@@ -43,8 +64,13 @@ class MergeRequest < ActiveRecord::Base
validates :target_branch, presence: true validates :target_branch, presence: true
validate :validate_branches validate :validate_branches
scope :merged, -> { with_state(:merged) }
class << self 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) def cared(user)
where('assignee_id = :user OR author_id = :user', user: user.id) where('assignee_id = :user OR author_id = :user', user: user.id)
end end
...@@ -58,13 +84,13 @@ class MergeRequest < ActiveRecord::Base ...@@ -58,13 +84,13 @@ class MergeRequest < ActiveRecord::Base
end end
end end
def human_state def human_merge_status
states = { merge_statuses = {
CAN_BE_MERGED => "can_be_merged", CAN_BE_MERGED => "can_be_merged",
CANNOT_BE_MERGED => "cannot_be_merged", CANNOT_BE_MERGED => "cannot_be_merged",
UNCHECKED => "unchecked" UNCHECKED => "unchecked"
} }
states[self.state] merge_statuses[self.merge_status]
end end
def validate_branches def validate_branches
...@@ -79,20 +105,20 @@ class MergeRequest < ActiveRecord::Base ...@@ -79,20 +105,20 @@ class MergeRequest < ActiveRecord::Base
end end
def unchecked? def unchecked?
state == UNCHECKED merge_status == UNCHECKED
end end
def mark_as_unchecked def mark_as_unchecked
self.state = UNCHECKED self.merge_status = UNCHECKED
self.save self.save
end end
def can_be_merged? def can_be_merged?
state == CAN_BE_MERGED merge_status == CAN_BE_MERGED
end end
def check_if_can_be_merged 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 CAN_BE_MERGED
else else
CANNOT_BE_MERGED CANNOT_BE_MERGED
...@@ -105,7 +131,7 @@ class MergeRequest < ActiveRecord::Base ...@@ -105,7 +131,7 @@ class MergeRequest < ActiveRecord::Base
end end
def reloaded_diffs def reloaded_diffs
if open? && unmerged_diffs.any? if opened? && unmerged_diffs.any?
self.st_diffs = unmerged_diffs self.st_diffs = unmerged_diffs
self.save self.save
end end
...@@ -135,10 +161,6 @@ class MergeRequest < ActiveRecord::Base ...@@ -135,10 +161,6 @@ class MergeRequest < ActiveRecord::Base
commits.first commits.first
end end
def merged?
merged && merge_event
end
def merge_event def merge_event
self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last self.project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last
end end
...@@ -153,26 +175,16 @@ class MergeRequest < ActiveRecord::Base ...@@ -153,26 +175,16 @@ class MergeRequest < ActiveRecord::Base
def probably_merged? def probably_merged?
unmerged_commits.empty? && unmerged_commits.empty? &&
commits.any? && open? commits.any? && opened?
end
def open?
!closed
end
def mark_as_merged!
self.merged = true
self.closed = true
save
end end
def mark_as_unmergable def mark_as_unmergable
self.state = CANNOT_BE_MERGED self.merge_status = CANNOT_BE_MERGED
self.save self.save
end end
def reloaded_commits def reloaded_commits
if open? && unmerged_commits.any? if opened? && unmerged_commits.any?
self.st_commits = unmerged_commits self.st_commits = unmerged_commits
save save
end end
...@@ -188,7 +200,8 @@ class MergeRequest < ActiveRecord::Base ...@@ -188,7 +200,8 @@ class MergeRequest < ActiveRecord::Base
end end
def merge!(user_id) def merge!(user_id)
self.mark_as_merged! self.merge
Event.create( Event.create(
project: self.project, project: self.project,
action: Event::MERGED, action: Event::MERGED,
......
...@@ -13,19 +13,32 @@ ...@@ -13,19 +13,32 @@
# #
class Milestone < ActiveRecord::Base 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 attr_accessor :author_id_of_changes
belongs_to :project belongs_to :project
has_many :issues has_many :issues
has_many :merge_requests has_many :merge_requests
scope :active, -> { where(closed: false) } scope :active, -> { with_state(:active) }
scope :closed, -> { where(closed: true) } scope :closed, -> { with_state(:closed) }
validates :title, presence: true validates :title, presence: true
validates :project, 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? def expired?
if due_date if due_date
...@@ -68,17 +81,13 @@ class Milestone < ActiveRecord::Base ...@@ -68,17 +81,13 @@ class Milestone < ActiveRecord::Base
end end
def can_be_closed? def can_be_closed?
open? && issues.opened.count.zero? active? && issues.opened.count.zero?
end end
def is_empty? def is_empty?
total_items_count.zero? total_items_count.zero?
end end
def open?
!closed
end
def author_id def author_id
author_id_of_changes author_id_of_changes
end end
......
...@@ -43,7 +43,7 @@ class Project < ActiveRecord::Base ...@@ -43,7 +43,7 @@ class Project < ActiveRecord::Base
has_many :events, dependent: :destroy has_many :events, dependent: :destroy
has_many :merge_requests, 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 :milestones, dependent: :destroy
has_many :users_projects, dependent: :destroy has_many :users_projects, dependent: :destroy
has_many :notes, dependent: :destroy has_many :notes, dependent: :destroy
......
...@@ -20,15 +20,23 @@ class ActivityObserver < ActiveRecord::Observer ...@@ -20,15 +20,23 @@ class ActivityObserver < ActiveRecord::Observer
end end
end end
def after_save(record) def after_close(record, transition)
if record.changed.include?("closed") && record.author_id_of_changes
Event.create( Event.create(
project: record.project, project: record.project,
target_id: record.id, target_id: record.id,
target_type: record.class.name, target_type: record.class.name,
action: (record.closed ? Event::CLOSED : Event::REOPENED), action: Event::CLOSED,
author_id: record.author_id_of_changes author_id: record.author_id_of_changes
) )
end 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 end
...@@ -7,22 +7,31 @@ class IssueObserver < ActiveRecord::Observer ...@@ -7,22 +7,31 @@ class IssueObserver < ActiveRecord::Observer
end end
end end
def after_update(issue) def after_close(issue, transition)
send_reassigned_email(issue) if issue.is_being_reassigned? send_reassigned_email(issue) if issue.is_being_reassigned?
status = nil create_note(issue)
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
def after_reopen(issue, transition)
send_reassigned_email(issue) if issue.is_being_reassigned?
create_note(issue)
end end
def after_update(issue)
send_reassigned_email(issue) if issue.is_being_reassigned?
end end
protected 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) def send_reassigned_email(issue)
recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id && id != current_user.id } recipient_ids = [issue.assignee_id, issue.assignee_id_was].keep_if {|id| id && id != current_user.id }
......
...@@ -7,15 +7,20 @@ class MergeRequestObserver < ActiveRecord::Observer ...@@ -7,15 +7,20 @@ class MergeRequestObserver < ActiveRecord::Observer
end end
end end
def after_update(merge_request) def after_close(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_reopen(merge_request, transition)
send_reassigned_email(merge_request) if merge_request.is_being_reassigned? send_reassigned_email(merge_request) if merge_request.is_being_reassigned?
status = nil Note.create_status_change_note(merge_request, current_user, merge_request.state)
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 end
def after_update(merge_request)
send_reassigned_email(merge_request) if merge_request.is_being_reassigned?
end end
protected protected
......
...@@ -8,10 +8,10 @@ ...@@ -8,10 +8,10 @@
%i.icon-comment %i.icon-comment
= issue.notes.count = issue.notes.count
- if can? current_user, :modify_issue, issue - if can? current_user, :modify_issue, issue
- if issue.closed - 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 = 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 - 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 = link_to edit_project_issue_path(issue.project, issue), class: "btn btn-small edit-issue-link grouped" do
%i.icon-edit %i.icon-edit
Edit Edit
......
...@@ -7,10 +7,10 @@ ...@@ -7,10 +7,10 @@
%span.pull-right %span.pull-right
- if can?(current_user, :admin_project, @project) || @issue.author == current_user - if can?(current_user, :admin_project, @project) || @issue.author == current_user
- if @issue.closed - if @issue.closed?
= link_to 'Reopen', project_issue_path(@project, @issue, issue: {closed: false }, status_only: true), method: :put, class: "btn grouped reopen_issue" = link_to 'Reopen', project_issue_path(@project, @issue, issue: {state_event: :reopen }, status_only: true), method: :put, class: "btn grouped reopen_issue"
- else - 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 - if can?(current_user, :admin_project, @project) || @issue.author == current_user
= link_to edit_project_issue_path(@project, @issue), class: "btn grouped" do = link_to edit_project_issue_path(@project, @issue), class: "btn grouped" do
%i.icon-edit %i.icon-edit
...@@ -27,7 +27,7 @@ ...@@ -27,7 +27,7 @@
.ui-box.ui-box-show .ui-box.ui-box-show
.ui-box-head .ui-box-head
%h4.box-title %h4.box-title
- if @issue.closed - if @issue.closed?
.error.status_info Closed .error.status_info Closed
= gfm escape_once(@issue.title) = gfm escape_once(@issue.title)
......
...@@ -29,10 +29,10 @@ ...@@ -29,10 +29,10 @@
$(function(){ $(function(){
merge_request = new MergeRequest({ merge_request = new MergeRequest({
url_to_automerge_check: "#{automerge_check_project_merge_request_path(@project, @merge_request)}", 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)}", url_to_ci_check: "#{ci_status_project_merge_request_path(@project, @merge_request)}",
ci_enable: #{@project.gitlab_ci? ? "true" : "false"}, ci_enable: #{@project.gitlab_ci? ? "true" : "false"},
current_state: "#{@merge_request.human_state}", current_status: "#{@merge_request.human_merge_status}",
action: "#{controller.action_name}" action: "#{controller.action_name}"
}); });
}); });
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
%strong Only masters can accept MR %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"} .automerge_widget.can_be_merged{style: "display:none"}
.alert.alert-success .alert.alert-success
%span %span
......
.ui-box.ui-box-show .ui-box.ui-box-show
.ui-box-head .ui-box-head
%h4.box-title %h4.box-title
- if @merge_request.merged - if @merge_request.merged?
.error.status_info .error.status_info
%i.icon-ok %i.icon-ok
Merged Merged
- elsif @merge_request.closed - elsif @merge_request.closed?
.error.status_info Closed .error.status_info Closed
= gfm escape_once(@merge_request.title) = gfm escape_once(@merge_request.title)
...@@ -21,14 +21,14 @@ ...@@ -21,14 +21,14 @@
%strong= link_to_gfm truncate(milestone.title, length: 20), project_milestone_path(milestone.project, milestone) %strong= link_to_gfm truncate(milestone.title, length: 20), project_milestone_path(milestone.project, milestone)
- if @merge_request.closed - if @merge_request.closed?
.ui-box-bottom .ui-box-bottom
%span
Closed by #{link_to_member(@project, @merge_request.closed_event.author)}
%small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago.
- if @merge_request.merged? - if @merge_request.merged?
.ui-box-bottom
%span %span
Merged by #{link_to_member(@project, @merge_request.merge_event.author)} Merged by #{link_to_member(@project, @merge_request.merge_event.author)}
%small #{time_ago_in_words(@merge_request.merge_event.created_at)} ago. %small #{time_ago_in_words(@merge_request.merge_event.created_at)} ago.
- elsif @merge_request.closed_event
%span
Closed by #{link_to_member(@project, @merge_request.closed_event.author)}
%small #{time_ago_in_words(@merge_request.closed_event.created_at)} ago.
- if @merge_request.open? && @commits.any? - if @merge_request.opened? && @commits.any?
.ci_widget.ci-success{style: "display:none"} .ci_widget.ci-success{style: "display:none"}
.alert.alert-success .alert.alert-success
%i.icon-ok %i.icon-ok
......
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
%span.pull-right %span.pull-right
- if can?(current_user, :modify_merge_request, @merge_request) - if can?(current_user, :modify_merge_request, @merge_request)
- if @merge_request.open? - if @merge_request.opened?
.left.btn-group .left.btn-group
%a.btn.grouped.dropdown-toggle{ data: {toggle: :dropdown} } %a.btn.grouped.dropdown-toggle{ data: {toggle: :dropdown} }
%i.icon-download-alt %i.icon-download-alt
...@@ -17,7 +17,7 @@ ...@@ -17,7 +17,7 @@
%li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch) %li= link_to "Email Patches", project_merge_request_path(@project, @merge_request, format: :patch)
%li= link_to "Plain Diff", project_merge_request_path(@project, @merge_request, format: :diff) %li= link_to "Plain Diff", project_merge_request_path(@project, @merge_request, format: :diff)
= link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {closed: true }, status_only: true), method: :put, class: "btn grouped btn-close", title: "Close merge request" = link_to 'Close', project_merge_request_path(@project, @merge_request, merge_request: {state_event: :close }), method: :put, class: "btn grouped btn-close", title: "Close merge request"
= link_to edit_project_merge_request_path(@project, @merge_request), class: "btn grouped" do = link_to edit_project_merge_request_path(@project, @merge_request), class: "btn grouped" do
%i.icon-edit %i.icon-edit
......
%li{class: "milestone milestone-#{milestone.closed ? 'closed' : 'open'}", id: dom_id(milestone) } %li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone) }
.pull-right .pull-right
- if can?(current_user, :admin_milestone, milestone.project) and milestone.open? - if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
= link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-small edit-milestone-link grouped" do = link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-small edit-milestone-link grouped" do
%i.icon-edit %i.icon-edit
Edit Edit
%h4 %h4
= link_to_gfm truncate(milestone.title, length: 100), project_milestone_path(milestone.project, milestone) = link_to_gfm truncate(milestone.title, length: 100), project_milestone_path(milestone.project, milestone)
- if milestone.expired? and not milestone.closed - if milestone.expired? and not milestone.closed?
%span.cred (Expired) %span.cred (Expired)
%small %small
= milestone.expires_at = milestone.expires_at
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
&larr; To milestones list &larr; To milestones list
.span6 .span6
.pull-right .pull-right
- unless @milestone.closed - unless @milestone.closed?
= link_to new_project_issue_path(@project, issue: { milestone_id: @milestone.id }), class: "btn btn-small grouped", title: "New Issue" do = link_to new_project_issue_path(@project, issue: { milestone_id: @milestone.id }), class: "btn btn-small grouped", title: "New Issue" do
%i.icon-plus %i.icon-plus
New Issue New Issue
...@@ -25,12 +25,12 @@ ...@@ -25,12 +25,12 @@
%hr %hr
%p %p
%span All issues for this milestone are closed. You may close milestone now. %span All issues for this milestone are closed. You may close milestone now.
= link_to 'Close Milestone', project_milestone_path(@project, @milestone, milestone: {closed: true }), method: :put, class: "btn btn-small btn-remove" = link_to 'Close Milestone', project_milestone_path(@project, @milestone, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
.ui-box.ui-box-show .ui-box.ui-box-show
.ui-box-head .ui-box-head
%h4.box-title %h4.box-title
- if @milestone.closed - if @milestone.closed?
.error.status_info Closed .error.status_info Closed
- elsif @milestone.expired? - elsif @milestone.expired?
.error.status_info Expired .error.status_info Expired
...@@ -63,7 +63,7 @@ ...@@ -63,7 +63,7 @@
%li=link_to('All Issues', '#') %li=link_to('All Issues', '#')
%ul.well-list %ul.well-list
- @issues.each do |issue| - @issues.each do |issue|
%li{data: {closed: issue.closed}} %li{data: {closed: issue.closed?}}
= link_to [@project, issue] do = link_to [@project, issue] do
%span.badge.badge-info ##{issue.id} %span.badge.badge-info ##{issue.id}
&ndash; &ndash;
...@@ -77,7 +77,7 @@ ...@@ -77,7 +77,7 @@
%li=link_to('All Merge Requests', '#') %li=link_to('All Merge Requests', '#')
%ul.well-list %ul.well-list
- @merge_requests.each do |merge_request| - @merge_requests.each do |merge_request|
%li{data: {closed: merge_request.closed}} %li{data: {closed: merge_request.closed?}}
= link_to [@project, merge_request] do = link_to [@project, merge_request] do
%span.badge.badge-info ##{merge_request.id} %span.badge.badge-info ##{merge_request.id}
&ndash; &ndash;
......
class RenameStateToMergeStatusInMilestone < ActiveRecord::Migration
def change
rename_column :merge_requests, :state, :merge_status
end
end
class AddStateToIssue < ActiveRecord::Migration
def change
add_column :issues, :state, :string
end
end
class AddStateToMergeRequest < ActiveRecord::Migration
def change
add_column :merge_requests, :state, :string
end
end
class AddStateToMilestone < ActiveRecord::Migration
def change
add_column :milestones, :state, :string
end
end
class ConvertClosedToStateInIssue < ActiveRecord::Migration
def up
Issue.transaction do
Issue.where(closed: true).update_all("state = 'closed'")
Issue.where(closed: false).update_all("state = 'opened'")
end
end
def down
Issue.transaction do
Issue.where(state: :closed).update_all("closed = 1")
end
end
end
class ConvertClosedToStateInMergeRequest < ActiveRecord::Migration
def up
MergeRequest.transaction do
MergeRequest.where("closed = 1 AND merged = 1").update_all("state = 'merged'")
MergeRequest.where("closed = 1 AND merged = 0").update_all("state = 'closed'")
MergeRequest.where("closed = 0").update_all("state = 'opened'")
end
end
def down
MergeRequest.transaction do
MergeRequest.where(state: :closed).update_all("closed = 1")
MergeRequest.where(state: :merged).update_all("closed = 1, merged = 1")
end
end
end
class ConvertClosedToStateInMilestone < ActiveRecord::Migration
def up
Milestone.transaction do
Milestone.where(closed: false).update_all("state = 'opened'")
Milestone.where(closed: false).update_all("state = 'active'")
end
end
def down
Milestone.transaction do
Milestone.where(state: :closed).update_all("closed = 1")
end
end
end
class RemoveMergedFromMergeRequest < ActiveRecord::Migration
def up
remove_column :merge_requests, :merged
end
def down
add_column :merge_requests, :merged, :boolean, default: true, null: false
end
end
class RemoveClosedFromIssue < ActiveRecord::Migration
def up
remove_column :issues, :closed
end
def down
add_column :issues, :closed, :boolean
end
end
class RemoveClosedFromMergeRequest < ActiveRecord::Migration
def up
remove_column :merge_requests, :closed
end
def down
add_column :merge_requests, :closed, :boolean
end
end
class RemoveClosedFromMilestone < ActiveRecord::Migration
def up
remove_column :milestones, :closed
end
def down
add_column :milestones, :closed, :boolean
end
end
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended to check this file into your version control system. # It's strongly recommended to check this file into your version control system.
ActiveRecord::Schema.define(:version => 20130131070232) do ActiveRecord::Schema.define(:version => 20130218141554) do
create_table "events", :force => true do |t| create_table "events", :force => true do |t|
t.string "target_type" t.string "target_type"
...@@ -39,16 +39,15 @@ ActiveRecord::Schema.define(:version => 20130131070232) do ...@@ -39,16 +39,15 @@ ActiveRecord::Schema.define(:version => 20130131070232) do
t.integer "project_id" t.integer "project_id"
t.datetime "created_at", :null => false t.datetime "created_at", :null => false
t.datetime "updated_at", :null => false t.datetime "updated_at", :null => false
t.boolean "closed", :default => false, :null => false
t.integer "position", :default => 0 t.integer "position", :default => 0
t.string "branch_name" t.string "branch_name"
t.text "description" t.text "description"
t.integer "milestone_id" t.integer "milestone_id"
t.string "state"
end end
add_index "issues", ["assignee_id"], :name => "index_issues_on_assignee_id" add_index "issues", ["assignee_id"], :name => "index_issues_on_assignee_id"
add_index "issues", ["author_id"], :name => "index_issues_on_author_id" add_index "issues", ["author_id"], :name => "index_issues_on_author_id"
add_index "issues", ["closed"], :name => "index_issues_on_closed"
add_index "issues", ["created_at"], :name => "index_issues_on_created_at" add_index "issues", ["created_at"], :name => "index_issues_on_created_at"
add_index "issues", ["milestone_id"], :name => "index_issues_on_milestone_id" add_index "issues", ["milestone_id"], :name => "index_issues_on_milestone_id"
add_index "issues", ["project_id"], :name => "index_issues_on_project_id" add_index "issues", ["project_id"], :name => "index_issues_on_project_id"
...@@ -75,19 +74,17 @@ ActiveRecord::Schema.define(:version => 20130131070232) do ...@@ -75,19 +74,17 @@ ActiveRecord::Schema.define(:version => 20130131070232) do
t.integer "author_id" t.integer "author_id"
t.integer "assignee_id" t.integer "assignee_id"
t.string "title" t.string "title"
t.boolean "closed", :default => false, :null => false
t.datetime "created_at", :null => false t.datetime "created_at", :null => false
t.datetime "updated_at", :null => false t.datetime "updated_at", :null => false
t.text "st_commits", :limit => 2147483647 t.text "st_commits", :limit => 2147483647
t.text "st_diffs", :limit => 2147483647 t.text "st_diffs", :limit => 2147483647
t.boolean "merged", :default => false, :null => false t.integer "merge_status", :default => 1, :null => false
t.integer "state", :default => 1, :null => false
t.integer "milestone_id" t.integer "milestone_id"
t.string "state"
end end
add_index "merge_requests", ["assignee_id"], :name => "index_merge_requests_on_assignee_id" add_index "merge_requests", ["assignee_id"], :name => "index_merge_requests_on_assignee_id"
add_index "merge_requests", ["author_id"], :name => "index_merge_requests_on_author_id" add_index "merge_requests", ["author_id"], :name => "index_merge_requests_on_author_id"
add_index "merge_requests", ["closed"], :name => "index_merge_requests_on_closed"
add_index "merge_requests", ["created_at"], :name => "index_merge_requests_on_created_at" add_index "merge_requests", ["created_at"], :name => "index_merge_requests_on_created_at"
add_index "merge_requests", ["milestone_id"], :name => "index_merge_requests_on_milestone_id" add_index "merge_requests", ["milestone_id"], :name => "index_merge_requests_on_milestone_id"
add_index "merge_requests", ["project_id"], :name => "index_merge_requests_on_project_id" add_index "merge_requests", ["project_id"], :name => "index_merge_requests_on_project_id"
...@@ -100,9 +97,9 @@ ActiveRecord::Schema.define(:version => 20130131070232) do ...@@ -100,9 +97,9 @@ ActiveRecord::Schema.define(:version => 20130131070232) do
t.integer "project_id", :null => false t.integer "project_id", :null => false
t.text "description" t.text "description"
t.date "due_date" t.date "due_date"
t.boolean "closed", :default => false, :null => false
t.datetime "created_at", :null => false t.datetime "created_at", :null => false
t.datetime "updated_at", :null => false t.datetime "updated_at", :null => false
t.string "state"
end end
add_index "milestones", ["due_date"], :name => "index_milestones_on_due_date" add_index "milestones", ["due_date"], :name => "index_milestones_on_due_date"
......
...@@ -122,10 +122,9 @@ class ProjectIssues < Spinach::FeatureSteps ...@@ -122,10 +122,9 @@ class ProjectIssues < Spinach::FeatureSteps
And 'project "Shop" have "Release 0.3" closed issue' do And 'project "Shop" have "Release 0.3" closed issue' do
project = Project.find_by_name("Shop") project = Project.find_by_name("Shop")
create(:issue, create(:closed_issue,
:title => "Release 0.3", :title => "Release 0.3",
:project => project, :project => project,
:author => project.users.first, :author => project.users.first)
:closed => true)
end end
end end
...@@ -26,7 +26,7 @@ class ProjectMergeRequests < Spinach::FeatureSteps ...@@ -26,7 +26,7 @@ class ProjectMergeRequests < Spinach::FeatureSteps
Then 'I should see closed merge request "Bug NS-04"' do Then 'I should see closed merge request "Bug NS-04"' do
mr = MergeRequest.find_by_title("Bug NS-04") mr = MergeRequest.find_by_title("Bug NS-04")
mr.closed.should be_true mr.closed?.should be_true
page.should have_content "Closed by" page.should have_content "Closed by"
end end
...@@ -80,11 +80,10 @@ class ProjectMergeRequests < Spinach::FeatureSteps ...@@ -80,11 +80,10 @@ class ProjectMergeRequests < Spinach::FeatureSteps
And 'project "Shop" have "Feature NS-03" closed merge request' do And 'project "Shop" have "Feature NS-03" closed merge request' do
project = Project.find_by_name("Shop") project = Project.find_by_name("Shop")
create(:merge_request, create(:closed_merge_request,
title: "Feature NS-03", title: "Feature NS-03",
project: project, project: project,
author: project.users.first, author: project.users.first)
closed: true)
end end
And 'I switch to the diff tab' do And 'I switch to the diff tab' do
......
...@@ -40,7 +40,6 @@ module Gitlab ...@@ -40,7 +40,6 @@ module Gitlab
expose :projects, using: Entities::Project expose :projects, using: Entities::Project
end end
class RepoObject < Grape::Entity class RepoObject < Grape::Entity
expose :name, :commit expose :name, :commit
expose :protected do |repo, options| expose :protected do |repo, options|
...@@ -63,7 +62,7 @@ module Gitlab ...@@ -63,7 +62,7 @@ module Gitlab
class Milestone < Grape::Entity class Milestone < Grape::Entity
expose :id expose :id
expose (:project_id) {|milestone| milestone.project.id} expose (:project_id) {|milestone| milestone.project.id}
expose :title, :description, :due_date, :closed, :updated_at, :created_at expose :title, :description, :due_date, :state, :updated_at, :created_at
end end
class Issue < Grape::Entity class Issue < Grape::Entity
...@@ -73,7 +72,7 @@ module Gitlab ...@@ -73,7 +72,7 @@ module Gitlab
expose :label_list, as: :labels expose :label_list, as: :labels
expose :milestone, using: Entities::Milestone expose :milestone, using: Entities::Milestone
expose :assignee, :author, using: Entities::UserBasic expose :assignee, :author, using: Entities::UserBasic
expose :closed, :updated_at, :created_at expose :state, :updated_at, :created_at
end end
class SSHKey < Grape::Entity class SSHKey < Grape::Entity
...@@ -81,7 +80,7 @@ module Gitlab ...@@ -81,7 +80,7 @@ module Gitlab
end end
class MergeRequest < Grape::Entity class MergeRequest < Grape::Entity
expose :id, :target_branch, :source_branch, :project_id, :title, :closed, :merged expose :id, :target_branch, :source_branch, :project_id, :title, :state
expose :author, :assignee, using: Entities::UserBasic expose :author, :assignee, using: Entities::UserBasic
end end
......
...@@ -69,14 +69,14 @@ module Gitlab ...@@ -69,14 +69,14 @@ module Gitlab
# assignee_id (optional) - The ID of a user to assign issue # assignee_id (optional) - The ID of a user to assign issue
# milestone_id (optional) - The ID of a milestone to assign issue # milestone_id (optional) - The ID of a milestone to assign issue
# labels (optional) - The labels of an issue # labels (optional) - The labels of an issue
# closed (optional) - The state of an issue (0 = false, 1 = true) # state (optional) - The state of an issue (close|reopen)
# Example Request: # Example Request:
# PUT /projects/:id/issues/:issue_id # PUT /projects/:id/issues/:issue_id
put ":id/issues/:issue_id" do put ":id/issues/:issue_id" do
@issue = user_project.issues.find(params[:issue_id]) @issue = user_project.issues.find(params[:issue_id])
authorize! :modify_issue, @issue authorize! :modify_issue, @issue
attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :closed] attrs = attributes_for_keys [:title, :description, :assignee_id, :milestone_id, :state_event]
attrs[:label_list] = params[:labels] if params[:labels].present? attrs[:label_list] = params[:labels] if params[:labels].present?
IssueObserver.current_user = current_user IssueObserver.current_user = current_user
if @issue.update_attributes attrs if @issue.update_attributes attrs
......
...@@ -73,12 +73,12 @@ module Gitlab ...@@ -73,12 +73,12 @@ module Gitlab
# target_branch - The target branch # target_branch - The target branch
# assignee_id - Assignee user ID # assignee_id - Assignee user ID
# title - Title of MR # title - Title of MR
# closed - Status of MR. true - closed # state_event - Status of MR. (close|reopen|merge)
# Example: # Example:
# PUT /projects/:id/merge_request/:merge_request_id # PUT /projects/:id/merge_request/:merge_request_id
# #
put ":id/merge_request/:merge_request_id" do put ":id/merge_request/:merge_request_id" do
attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :closed] attrs = attributes_for_keys [:source_branch, :target_branch, :assignee_id, :title, :state_event]
merge_request = user_project.merge_requests.find(params[:merge_request_id]) merge_request = user_project.merge_requests.find(params[:merge_request_id])
authorize! :modify_merge_request, merge_request authorize! :modify_merge_request, merge_request
......
...@@ -59,14 +59,14 @@ module Gitlab ...@@ -59,14 +59,14 @@ module Gitlab
# title (optional) - The title of a milestone # title (optional) - The title of a milestone
# description (optional) - The description of a milestone # description (optional) - The description of a milestone
# due_date (optional) - The due date of a milestone # due_date (optional) - The due date of a milestone
# closed (optional) - The status of the milestone # state (optional) - The status of the milestone (close|activate)
# Example Request: # Example Request:
# PUT /projects/:id/milestones/:milestone_id # PUT /projects/:id/milestones/:milestone_id
put ":id/milestones/:milestone_id" do put ":id/milestones/:milestone_id" do
authorize! :admin_milestone, user_project authorize! :admin_milestone, user_project
@milestone = user_project.milestones.find(params[:milestone_id]) @milestone = user_project.milestones.find(params[:milestone_id])
attrs = attributes_for_keys [:title, :description, :due_date, :closed] attrs = attributes_for_keys [:title, :description, :due_date, :state_event]
if @milestone.update_attributes attrs if @milestone.update_attributes attrs
present @milestone, with: Entities::Milestone present @milestone, with: Entities::Milestone
else else
......
...@@ -54,10 +54,15 @@ FactoryGirl.define do ...@@ -54,10 +54,15 @@ FactoryGirl.define do
project project
trait :closed do trait :closed do
closed true state :closed
end
trait :reopened do
state :reopened
end end
factory :closed_issue, traits: [:closed] factory :closed_issue, traits: [:closed]
factory :reopened_issue, traits: [:reopened]
end end
factory :merge_request do factory :merge_request do
...@@ -67,10 +72,6 @@ FactoryGirl.define do ...@@ -67,10 +72,6 @@ FactoryGirl.define do
source_branch "master" source_branch "master"
target_branch "stable" target_branch "stable"
trait :closed do
closed true
end
# pick 3 commits "at random" (from bcf03b5d~3 to bcf03b5d) # pick 3 commits "at random" (from bcf03b5d~3 to bcf03b5d)
trait :with_diffs do trait :with_diffs do
target_branch "master" # pretend bcf03b5d~3 target_branch "master" # pretend bcf03b5d~3
...@@ -85,7 +86,16 @@ FactoryGirl.define do ...@@ -85,7 +86,16 @@ FactoryGirl.define do
end end
end end
trait :closed do
state :closed
end
trait :reopened do
state :reopened
end
factory :closed_merge_request, traits: [:closed] factory :closed_merge_request, traits: [:closed]
factory :reopened_merge_request, traits: [:reopened]
factory :merge_request_with_diffs, traits: [:with_diffs] factory :merge_request_with_diffs, traits: [:with_diffs]
end end
...@@ -159,6 +169,12 @@ FactoryGirl.define do ...@@ -159,6 +169,12 @@ FactoryGirl.define do
factory :milestone do factory :milestone do
title title
project project
trait :closed do
state :closed
end
factory :closed_milestone, traits: [:closed]
end end
factory :system_hook do factory :system_hook do
......
...@@ -15,7 +15,6 @@ describe Issue, "Issuable" do ...@@ -15,7 +15,6 @@ describe Issue, "Issuable" do
it { should validate_presence_of(:author) } it { should validate_presence_of(:author) }
it { should validate_presence_of(:title) } it { should validate_presence_of(:title) }
it { should ensure_length_of(:title).is_at_least(0).is_at_most(255) } it { should ensure_length_of(:title).is_at_least(0).is_at_most(255) }
it { should ensure_inclusion_of(:closed).in_array([true, false]) }
end end
describe "Scope" do describe "Scope" do
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
# project_id :integer # project_id :integer
# created_at :datetime not null # created_at :datetime not null
# updated_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) # position :integer default(0)
# branch_name :string(255) # branch_name :string(255)
# description :text # description :text
...@@ -44,34 +44,15 @@ describe Issue do ...@@ -44,34 +44,15 @@ describe Issue do
end end
end end
describe '#is_being_closed?' do describe '#is_being_reassigned?' do
it 'returns true if the closed attribute has changed and is now true' do it 'returnes issues assigned to user' do
subject.closed = true user = create :user
subject.is_being_closed?.should be_true
end
it 'returns false if the closed attribute has changed and is now false' do
issue = create(:closed_issue)
issue.closed = false
issue.is_being_closed?.should be_false
end
it 'returns false if the closed attribute has not changed' do
subject.is_being_closed?.should be_false
end
end
describe '#is_being_reopened?' do 2.times do
it 'returns true if the closed attribute has changed and is now false' do issue = create :issue, assignee: user
issue = create(:closed_issue)
issue.closed = false
issue.is_being_reopened?.should be_true
end end
it 'returns false if the closed attribute has changed and is now true' do
subject.closed = true Issue.open_for(user).count.should eq 2
subject.is_being_reopened?.should be_false
end
it 'returns false if the closed attribute has not changed' do
subject.is_being_reopened?.should be_false
end end
end end
end end
...@@ -15,7 +15,7 @@ ...@@ -15,7 +15,7 @@
# st_commits :text(2147483647) # st_commits :text(2147483647)
# st_diffs :text(2147483647) # st_diffs :text(2147483647)
# merged :boolean default(FALSE), not null # merged :boolean default(FALSE), not null
# state :integer default(1), not null # merge_status :integer default(1), not null
# milestone_id :integer # milestone_id :integer
# #
...@@ -36,6 +36,10 @@ describe MergeRequest do ...@@ -36,6 +36,10 @@ describe MergeRequest do
it { should include_module(Issuable) } it { should include_module(Issuable) }
end end
describe "#mr_and_commit_notes" do
end
describe "#mr_and_commit_notes" do describe "#mr_and_commit_notes" do
let!(:merge_request) { create(:merge_request) } let!(:merge_request) { create(:merge_request) }
...@@ -62,35 +66,4 @@ describe MergeRequest do ...@@ -62,35 +66,4 @@ describe MergeRequest do
subject.is_being_reassigned?.should be_false subject.is_being_reassigned?.should be_false
end end
end end
describe '#is_being_closed?' do
it 'returns true if the closed attribute has changed and is now true' do
subject.closed = true
subject.is_being_closed?.should be_true
end
it 'returns false if the closed attribute has changed and is now false' do
merge_request = create(:closed_merge_request)
merge_request.closed = false
merge_request.is_being_closed?.should be_false
end
it 'returns false if the closed attribute has not changed' do
subject.is_being_closed?.should be_false
end
end
describe '#is_being_reopened?' do
it 'returns true if the closed attribute has changed and is now false' do
merge_request = create(:closed_merge_request)
merge_request.closed = false
merge_request.is_being_reopened?.should be_true
end
it 'returns false if the closed attribute has changed and is now true' do
subject.closed = true
subject.is_being_reopened?.should be_false
end
it 'returns false if the closed attribute has not changed' do
subject.is_being_reopened?.should be_false
end
end
end end
...@@ -7,7 +7,7 @@ ...@@ -7,7 +7,7 @@
# project_id :integer not null # project_id :integer not null
# description :text # description :text
# due_date :date # due_date :date
# closed :boolean default(FALSE), not null # state :string default(FALSE), not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# #
...@@ -27,7 +27,6 @@ describe Milestone do ...@@ -27,7 +27,6 @@ describe Milestone do
describe "Validation" do describe "Validation" do
it { should validate_presence_of(:title) } it { should validate_presence_of(:title) }
it { should validate_presence_of(:project) } it { should validate_presence_of(:project) }
it { should ensure_inclusion_of(:closed).in_array([true, false]) }
end end
let(:milestone) { create(:milestone) } let(:milestone) { create(:milestone) }
...@@ -41,7 +40,7 @@ describe Milestone do ...@@ -41,7 +40,7 @@ describe Milestone do
it "should count closed issues" do it "should count closed issues" do
IssueObserver.current_user = issue.author IssueObserver.current_user = issue.author
issue.update_attributes(closed: true) issue.close
milestone.issues << issue milestone.issues << issue
milestone.percent_complete.should == 100 milestone.percent_complete.should == 100
end end
...@@ -96,7 +95,7 @@ describe Milestone do ...@@ -96,7 +95,7 @@ describe Milestone do
describe :items_count do describe :items_count do
before do before do
milestone.issues << create(:issue) milestone.issues << create(:issue)
milestone.issues << create(:issue, closed: true) milestone.issues << create(:closed_issue)
milestone.merge_requests << create(:merge_request) milestone.merge_requests << create(:merge_request)
end end
...@@ -110,7 +109,35 @@ describe Milestone do ...@@ -110,7 +109,35 @@ describe Milestone do
it { milestone.can_be_closed?.should be_true } it { milestone.can_be_closed?.should be_true }
end end
describe :open? do describe :is_empty? do
it { milestone.open?.should be_true } before do
issue = create :closed_issue, milestone: milestone
merge_request = create :merge_request, milestone: milestone
end
it 'Should return total count of issues and merge requests assigned to milestone' do
milestone.total_items_count.should eq 2
end
end
describe :can_be_closed? do
before do
milestone = create :milestone
create :closed_issue, milestone: milestone
issue = create :issue
end
it 'should be true if milestone active and all nestied issues closed' do
milestone.can_be_closed?.should be_true
end
it 'should be false if milestone active and not all nestied issues closed' do
issue.milestone = milestone
issue.save
milestone.can_be_closed?.should be_false
end end
end
end end
...@@ -121,10 +121,7 @@ describe Project do ...@@ -121,10 +121,7 @@ describe Project do
let(:project) { create(:project) } let(:project) { create(:project) }
before do before do
@merge_request = create(:merge_request, @merge_request = create(:merge_request, project: project)
project: project,
merged: false,
closed: false)
@key = create(:key, user_id: project.owner.id) @key = create(:key, user_id: project.owner.id)
end end
...@@ -133,8 +130,7 @@ describe Project do ...@@ -133,8 +130,7 @@ describe Project do
@merge_request.last_commit.id.should == "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a" @merge_request.last_commit.id.should == "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a"
project.update_merge_requests("8716fc78f3c65bbf7bcf7b574febd583bc5d2812", "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a", "refs/heads/stable", @key.user) project.update_merge_requests("8716fc78f3c65bbf7bcf7b574febd583bc5d2812", "bcf03b5de6c33f3869ef70d68cf06e679d1d7f9a", "refs/heads/stable", @key.user)
@merge_request.reload @merge_request.reload
@merge_request.merged.should be_true @merge_request.merged?.should be_true
@merge_request.closed.should be_true
end end
it "should update merge request commits with new one if pushed to source branch" do it "should update merge request commits with new one if pushed to source branch" do
......
This diff is collapsed.
require 'spec_helper' require 'spec_helper'
describe MergeRequestObserver do describe MergeRequestObserver do
let(:some_user) { double(:user, id: 1) } let(:some_user) { create :user }
let(:assignee) { double(:user, id: 2) } let(:assignee) { create :user }
let(:author) { double(:user, id: 3) } let(:author) { create :user }
let(:mr) { double(:merge_request, id: 42, assignee: assignee, author: author) } let(:mr_mock) { double(:merge_request, id: 42, assignee: assignee, author: author) }
let(:assigned_mr) { create(:merge_request, assignee: assignee, author: author) }
let(:unassigned_mr) { create(:merge_request, author: author) }
let(:closed_assigned_mr) { create(:closed_merge_request, assignee: assignee, author: author) }
let(:closed_unassigned_mr) { create(:closed_merge_request, author: author) }
before(:each) { subject.stub(:current_user).and_return(some_user) } before(:each) { subject.stub(:current_user).and_return(some_user) }
...@@ -21,23 +25,21 @@ describe MergeRequestObserver do ...@@ -21,23 +25,21 @@ describe MergeRequestObserver do
end end
it 'sends an email to the assignee' do it 'sends an email to the assignee' do
Notify.should_receive(:new_merge_request_email).with(mr.id) Notify.should_receive(:new_merge_request_email).with(mr_mock.id)
subject.after_create(mr) subject.after_create(mr_mock)
end end
it 'does not send an email to the assignee if assignee created the merge request' do it 'does not send an email to the assignee if assignee created the merge request' do
subject.stub(:current_user).and_return(assignee) subject.stub(:current_user).and_return(assignee)
Notify.should_not_receive(:new_merge_request_email) Notify.should_not_receive(:new_merge_request_email)
subject.after_create(mr) subject.after_create(mr_mock)
end end
end end
context '#after_update' do context '#after_update' do
before(:each) do before(:each) do
mr.stub(:is_being_reassigned?).and_return(false) mr_mock.stub(:is_being_reassigned?).and_return(false)
mr.stub(:is_being_closed?).and_return(false)
mr.stub(:is_being_reopened?).and_return(false)
end end
it 'is called when a merge request is changed' do it 'is called when a merge request is changed' do
...@@ -52,97 +54,50 @@ describe MergeRequestObserver do ...@@ -52,97 +54,50 @@ describe MergeRequestObserver do
context 'a reassigned email' do context 'a reassigned email' do
it 'is sent if the merge request is being reassigned' do it 'is sent if the merge request is being reassigned' do
mr.should_receive(:is_being_reassigned?).and_return(true) mr_mock.should_receive(:is_being_reassigned?).and_return(true)
subject.should_receive(:send_reassigned_email).with(mr) subject.should_receive(:send_reassigned_email).with(mr_mock)
subject.after_update(mr) subject.after_update(mr_mock)
end end
it 'is not sent if the merge request is not being reassigned' do it 'is not sent if the merge request is not being reassigned' do
mr.should_receive(:is_being_reassigned?).and_return(false) mr_mock.should_receive(:is_being_reassigned?).and_return(false)
subject.should_not_receive(:send_reassigned_email) subject.should_not_receive(:send_reassigned_email)
subject.after_update(mr) subject.after_update(mr_mock)
end end
end end
context 'a status "closed"' do
it 'note is created if the merge request is being closed' do
mr.should_receive(:is_being_closed?).and_return(true)
Note.should_receive(:create_status_change_note).with(mr, some_user, 'closed')
subject.after_update(mr)
end
it 'note is not created if the merge request is not being closed' do
mr.should_receive(:is_being_closed?).and_return(false)
Note.should_not_receive(:create_status_change_note).with(mr, some_user, 'closed')
subject.after_update(mr)
end end
it 'notification is delivered if the merge request being closed' do context '#after_close' do
mr.stub(:is_being_closed?).and_return(true) context 'a status "closed"' do
Note.should_receive(:create_status_change_note).with(mr, some_user, 'closed') it 'note is created if the merge request is being closed' do
Note.should_receive(:create_status_change_note).with(assigned_mr, some_user, 'closed')
subject.after_update(mr)
end
it 'notification is not delivered if the merge request not being closed' do
mr.stub(:is_being_closed?).and_return(false)
Note.should_not_receive(:create_status_change_note).with(mr, some_user, 'closed')
subject.after_update(mr) assigned_mr.close
end end
it 'notification is delivered only to author if the merge request is being closed' do it 'notification is delivered only to author if the merge request is being closed' do
mr_without_assignee = double(:merge_request, id: 42, author: author, assignee: nil) Note.should_receive(:create_status_change_note).with(unassigned_mr, some_user, 'closed')
mr_without_assignee.stub(:is_being_reassigned?).and_return(false)
mr_without_assignee.stub(:is_being_closed?).and_return(true)
mr_without_assignee.stub(:is_being_reopened?).and_return(false)
Note.should_receive(:create_status_change_note).with(mr_without_assignee, some_user, 'closed')
subject.after_update(mr_without_assignee) unassigned_mr.close
end
end end
end end
context '#after_reopen' do
context 'a status "reopened"' do context 'a status "reopened"' do
it 'note is created if the merge request is being reopened' do it 'note is created if the merge request is being reopened' do
mr.should_receive(:is_being_reopened?).and_return(true) Note.should_receive(:create_status_change_note).with(closed_assigned_mr, some_user, 'reopened')
Note.should_receive(:create_status_change_note).with(mr, some_user, 'reopened')
subject.after_update(mr)
end
it 'note is not created if the merge request is not being reopened' do
mr.should_receive(:is_being_reopened?).and_return(false)
Note.should_not_receive(:create_status_change_note).with(mr, some_user, 'reopened')
subject.after_update(mr)
end
it 'notification is delivered if the merge request being reopened' do
mr.stub(:is_being_reopened?).and_return(true)
Note.should_receive(:create_status_change_note).with(mr, some_user, 'reopened')
subject.after_update(mr)
end
it 'notification is not delivered if the merge request is not being reopened' do
mr.stub(:is_being_reopened?).and_return(false)
Note.should_not_receive(:create_status_change_note).with(mr, some_user, 'reopened')
subject.after_update(mr) closed_assigned_mr.reopen
end end
it 'notification is delivered only to author if the merge request is being reopened' do it 'notification is delivered only to author if the merge request is being reopened' do
mr_without_assignee = double(:merge_request, id: 42, author: author, assignee: nil) Note.should_receive(:create_status_change_note).with(closed_unassigned_mr, some_user, 'reopened')
mr_without_assignee.stub(:is_being_reassigned?).and_return(false)
mr_without_assignee.stub(:is_being_closed?).and_return(false)
mr_without_assignee.stub(:is_being_reopened?).and_return(true)
Note.should_receive(:create_status_change_note).with(mr_without_assignee, some_user, 'reopened')
subject.after_update(mr_without_assignee) closed_unassigned_mr.reopen
end end
end end
end end
...@@ -151,23 +106,23 @@ describe MergeRequestObserver do ...@@ -151,23 +106,23 @@ describe MergeRequestObserver do
let(:previous_assignee) { double(:user, id: 3) } let(:previous_assignee) { double(:user, id: 3) }
before(:each) do before(:each) do
mr.stub(:assignee_id).and_return(assignee.id) mr_mock.stub(:assignee_id).and_return(assignee.id)
mr.stub(:assignee_id_was).and_return(previous_assignee.id) mr_mock.stub(:assignee_id_was).and_return(previous_assignee.id)
end end
def it_sends_a_reassigned_email_to(recipient) def it_sends_a_reassigned_email_to(recipient)
Notify.should_receive(:reassigned_merge_request_email).with(recipient, mr.id, previous_assignee.id) Notify.should_receive(:reassigned_merge_request_email).with(recipient, mr_mock.id, previous_assignee.id)
end end
def it_does_not_send_a_reassigned_email_to(recipient) def it_does_not_send_a_reassigned_email_to(recipient)
Notify.should_not_receive(:reassigned_merge_request_email).with(recipient, mr.id, previous_assignee.id) Notify.should_not_receive(:reassigned_merge_request_email).with(recipient, mr_mock.id, previous_assignee.id)
end end
it 'sends a reassigned email to the previous and current assignees' do it 'sends a reassigned email to the previous and current assignees' do
it_sends_a_reassigned_email_to assignee.id it_sends_a_reassigned_email_to assignee.id
it_sends_a_reassigned_email_to previous_assignee.id it_sends_a_reassigned_email_to previous_assignee.id
subject.send(:send_reassigned_email, mr) subject.send(:send_reassigned_email, mr_mock)
end end
context 'does not send an email to the user who made the reassignment' do context 'does not send an email to the user who made the reassignment' do
...@@ -176,14 +131,14 @@ describe MergeRequestObserver do ...@@ -176,14 +131,14 @@ describe MergeRequestObserver do
it_sends_a_reassigned_email_to previous_assignee.id it_sends_a_reassigned_email_to previous_assignee.id
it_does_not_send_a_reassigned_email_to assignee.id it_does_not_send_a_reassigned_email_to assignee.id
subject.send(:send_reassigned_email, mr) subject.send(:send_reassigned_email, mr_mock)
end end
it 'if the user is the previous assignee' do it 'if the user is the previous assignee' do
subject.stub(:current_user).and_return(previous_assignee) subject.stub(:current_user).and_return(previous_assignee)
it_sends_a_reassigned_email_to assignee.id it_sends_a_reassigned_email_to assignee.id
it_does_not_send_a_reassigned_email_to previous_assignee.id it_does_not_send_a_reassigned_email_to previous_assignee.id
subject.send(:send_reassigned_email, mr) subject.send(:send_reassigned_email, mr_mock)
end end
end end
end end
......
...@@ -54,14 +54,24 @@ describe Gitlab::API do ...@@ -54,14 +54,24 @@ describe Gitlab::API do
end end
end end
describe "PUT /projects/:id/issues/:issue_id" do describe "PUT /projects/:id/issues/:issue_id to update only title" do
it "should update a project issue" do it "should update a project issue" do
put api("/projects/#{project.id}/issues/#{issue.id}", user), put api("/projects/#{project.id}/issues/#{issue.id}", user),
title: 'updated title', labels: 'label2', closed: 1 title: 'updated title'
response.status.should == 200 response.status.should == 200
json_response['title'].should == 'updated title' json_response['title'].should == 'updated title'
end
end
describe "PUT /projects/:id/issues/:issue_id to update state and label" do
it "should update a project issue" do
put api("/projects/#{project.id}/issues/#{issue.id}", user),
labels: 'label2', state_event: "close"
response.status.should == 200
json_response['labels'].should == ['label2'] json_response['labels'].should == ['label2']
json_response['closed'].should be_true json_response['state'].should eq "closed"
end end
end end
......
...@@ -43,6 +43,23 @@ describe Gitlab::API do ...@@ -43,6 +43,23 @@ describe Gitlab::API do
end end
end end
describe "PUT /projects/:id/merge_request/:merge_request_id to close MR" do
it "should return merge_request" do
put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), state_event: "close"
response.status.should == 200
json_response['state'].should == 'closed'
end
end
describe "PUT /projects/:id/merge_request/:merge_request_id to merge MR" do
it "should return merge_request" do
put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), state_event: "merge"
response.status.should == 200
json_response['state'].should == 'merged'
end
end
describe "PUT /projects/:id/merge_request/:merge_request_id" do describe "PUT /projects/:id/merge_request/:merge_request_id" do
it "should return merge_request" do it "should return merge_request" do
put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), title: "New title" put api("/projects/#{project.id}/merge_request/#{merge_request.id}", user), title: "New title"
......
...@@ -44,4 +44,14 @@ describe Gitlab::API do ...@@ -44,4 +44,14 @@ describe Gitlab::API do
json_response['title'].should == 'updated title' json_response['title'].should == 'updated title'
end end
end end
describe "PUT /projects/:id/milestones/:milestone_id to close milestone" do
it "should update a project milestone" do
put api("/projects/#{project.id}/milestones/#{milestone.id}", user),
state_event: 'close'
response.status.should == 200
json_response['state'].should == 'closed'
end
end
end end
...@@ -58,8 +58,7 @@ describe "Issues" do ...@@ -58,8 +58,7 @@ describe "Issues" do
it "should be able to search on different statuses" do it "should be able to search on different statuses" do
issue = Issue.first # with title 'foobar' issue = Issue.first # with title 'foobar'
issue.closed = true issue.close
issue.save
visit project_issues_path(project) visit project_issues_path(project)
click_link 'Closed' click_link 'Closed'
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment