summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelipe Artur <fcardozo@gitlab.com>2017-07-07 15:08:49 +0000
committerSean McGivern <sean@mcgivern.me.uk>2017-07-07 15:08:49 +0000
commitb5f596c3ffb655b6e4fee127fa9336c829198b5b (patch)
tree202d725fcd56434b82c37037645f88839013ba53
parent1a3edcec4363239a4d080bc9af53d9d455dccfb4 (diff)
downloadgitlab-ce-b5f596c3ffb655b6e4fee127fa9336c829198b5b.tar.gz
Native group milestones
-rw-r--r--app/controllers/groups/milestones_controller.rb77
-rw-r--r--app/controllers/projects/milestones_controller.rb28
-rw-r--r--app/finders/issuable_finder.rb17
-rw-r--r--app/finders/milestones_finder.rb59
-rw-r--r--app/helpers/milestones_helper.rb14
-rw-r--r--app/models/concerns/internal_id.rb3
-rw-r--r--app/models/concerns/issuable.rb1
-rw-r--r--app/models/concerns/milestoneish.rb16
-rw-r--r--app/models/dashboard_milestone.rb4
-rw-r--r--app/models/global_milestone.rb47
-rw-r--r--app/models/group.rb1
-rw-r--r--app/models/group_milestone.rb4
-rw-r--r--app/models/milestone.rb72
-rw-r--r--app/services/issuable_base_service.rb15
-rw-r--r--app/services/issues/move_service.rb14
-rw-r--r--app/services/milestones/base_service.rb6
-rw-r--r--app/services/milestones/close_service.rb2
-rw-r--r--app/services/milestones/create_service.rb4
-rw-r--r--app/services/milestones/reopen_service.rb2
-rw-r--r--app/services/milestones/update_service.rb4
-rw-r--r--app/views/groups/milestones/_form.html.haml27
-rw-r--r--app/views/groups/milestones/_milestone.html.haml3
-rw-r--r--app/views/groups/milestones/edit.html.haml7
-rw-r--r--app/views/groups/milestones/index.html.haml5
-rw-r--r--app/views/groups/milestones/new.html.haml38
-rw-r--r--app/views/groups/milestones/show.html.haml2
-rw-r--r--app/views/shared/milestones/_milestone.html.haml35
-rw-r--r--app/views/shared/milestones/_top.html.haml64
-rw-r--r--changelogs/unreleased/issue_30126_be.yml4
-rw-r--r--config/routes/group.rb2
-rw-r--r--db/migrate/20170723183807_add_group_id_to_milestones.rb18
-rw-r--r--db/migrate/20170724184243_add_group_milestone_id_indexes.rb19
-rw-r--r--db/schema.rb7
-rw-r--r--doc/user/project/milestones/index.md13
-rw-r--r--features/group/milestones.feature4
-rw-r--r--features/steps/group/milestones.rb9
-rw-r--r--lib/api/entities.rb9
-rw-r--r--spec/controllers/groups/milestones_controller_spec.rb118
-rw-r--r--spec/controllers/projects/milestones_controller_spec.rb34
-rw-r--r--spec/factories/milestones.rb22
-rw-r--r--spec/features/groups/milestone_spec.rb28
-rw-r--r--spec/features/milestone_spec.rb23
-rw-r--r--spec/finders/issues_finder_spec.rb17
-rw-r--r--spec/finders/merge_requests_finder_spec.rb19
-rw-r--r--spec/finders/milestones_finder_spec.rb90
-rw-r--r--spec/fixtures/api/schemas/public_api/v3/issues.json3
-rw-r--r--spec/fixtures/api/schemas/public_api/v3/merge_requests.json3
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/issues.json3
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/merge_requests.json3
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml1
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/models/milestone_spec.rb44
-rw-r--r--spec/services/issues/move_service_spec.rb65
-rw-r--r--spec/services/issues/update_service_spec.rb6
-rw-r--r--spec/services/merge_requests/update_service_spec.rb6
-rw-r--r--spec/support/issuable_shared_examples.rb31
56 files changed, 935 insertions, 238 deletions
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index 6b1d418fc9a..5c10d7bc261 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -2,13 +2,13 @@ class Groups::MilestonesController < Groups::ApplicationController
include MilestoneActions
before_action :group_projects
- before_action :milestone, only: [:show, :update, :merge_requests, :participants, :labels]
- before_action :authorize_admin_milestones!, only: [:new, :create, :update]
+ before_action :milestone, only: [:edit, :show, :update, :merge_requests, :participants, :labels]
+ before_action :authorize_admin_milestones!, only: [:edit, :new, :create, :update]
def index
respond_to do |format|
format.html do
- @milestone_states = GlobalMilestone.states_count(@projects)
+ @milestone_states = GlobalMilestone.states_count(group_projects, group)
@milestones = Kaminari.paginate_array(milestones).page(params[:page])
end
format.json do
@@ -22,49 +22,41 @@ class Groups::MilestonesController < Groups::ApplicationController
end
def create
- project_ids = params[:milestone][:project_ids].reject(&:blank?)
- title = milestone_params[:title]
+ @milestone = Milestones::CreateService.new(group, current_user, milestone_params).execute
- if create_milestones(project_ids)
- redirect_to milestone_path(title)
+ if @milestone.persisted?
+ redirect_to milestone_path
else
- render_new_with_error(project_ids.empty?)
+ render "new"
end
end
def show
end
- def update
- @milestone.milestones.each do |milestone|
- Milestones::UpdateService.new(milestone.project, current_user, milestone_params).execute(milestone)
- end
-
- redirect_back_or_default(default: milestone_path(@milestone.title))
+ def edit
+ render_404 if @milestone.is_legacy_group_milestone?
end
- private
-
- def create_milestones(project_ids)
- return false unless project_ids.present?
+ def update
+ # Keep this compatible with legacy group milestones where we have to update
+ # all projects milestones states at once.
+ if @milestone.is_legacy_group_milestone?
+ update_params = milestone_params.select { |key| key == "state_event" }
+ milestones = @milestone.milestones
+ else
+ update_params = milestone_params
+ milestones = [@milestone]
+ end
- ActiveRecord::Base.transaction do
- @projects.where(id: project_ids).each do |project|
- Milestones::CreateService.new(project, current_user, milestone_params).execute
- end
+ milestones.each do |milestone|
+ Milestones::UpdateService.new(milestone.parent, current_user, update_params).execute(milestone)
end
- true
- rescue ActiveRecord::ActiveRecordError => e
- flash.now[:alert] = "An error occurred while creating the milestone: #{e.message}"
- false
+ redirect_to milestone_path
end
- def render_new_with_error(empty_project_ids)
- @milestone = Milestone.new(milestone_params)
- @milestone.errors.add(:base, "Please select at least one project.") if empty_project_ids
- render :new
- end
+ private
def authorize_admin_milestones!
return render_404 unless can?(current_user, :admin_milestones, group)
@@ -74,16 +66,31 @@ class Groups::MilestonesController < Groups::ApplicationController
params.require(:milestone).permit(:title, :description, :start_date, :due_date, :state_event)
end
- def milestone_path(title)
- group_milestone_path(@group, title.to_slug.to_s, title: title)
+ def milestone_path
+ if @milestone.is_legacy_group_milestone?
+ group_milestone_path(group, @milestone.safe_title, title: @milestone.title)
+ else
+ group_milestone_path(group, @milestone.iid)
+ end
end
def milestones
- @milestones = GroupMilestone.build_collection(@group, @projects, params)
+ search_params = params.merge(group_ids: group.id)
+
+ milestones = MilestonesFinder.new(search_params).execute
+ legacy_milestones = GroupMilestone.build_collection(group, group_projects, params)
+
+ milestones + legacy_milestones
end
def milestone
- @milestone = GroupMilestone.build(@group, @projects, params[:title])
+ @milestone =
+ if params[:title]
+ GroupMilestone.build(group, group_projects, params[:title])
+ else
+ group.milestones.find_by_iid(params[:id])
+ end
+
render_404 unless @milestone
end
end
diff --git a/app/controllers/projects/milestones_controller.rb b/app/controllers/projects/milestones_controller.rb
index c4723c72136..c94384d2a1a 100644
--- a/app/controllers/projects/milestones_controller.rb
+++ b/app/controllers/projects/milestones_controller.rb
@@ -13,20 +13,16 @@ class Projects::MilestonesController < Projects::ApplicationController
respond_to :html
def index
- @milestones =
- case params[:state]
- when 'all' then @project.milestones
- when 'closed' then @project.milestones.closed
- else @project.milestones.active
- end
-
@sort = params[:sort] || 'due_date_asc'
- @milestones = @milestones.sort(@sort)
+ @milestones = milestones.sort(@sort)
respond_to do |format|
format.html do
@project_namespace = @project.namespace.becomes(Namespace)
- @milestones = @milestones.includes(:project)
+ # We need to show group milestones in the JSON response
+ # so that people can filter by and assign group milestones,
+ # but we don't need to show them on the project milestones page itself.
+ @milestones = @milestones.for_projects
@milestones = @milestones.page(params[:page])
end
format.json do
@@ -51,7 +47,7 @@ class Projects::MilestonesController < Projects::ApplicationController
def create
@milestone = Milestones::CreateService.new(project, current_user, milestone_params).execute
- if @milestone.save
+ if @milestone.valid?
redirect_to project_milestone_path(@project, @milestone)
else
render "new"
@@ -86,6 +82,18 @@ class Projects::MilestonesController < Projects::ApplicationController
protected
+ def milestones
+ @milestones ||= begin
+ if @project.group && can?(current_user, :read_group, @project.group)
+ group = @project.group
+ end
+
+ search_params = params.merge(project_ids: @project.id, group_ids: group&.id)
+
+ MilestonesFinder.new(search_params).execute
+ end
+ end
+
def milestone
@milestone ||= @project.milestones.find_by!(iid: params[:id])
end
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index 7bc2117f61e..d81e9ed17d4 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -147,9 +147,17 @@ class IssuableFinder
@milestones =
if milestones?
- scope = Milestone.where(project_id: projects)
+ if project?
+ group_id = project.group&.id
+ project_id = project.id
+ end
+
+ group_id = group.id if group
- scope.where(title: params[:milestone_title])
+ search_params =
+ { title: params[:milestone_title], project_ids: project_id, group_ids: group_id }
+
+ MilestonesFinder.new(search_params).execute
else
Milestone.none
end
@@ -331,11 +339,6 @@ class IssuableFinder
items = items.left_joins_milestones.where('milestones.start_date <= NOW()')
else
items = items.with_milestone(params[:milestone_title])
- items_projects = projects(items)
-
- if items_projects
- items = items.where(milestones: { project_id: items_projects })
- end
end
end
diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb
index 630c73c2a94..23c42a5f662 100644
--- a/app/finders/milestones_finder.rb
+++ b/app/finders/milestones_finder.rb
@@ -1,12 +1,55 @@
+# Search for milestones
+#
+# params - Hash
+# project_ids: Array of project ids or single project id.
+# group_ids: Array of group ids or single group id.
+# order - Orders by field default due date asc.
+# title - filter by title.
+# state - filters by state.
+
class MilestonesFinder
- def execute(projects, params)
- milestones = Milestone.of_projects(projects)
- milestones = milestones.reorder("due_date ASC")
-
- case params[:state]
- when 'closed' then milestones.closed
- when 'all' then milestones
- else milestones.active
+ attr_reader :params, :project_ids, :group_ids
+
+ def initialize(params = {})
+ @project_ids = Array(params[:project_ids])
+ @group_ids = Array(params[:group_ids])
+ @params = params
+ end
+
+ def execute
+ return Milestone.none if project_ids.empty? && group_ids.empty?
+
+ items = Milestone.all
+ items = by_groups_and_projects(items)
+ items = by_title(items)
+ items = by_state(items)
+
+ order(items)
+ end
+
+ private
+
+ def by_groups_and_projects(items)
+ items.for_projects_and_groups(project_ids, group_ids)
+ end
+
+ def by_title(items)
+ if params[:title]
+ items.where(title: params[:title])
+ else
+ items
+ end
+ end
+
+ def by_state(items)
+ Milestone.filter_by_state(items, params[:state])
+ end
+
+ def order(items)
+ if params.has_key?(:order)
+ items.reorder(params[:order])
+ else
+ items.reorder('due_date ASC')
end
end
end
diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb
index 8c7851dcfc2..f8860bfee99 100644
--- a/app/helpers/milestones_helper.rb
+++ b/app/helpers/milestones_helper.rb
@@ -54,8 +54,10 @@ module MilestonesHelper
def milestone_class_for_state(param, check, match_blank_param = false)
if match_blank_param
'active' if param.blank? || param == check
+ elsif param == check
+ 'active'
else
- 'active' if param == check
+ check
end
end
@@ -147,4 +149,14 @@ module MilestonesHelper
labels_dashboard_milestone_path(milestone, title: milestone.title, format: :json)
end
end
+
+ def group_milestone_route(milestone, params = {})
+ params = nil if params.empty?
+
+ if milestone.is_legacy_group_milestone?
+ group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: params)
+ else
+ group_milestone_path(@group, milestone.iid, milestone: params)
+ end
+ end
end
diff --git a/app/models/concerns/internal_id.rb b/app/models/concerns/internal_id.rb
index 5382dde6765..67a0adfcd56 100644
--- a/app/models/concerns/internal_id.rb
+++ b/app/models/concerns/internal_id.rb
@@ -8,7 +8,8 @@ module InternalId
def set_iid
if iid.blank?
- records = project.send(self.class.name.tableize)
+ parent = project || group
+ records = parent.send(self.class.name.tableize)
records = records.with_deleted if self.paranoid?
max_iid = records.maximum(:iid)
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 23cb85600da..13fe9d09c69 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -30,6 +30,7 @@ module Issuable
belongs_to :updated_by, class_name: "User"
belongs_to :last_edited_by, class_name: 'User'
belongs_to :milestone
+
has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do # rubocop:disable Cop/ActiveRecordDependent
def authors_loaded?
# We check first if we're loaded to not load unnecessarily.
diff --git a/app/models/concerns/milestoneish.rb b/app/models/concerns/milestoneish.rb
index 01599ce49c6..f0998465822 100644
--- a/app/models/concerns/milestoneish.rb
+++ b/app/models/concerns/milestoneish.rb
@@ -70,6 +70,22 @@ module Milestoneish
due_date && due_date.past?
end
+ def is_group_milestone?
+ false
+ end
+
+ def is_project_milestone?
+ false
+ end
+
+ def is_legacy_group_milestone?
+ false
+ end
+
+ def is_dashboard_milestone?
+ false
+ end
+
private
def count_issues_by_state(user)
diff --git a/app/models/dashboard_milestone.rb b/app/models/dashboard_milestone.rb
index 646c1e5ce1a..fac7c5e5c85 100644
--- a/app/models/dashboard_milestone.rb
+++ b/app/models/dashboard_milestone.rb
@@ -2,4 +2,8 @@ class DashboardMilestone < GlobalMilestone
def issues_finder_params
{ authorized_only: true }
end
+
+ def is_dashboard_milestone?
+ true
+ end
end
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index 538615130a7..c0864769314 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -2,6 +2,7 @@ class GlobalMilestone
include Milestoneish
EPOCH = DateTime.parse('1970-01-01')
+ STATE_COUNT_HASH = { opened: 0, closed: 0, all: 0 }.freeze
attr_accessor :title, :milestones
alias_attribute :name, :title
@@ -11,7 +12,10 @@ class GlobalMilestone
end
def self.build_collection(projects, params)
- child_milestones = MilestonesFinder.new.execute(projects, params)
+ params =
+ { project_ids: projects.map(&:id), state: params[:state] }
+
+ child_milestones = MilestonesFinder.new(params).execute
milestones = child_milestones.select(:id, :title).group_by(&:title).map do |title, grouped|
milestones_relation = Milestone.where(id: grouped.map(&:id))
@@ -28,13 +32,42 @@ class GlobalMilestone
new(title, child_milestones)
end
- def self.states_count(projects)
- relation = MilestonesFinder.new.execute(projects, state: 'all')
- milestones_by_state_and_title = relation.reorder(nil).group(:state, :title).count
+ def self.states_count(projects, group = nil)
+ legacy_group_milestones_count = legacy_group_milestone_states_count(projects)
+ group_milestones_count = group_milestones_states_count(group)
+
+ legacy_group_milestones_count.merge(group_milestones_count) do |k, legacy_group_milestones_count, group_milestones_count|
+ legacy_group_milestones_count + group_milestones_count
+ end
+ end
+
+ def self.group_milestones_states_count(group)
+ return STATE_COUNT_HASH unless group
+
+ params = { group_ids: [group.id], state: 'all', order: nil }
+
+ relation = MilestonesFinder.new(params).execute
+ grouped_by_state = relation.group(:state).count
+
+ {
+ opened: grouped_by_state['active'] || 0,
+ closed: grouped_by_state['closed'] || 0,
+ all: grouped_by_state.values.sum
+ }
+ end
+
+ # Counts the legacy group milestones which must be grouped by title
+ def self.legacy_group_milestone_states_count(projects)
+ return STATE_COUNT_HASH unless projects
+
+ params = { project_ids: projects.map(&:id), state: 'all', order: nil }
+
+ relation = MilestonesFinder.new(params).execute
+ project_milestones_by_state_and_title = relation.group(:state, :title).count
- opened = count_by_state(milestones_by_state_and_title, 'active')
- closed = count_by_state(milestones_by_state_and_title, 'closed')
- all = milestones_by_state_and_title.map { |(_, title), _| title }.uniq.count
+ opened = count_by_state(project_milestones_by_state_and_title, 'active')
+ closed = count_by_state(project_milestones_by_state_and_title, 'closed')
+ all = project_milestones_by_state_and_title.map { |(_, title), _| title }.uniq.count
{
opened: opened,
diff --git a/app/models/group.rb b/app/models/group.rb
index f29e642ac91..70a4ceeffd8 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -18,6 +18,7 @@ class Group < Namespace
has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent
+ has_many :milestones
has_many :project_group_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :shared_projects, through: :project_group_links, source: :project
has_many :notification_settings, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
diff --git a/app/models/group_milestone.rb b/app/models/group_milestone.rb
index 86d38e5468b..65249bd7bfc 100644
--- a/app/models/group_milestone.rb
+++ b/app/models/group_milestone.rb
@@ -16,4 +16,8 @@ class GroupMilestone < GlobalMilestone
def issues_finder_params
{ group_id: group.id }
end
+
+ def is_legacy_group_milestone?
+ true
+ end
end
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index c0ccbf8e27e..48d00764965 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -18,17 +18,32 @@ class Milestone < ActiveRecord::Base
cache_markdown_field :description
belongs_to :project
+ belongs_to :group
+
has_many :issues
has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues
has_many :merge_requests
has_many :events, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+ scope :of_projects, ->(ids) { where(project_id: ids) }
+ scope :of_groups, ->(ids) { where(group_id: ids) }
scope :active, -> { with_state(:active) }
scope :closed, -> { with_state(:closed) }
- scope :of_projects, ->(ids) { where(project_id: ids) }
+ scope :for_projects, -> { where(group: nil).includes(:project) }
+
+ scope :for_projects_and_groups, -> (project_ids, group_ids) do
+ conditions = []
+ conditions << arel_table[:project_id].in(project_ids) if project_ids.compact.any?
+ conditions << arel_table[:group_id].in(group_ids) if group_ids.compact.any?
+
+ where(conditions.reduce(:or))
+ end
+
+ validates :group, presence: true, unless: :project
+ validates :project, presence: true, unless: :group
- validates :title, presence: true, uniqueness: { scope: :project_id }
- validates :project, presence: true
+ validate :uniqueness_of_title, if: :title_changed?
+ validate :milestone_type_check
validate :start_date_should_be_less_than_due_date, if: proc { |m| m.start_date.present? && m.due_date.present? }
strip_attributes :title
@@ -63,6 +78,14 @@ class Milestone < ActiveRecord::Base
where(t[:title].matches(pattern).or(t[:description].matches(pattern)))
end
+
+ def filter_by_state(milestones, state)
+ case state
+ when 'closed' then milestones.closed
+ when 'all' then milestones
+ else milestones.active
+ end
+ end
end
def self.reference_prefix
@@ -138,6 +161,8 @@ class Milestone < ActiveRecord::Base
# Milestone.first.to_reference(same_namespace_project) # => "gitlab-ce%1"
#
def to_reference(from_project = nil, format: :iid, full: false)
+ return if is_group_milestone?
+
format_reference = milestone_format_reference(format)
reference = "#{self.class.reference_prefix}#{format_reference}"
@@ -152,6 +177,10 @@ class Milestone < ActiveRecord::Base
id
end
+ def for_display
+ self
+ end
+
def can_be_closed?
active? && issues.opened.count.zero?
end
@@ -164,8 +193,45 @@ class Milestone < ActiveRecord::Base
write_attribute(:title, sanitize_title(value)) if value.present?
end
+ def safe_title
+ title.to_slug.normalize.to_s
+ end
+
+ def parent
+ group || project
+ end
+
+ def is_group_milestone?
+ group_id.present?
+ end
+
+ def is_project_milestone?
+ project_id.present?
+ end
+
private
+ # Milestone titles must be unique across project milestones and group milestones
+ def uniqueness_of_title
+ if project
+ relation = Milestone.for_projects_and_groups([project_id], [project.group&.id])
+ elsif group
+ project_ids = group.projects.map(&:id)
+ relation = Milestone.for_projects_and_groups(project_ids, [group.id])
+ end
+
+ title_exists = relation.find_by_title(title)
+ errors.add(:title, "already being used for another group or project milestone.") if title_exists
+ end
+
+ # Milestone should be either a project milestone or a group milestone
+ def milestone_type_check
+ if group_id && project_id
+ field = project_id_changed? ? :project_id : :group_id
+ errors.add(field, "milestone should belong either to a project or a group.")
+ end
+ end
+
def milestone_format_reference(format = :iid)
raise ArgumentError, 'Unknown format' unless [:iid, :name].include?(format)
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 8dd0846f3bc..a03a7abfeb1 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -2,8 +2,11 @@ class IssuableBaseService < BaseService
private
def create_milestone_note(issuable)
+ milestone = issuable.milestone
+ return if milestone && milestone.is_group_milestone?
+
SystemNoteService.change_milestone(
- issuable, issuable.project, current_user, issuable.milestone)
+ issuable, issuable.project, current_user, milestone)
end
def create_labels_note(issuable, old_labels)
@@ -89,10 +92,12 @@ class IssuableBaseService < BaseService
milestone_id = params[:milestone_id]
return unless milestone_id
- if milestone_id == IssuableFinder::NONE ||
- project.milestones.find_by(id: milestone_id).nil?
- params[:milestone_id] = ''
- end
+ params[:milestone_id] = '' if milestone_id == IssuableFinder::NONE
+
+ milestone =
+ Milestone.for_projects_and_groups([project.id], [project.group&.id]).find_by_id(milestone_id)
+
+ params[:milestone_id] = '' unless milestone
end
def filter_labels
diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index 711f4035c55..29def25719d 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -61,8 +61,18 @@ module Issues
end
def cloneable_milestone_id
- @new_project.milestones
- .find_by(title: @old_issue.milestone.try(:title)).try(:id)
+ title = @old_issue.milestone&.title
+ return unless title
+
+ if @new_project.group && can?(current_user, :read_group, @new_project.group)
+ group_id = @new_project.group.id
+ end
+
+ params =
+ { title: title, project_ids: @new_project.id, group_ids: group_id }
+
+ milestones = MilestonesFinder.new(params).execute
+ milestones.first&.id
end
def rewrite_notes
diff --git a/app/services/milestones/base_service.rb b/app/services/milestones/base_service.rb
index 176ab9f1ab5..4963601ea8b 100644
--- a/app/services/milestones/base_service.rb
+++ b/app/services/milestones/base_service.rb
@@ -1,4 +1,10 @@
module Milestones
class BaseService < ::BaseService
+ # Parent can either a group or a project
+ attr_accessor :parent, :current_user, :params
+
+ def initialize(parent, user, params = {})
+ @parent, @current_user, @params = parent, user, params.dup
+ end
end
end
diff --git a/app/services/milestones/close_service.rb b/app/services/milestones/close_service.rb
index 608fc49d766..776ec4b287b 100644
--- a/app/services/milestones/close_service.rb
+++ b/app/services/milestones/close_service.rb
@@ -1,7 +1,7 @@
module Milestones
class CloseService < Milestones::BaseService
def execute(milestone)
- if milestone.close
+ if milestone.close && milestone.is_project_milestone?
event_service.close_milestone(milestone, current_user)
end
diff --git a/app/services/milestones/create_service.rb b/app/services/milestones/create_service.rb
index b8e08c9f1eb..aef3124c7e3 100644
--- a/app/services/milestones/create_service.rb
+++ b/app/services/milestones/create_service.rb
@@ -1,9 +1,9 @@
module Milestones
class CreateService < Milestones::BaseService
def execute
- milestone = project.milestones.new(params)
+ milestone = parent.milestones.new(params)
- if milestone.save
+ if milestone.save && milestone.is_project_milestone?
event_service.open_milestone(milestone, current_user)
end
diff --git a/app/services/milestones/reopen_service.rb b/app/services/milestones/reopen_service.rb
index 573f9ee5c21..5b8b682caaf 100644
--- a/app/services/milestones/reopen_service.rb
+++ b/app/services/milestones/reopen_service.rb
@@ -1,7 +1,7 @@
module Milestones
class ReopenService < Milestones::BaseService
def execute(milestone)
- if milestone.activate
+ if milestone.activate && milestone.is_project_milestone?
event_service.reopen_milestone(milestone, current_user)
end
diff --git a/app/services/milestones/update_service.rb b/app/services/milestones/update_service.rb
index ed64847f429..31b441ed476 100644
--- a/app/services/milestones/update_service.rb
+++ b/app/services/milestones/update_service.rb
@@ -5,9 +5,9 @@ module Milestones
case state
when 'activate'
- Milestones::ReopenService.new(project, current_user, {}).execute(milestone)
+ Milestones::ReopenService.new(parent, current_user, {}).execute(milestone)
when 'close'
- Milestones::CloseService.new(project, current_user, {}).execute(milestone)
+ Milestones::CloseService.new(parent, current_user, {}).execute(milestone)
end
if params.present?
diff --git a/app/views/groups/milestones/_form.html.haml b/app/views/groups/milestones/_form.html.haml
new file mode 100644
index 00000000000..7f450cd9a93
--- /dev/null
+++ b/app/views/groups/milestones/_form.html.haml
@@ -0,0 +1,27 @@
+= form_for [@group, @milestone], html: { class: 'form-horizontal milestone-form common-note-form js-quick-submit js-requires-input' } do |f|
+ .row
+ = form_errors(@milestone)
+
+ .col-md-6
+ .form-group
+ = f.label :title, "Title", class: "control-label"
+ .col-sm-10
+ = f.text_field :title, maxlength: 255, class: "form-control", required: true, autofocus: true
+ .form-group.milestone-description
+ = f.label :description, "Description", class: "control-label"
+ .col-sm-10
+ = render layout: 'projects/md_preview', locals: { url: '' } do
+ = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: 'Write milestone description...'
+ .clearfix
+ .error-alert
+
+ = render "shared/milestones/form_dates", f: f
+
+ .form-actions
+ - if @milestone.new_record?
+ = f.submit 'Create milestone', class: "btn-create btn"
+ = link_to "Cancel", group_milestones_path(@group), class: "btn btn-cancel"
+ - else
+ = f.submit 'Update milestone', class: "btn-create btn"
+ = link_to "Cancel", group_milestone_path(@group, @milestone), class: "btn btn-cancel"
+
diff --git a/app/views/groups/milestones/_milestone.html.haml b/app/views/groups/milestones/_milestone.html.haml
index 4c4e0a26728..bae8997e24c 100644
--- a/app/views/groups/milestones/_milestone.html.haml
+++ b/app/views/groups/milestones/_milestone.html.haml
@@ -1,5 +1,6 @@
+
= render 'shared/milestones/milestone',
- milestone_path: group_milestone_path(@group, milestone.safe_title, title: milestone.title),
+ milestone_path: group_milestone_route(milestone),
issues_path: issues_group_path(@group, milestone_title: milestone.title),
merge_requests_path: merge_requests_group_path(@group, milestone_title: milestone.title),
milestone: milestone
diff --git a/app/views/groups/milestones/edit.html.haml b/app/views/groups/milestones/edit.html.haml
new file mode 100644
index 00000000000..5f6d7d209d0
--- /dev/null
+++ b/app/views/groups/milestones/edit.html.haml
@@ -0,0 +1,7 @@
+- page_title "Milestones"
+- render "header_title"
+
+%h3.page-title
+ Edit Milestone
+
+= render "form"
diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml
index f91bee0b610..6ceb4092307 100644
--- a/app/views/groups/milestones/index.html.haml
+++ b/app/views/groups/milestones/index.html.haml
@@ -9,11 +9,6 @@
= link_to new_group_milestone_path(@group), class: "btn btn-new" do
New milestone
-.row-content-block
- Only milestones from
- %strong= @group.name
- group are listed here.
-
.milestones
%ul.content-list
- if @milestones.blank?
diff --git a/app/views/groups/milestones/new.html.haml b/app/views/groups/milestones/new.html.haml
index 7c7573862d0..e24844661ee 100644
--- a/app/views/groups/milestones/new.html.haml
+++ b/app/views/groups/milestones/new.html.haml
@@ -4,40 +4,4 @@
%h3.page-title
New Milestone
-%p.light
- This will create milestone in every selected project
-%hr
-
-= form_for @milestone, url: group_milestones_path(@group), html: { class: 'form-horizontal milestone-form common-note-form js-quick-submit js-requires-input' } do |f|
- .row
- - if @milestone.errors.any?
- #error_explanation
- .alert.alert-danger
- %ul
- - @milestone.errors.full_messages.each do |msg|
- %li
- = msg
-
- .col-md-6
- .form-group
- = f.label :title, "Title", class: "control-label"
- .col-sm-10
- = f.text_field :title, maxlength: 255, class: "form-control", required: true, autofocus: true
- .form-group.milestone-description
- = f.label :description, "Description", class: "control-label"
- .col-sm-10
- = render layout: 'projects/md_preview', locals: { url: '' } do
- = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: 'Write milestone description...'
- .clearfix
- .error-alert
- .form-group
- = f.label :projects, "Projects", class: "control-label"
- .col-sm-10
- = f.collection_select :project_ids, @group.projects.non_archived, :id, :name,
- { selected: @group.projects.non_archived.pluck(:id) }, required: true, multiple: true, class: 'select2'
-
- = render "shared/milestones/form_dates", f: f
-
- .form-actions
- = f.submit 'Create milestone', class: "btn-create btn"
- = link_to "Cancel", group_milestones_path(@group), class: "btn btn-cancel"
+= render "form"
diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml
index 33e68bc766e..54b1b7a734a 100644
--- a/app/views/groups/milestones/show.html.haml
+++ b/app/views/groups/milestones/show.html.haml
@@ -1,4 +1,4 @@
= render "header_title"
= render 'shared/milestones/top', milestone: @milestone, group: @group
-= render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true
+= render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true if @milestone.is_legacy_group_milestone?
= render 'shared/milestones/sidebar', milestone: @milestone, affix_offset: 102
diff --git a/app/views/shared/milestones/_milestone.html.haml b/app/views/shared/milestones/_milestone.html.haml
index ecc8b42979c..6f6a036b13f 100644
--- a/app/views/shared/milestones/_milestone.html.haml
+++ b/app/views/shared/milestones/_milestone.html.haml
@@ -1,10 +1,15 @@
- dashboard = local_assigns[:dashboard]
-- custom_dom_id = dom_id(@project ? milestone : milestone.milestones.first)
+- custom_dom_id = dom_id(milestone.try(:milestones) ? milestone.milestones.first : milestone)
%li{ class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: custom_dom_id }
.row
.col-sm-6
%strong= link_to truncate(milestone.title, length: 100), milestone_path
+ - if milestone.is_group_milestone?
+ %span - Group Milestone
+ - else
+ %span - Project Milestone
+
.col-sm-6
.pull-right.light #{milestone.percent_complete(current_user)}% complete
.row
@@ -13,26 +18,32 @@
&middot;
= link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path
.col-sm-6= milestone_progress_bar(milestone)
- - if milestone.is_a?(GlobalMilestone)
+ - if milestone.is_a?(GlobalMilestone) || milestone.is_group_milestone?
.row
.col-sm-6
- .expiration= render('shared/milestone_expired', milestone: milestone)
- .projects
- - milestone.milestones.each do |milestone|
- = link_to milestone_path(milestone) do
- %span.label.label-gray
- = dashboard ? milestone.project.name_with_namespace : milestone.project.name
+ - if milestone.is_legacy_group_milestone?
+ .expiration= render('shared/milestone_expired', milestone: milestone)
+ .projects
+ - milestone.milestones.each do |milestone|
+ = link_to milestone_path(milestone) do
+ %span.label.label-gray
+ = dashboard ? milestone.project.name_with_namespace : milestone.project.name
- if @group
- .col-sm-6
+ .col-sm-6.milestone-actions
- if can?(current_user, :admin_milestones, @group)
+ - if milestone.is_group_milestone?
+ = link_to edit_group_milestone_path(@group, milestone.id), class: "btn btn-xs btn-grouped" do
+ Edit
+ \
- if milestone.closed?
- = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-xs btn-grouped btn-reopen"
+ = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-xs btn-grouped btn-reopen"
- else
- = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-xs btn-close"
+ = link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-xs btn-grouped btn-close"
- if @project
.row
- .col-sm-6= render('shared/milestone_expired', milestone: milestone)
+ .col-sm-6
+ = render('shared/milestone_expired', milestone: milestone)
.col-sm-6.milestone-actions
- if can?(current_user, :admin_milestone, milestone.project) and milestone.active?
= link_to edit_project_milestone_path(milestone.project, milestone), class: "btn btn-xs btn-grouped" do
diff --git a/app/views/shared/milestones/_top.html.haml b/app/views/shared/milestones/_top.html.haml
index 20a12613cfc..b93837e3087 100644
--- a/app/views/shared/milestones/_top.html.haml
+++ b/app/views/shared/milestones/_top.html.haml
@@ -22,39 +22,55 @@
- if group
.pull-right
- if can?(current_user, :admin_milestones, group)
+ - if milestone.is_group_milestone?
+ = link_to edit_group_milestone_path(group, milestone.iid), class: "btn btn btn-grouped" do
+ Edit
- if milestone.active?
- = link_to 'Close Milestone', group_milestone_path(group, milestone.safe_title, title: milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-grouped btn-close"
+ = link_to 'Close Milestone', group_milestone_route(milestone, {state_event: :close }), method: :put, class: "btn btn-grouped btn-close"
- else
- = link_to 'Reopen Milestone', group_milestone_path(group, milestone.safe_title, title: milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen"
+ = link_to 'Reopen Milestone', group_milestone_route(milestone, {state_event: :activate }), method: :put, class: "btn btn-grouped btn-reopen"
.detail-page-description.milestone-detail
%h2.title
= markdown_field(milestone, :title)
+ - if @milestone.is_group_milestone? && @milestone.description.present?
+ %div
+ .description
+ .wiki
+ = markdown_field(@milestone, :description)
- if milestone.complete?(current_user) && milestone.active?
.alert.alert-success.prepend-top-default
- close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.'
%span All issues for this milestone are closed. #{close_msg}
-.table-holder
- %table.table
- %thead
- %tr
- %th Project
- %th Open issues
- %th State
- %th Due date
- - milestone.milestones.each do |ms|
- %tr
- %td
- - project_name = group ? ms.project.name : ms.project.name_with_namespace
- = link_to project_name, project_milestone_path(ms.project, ms)
- %td
- = ms.issues_visible_to_user(current_user).opened.count
- %td
- - if ms.closed?
- Closed
- - else
- Open
- %td
- = ms.expires_at
+- if @milestone.is_legacy_group_milestone? || @milestone.is_dashboard_milestone?
+ .table-holder
+ %table.table
+ %thead
+ %tr
+ %th Project
+ %th Open issues
+ %th State
+ %th Due date
+ - milestone.milestones.each do |ms|
+ %tr
+ %td
+ - project_name = group ? ms.project.name : ms.project.name_with_namespace
+ = link_to project_name, project_milestone_path(ms.project, ms)
+ %td
+ = ms.issues_visible_to_user(current_user).opened.count
+ %td
+ - if ms.closed?
+ Closed
+ - else
+ Open
+ %td
+ = ms.expires_at
+- elsif @milestone.is_group_milestone?
+ %br
+ View
+ = link_to 'Issues', issues_group_path(@group, milestone_title: milestone.title)
+ or
+ = link_to 'Merge Requests', merge_requests_group_path(@group, milestone_title: milestone.title)
+ in this milestone
diff --git a/changelogs/unreleased/issue_30126_be.yml b/changelogs/unreleased/issue_30126_be.yml
new file mode 100644
index 00000000000..96bb8d9574b
--- /dev/null
+++ b/changelogs/unreleased/issue_30126_be.yml
@@ -0,0 +1,4 @@
+---
+title: Add native group milestones
+merge_request:
+author:
diff --git a/config/routes/group.rb b/config/routes/group.rb
index e578dd8b082..23052a6c6dc 100644
--- a/config/routes/group.rb
+++ b/config/routes/group.rb
@@ -12,7 +12,7 @@ scope(path: 'groups/*group_id',
end
resource :avatar, only: [:destroy]
- resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :update, :new, :create] do
+ resources :milestones, constraints: { id: /[^\/]+/ }, only: [:index, :show, :edit, :update, :new, :create] do
member do
get :merge_requests
get :participants
diff --git a/db/migrate/20170723183807_add_group_id_to_milestones.rb b/db/migrate/20170723183807_add_group_id_to_milestones.rb
new file mode 100644
index 00000000000..e46fc4f80f0
--- /dev/null
+++ b/db/migrate/20170723183807_add_group_id_to_milestones.rb
@@ -0,0 +1,18 @@
+class AddGroupIdToMilestones < ActiveRecord::Migration
+ DOWNTIME = false
+
+ def up
+ change_column_null :milestones, :project_id, true
+
+ add_column :milestones, :group_id, :integer
+ end
+
+ def down
+ # We cannot rollback project_id not null constraint if there are records
+ # with null values.
+ execute "DELETE from milestones WHERE project_id IS NULL"
+
+ remove_column :milestones, :group_id
+ change_column :milestones, :project_id, :integer, null: false
+ end
+end
diff --git a/db/migrate/20170724184243_add_group_milestone_id_indexes.rb b/db/migrate/20170724184243_add_group_milestone_id_indexes.rb
new file mode 100644
index 00000000000..d48b1884179
--- /dev/null
+++ b/db/migrate/20170724184243_add_group_milestone_id_indexes.rb
@@ -0,0 +1,19 @@
+class AddGroupMilestoneIdIndexes < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ disable_ddl_transaction!
+
+ DOWNTIME = false
+
+ def up
+ add_concurrent_foreign_key :milestones, :namespaces, column: :group_id, on_delete: :cascade
+
+ add_concurrent_index :milestones, :group_id
+ end
+
+ def down
+ remove_foreign_key :milestones, column: :group_id
+
+ remove_concurrent_index :milestones, :group_id
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index f4d83a4dd9c..d50e623d0f0 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20170703102400) do
+ActiveRecord::Schema.define(version: 20170724184243) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -829,7 +829,7 @@ ActiveRecord::Schema.define(version: 20170703102400) do
create_table "milestones", force: :cascade do |t|
t.string "title", null: false
- t.integer "project_id", null: false
+ t.integer "project_id"
t.text "description"
t.date "due_date"
t.datetime "created_at"
@@ -840,10 +840,12 @@ ActiveRecord::Schema.define(version: 20170703102400) do
t.text "description_html"
t.date "start_date"
t.integer "cached_markdown_version"
+ t.integer "group_id"
end
add_index "milestones", ["description"], name: "index_milestones_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
add_index "milestones", ["due_date"], name: "index_milestones_on_due_date", using: :btree
+ add_index "milestones", ["group_id"], name: "index_milestones_on_group_id", using: :btree
add_index "milestones", ["project_id", "iid"], name: "index_milestones_on_project_id_and_iid", unique: true, using: :btree
add_index "milestones", ["title"], name: "index_milestones_on_title", using: :btree
add_index "milestones", ["title"], name: "index_milestones_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
@@ -1601,6 +1603,7 @@ ActiveRecord::Schema.define(version: 20170703102400) do
add_foreign_key "merge_requests", "projects", column: "target_project_id", name: "fk_a6963e8447", on_delete: :cascade
add_foreign_key "merge_requests_closing_issues", "issues", on_delete: :cascade
add_foreign_key "merge_requests_closing_issues", "merge_requests", on_delete: :cascade
+ add_foreign_key "milestones", "namespaces", column: "group_id", name: "fk_95650a40d4", on_delete: :cascade
add_foreign_key "milestones", "projects", name: "fk_9bd0a0c791", on_delete: :cascade
add_foreign_key "notes", "projects", name: "fk_99e097b079", on_delete: :cascade
add_foreign_key "oauth_openid_requests", "oauth_access_grants", column: "access_grant_id", name: "fk_oauth_openid_requests_oauth_access_grants_access_grant_id"
diff --git a/doc/user/project/milestones/index.md b/doc/user/project/milestones/index.md
index 99233ed5ae2..1848514e2dd 100644
--- a/doc/user/project/milestones/index.md
+++ b/doc/user/project/milestones/index.md
@@ -21,14 +21,11 @@ Once you fill in all the details, hit the **Create milestone** button.
>**Note:**
You need [Master permissions](../../permissions.md) in order to create a milestone.
-You can create a milestone for several projects in the same group simultaneously.
-On the group's **Issues ➔ Milestones** page, you will be able to see the status
-of that milestone across all of the selected projects. To create a new milestone
-for selected projects in the group, click the **New milestone** button. The
-form is the same as when creating a milestone for a specific project with the
-addition of the selection of the projects you want to inherit this milestone.
-
-![Creating a group milestone](img/milestone_group_create.png)
+You can create a milestone for a group that will be shared across group projects.
+On the group's **Issues ➔ Milestones** page, you will be able to see the state
+of that milestone and the issues/merge requests count that it shares across the group projects. To create a new milestone click the **New milestone** button. The form is the same as when creating a milestone for a specific project which you can find in the previous item.
+
+In addition to that you will be able to filter issues or merge requests by group milestones in all projects that belongs to the milestone group.
## Special milestone filters
diff --git a/features/group/milestones.feature b/features/group/milestones.feature
index 1c1539b3e12..2211acfee20 100644
--- a/features/group/milestones.feature
+++ b/features/group/milestones.feature
@@ -22,12 +22,12 @@ Feature: Group Milestones
Then I should see group milestone with descriptions and expiry date
And I should see group milestone with all issues and MRs assigned to that milestone
- Scenario: Create multiple milestones with one form
+ Scenario: Create group milestones
Given I visit group "Owned" milestones page
And I click new milestone button
And I fill milestone name
When I press create mileston button
- Then milestone in each project should be created
+ Then group milestone should be created
Scenario: I should see Issues listed with labels
Given Group has projects with milestones
diff --git a/features/steps/group/milestones.rb b/features/steps/group/milestones.rb
index 7288dc87005..915d766dd60 100644
--- a/features/steps/group/milestones.rb
+++ b/features/steps/group/milestones.rb
@@ -54,14 +54,9 @@ class Spinach::Features::GroupMilestones < Spinach::FeatureSteps
click_button "Create milestone"
end
- step 'milestone in each project should be created' do
+ step 'group milestone should be created' do
group = Group.find_by(name: 'Owned')
- expect(page).to have_content "Milestone v2.9.0"
- expect(group.projects).to be_present
-
- group.projects.each do |project|
- expect(page).to have_content project.name
- end
+ expect(page).to have_content group.milestones.find_by_title('v2.9.0').title
end
step 'I should see the "bug" label' do
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 94168fa4ebc..fdc0c562248 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -255,7 +255,7 @@ module API
class ProjectEntity < Grape::Entity
expose :id, :iid
- expose(:project_id) { |entity| entity.project.id }
+ expose(:project_id) { |entity| entity&.project.try(:id) }
expose :title, :description
expose :state, :created_at, :updated_at
end
@@ -267,7 +267,12 @@ module API
expose :deleted_file?, as: :deleted_file
end
- class Milestone < ProjectEntity
+ class Milestone < Grape::Entity
+ expose :id, :iid
+ expose(:project_id) { |entity| entity&.project_id }
+ expose(:group_id) { |entity| entity&.group_id }
+ expose :title, :description
+ expose :state, :created_at, :updated_at
expose :due_date
expose :start_date
end
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
index c6e5fb61cf9..aad67dd0164 100644
--- a/spec/controllers/groups/milestones_controller_spec.rb
+++ b/spec/controllers/groups/milestones_controller_spec.rb
@@ -2,8 +2,8 @@ require 'spec_helper'
describe Groups::MilestonesController do
let(:group) { create(:group) }
- let(:project) { create(:empty_project, group: group) }
- let(:project2) { create(:empty_project, group: group) }
+ let!(:project) { create(:empty_project, group: group) }
+ let!(:project2) { create(:empty_project, group: group) }
let(:user) { create(:user) }
let(:title) { '肯定不是中文的问题' }
let(:milestone) do
@@ -17,24 +17,67 @@ describe Groups::MilestonesController do
end
let(:milestone_path) { group_milestone_path(group, milestone.safe_title, title: milestone.title) }
+ let(:milestone_params) do
+ {
+ title: title,
+ start_date: Date.today,
+ due_date: 1.month.from_now.to_date
+ }
+ end
+
before do
sign_in(user)
group.add_owner(user)
project.team << [user, :master]
end
- describe "#index" do
+ describe '#index' do
it 'shows group milestones page' do
get :index, group_id: group.to_param
expect(response).to have_http_status(200)
end
- it 'shows group milestones JSON' do
- get :index, group_id: group.to_param, format: :json
+ context 'as JSON' do
+ let!(:milestone) { create(:milestone, group: group, title: 'group milestone') }
+ let!(:legacy_milestone1) { create(:milestone, project: project, title: 'legacy') }
+ let!(:legacy_milestone2) { create(:milestone, project: project2, title: 'legacy') }
- expect(response).to have_http_status(200)
- expect(response.content_type).to eq 'application/json'
+ it 'lists legacy group milestones and group milestones' do
+ get :index, group_id: group.to_param, format: :json
+
+ milestones = JSON.parse(response.body)
+
+ expect(milestones.count).to eq(2)
+ expect(milestones.first["title"]).to eq("group milestone")
+ expect(milestones.second["title"]).to eq("legacy")
+ expect(response).to have_http_status(200)
+ expect(response.content_type).to eq 'application/json'
+ end
+ end
+ end
+
+ describe '#show' do
+ let(:milestone1) { create(:milestone, project: project, title: 'legacy') }
+ let(:milestone2) { create(:milestone, project: project, title: 'legacy') }
+ let(:group_milestone) { create(:milestone, group: group) }
+
+ context 'when there is a title parameter' do
+ it 'searchs for a legacy group milestone' do
+ expect(GlobalMilestone).to receive(:build)
+ expect(Milestone).not_to receive(:find_by_iid)
+
+ get :show, group_id: group.to_param, id: title, title: milestone1.safe_title
+ end
+ end
+
+ context 'when there is not a title parameter' do
+ it 'searchs for a group milestone' do
+ expect(GlobalMilestone).not_to receive(:build)
+ expect(Milestone).to receive(:find_by_iid)
+
+ get :show, group_id: group.to_param, id: group_milestone.id
+ end
end
end
@@ -44,16 +87,57 @@ describe Groups::MilestonesController do
it "creates group milestone with Chinese title" do
post :create,
group_id: group.to_param,
- milestone: { project_ids: [project.id, project2.id], title: title }
+ milestone: milestone_params
- expect(response).to redirect_to(group_milestone_path(group, title.to_slug.to_s, title: title))
- expect(Milestone.where(title: title).count).to eq(2)
+ milestone = Milestone.find_by_title(title)
+
+ expect(response).to redirect_to(group_milestone_path(group, milestone.iid))
+ expect(milestone.group_id).to eq(group.id)
+ expect(milestone.due_date).to eq(milestone_params[:due_date])
+ expect(milestone.start_date).to eq(milestone_params[:start_date])
+ end
+ end
+
+ describe "#update" do
+ let(:milestone) { create(:milestone, group: group) }
+
+ it "updates group milestone" do
+ milestone_params[:title] = "title changed"
+
+ put :update,
+ id: milestone.iid,
+ group_id: group.to_param,
+ milestone: milestone_params
+
+ milestone.reload
+ expect(response).to redirect_to(group_milestone_path(group, milestone.iid))
+ expect(milestone.title).to eq("title changed")
end
- it "redirects to new when there are no project ids" do
- post :create, group_id: group.to_param, milestone: { title: title, project_ids: [""] }
- expect(response).to render_template :new
- expect(assigns(:milestone).errors).not_to be_nil
+ context "legacy group milestones" do
+ let!(:milestone1) { create(:milestone, project: project, title: 'legacy milestone', description: "old description") }
+ let!(:milestone2) { create(:milestone, project: project2, title: 'legacy milestone', description: "old description") }
+
+ it "updates only group milestones state" do
+ milestone_params[:title] = "title changed"
+ milestone_params[:description] = "description changed"
+ milestone_params[:state_event] = "close"
+
+ put :update,
+ id: milestone1.title.to_slug.to_s,
+ group_id: group.to_param,
+ milestone: milestone_params,
+ title: milestone1.title
+
+ expect(response).to redirect_to(group_milestone_path(group, milestone1.safe_title, title: milestone1.title))
+
+ [milestone1, milestone2].each do |milestone|
+ milestone.reload
+ expect(milestone.title).to eq("legacy milestone")
+ expect(milestone.description).to eq("old description")
+ expect(milestone.state).to eq("closed")
+ end
+ end
end
end
@@ -156,7 +240,7 @@ describe Groups::MilestonesController do
it 'does not 404' do
post :create,
group_id: group.to_param,
- milestone: { project_ids: [project.id, project2.id], title: title }
+ milestone: { title: title }
expect(response).not_to have_http_status(404)
end
@@ -164,7 +248,7 @@ describe Groups::MilestonesController do
it 'does not redirect to the correct casing' do
post :create,
group_id: group.to_param,
- milestone: { project_ids: [project.id, project2.id], title: title }
+ milestone: { title: title }
expect(response).not_to have_http_status(301)
end
@@ -176,7 +260,7 @@ describe Groups::MilestonesController do
it 'returns not found' do
post :create,
group_id: redirect_route.path,
- milestone: { project_ids: [project.id, project2.id], title: title }
+ milestone: { title: title }
expect(response).to have_http_status(404)
end
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb
index 84a61b2784e..bb5a340cd96 100644
--- a/spec/controllers/projects/milestones_controller_spec.rb
+++ b/spec/controllers/projects/milestones_controller_spec.rb
@@ -31,6 +31,40 @@ describe Projects::MilestonesController do
end
end
+ describe "#index" do
+ context "as html" do
+ before do
+ get :index, namespace_id: project.namespace.id, project_id: project.id
+ end
+
+ it "queries only projects milestones" do
+ milestones = assigns(:milestones)
+
+ expect(milestones.count).to eq(1)
+ expect(milestones.where(project_id: nil)).to be_empty
+ end
+ end
+
+ context "as json" do
+ let!(:group) { create(:group, :public) }
+ let!(:group_milestone) { create(:milestone, group: group) }
+ let!(:group_member) { create(:group_member, group: group, user: user) }
+
+ before do
+ project.update(namespace: group)
+ get :index, namespace_id: project.namespace.id, project_id: project.id, format: :json
+ end
+
+ it "queries projects milestones and groups milestones" do
+ milestones = assigns(:milestones)
+
+ expect(milestones.count).to eq(2)
+ expect(milestones.where(project_id: nil).first).to eq(group_milestone)
+ expect(milestones.where(group_id: nil).first).to eq(milestone)
+ end
+ end
+ end
+
describe "#destroy" do
it "removes milestone" do
expect(issue.milestone_id).to eq(milestone.id)
diff --git a/spec/factories/milestones.rb b/spec/factories/milestones.rb
index 841ab3c73b8..113665ff11b 100644
--- a/spec/factories/milestones.rb
+++ b/spec/factories/milestones.rb
@@ -1,7 +1,13 @@
FactoryGirl.define do
factory :milestone do
title
- project factory: :empty_project
+
+ transient do
+ project nil
+ group nil
+ project_id nil
+ group_id nil
+ end
trait :active do
state "active"
@@ -11,6 +17,20 @@ FactoryGirl.define do
state "closed"
end
+ after(:build) do |milestone, evaluator|
+ if evaluator.group
+ milestone.group = evaluator.group
+ elsif evaluator.group_id
+ milestone.group_id = evaluator.group_id
+ elsif evaluator.project
+ milestone.project = evaluator.project
+ elsif evaluator.project_id
+ milestone.project_id = evaluator.project_id
+ else
+ milestone.project = create(:empty_project)
+ end
+ end
+
factory :active_milestone, traits: [:active]
factory :closed_milestone, traits: [:closed]
end
diff --git a/spec/features/groups/milestone_spec.rb b/spec/features/groups/milestone_spec.rb
index 330310eae6b..9b6eb946f4b 100644
--- a/spec/features/groups/milestone_spec.rb
+++ b/spec/features/groups/milestone_spec.rb
@@ -33,4 +33,32 @@ feature 'Group milestones', :feature, :js do
expect(find('.start_date')).to have_content(Date.today.at_beginning_of_month.strftime('%b %-d, %Y'))
end
end
+
+ context 'milestones list' do
+ let!(:other_project) { create(:project_empty_repo, group: group) }
+
+ let!(:active_group_milestone) { create(:milestone, group: group, state: 'active') }
+ let!(:active_project_milestone1) { create(:milestone, project: project, state: 'active', title: 'v1.0') }
+ let!(:active_project_milestone2) { create(:milestone, project: other_project, state: 'active', title: 'v1.0') }
+ let!(:closed_group_milestone) { create(:milestone, group: group, state: 'closed') }
+ let!(:closed_project_milestone1) { create(:milestone, project: project, state: 'closed', title: 'v2.0') }
+ let!(:closed_project_milestone2) { create(:milestone, project: other_project, state: 'closed', title: 'v2.0') }
+
+ before do
+ visit group_milestones_path(group)
+ end
+
+ it 'counts milestones correctly' do
+ expect(find('.top-area .active .badge').text).to eq("2")
+ expect(find('.top-area .closed .badge').text).to eq("2")
+ expect(find('.top-area .all .badge').text).to eq("4")
+ end
+
+ it 'lists legacy group milestones and group milestones' do
+ legacy_milestone = GroupMilestone.build_collection(group, group.projects, { state: 'active' }).first
+
+ expect(page).to have_selector("#milestone_#{active_group_milestone.id}", count: 1)
+ expect(page).to have_selector("#milestone_#{legacy_milestone.milestones.first.id}", count: 1)
+ end
+ end
end
diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb
index 880c53343bc..2afdd0d321c 100644
--- a/spec/features/milestone_spec.rb
+++ b/spec/features/milestone_spec.rb
@@ -1,10 +1,12 @@
require 'rails_helper'
feature 'Milestone', feature: true do
- let(:project) { create(:empty_project, :public) }
+ let(:group) { create(:group, :public) }
+ let(:project) { create(:empty_project, :public, namespace: group) }
let(:user) { create(:user) }
before do
+ create(:group_member, group: group, user: user)
project.team << [user, :master]
gitlab_sign_in(user)
end
@@ -37,8 +39,8 @@ feature 'Milestone', feature: true do
end
end
- feature 'Open a milestone with an existing title' do
- scenario 'displays validation message' do
+ feature 'Open a project milestone with an existing title' do
+ scenario 'displays validation message when there is a project milestone with same title' do
milestone = create(:milestone, project: project, title: 8.7)
visit new_project_milestone_path(project)
@@ -47,7 +49,20 @@ feature 'Milestone', feature: true do
end
find('input[name="commit"]').click
- expect(find('.alert-danger')).to have_content('Title has already been taken')
+ expect(find('.alert-danger')).to have_content('already being used for another group or project milestone.')
+ end
+
+ scenario 'displays validation message when there is a group milestone with same title' do
+ milestone = create(:milestone, project_id: nil, group: project.group, title: 8.7)
+
+ visit new_group_milestone_path(project.group)
+
+ page.within '.milestone-form' do
+ fill_in "milestone_title", with: milestone.title
+ end
+ find('input[name="commit"]').click
+
+ expect(find('.alert-danger')).to have_content('already being used for another group or project milestone.')
end
end
end
diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb
index 4a52f0d5c58..bef4fd44331 100644
--- a/spec/finders/issues_finder_spec.rb
+++ b/spec/finders/issues_finder_spec.rb
@@ -59,6 +59,23 @@ describe IssuesFinder do
end
end
+ context 'filtering by group milestone' do
+ let!(:group) { create(:group, :public) }
+ let(:group_milestone) { create(:milestone, group: group) }
+ let!(:group_member) { create(:group_member, group: group, user: user) }
+ let(:params) { { milestone_title: group_milestone.title } }
+
+ before do
+ project2.update(namespace: group)
+ issue2.update(milestone: group_milestone)
+ issue3.update(milestone: group_milestone)
+ end
+
+ it 'returns issues assigned to that group milestone' do
+ expect(issues).to contain_exactly(issue2, issue3)
+ end
+ end
+
context 'filtering by no milestone' do
let(:params) { { milestone_title: Milestone::None.title } }
diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb
index 5eb26de6c92..b46218bf72e 100644
--- a/spec/finders/merge_requests_finder_spec.rb
+++ b/spec/finders/merge_requests_finder_spec.rb
@@ -47,6 +47,25 @@ describe MergeRequestsFinder do
expect(merge_requests).to contain_exactly(merge_request1)
end
+ context 'filtering by group milestone' do
+ let!(:group) { create(:group, :public) }
+ let(:group_milestone) { create(:milestone, group: group) }
+ let!(:group_member) { create(:group_member, group: group, user: user) }
+ let(:params) { { milestone_title: group_milestone.title } }
+
+ before do
+ project2.update(namespace: group)
+ merge_request2.update(milestone: group_milestone)
+ merge_request3.update(milestone: group_milestone)
+ end
+
+ it 'returns issues assigned to that group milestone' do
+ merge_requests = described_class.new(user, params).execute
+
+ expect(merge_requests).to contain_exactly(merge_request2, merge_request3)
+ end
+ end
+
context 'with created_after and created_before params' do
let(:project4) { create(:empty_project, forked_from_project: project1) }
diff --git a/spec/finders/milestones_finder_spec.rb b/spec/finders/milestones_finder_spec.rb
new file mode 100644
index 00000000000..32ec983c5b8
--- /dev/null
+++ b/spec/finders/milestones_finder_spec.rb
@@ -0,0 +1,90 @@
+require 'spec_helper'
+
+describe MilestonesFinder do
+ let(:group) { create(:group) }
+ let(:project_1) { create(:empty_project, namespace: group) }
+ let(:project_2) { create(:empty_project, namespace: group) }
+ let!(:milestone_1) { create(:milestone, group: group, title: 'one test', due_date: Date.today) }
+ let!(:milestone_2) { create(:milestone, group: group) }
+ let!(:milestone_3) { create(:milestone, project: project_1, state: 'active', due_date: Date.tomorrow) }
+ let!(:milestone_4) { create(:milestone, project: project_2, state: 'active') }
+
+ it 'it returns milestones for projects' do
+ result = described_class.new(project_ids: [project_1.id, project_2.id], state: 'all').execute
+
+ expect(result).to contain_exactly(milestone_3, milestone_4)
+ end
+
+ it 'returns milestones for groups' do
+ result = described_class.new(group_ids: group.id, state: 'all').execute
+
+ expect(result).to contain_exactly(milestone_1, milestone_2)
+ end
+
+ it 'returns milestones for groups and projects' do
+ result = described_class.new(project_ids: [project_1.id, project_2.id], group_ids: group.id, state: 'all').execute
+
+ expect(result).to contain_exactly(milestone_1, milestone_2, milestone_3, milestone_4)
+ end
+
+ context 'with filters' do
+ let(:params) do
+ {
+ project_ids: [project_1.id, project_2.id],
+ group_ids: group.id,
+ state: 'all'
+ }
+ end
+
+ before do
+ milestone_1.close
+ milestone_3.close
+ end
+
+ it 'filters by active state' do
+ params[:state] = 'active'
+ result = described_class.new(params).execute
+
+ expect(result).to contain_exactly(milestone_2, milestone_4)
+ end
+
+ it 'filters by closed state' do
+ params[:state] = 'closed'
+ result = described_class.new(params).execute
+
+ expect(result).to contain_exactly(milestone_1, milestone_3)
+ end
+
+ it 'filters by title' do
+ result = described_class.new(params.merge(title: 'one test')).execute
+
+ expect(result.to_a).to contain_exactly(milestone_1)
+ end
+ end
+
+ context 'with order' do
+ let(:params) do
+ {
+ project_ids: [project_1.id, project_2.id],
+ group_ids: group.id,
+ state: 'all'
+ }
+ end
+
+ it "default orders by due date" do
+ result = described_class.new(params).execute
+
+ expect(result.first).to eq(milestone_1)
+ expect(result.second).to eq(milestone_3)
+ end
+
+ it "orders by parameter" do
+ result = described_class.new(params.merge(order: 'id DESC')).execute
+
+ expect(result.first).to eq(milestone_4)
+ expect(result.second).to eq(milestone_3)
+ expect(result.third).to eq(milestone_2)
+ expect(result.fourth).to eq(milestone_1)
+ end
+ end
+end
diff --git a/spec/fixtures/api/schemas/public_api/v3/issues.json b/spec/fixtures/api/schemas/public_api/v3/issues.json
index f2ee9c925ae..51b0822bc66 100644
--- a/spec/fixtures/api/schemas/public_api/v3/issues.json
+++ b/spec/fixtures/api/schemas/public_api/v3/issues.json
@@ -22,7 +22,8 @@
"properties": {
"id": { "type": "integer" },
"iid": { "type": "integer" },
- "project_id": { "type": "integer" },
+ "project_id": { "type": ["integer", "null"] },
+ "group_id": { "type": ["integer", "null"] },
"title": { "type": "string" },
"description": { "type": ["string", "null"] },
"state": { "type": "string" },
diff --git a/spec/fixtures/api/schemas/public_api/v3/merge_requests.json b/spec/fixtures/api/schemas/public_api/v3/merge_requests.json
index 01f9fbb2c89..b5c74bcc26e 100644
--- a/spec/fixtures/api/schemas/public_api/v3/merge_requests.json
+++ b/spec/fixtures/api/schemas/public_api/v3/merge_requests.json
@@ -53,7 +53,8 @@
"properties": {
"id": { "type": "integer" },
"iid": { "type": "integer" },
- "project_id": { "type": "integer" },
+ "project_id": { "type": ["integer", "null"] },
+ "group_id": { "type": ["integer", "null"] },
"title": { "type": "string" },
"description": { "type": ["string", "null"] },
"state": { "type": "string" },
diff --git a/spec/fixtures/api/schemas/public_api/v4/issues.json b/spec/fixtures/api/schemas/public_api/v4/issues.json
index 2d1c84ee93d..bd6bfc03199 100644
--- a/spec/fixtures/api/schemas/public_api/v4/issues.json
+++ b/spec/fixtures/api/schemas/public_api/v4/issues.json
@@ -22,7 +22,8 @@
"properties": {
"id": { "type": "integer" },
"iid": { "type": "integer" },
- "project_id": { "type": "integer" },
+ "project_id": { "type": ["integer", "null"] },
+ "group_id": { "type": ["integer", "null"] },
"title": { "type": "string" },
"description": { "type": ["string", "null"] },
"state": { "type": "string" },
diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json
index 51642e8cbb8..60aa47c1259 100644
--- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json
+++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json
@@ -53,7 +53,8 @@
"properties": {
"id": { "type": "integer" },
"iid": { "type": "integer" },
- "project_id": { "type": "integer" },
+ "project_id": { "type": ["integer", "null"] },
+ "group_id": { "type": ["integer", "null"] },
"title": { "type": "string" },
"description": { "type": ["string", "null"] },
"state": { "type": "string" },
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 562f0c2991c..83fe26668cb 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -45,6 +45,7 @@ label:
- merge_requests
- priorities
milestone:
+- group
- project
- issues
- labels
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index b4a7e956686..4ef3db3721f 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -82,6 +82,7 @@ Milestone:
- id
- title
- project_id
+- group_id
- description
- due_date
- start_date
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 45953023a36..2649d04bee3 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -6,9 +6,6 @@ describe Milestone, models: true do
allow(subject).to receive(:set_iid).and_return(false)
end
- it { is_expected.to validate_presence_of(:title) }
- it { is_expected.to validate_presence_of(:project) }
-
describe 'start_date' do
it 'adds an error when start_date is greated then due_date' do
milestone = build(:milestone, start_date: Date.tomorrow, due_date: Date.yesterday)
@@ -37,17 +34,42 @@ describe Milestone, models: true do
end
end
- describe "unique milestone title per project" do
- it "does not accept the same title in a project twice" do
- new_milestone = Milestone.new(project: milestone.project, title: milestone.title)
- expect(new_milestone).not_to be_valid
+ describe "unique milestone title" do
+ context "per project" do
+ it "does not accept the same title in a project twice" do
+ new_milestone = Milestone.new(project: milestone.project, title: milestone.title)
+ expect(new_milestone).not_to be_valid
+ end
+
+ it "accepts the same title in another project" do
+ project = create(:empty_project)
+ new_milestone = Milestone.new(project: project, title: milestone.title)
+
+ expect(new_milestone).to be_valid
+ end
end
- it "accepts the same title in another project" do
- project = build(:empty_project)
- new_milestone = Milestone.new(project: project, title: milestone.title)
+ context "per group" do
+ let(:group) { create(:group) }
+ let(:milestone) { create(:milestone, group: group) }
+
+ before do
+ project.update(group: group)
+ end
+
+ it "does not accept the same title in a group twice" do
+ new_milestone = Milestone.new(group: group, title: milestone.title)
+
+ expect(new_milestone).not_to be_valid
+ end
- expect(new_milestone).to be_valid
+ it "does not accept the same title of a child project milestone" do
+ create(:milestone, project: group.projects.first)
+
+ new_milestone = Milestone.new(group: group, title: milestone.title)
+
+ expect(new_milestone).not_to be_valid
+ end
end
end
diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb
index d1dd1466d95..36d5038fb95 100644
--- a/spec/services/issues/move_service_spec.rb
+++ b/spec/services/issues/move_service_spec.rb
@@ -37,9 +37,6 @@ describe Issues::MoveService, services: true do
describe '#execute' do
shared_context 'issue move executed' do
- let!(:milestone2) do
- create(:milestone, project_id: new_project.id, title: 'v9.0')
- end
let!(:award_emoji) { create(:award_emoji, awardable: old_issue) }
let!(:new_issue) { move_service.execute(old_issue, new_project) }
@@ -48,6 +45,63 @@ describe Issues::MoveService, services: true do
context 'issue movable' do
include_context 'user can move issue'
+ context 'move to new milestone' do
+ let(:new_issue) { move_service.execute(old_issue, new_project) }
+
+ context 'project milestone' do
+ let!(:milestone2) do
+ create(:milestone, project_id: new_project.id, title: 'v9.0')
+ end
+
+ it 'assigns milestone to new issue' do
+ expect(new_issue.reload.milestone.title).to eq 'v9.0'
+ expect(new_issue.reload.milestone).to eq(milestone2)
+ end
+ end
+
+ context 'group milestones' do
+ let!(:group) { create(:group, :private) }
+ let!(:group_milestone_1) do
+ create(:milestone, group_id: group.id, title: 'v9.0_group')
+ end
+
+ before do
+ old_issue.update(milestone: group_milestone_1)
+ old_project.update(namespace: group)
+ new_project.update(namespace: group)
+
+ group.add_users([user], GroupMember::DEVELOPER)
+ end
+
+ context 'when moving to a project of the same group' do
+ it 'keeps the same group milestone' do
+ expect(new_issue.reload.project).to eq(new_project)
+ expect(new_issue.reload.milestone).to eq(group_milestone_1)
+ end
+ end
+
+ context 'when moving to a project of a different group' do
+ let!(:group_2) { create(:group, :private) }
+
+ let!(:group_milestone_2) do
+ create(:milestone, group_id: group_2.id, title: 'v9.0_group')
+ end
+
+ before do
+ old_issue.update(milestone: group_milestone_1)
+ new_project.update(namespace: group_2)
+
+ group_2.add_users([user], GroupMember::DEVELOPER)
+ end
+
+ it 'assigns to new group milestone of same title' do
+ expect(new_issue.reload.project).to eq(new_project)
+ expect(new_issue.reload.milestone).to eq(group_milestone_2)
+ end
+ end
+ end
+ end
+
context 'generic issue' do
include_context 'issue move executed'
@@ -55,11 +109,6 @@ describe Issues::MoveService, services: true do
expect(new_issue.project).to eq new_project
end
- it 'assigns milestone to new issue' do
- expect(new_issue.reload.milestone.title).to eq 'v9.0'
- expect(new_issue.reload.milestone).to eq(milestone2)
- end
-
it 'assign labels to new issue' do
expected_label_titles = new_issue.reload.labels.map(&:title)
expect(expected_label_titles).to include 'label1'
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index c26642f5015..d0b991f19ab 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -253,13 +253,13 @@ describe Issues::UpdateService, services: true do
end
context 'when the milestone change' do
- before do
+ it 'marks todos as done' do
update_issue(milestone: create(:milestone))
- end
- it 'marks todos as done' do
expect(todo.reload.done?).to eq true
end
+
+ it_behaves_like 'system notes for milestones'
end
context 'when the labels change' do
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index ec15b5cac14..be62584ec0e 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -296,13 +296,13 @@ describe MergeRequests::UpdateService, services: true do
end
context 'when the milestone change' do
- before do
+ it 'marks pending todos as done' do
update_merge_request({ milestone: create(:milestone) })
- end
- it 'marks pending todos as done' do
expect(pending_todo.reload).to be_done
end
+
+ it_behaves_like 'system notes for milestones'
end
context 'when the labels change' do
diff --git a/spec/support/issuable_shared_examples.rb b/spec/support/issuable_shared_examples.rb
index 03011535351..970fe10db2b 100644
--- a/spec/support/issuable_shared_examples.rb
+++ b/spec/support/issuable_shared_examples.rb
@@ -5,3 +5,34 @@ shared_examples 'cache counters invalidator' do
described_class.new(project, user, {}).execute(merge_request)
end
end
+
+shared_examples 'system notes for milestones' do
+ def update_issuable(opts)
+ issuable = try(:issue) || try(:merge_request)
+ described_class.new(project, user, opts).execute(issuable)
+ end
+
+ context 'group milestones' do
+ let(:group) { create(:group) }
+ let(:group_milestone) { create(:milestone, group: group) }
+
+ before do
+ project.update(namespace: group)
+ create(:group_member, group: group, user: user)
+ end
+
+ it 'does not create system note' do
+ expect do
+ update_issuable(milestone: group_milestone)
+ end.not_to change { Note.system.count }
+ end
+ end
+
+ context 'project milestones' do
+ it 'creates system note' do
+ expect do
+ update_issuable(milestone: create(:milestone))
+ end.to change { Note.system.count }.by(1)
+ end
+ end
+end