Commit f99ac4cc authored by Nick Thomas's avatar Nick Thomas

Merge branch 'osw-multi-assignees-merge-requests-ee' into 'master'

Support multiple assignees for merge requests

Closes #2004

See merge request gitlab-org/gitlab-ee!10161
parents 9cfb02f8 b73853a9
......@@ -81,9 +81,6 @@ export default {
const formData = {
update: {
state_event: this.form.find('input[name="update[state_event]"]').val(),
// For Merge Requests
assignee_id: this.form.find('input[name="update[assignee_id]"]').val(),
// For Issues
assignee_ids: [this.form.find('input[name="update[assignee_ids][]"]').val()],
milestone_id: this.form.find('input[name="update[milestone_id]"]').val(),
issuable_ids: this.form.find('input[name="update[issuable_ids]"]').val(),
......
......@@ -74,8 +74,7 @@ export default {
}
if (!this.users.length) {
const emptyTooltipLabel =
this.issuableType === 'issue' ? __('Assignee(s)') : __('Assignee');
const emptyTooltipLabel = __('Assignee(s)');
names.push(emptyTooltipLabel);
}
......@@ -90,6 +89,27 @@ export default {
return counter;
},
mergeNotAllowedTooltipMessage() {
const assigneesCount = this.users.length;
if (this.issuableType !== 'merge_request' || assigneesCount === 0) {
return null;
}
const cannotMergeCount = this.users.filter(u => u.can_merge === false).length;
const canMergeCount = assigneesCount - cannotMergeCount;
if (canMergeCount === assigneesCount) {
// Everyone can merge
return null;
} else if (cannotMergeCount === assigneesCount && assigneesCount > 1) {
return 'No one can merge';
} else if (assigneesCount === 1) {
return 'Cannot merge';
}
return `${canMergeCount}/${assigneesCount} can merge`;
},
},
methods: {
assignSelf() {
......@@ -154,6 +174,15 @@ export default {
</button>
</div>
<div class="value hide-collapsed">
<span
v-if="mergeNotAllowedTooltipMessage"
v-tooltip
:title="mergeNotAllowedTooltipMessage"
data-placement="left"
class="float-right cannot-be-merged"
>
<i aria-hidden="true" data-hidden="true" class="fa fa-exclamation-triangle"></i>
</span>
<template v-if="hasNoUsers">
<span class="assign-yourself no-value">
No assignee
......
......@@ -498,6 +498,16 @@
flex: 1;
}
.issuable-meta {
.author-link {
display: inline-block;
}
.issuable-comments {
height: 18px;
}
}
.merge-request-title {
margin-bottom: 2px;
......
......@@ -1158,6 +1158,8 @@ pre.light-well {
.cannot-be-merged:hover {
color: $red-500;
margin-top: 2px;
position: relative;
z-index: 2;
}
.private-forks-notice .private-fork-icon {
......
......@@ -192,12 +192,7 @@ module IssuableActions
def bulk_update_params
permitted_keys_array = permitted_keys.dup
if resource_name == 'issue'
permitted_keys_array << { assignee_ids: [] }
else
permitted_keys_array.unshift(:assignee_id)
end
params.require(:update).permit(permitted_keys_array)
end
......
......@@ -190,17 +190,17 @@ module IssuableCollections
end
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def preload_for_collection
common_attributes = [:author, :assignees, :labels, :milestone]
@preload_for_collection ||= case collection_type
when 'Issue'
[:project, :author, :assignees, :labels, :milestone, project: :namespace]
common_attributes + [:project, project: :namespace]
when 'MergeRequest'
[
:target_project, :author, :assignee, :labels, :milestone,
source_project: :route, head_pipeline: :project, target_project: :namespace, latest_merge_request_diff: :merge_request_diff_commits
]
common_attributes + [:target_project, source_project: :route, head_pipeline: :project, target_project: :namespace, latest_merge_request_diff: :merge_request_diff_commits]
end
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
end
IssuableCollections.prepend(EE::IssuableCollections)
......@@ -20,7 +20,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
def merge_request_params_attributes
[
:allow_collaboration,
:assignee_id,
:description,
:force_remove_source_branch,
:lock_version,
......@@ -35,6 +34,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:title,
:discussion_locked,
label_ids: [],
assignee_ids: [],
update_task: [:index, :checked, :line_number, :line_source]
]
end
......
......@@ -439,22 +439,6 @@ class IssuableFinder
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def by_assignee(items)
if filter_by_no_assignee?
items.where(assignee_id: nil)
elsif filter_by_any_assignee?
items.where('assignee_id IS NOT NULL')
elsif assignee
items.where(assignee_id: assignee.id)
elsif assignee_id? || assignee_username? # assignee not found
items.none
else
items
end
end
# rubocop: enable CodeReuse/ActiveRecord
def filter_by_no_assignee?
# Assignee_id takes precedence over assignee_username
[NONE, FILTER_NONE].include?(params[:assignee_id].to_s.downcase) || params[:assignee_username].to_s == NONE
......@@ -478,6 +462,20 @@ class IssuableFinder
end
# rubocop: enable CodeReuse/ActiveRecord
def by_assignee(items)
if filter_by_no_assignee?
items.unassigned
elsif filter_by_any_assignee?
items.assigned
elsif assignee
items.assigned_to(assignee)
elsif assignee_id? || assignee_username? # assignee not found
items.none
else
items
end
end
# rubocop: disable CodeReuse/ActiveRecord
def by_milestone(items)
if milestones?
......
......@@ -144,20 +144,6 @@ class IssuesFinder < IssuableFinder
current_user.blank?
end
def by_assignee(items)
if filter_by_no_assignee?
items.unassigned
elsif filter_by_any_assignee?
items.assigned
elsif assignee
items.assigned_to(assignee)
elsif assignee_id? || assignee_username? # assignee not found
items.none
else
items
end
end
end
IssuesFinder.prepend(EE::IssuesFinder)
......@@ -69,7 +69,7 @@ module BoardsHelper
end
def board_sidebar_user_data
dropdown_options = issue_assignees_dropdown_options
dropdown_options = assignees_dropdown_options('issue')
{
toggle: 'dropdown',
......
......@@ -19,8 +19,8 @@ module FormHelper
end
end
def issue_assignees_dropdown_options
{
def assignees_dropdown_options(issuable_type)
dropdown_data = {
toggle_class: 'js-user-search js-assignee-search js-multiselect js-save-user-data',
title: 'Select assignee',
filter: true,
......@@ -30,8 +30,8 @@ module FormHelper
first_user: current_user&.username,
null_user: true,
current_user: true,
project_id: @project&.id,
field_name: 'issue[assignee_ids][]',
project_id: (@target_project || @project)&.id,
field_name: "#{issuable_type}[assignee_ids][]",
default_label: 'Unassigned',
'max-select': 1,
'dropdown-header': 'Assignee',
......@@ -41,5 +41,36 @@ module FormHelper
current_user_info: UserSerializer.new.represent(current_user)
}
}
type = issuable_type.to_s
if type == 'issue' && issue_supports_multiple_assignees? ||
type == 'merge_request' && merge_request_supports_multiple_assignees?
dropdown_data = multiple_assignees_dropdown_options(dropdown_data)
end
dropdown_data
end
# Overwritten
def issue_supports_multiple_assignees?
false
end
# Overwritten
def merge_request_supports_multiple_assignees?
false
end
private
def multiple_assignees_dropdown_options(options)
new_options = options.dup
new_options[:title] = 'Select assignee(s)'
new_options[:data][:'dropdown-header'] = 'Assignee(s)'
new_options[:data].delete(:'max-select')
new_options
end
end
......@@ -15,11 +15,14 @@ module IssuablesHelper
sidebar_gutter_collapsed? ? _('Expand sidebar') : _('Collapse sidebar')
end
def sidebar_assignee_tooltip_label(issuable)
if issuable.assignee
issuable.assignee.name
def assignees_label(issuable, include_value: true)
label = 'Assignee'.pluralize(issuable.assignees.count)
if include_value
sanitized_list = sanitize_name(issuable.assignee_list)
"#{label}: #{sanitized_list}"
else
issuable.allows_multiple_assignees? ? _('Assignee(s)') : _('Assignee')
label
end
end
......
......@@ -24,10 +24,12 @@ module Emails
end
# rubocop: disable CodeReuse/ActiveRecord
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id, reason = nil)
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_ids, updated_by_user_id, reason = nil)
setup_merge_request_mail(merge_request_id, recipient_id)
@previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
@previous_assignees = []
@previous_assignees = User.where(id: previous_assignee_ids) if previous_assignee_ids.any?
mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id, reason))
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -4,6 +4,7 @@ class Notify < BaseMailer
include ActionDispatch::Routing::PolymorphicRoutes
include GitlabRoutingHelper
include EmailsHelper
include IssuablesHelper
include Emails::Issues
include Emails::MergeRequests
......@@ -24,6 +25,7 @@ class Notify < BaseMailer
helper MembersHelper
helper AvatarsHelper
helper GitlabRoutingHelper
helper IssuablesHelper
def test_email(recipient_email, subject, body)
mail(to: recipient_email,
......
# frozen_string_literal: true
# This module handles backward compatibility for import/export of Merge Requests after
# multiple assignees feature was introduced. Also, it handles the scenarios where
# the #26496 background migration hasn't finished yet.
# Ideally, most of this code should be removed at #59457.
module DeprecatedAssignee
extend ActiveSupport::Concern
def assignee_ids=(ids)
nullify_deprecated_assignee
super
end
def assignees=(users)
nullify_deprecated_assignee
super
end
def assignee_id=(id)
self.assignee_ids = Array(id)
end
def assignee=(user)
self.assignees = Array(user)
end
def assignee
assignees.first
end
def assignee_id
assignee_ids.first
end
def assignee_ids
if Gitlab::Database.read_only? && pending_assignees_population?
return Array(deprecated_assignee_id)
end
update_assignees_relation
super
end
def assignees
if Gitlab::Database.read_only? && pending_assignees_population?
return User.where(id: deprecated_assignee_id)
end
update_assignees_relation
super
end
private
# This will make the background migration process quicker (#26496) as it'll have less
# assignee_id rows to look through.
def nullify_deprecated_assignee
return unless persisted? && Gitlab::Database.read_only?
update_column(:assignee_id, nil)
end
# This code should be removed in the clean-up phase of the
# background migration (#59457).
def pending_assignees_population?
persisted? && deprecated_assignee_id && merge_request_assignees.empty?
end
# If there's an assignee_id and no relation, it means the background
# migration at #26496 didn't reach this merge request yet.
# This code should be removed in the clean-up phase of the
# background migration (#59457).
def update_assignees_relation
if pending_assignees_population?
transaction do
merge_request_assignees.create!(user_id: deprecated_assignee_id, merge_request_id: id)
update_column(:assignee_id, nil)
end
end
end
def deprecated_assignee_id
read_attribute(:assignee_id)
end
end
......@@ -67,13 +67,6 @@ module Issuable
allow_nil: true,
prefix: true
delegate :name,
:email,
:public_email,
to: :assignee,
allow_nil: true,
prefix: true
validates :author, presence: true
validates :title, presence: true, length: { maximum: 255 }
validate :milestone_is_valid
......@@ -88,6 +81,19 @@ module Issuable
scope :only_opened, -> { with_state(:opened) }
scope :closed, -> { with_state(:closed) }
# rubocop:disable GitlabSecurity/SqlInjection
# The `to_ability_name` method is not an user input.
scope :assigned, -> do
where("EXISTS (SELECT TRUE FROM #{to_ability_name}_assignees WHERE #{to_ability_name}_id = #{to_ability_name}s.id)")
end
scope :unassigned, -> do
where("NOT EXISTS (SELECT TRUE FROM #{to_ability_name}_assignees WHERE #{to_ability_name}_id = #{to_ability_name}s.id)")
end
scope :assigned_to, ->(u) do
where("EXISTS (SELECT TRUE FROM #{to_ability_name}_assignees WHERE user_id = ? AND #{to_ability_name}_id = #{to_ability_name}s.id)", u.id)
end
# rubocop:enable GitlabSecurity/SqlInjection
scope :left_joins_milestones, -> { joins("LEFT OUTER JOIN milestones ON #{table_name}.milestone_id = milestones.id") }
scope :order_milestone_due_desc, -> { left_joins_milestones.reorder('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date DESC') }
scope :order_milestone_due_asc, -> { left_joins_milestones.reorder('milestones.due_date IS NULL, milestones.id IS NULL, milestones.due_date ASC') }
......@@ -104,6 +110,7 @@ module Issuable
participant :author
participant :notes_with_associations
participant :assignees
strip_attributes :title
......@@ -272,6 +279,10 @@ module Issuable
end
end
def assignee_or_author?(user)
author_id == user.id || assignees.exists?(user.id)
end
def today?
Date.today == created_at.to_date
end
......@@ -316,11 +327,7 @@ module Issuable
end
if old_assignees != assignees
if self.is_a?(Issue)
changes[:assignees] = [old_assignees.map(&:hook_attrs), assignees.map(&:hook_attrs)]
else
changes[:assignee] = [old_assignees&.first&.hook_attrs, assignee&.hook_attrs]
end
end
if self.respond_to?(:total_time_spent)
......@@ -357,10 +364,18 @@ module Issuable
def card_attributes
{
'Author' => author.try(:name),
'Assignee' => assignee.try(:name)
'Assignee' => assignee_list
}
end
def assignee_list
assignees.map(&:name).to_sentence
end
def assignee_username_list
assignees.map(&:username).to_sentence
end
def notes_with_associations
# If A has_many Bs, and B has_many Cs, and you do
# `A.includes(b: :c).each { |a| a.b.includes(:c) }`, sadly ActiveRecord
......
......@@ -49,10 +49,6 @@ class Issue < ApplicationRecord
scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
scope :assigned, -> { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
scope :unassigned, -> { where('NOT EXISTS (SELECT TRUE FROM issue_assignees WHERE issue_id = issues.id)') }
scope :assigned_to, ->(u) { where('EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = ? AND issue_id = issues.id)', u.id)}
scope :with_due_date, -> { where.not(due_date: nil) }
scope :without_due_date, -> { where(due_date: nil) }
scope :due_before, ->(date) { where('issues.due_date < ?', date) }
......@@ -75,8 +71,6 @@ class Issue < ApplicationRecord
attr_spammable :title, spam_title: true
attr_spammable :description, spam_description: true
participant :assignees
state_machine :state, initial: :opened do
event :close do
transition [:opened] => :closed
......@@ -155,22 +149,6 @@ class Issue < ApplicationRecord
Gitlab::HookData::IssueBuilder.new(self).build
end
# Returns a Hash of attributes to be used for Twitter card metadata
def card_attributes
{
'Author' => author.try(:name),
'Assignee' => assignee_list
}
end
def assignee_or_author?(user)
author_id == user.id || assignees.exists?(user.id)
end
def assignee_list
assignees.map(&:name).to_sentence
end
# `from` argument can be a Namespace or Project.
def to_reference(from = nil, full: false)
reference = "#{self.class.reference_prefix}#{iid}"
......
......@@ -16,6 +16,7 @@ class MergeRequest < ApplicationRecord
include LabelEventable
include ReactiveCaching
include FromUnion
include DeprecatedAssignee
self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
self.reactive_cache_refresh_interval = 10.minutes
......@@ -71,8 +72,7 @@ class MergeRequest < ApplicationRecord
has_many :suggestions, through: :notes
has_many :merge_request_assignees
# Will be deprecated at https://gitlab.com/gitlab-org/gitlab-ce/issues/59457
belongs_to :assignee, class_name: "User"
has_many :assignees, class_name: "User", through: :merge_request_assignees
serialize :merge_params, Hash # rubocop:disable Cop/ActiveRecordSerialize
......@@ -81,10 +81,6 @@ class MergeRequest < ApplicationRecord
after_update :reload_diff_if_branch_changed
after_save :ensure_metrics
# Required until the codebase starts using this relation for single or multiple assignees.
# TODO: Remove at gitlab-ee#2004 implementation.
after_save :refresh_merge_request_assignees, if: :assignee_id_changed?
# When this attribute is true some MR validation is ignored
# It allows us to close or modify broken merge requests
attr_accessor :allow_broken
......@@ -190,19 +186,14 @@ class MergeRequest < ApplicationRecord
end
scope :join_project, -> { joins(:target_project) }
scope :references_project, -> { references(:target_project) }
scope :assigned, -> { where("assignee_id IS NOT NULL") }
scope :unassigned, -> { where("assignee_id IS NULL") }
scope :assigned_to, ->(u) { where(assignee_id: u.id)}
scope :with_api_entity_associations, -> {
preload(:author, :assignee, :notes, :labels, :milestone, :timelogs,
preload(:assignees, :author, :notes, :labels, :milestone, :timelogs,
latest_merge_request_diff: [:merge_request_diff_commits],
metrics: [:latest_closed_by, :merged_by],
target_project: [:route, { namespace: :route }],
source_project: [:route, { namespace: :route }])
}
participant :assignee
after_save :keep_around_commit
alias_attribute :project, :target_project
......@@ -339,31 +330,6 @@ class MergeRequest < ApplicationRecord
Gitlab::HookData::MergeRequestBuilder.new(self).build
end
# Returns a Hash of attributes to be used for Twitter card metadata
def card_attributes
{
'Author' => author.try(:name),
'Assignee' => assignee.try(:name)
}
end
# These method are needed for compatibility with issues to not mess view and other code
def assignees
Array(assignee)
end
def assignee_ids
Array(assignee_id)
end
def assignee_ids=(ids)
write_attribute(:assignee_id, ids.last)
end
def assignee_or_author?(user)
author_id == user.id || assignee_id == user.id
end
# `from` argument can be a Namespace or Project.
def to_reference(from = nil, full: false)
reference = "#{self.class.reference_prefix}#{iid}"
......@@ -684,15 +650,6 @@ class MergeRequest < ApplicationRecord
merge_request_diff || create_merge_request_diff
end
def refresh_merge_request_assignees
transaction do
# Using it instead relation.delete_all in order to avoid adding a
# dependent: :delete_all (we already have foreign key cascade deletion).
MergeRequestAssignee.where(merge_request_id: self).delete_all
merge_request_assignees.create(user_id: assignee_id) if assignee_id
end
end
def create_merge_request_diff
fetch_ref!
......@@ -1210,7 +1167,7 @@ class MergeRequest < ApplicationRecord
variables.append(key: 'CI_MERGE_REQUEST_PROJECT_URL', value: project.web_url)
variables.append(key: 'CI_MERGE_REQUEST_TARGET_BRANCH_NAME', value: target_branch.to_s)
variables.append(key: 'CI_MERGE_REQUEST_TITLE', value: title)
variables.append(key: 'CI_MERGE_REQUEST_ASSIGNEES', value: assignee.username) if assignee
variables.append(key: 'CI_MERGE_REQUEST_ASSIGNEES', value: assignee_username_list) if assignees.any?
variables.append(key: 'CI_MERGE_REQUEST_MILESTONE', value: milestone.title) if milestone
variables.append(key: 'CI_MERGE_REQUEST_LABELS', value: label_names.join(',')) if labels.present?
variables.concat(source_project_variables)
......
......@@ -674,6 +674,10 @@ class Project < ApplicationRecord
{ scope: :project, status: auto_devops&.enabled || Feature.enabled?(:force_autodevops_on_by_default, self) }
end
def multiple_mr_assignees_enabled?
Feature.enabled?(:multiple_merge_request_assignees, self)
end
def daily_statistics_enabled?
Feature.enabled?(:project_daily_statistics, self, default_enabled: true)
end
......
......@@ -11,4 +11,6 @@ class IssuableSidebarExtrasEntity < Grape::Entity
expose :subscribed do |issuable|
issuable.subscribed?(request.current_user, issuable.project)
end
expose :assignees, using: API::Entities::UserBasic
end
# frozen_string_literal: true
class IssueSidebarExtrasEntity < IssuableSidebarExtrasEntity
expose :assignees, using: API::Entities::UserBasic
end
IssueSidebarExtrasEntity.prepend(EE::IssueSidebarExtrasEntity)
# frozen_string_literal: true
class MergeRequestAssigneeEntity < ::API::Entities::UserBasic
expose :can_merge do |assignee, options|
options[:merge_request]&.can_be_merged_by?(assignee)
end
end
# frozen_string_literal: true
class MergeRequestBasicEntity < Grape::Entity
expose :assignee_id
expose :merge_status
expose :merge_error
expose :state
......@@ -9,7 +8,7 @@ class MergeRequestBasicEntity < Grape::Entity
expose :rebase_in_progress?, as: :rebase_in_progress
expose :milestone, using: API::Entities::Milestone
expose :labels, using: LabelEntity
expose :assignee, using: API::Entities::UserBasic
expose :assignees, using: API::Entities::UserBasic
expose :task_status, :task_status_short
expose :lock_version, :lock_version
end
......@@ -8,9 +8,9 @@ class MergeRequestSerializer < BaseSerializer
entity =
case opts[:serializer]
when 'sidebar'
MergeRequestSidebarBasicEntity
IssuableSidebarBasicEntity
when 'sidebar_extras'
IssuableSidebarExtrasEntity
MergeRequestSidebarExtrasEntity
when 'basic'
MergeRequestBasicEntity
else
......
# frozen_string_literal: true
class MergeRequestSidebarBasicEntity < IssuableSidebarBasicEntity
expose :assignee, if: lambda { |issuable| issuable.assignee } do
expose :assignee, merge: true, using: API::Entities::UserBasic
expose :can_merge do |issuable|
issuable.can_be_merged_by?(issuable.assignee)
end
end
end
# frozen_string_literal: true
class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity
expose :assignees do |merge_request|
MergeRequestAssigneeEntity.represent(merge_request.assignees, merge_request: merge_request)
end
end
......@@ -34,14 +34,20 @@ class IssuableBaseService < BaseService
end
def filter_assignee(issuable)
return unless params[:assignee_id].present?
return if params[:assignee_ids].blank?
assignee_id = params[:assignee_id]
unless issuable.allows_multiple_assignees?
params[:assignee_ids] = params[:assignee_ids].first(1)
end
assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) }
if assignee_id.to_s == IssuableFinder::NONE
params[:assignee_id] = ""
if params[:assignee_ids].map(&:to_s) == [IssuableFinder::NONE]
params[:assignee_ids] = []
elsif assignee_ids.any?
params[:assignee_ids] = assignee_ids
else
params.delete(:assignee_id) unless assignee_can_read?(issuable, assignee_id)
params.delete(:assignee_ids)
end
end
......@@ -352,7 +358,7 @@ class IssuableBaseService < BaseService
end
def has_changes?(issuable, old_labels: [], old_assignees: [])
valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch]
valid_attrs = [:title, :description, :assignee_ids, :milestone_id, :target_branch]
attrs_changed = valid_attrs.any? do |attr|
issuable.previous_changes.include?(attr.to_s)
......
......@@ -20,7 +20,7 @@ module Issues
private
def create_assignee_note(issue, old_assignees)
SystemNoteService.change_issue_assignees(
SystemNoteService.change_issuable_assignees(
issue, issue.project, current_user, old_assignees)
end
......@@ -31,26 +31,6 @@ module Issues
issue.project.execute_services(issue_data, hooks_scope)
end
# rubocop: disable CodeReuse/ActiveRecord
def filter_assignee(issuable)
return if params[:assignee_ids].blank?
unless issuable.allows_multiple_assignees?
params[:assignee_ids] = params[:assignee_ids].take(1)
end
assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) }
if params[:assignee_ids].map(&:to_s) == [IssuableFinder::NONE]
params[:assignee_ids] = []
elsif assignee_ids.any?
params[:assignee_ids] = assignee_ids
else
params.delete(:assignee_ids)
end
end
# rubocop: enable CodeReuse/ActiveRecord
def update_project_counter_caches?(issue)
super || issue.confidential_changed?
end
......
......@@ -39,7 +39,7 @@ module Issues
if issue.assignees != old_assignees
create_assignee_note(issue, old_assignees)
notification_service.async.reassigned_issue(issue, current_user, old_assignees)
todo_service.reassigned_issue(issue, current_user, old_assignees)
todo_service.reassigned_issuable(issue, current_user, old_assignees)
end
if issue.previous_changes.include?('confidential')
......
......@@ -49,9 +49,9 @@ module MergeRequests
MergeRequestMetricsService.new(merge_request.metrics)
end
def create_assignee_note(merge_request)
SystemNoteService.change_assignee(
merge_request, merge_request.project, current_user, merge_request.assignee)
def create_assignee_note(merge_request, old_assignees)
SystemNoteService.change_issuable_assignees(
merge_request, merge_request.project, current_user, old_assignees)
end
def create_pipeline_for(merge_request, user)
......
......@@ -24,13 +24,13 @@ module MergeRequests
update_task_event(merge_request) || update(merge_request)
end
# rubocop:disable Metrics/AbcSize
def handle_changes(merge_request, options)
old_associations = options.fetch(:old_associations, {})
old_labels = old_associations.fetch(:labels, [])
old_mentioned_users = old_associations.fetch(:mentioned_users, [])
old_assignees = old_associations.fetch(:assignees, [])
if has_changes?(merge_request, old_labels: old_labels)
if has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees)
todo_service.mark_pending_todos_as_done(merge_request, current_user)
end
......@@ -45,15 +45,10 @@ module MergeRequests
merge_request.target_branch)
end
if merge_request.previous_changes.include?('assignee_id')
reassigned_merge_request_args = [merge_request, current_user]
old_assignee_id = merge_request.previous_changes['assignee_id'].first
reassigned_merge_request_args << User.find(old_assignee_id) if old_assignee_id
create_assignee_note(merge_request)
notification_service.async.reassigned_merge_request(*reassigned_merge_request_args)
todo_service.reassigned_merge_request(merge_request, current_user)
if merge_request.assignees != old_assignees
create_assignee_note(merge_request, old_assignees)
notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignees)
todo_service.reassigned_issuable(merge_request, current_user, old_assignees)
end
if merge_request.previous_changes.include?('target_branch') ||
......@@ -81,7 +76,6 @@ module MergeRequests
)
end
end
# rubocop:enable Metrics/AbcSize
def handle_task_changes(merge_request)
todo_service.mark_pending_todos_as_done(merge_request, current_user)
......
......@@ -247,15 +247,15 @@ module NotificationRecipientService
attr_reader :target
attr_reader :current_user
attr_reader :action
attr_reader :previous_assignee
attr_reader :previous_assignees
attr_reader :skip_current_user
def initialize(target, current_user, action:, custom_action: nil, previous_assignee: nil, skip_current_user: true)
def initialize(target, current_user, action:, custom_action: nil, previous_assignees: nil, skip_current_user: true)
@target = target
@current_user = current_user
@action = action
@custom_action = custom_action
@previous_assignee = previous_assignee
@previous_assignees = previous_assignees
@skip_current_user = skip_current_user
end
......@@ -270,11 +270,7 @@ module NotificationRecipientService
# Re-assign is considered as a mention of the new assignee
case custom_action
when :reassign_merge_request
add_recipients(previous_assignee, :mention, nil)
add_recipients(target.assignee, :mention, NotificationReason::ASSIGNED)
when :reassign_issue
previous_assignees = Array(previous_assignee)
when :reassign_merge_request, :reassign_issue
add_recipients(previous_assignees, :mention, nil)
add_recipients(target.assignees, :mention, NotificationReason::ASSIGNED)
end
......@@ -287,17 +283,11 @@ module NotificationRecipientService
# receive them, too.
add_mentions(current_user, target: target)
# Add the assigned users, if any
assignees = case custom_action
when :new_issue
target.assignees
else
target.assignee
end
# We use the `:participating` notification level in order to match existing legacy behavior as captured
# in existing specs (notification_service_spec.rb ~ line 507)
add_recipients(assignees, :participating, NotificationReason::ASSIGNED) if assignees
if target.is_a?(Issuable)
add_recipients(target.assignees, :participating, NotificationReason::ASSIGNED)
end
add_labels_subscribers
end
......
......@@ -95,8 +95,8 @@ class NotificationService
# When we reassign an issue we should send an email to:
#
# * issue old assignee if their notification level is not Disabled
# * issue new assignee if their notification level is not Disabled
# * issue old assignees if their notification level is not Disabled
# * issue new assignees if their notification level is not Disabled
# * users with custom level checked with "reassign issue"
#
def reassigned_issue(issue, current_user, previous_assignees = [])
......@@ -104,7 +104,7 @@ class NotificationService
issue,
current_user,
action: "reassign",
previous_assignee: previous_assignees
previous_assignees: previous_assignees
)
previous_assignee_ids = previous_assignees.map(&:id)
......@@ -140,7 +140,7 @@ class NotificationService
# When create a merge request we should send an email to:
#
# * mr author
# * mr assignee if their notification level is not Disabled
# * mr assignees if their notification level is not Disabled
# * project team members with notification level higher then Participating
# * watchers of the mr's labels
# * users with custom level checked with "new merge request"
......@@ -184,23 +184,25 @@ class NotificationService
# When we reassign a merge_request we should send an email to:
#
# * merge_request old assignee if their notification level is not Disabled
# * merge_request assignee if their notification level is not Disabled
# * merge_request old assignees if their notification level is not Disabled
# * merge_request new assignees if their notification level is not Disabled
# * users with custom level checked with "reassign merge request"
#
def reassigned_merge_request(merge_request, current_user, previous_assignee = nil)
def reassigned_merge_request(merge_request, current_user, previous_assignees = [])
recipients = NotificationRecipientService.build_recipients(
merge_request,
current_user,
action: "reassign",
previous_assignee: previous_assignee
previous_assignees: previous_assignees
)
previous_assignee_ids = previous_assignees.map(&:id)
recipients.each do |recipient|
mailer.reassigned_merge_request_email(
recipient.user.id,
merge_request.id,
previous_assignee&.id,
previous_assignee_ids,
current_user.id,
recipient.reason
).deliver_later
......
......@@ -69,7 +69,7 @@ module SystemNoteService
# Called when the assignees of an Issue is changed or removed
#
# issue - Issue object
# issuable - Issuable object (responds to assignees)
# project - Project owning noteable
# author - User performing the change
# assignees - Users being assigned, or nil
......@@ -85,9 +85,9 @@ module SystemNoteService
# "assigned to @user1 and @user2"
#
# Returns the created Note object
def change_issue_assignees(issue, project, author, old_assignees)
unassigned_users = old_assignees - issue.assignees
added_users = issue.assignees.to_a - old_assignees
def change_issuable_assignees(issuable, project, author, old_assignees)
unassigned_users = old_assignees - issuable.assignees
added_users = issuable.assignees.to_a - old_assignees
text_parts = []
text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any?
......@@ -95,7 +95,7 @@ module SystemNoteService
body = text_parts.join(' and ')
create_note(NoteSummary.new(issue, project, author, body, action: 'assignee'))
create_note(NoteSummary.new(issuable, project, author, body, action: 'assignee'))
end
# Called when the milestone of a Noteable is changed
......
......@@ -49,12 +49,12 @@ class TodoService
todo_users.each(&:update_todos_count_cache)
end
# When we reassign an issue we should:
# When we reassign an issuable we should:
#
# * create a pending todo for new assignee if issue is assigned
# * create a pending todo for new assignee if issuable is assigned
#
def reassigned_issue(issue, current_user, old_assignees = [])
create_assignment_todo(issue, current_user, old_assignees)
def reassigned_issuable(issuable, current_user, old_assignees = [])
create_assignment_todo(issuable, current_user, old_assignees)
end
# When create a merge request we should:
......@@ -82,14 +82,6 @@ class TodoService
mark_pending_todos_as_done(merge_request, current_user)
end
# When we reassign a merge request we should:
#
# * creates a pending todo for new assignee if merge request is assigned
#
def reassigned_merge_request(merge_request, current_user)
create_assignment_todo(merge_request, current_user)
end
# When merge a merge request we should:
#
# * mark all pending todos related to the target for the current user as done
......
%p
Assignee changed
- if previous_assignees.any?
from
%strong= sanitize_name(previous_assignees.map(&:name).to_sentence)
to
- if issuable.assignees.any?
%strong= sanitize_name(issuable.assignee_list)
- else
%strong Unassigned
......@@ -5,4 +5,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
= merge_path_description(@merge_request, 'to')
Author: #{sanitize_name(@merge_request.author_name)}
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
......@@ -3,7 +3,7 @@
- if @issue.assignees.any?
%p
Assignee: #{@issue.assignee_list}
= assignees_label(@issue)
%p
This issue is due on: #{@issue.due_date.to_s(:medium)}
......
......@@ -2,6 +2,6 @@ The following issue is due on <%= @issue.due_date %>:
Issue <%= @issue.iid %>: <%= url_for(project_issue_url(@issue.project, @issue)) %>
Author: <%= @issue.author_name %>
Assignee: <%= @issue.assignee_list %>
<%= assignees_label(@issue) %>
<%= @issue.description %>
......@@ -5,4 +5,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
= merge_path_description(@merge_request, 'to')
Author: #{sanitize_name(@merge_request.author_name)}
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
......@@ -5,4 +5,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
= merge_path_description(@merge_request, 'to')
Author: #{sanitize_name(@merge_request.author_name)}
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
......@@ -5,4 +5,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
= merge_path_description(@merge_request, 'to')
Author: #{sanitize_name(@merge_request.author_name)}
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
......@@ -4,7 +4,7 @@
- if @issue.assignees.any?
%p
Assignee: #{@issue.assignee_list}
= assignees_label(@issue)
- if @issue.description
%div
......
......@@ -2,6 +2,6 @@ New Issue was created.
Issue <%= @issue.iid %>: <%= url_for(project_issue_url(@issue.project, @issue)) %>
Author: <%= sanitize_name(@issue.author_name) %>
Assignee: <%= @issue.assignee_list %>
<%= assignees_label(@issue) %>
<%= @issue.description %>
......@@ -2,6 +2,6 @@ You have been mentioned in an issue.
Issue <%= @issue.iid %>: <%= url_for(project_issue_url(@issue.project, @issue)) %>
Author: <%= sanitize_name(@issue.author_name) %>
Assignee: <%= sanitize_name(@issue.assignee_list) %>
<%= assignees_label(@issue) %>
<%= @issue.description %>
......@@ -4,6 +4,6 @@ You have been mentioned in Merge Request <%= @merge_request.to_reference %>
<%= merge_path_description(@merge_request, 'to') %>
Author: <%= sanitize_name(@merge_request.author_name) %>
Assignee: <%= sanitize_name(@merge_request.assignee_name) %>
= assignees_label(@merge_request)
<%= @merge_request.description %>
......@@ -5,9 +5,9 @@
%p.details
!= merge_path_description(@merge_request, '&rarr;')
- if @merge_request.assignee_id.present?
- if @merge_request.assignees.any?
%p
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter
......
......@@ -4,7 +4,7 @@ New Merge Request <%= @merge_request.to_reference %>
<%= merge_path_description(@merge_request, 'to') %>
Author: <%= @merge_request.author_name %>
Assignee: <%= @merge_request.assignee_name %>
<%= assignees_label(@merge_request) %>
<%= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter %>
<%= @merge_request.description %>
%p
Assignee changed
- if @previous_assignees.any?
from
%strong= sanitize_name(@previous_assignees.map(&:name).to_sentence)
to
- if @issue.assignees.any?
%strong= @issue.assignee_list
- else
%strong Unassigned
= render 'reassigned_issuable_email', issuable: @issue, previous_assignees: @previous_assignees
%p
Assignee changed
- if @previous_assignee
from
%strong= sanitize_name(@previous_assignee.name)
to
- if @merge_request.assignee_id
%strong= sanitize_name(@merge_request.assignee_name)
- else
%strong Unassigned
= render 'reassigned_issuable_email', issuable: @merge_request, previous_assignees: @previous_assignees
......@@ -2,5 +2,5 @@ Reassigned Merge Request <%= @merge_request.iid %>
<%= url_for([@merge_request.project.namespace.becomes(Namespace), @merge_request.project, @merge_request, { only_path: false }]) %>
Assignee changed <%= "from #{sanitize_name(@previous_assignee.name)}" if @previous_assignee -%>
to <%= "#{@merge_request.assignee_id ? sanitize_name(@merge_request.assignee_name) : 'Unassigned'}" %>
Assignee changed <%= "from #{sanitize_name(@previous_assignees.map(&:name).to_sentence)}" if @previous_assignees.any? -%>
to <%= "#{@merge_request.assignees.any? ? @merge_request.assignee_list : 'Unassigned'}" %>
......@@ -52,7 +52,7 @@
CLOSED
- if issue.assignees.any?
%li
= render 'shared/issuable/assignees', project: @project, issue: issue
= render 'shared/issuable/assignees', project: @project, issuable: issue
= render 'shared/issuable_meta_data', issuable: issue
......
......@@ -53,9 +53,9 @@
%li.issuable-pipeline-broken.d-none.d-sm-inline-block
= link_to merge_request_path(merge_request), class: "has-tooltip", title: _('Cannot be merged automatically') do
= icon('exclamation-triangle')
- if merge_request.assignee
- if merge_request.assignees.any?
%li
= link_to_member(merge_request.source_project, merge_request.assignee, name: false, title: _('Assigned to :name'))
= render 'shared/issuable/assignees', project: merge_request.project, issuable: merge_request
= render_if_exists 'projects/merge_requests/approvals_count', merge_request: merge_request
= render 'shared/issuable_meta_data', issuable: merge_request
......
......@@ -19,7 +19,7 @@
":data-name" => "assignee.name",
":data-username" => "assignee.username" }
.dropdown
- dropdown_options = issue_assignees_dropdown_options
- dropdown_options = assignees_dropdown_options('issue')
%button.dropdown-menu-toggle.js-user-search.js-author-search.js-multiselect.js-save-user-data.js-issue-board-sidebar{ type: 'button', ref: 'assigneeDropdown', data: board_sidebar_user_data,
":data-issuable-id" => "issue.iid" }
= dropdown_options[:title]
......
- max_render = 4
- assignees_rendering_overflow = issue.assignees.size > max_render
- assignees_rendering_overflow = issuable.assignees.size > max_render
- render_count = assignees_rendering_overflow ? max_render - 1 : max_render
- more_assignees_count = issue.assignees.size - render_count
- more_assignees_count = issuable.assignees.size - render_count
- issue.assignees.take(render_count).each do |assignee| # rubocop: disable CodeReuse/ActiveRecord
- issuable.assignees.take(render_count).each do |assignee| # rubocop: disable CodeReuse/ActiveRecord
= link_to_member(@project, assignee, name: false, title: "Assigned to :name")
- if more_assignees_count.positive?
......
......@@ -21,10 +21,7 @@
.title
Assignee
.filter-item
- if type == :issues
- field_name = "update[assignee_ids][]"
- else
- field_name = "update[assignee_id]"
= dropdown_tag("Select assignee", options: { toggle_class: "js-user-search js-update-assignee js-filter-submit js-filter-bulk-update", title: "Assign to", filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable",
placeholder: "Search authors", data: { first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: @project.id, field_name: field_name } })
.block
......
- issuable_type = issuable_sidebar[:type]
- signed_in = !!issuable_sidebar.dig(:current_user, :id)
- can_edit_issuable = issuable_sidebar.dig(:current_user, :can_edit)
- if issuable_type == "issue"
#js-vue-sidebar-assignees{ data: { field: "#{issuable_type}[assignee_ids]", signed_in: signed_in } }
#js-vue-sidebar-assignees{ data: { field: "#{issuable_type}[assignee_ids]", signed_in: signed_in } }
.title.hide-collapsed
= _('Assignee')
= icon('spinner spin')
- else
- assignee = assignees.first
.sidebar-collapsed-icon.sidebar-collapsed-user{ data: { toggle: "tooltip", placement: "left", container: "body", boundary: 'viewport' }, title: (issuable_sidebar.dig(:assignee, :name) || _('Assignee')) }
- if issuable_sidebar[:assignee]
= link_to_member(@project, assignee, size: 24)
- else
= icon('user', 'aria-hidden': 'true')
.title.hide-collapsed
= _('Assignee')
= icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true')
- if can_edit_issuable
= link_to _('Edit'), '#', class: 'js-sidebar-dropdown-toggle edit-link float-right'
- if !signed_in
%a.gutter-toggle.float-right.js-sidebar-toggle{ role: "button", href: "#", "aria-label" => _('Toggle sidebar') }
= sidebar_gutter_toggle_icon
.value.hide-collapsed
- if issuable_sidebar[:assignee]
= link_to_member(@project, assignee, size: 32, extra_class: 'bold') do
- unless issuable_sidebar[:assignee][:can_merge]
%span.float-right.cannot-be-merged{ data: { toggle: 'tooltip', placement: 'left' }, title: _('Not allowed to merge') }
= icon('exclamation-triangle', 'aria-hidden': 'true')
%span.username
@#{issuable_sidebar[:assignee][:username]}
- else
%span.assign-yourself.no-value
= _('No assignee')
- if can_edit_issuable
\-
%a.js-assign-yourself{ href: '#' }
= _('assign yourself')
.selectbox.hide-collapsed
- if assignees.none?
......@@ -59,10 +27,8 @@
ability_name: issuable_type,
null_user: true,
display: 'static' } }
- title = _('Select assignee')
- if issuable_type == "issue"
- dropdown_options = issue_assignees_dropdown_options
- dropdown_options = assignees_dropdown_options(issuable_type)
- title = dropdown_options[:title]
- options[:toggle_class] += ' js-multiselect js-save-user-data'
- data = { field_name: "#{issuable_type}[assignee_ids][]" }
......
- merge_request = issuable
.block.assignee
.sidebar-collapsed-icon.sidebar-collapsed-user{ data: { toggle: "tooltip", placement: "left", container: "body" }, title: sidebar_assignee_tooltip_label(issuable) }
- if merge_request.assignee
= link_to_member(@project, merge_request.assignee, size: 24)
- else
= icon('user', 'aria-hidden': 'true')
.title.hide-collapsed
Assignee
= icon('spinner spin', class: 'hidden block-loading', 'aria-hidden': 'true')
- if can_edit_issuable
= link_to 'Edit', '#', class: 'js-sidebar-dropdown-toggle edit-link float-right'
.value.hide-collapsed
- if merge_request.assignee
= link_to_member(@project, merge_request.assignee, size: 32, extra_class: 'bold') do
- unless merge_request.can_be_merged_by?(merge_request.assignee)
%span.float-right.cannot-be-merged{ data: { toggle: 'tooltip', placement: 'left' }, title: 'Not allowed to merge' }
= icon('exclamation-triangle', 'aria-hidden': 'true')
%span.username
= merge_request.assignee.to_reference
- else
%span.assign-yourself.no-value
No assignee
- if can_edit_issuable
\-
%a.js-assign-yourself{ href: '#' }
assign yourself
.selectbox.hide-collapsed
= f.hidden_field 'assignee_id', value: merge_request.assignee_id, id: 'issue_assignee_id'
= dropdown_tag('Select assignee', options: { toggle_class: 'js-user-search js-author-search', title: 'Assign to', filter: true, dropdown_class: 'dropdown-menu-user dropdown-menu-selectable dropdown-menu-author', placeholder: 'Search users', data: { first_user: (current_user.username if current_user), current_user: true, project_id: @project&.id, author_id: merge_request.author_id, field_name: 'merge_request[assignee_id]', issue_update: issuable_json_path(merge_request), ability_name: 'merge_request', null_user: true } })
......@@ -8,11 +8,8 @@
%hr
.row
%div{ class: (has_due_date ? "col-lg-6" : "col-12") }
.form-group.row.issue-assignee
- if issuable.is_a?(Issue)
= render "shared/issuable/form/metadata_issue_assignee", issuable: issuable, form: form, has_due_date: has_due_date
- else
= render "shared/issuable/form/metadata_merge_request_assignee", issuable: issuable, form: form, has_due_date: has_due_date
.form-group.row.merge-request-assignee
= render "shared/issuable/form/metadata_issuable_assignee", issuable: issuable, form: form, has_due_date: has_due_date
.form-group.row.issue-milestone
= form.label :milestone_id, "Milestone", class: "col-form-label #{has_due_date ? "col-md-2 col-lg-4" : "col-sm-2"}"
.col-sm-10{ class: ("col-md-8" if has_due_date) }
......
= form.label :assignee_ids, "Assignee", class: "col-form-label #{"col-md-2 col-lg-4" if has_due_date}"
= form.label :assignee_id, "Assignee", class: "col-form-label #{has_due_date ? "col-lg-4" : "col-sm-2"}"
.col-sm-10{ class: ("col-md-8" if has_due_date) }
.issuable-form-select-holder.selectbox
- issuable.assignees.each do |assignee|
......@@ -7,5 +7,5 @@
- if issuable.assignees.length === 0
= hidden_field_tag "#{issuable.to_ability_name}[assignee_ids][]", 0, id: nil, data: { meta: '' }
= dropdown_tag(users_dropdown_label(issuable.assignees), options: issue_assignees_dropdown_options)
= link_to 'Assign to me', '#', class: "assign-to-me-link #{'hide' if issuable.assignees.include?(current_user)}"
= dropdown_tag(users_dropdown_label(issuable.assignees), options: assignees_dropdown_options(issuable.to_ability_name))
= link_to 'Assign to me', '#', class: "assign-to-me-link qa-assign-to-me-link #{'hide' if issuable.assignees.include?(current_user)}"
......@@ -21,7 +21,7 @@ Gitlab::Seeder.quiet do
title: FFaker::Lorem.sentence(6),
description: FFaker::Lorem.sentences(3).join(" "),
milestone: project.milestones.sample,
assignee: project.team.users.sample,
assignees: [project.team.users.sample],
label_ids: label_ids
}
......
......@@ -2,16 +2,12 @@
module EE
module FormHelper
def issue_assignees_dropdown_options
options = super
if current_board_parent.feature_available?(:multiple_issue_assignees)
options[:title] = 'Select assignee(s)'
options[:data][:'dropdown-header'] = 'Assignee(s)'
options[:data].delete(:'max-select')
def issue_supports_multiple_assignees?
current_board_parent.feature_available?(:multiple_issue_assignees)
end
options
def merge_request_supports_multiple_assignees?
@merge_request&.allows_multiple_assignees?
end
end
end
......@@ -54,6 +54,11 @@ module EE
super
end
def allows_multiple_assignees?
project.multiple_mr_assignees_enabled? &&
project.feature_available?(:multiple_merge_request_assignees)
end
def supports_weight?
false
end
......
......@@ -26,6 +26,7 @@ class License < ApplicationRecord
merge_request_approvers
multiple_ldap_servers
multiple_issue_assignees
multiple_merge_request_assignees
multiple_project_issue_boards
push_rules
protected_refs_for_users
......
......@@ -4,9 +4,13 @@
%p.details
!= merge_path_description(@merge_request, '&rarr;')
- if @merge_request.assignee_id.present?
- if @merge_request.assignees.any?
%p
Assignee: #{@merge_request.author_name} &rarr; #{@merge_request.assignee_name}
- label = assignees_label(@merge_request, include_value: false)
- author_name = sanitize_name(@merge_request.author_name)
- assignee_list = sanitize_name(@merge_request.assignee_list)
"#{label}: #{author_name} &rarr; #{assignee_list}"
= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter
......
......@@ -4,5 +4,5 @@
<%= merge_path_description(@merge_request, 'to') %>
Author: <%= sanitize_name(@merge_request.author_name) %>
Assignee: <%= sanitize_name(@merge_request.assignee_name) %>
<%= assignees_label(@merge_request) %>
<%= render_if_exists 'notify/merge_request_approvers', presenter: @mr_presenter %>
......@@ -41,6 +41,19 @@
padding-right: 10px !important;
}
}
ul.assignees-list {
list-style: none;
padding: 0px;
display: block;
margin-top: 0px;
}
ul.assignees-list li {
display: inline-block;
padding-right: 12px;
padding-top: 8px;
}
%body{ style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;height:100%;font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" }
%table#body{ border: "0", cellpadding: "0", cellspacing: "0", style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;" }
%tbody
......@@ -122,18 +135,17 @@
%a.muted{ href: user_url(@merge_request.author), style: "color:#333333;text-decoration:none;" }
= @merge_request.author.name
- if @merge_request.assignee
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Assignee
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
%table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" }
%tbody
- if @merge_request.assignees.any?
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon_for_user(@merge_request.assignee, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
%a.muted{ href: user_url(@merge_request.assignee), style: "color:#333333;text-decoration:none;" }
= @merge_request.assignee.name
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" }
= assignees_label(@merge_request, include_value: false)
%td{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; margin: 0; padding: 14px 0 0px 5px; font-size: 15px; line-height: 1.4; color: #333333; font-weight: 400; width: 75%; border-top-style: solid; border-top-color: #ededed; border-top-width: 1px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
%ul.assignees-list{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; font-size: 15px; line-height: 1.4; padding-right: 5px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
- @merge_request.assignees.each do |assignee|
%li
%img.avatar{ alt: "Avatar", height: "24", src: avatar_icon_for_user(assignee, 24, only_path: false), style: "border-radius: 12px; max-width: 100%; height: auto; -ms-interpolation-mode: bicubic; margin: -2px 0;", width: "24" }/
%a.muted{ href: user_url(assignee), style: "color: #333333; text-decoration: none; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; vertical-align: top;" }
= assignee.name
-# EE-specific start
= render 'layouts/mailer/additional_text'
......
......@@ -5,4 +5,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
= merge_path_description(@merge_request, 'to')
Author: #{sanitize_name(@merge_request.author_name)}
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
......@@ -41,6 +41,19 @@
padding-right: 10px !important;
}
}
ul.assignees-list {
list-style: none;
padding: 0px;
display: block;
margin-top: 0px;
}
ul.assignees-list li {
display: inline-block;
padding-right: 12px;
padding-top: 8px;
}
%body{ style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;height:100%;font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;" }
%table#body{ border: "0", cellpadding: "0", cellspacing: "0", style: "background-color:#fafafa;margin:0;padding:0;text-align:center;min-width:640px;width:100%;" }
%tbody
......@@ -121,18 +134,17 @@
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
%a.muted{ href: user_url(@merge_request.author), style: "color:#333333;text-decoration:none;" }
= @merge_request.author.name
- if @merge_request.assignee
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" } Assignee
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;color:#333333;font-weight:400;width:75%;padding-left:5px;border-top:1px solid #ededed;" }
%table.img{ border: "0", cellpadding: "0", cellspacing: "0", style: "border-collapse:collapse;" }
%tbody
- if @merge_request.assignees.any?
%tr
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" }
%img.avatar{ height: "24", src: avatar_icon_for_user(@merge_request.assignee, 24), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "Avatar" }/
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" }
%a.muted{ href: user_url(@merge_request.assignee), style: "color:#333333;text-decoration:none;" }
= @merge_request.assignee.name
%td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;color:#8c8c8c;font-weight:300;padding:14px 0;margin:0;border-top:1px solid #ededed;" }
= assignees_label(@merge_request, include_value: false)
%td{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; margin: 0; padding: 14px 0 0px 5px; font-size: 15px; line-height: 1.4; color: #333333; font-weight: 400; width: 75%; border-top-style: solid; border-top-color: #ededed; border-top-width: 1px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
%ul.assignees-list{ style: "font-family: 'Helvetica Neue',Helvetica,Arial,sans-serif; font-size: 15px; line-height: 1.4; padding-right: 5px; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; mso-table-lspace: 0pt; mso-table-rspace: 0pt;" }
- @merge_request.assignees.each do |assignee|
%li
%img.avatar{ alt: "Avatar", height: "24", src: avatar_icon_for_user(assignee, 24, only_path: false), style: "border-radius: 12px; max-width: 100%; height: auto; -ms-interpolation-mode: bicubic; margin: -2px 0;", width: "24" }/
%a.muted{ href: user_url(assignee), style: "color: #333333; text-decoration: none; -webkit-text-size-adjust: 100%; -ms-text-size-adjust: 100%; vertical-align: top;" }
= assignee.name
-# EE-specific start
= render 'layouts/mailer/additional_text'
......
......@@ -5,4 +5,4 @@ Merge Request url: #{project_merge_request_url(@merge_request.target_project, @m
= merge_path_description(@merge_request, 'to')
Author: #{sanitize_name(@merge_request.author_name)}
Assignee: #{sanitize_name(@merge_request.assignee_name)}
= assignees_label(@merge_request)
---
title: Support multiple assignees for merge requests
merge_request: 10161
author:
type: added
......@@ -135,7 +135,9 @@ module API
class PullRequest < Grape::Entity
expose :title
expose :assignee, using: User
expose :assignee, using: User do |merge_request|
merge_request.assignee
end
expose :author, as: :user, using: User
expose :created_at
expose :description, as: :body
......
......@@ -17,7 +17,7 @@ describe Projects::MergeRequests::DraftsController do
before do
sign_in(user)
stub_licensed_features(batch_comments: true)
stub_licensed_features(batch_comments: true, multiple_merge_request_assignees: true)
stub_commonmark_sourcepos_disabled
end
......@@ -237,15 +237,17 @@ describe Projects::MergeRequests::DraftsController do
end
it 'publishes a draft note with quick actions and applies them' do
create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user.to_reference}")
project.add_developer(user2)
create(:draft_note, merge_request: merge_request, author: user,
note: "/assign #{user.to_reference} #{user2.to_reference}")
expect(merge_request.assignee_id).to be_nil
expect(merge_request.assignees).to be_empty
expect { post :publish, params: params }.to change { Note.count }.by(1)
.and change { DraftNote.count }.by(-1)
expect(response).to have_gitlab_http_status(200)
expect(merge_request.reload.assignee_id).to eq(user.id)
expect(merge_request.reload.assignee_ids).to match_array([user.id, user2.id])
expect(Note.last.system?).to be true
end
......
......@@ -35,8 +35,8 @@ describe 'New/edit issue', :js do
# the original method, resulting in infinite recurison when called.
# This is likely a bug with helper modules included into dynamically generated view classes.
# To work around this, we have to hold on to and call to the original implementation manually.
original_issue_dropdown_options = EE::FormHelper.instance_method(:issue_assignees_dropdown_options)
allow_any_instance_of(EE::FormHelper).to receive(:issue_assignees_dropdown_options).and_wrap_original do |original, *args|
original_issue_dropdown_options = FormHelper.instance_method(:assignees_dropdown_options)
allow_any_instance_of(FormHelper).to receive(:assignees_dropdown_options).and_wrap_original do |original, *args|
options = original_issue_dropdown_options.bind(original.receiver).call(*args)
options[:data][:per_page] = 2
......
# frozen_string_literal: true
require 'spec_helper'
describe 'Merge request > User creates MR with multiple assignees' do
include_context 'merge request create context'
before do
stub_licensed_features(multiple_merge_request_assignees: true)
end
it_behaves_like 'multiple assignees merge request', 'creates', 'Submit merge request'
end
# frozen_string_literal: true
require 'spec_helper'
describe 'Merge request > User edits MR with multiple assignees' do
include_context 'merge request edit context'
before do
stub_licensed_features(multiple_merge_request_assignees: true)
end
it_behaves_like 'multiple assignees merge request', 'updates', 'Save changes'
end
......@@ -12,12 +12,13 @@ describe Notify do
set(:user) { create(:user) }
set(:current_user) { create(:user, email: "current@email.com") }
set(:assignee) { create(:user, email: 'assignee@example.com', name: 'John Doe') }
set(:assignee2) { create(:user, email: 'assignee2@example.com', name: 'Jane Doe') }
set(:merge_request) do
create(:merge_request, source_project: project,
target_project: project,
author: current_user,
assignee: assignee,
assignees: [assignee, assignee2],
description: 'Awesome description')
end
......@@ -85,9 +86,7 @@ describe Notify do
end
subject do
described_class.new_merge_request_email(
merge_request.assignee_id, merge_request.id
)
described_class.new_merge_request_email(assignee.id, merge_request.id)
end
it "contains the approvers list" do
......@@ -100,7 +99,7 @@ describe Notify do
subject { described_class.approved_merge_request_email(recipient.id, merge_request.id, last_approver.id) }
before do
merge_request.approvals.create(user: merge_request.assignee)
merge_request.approvals.create(user: assignee)
merge_request.approvals.create(user: last_approver)
end
......@@ -130,13 +129,20 @@ describe Notify do
end
it 'contains the names of all of the approvers' do
is_expected.to have_body_text /#{merge_request.assignee.name}/
is_expected.to have_body_text /#{last_approver.name}/
merge_request.approvals.each do |approval|
is_expected.to have_body_text /#{approval.user.name}/
end
end
it 'contains the names of all assignees' do
merge_request.assignees.each do |assignee|
is_expected.to have_body_text /#{assignee.name}/
end
end
context 'when merge request has no assignee' do
before do
merge_request.update(assignee: nil)
merge_request.update(assignees: [])
end
it 'does not show the assignee' do
......@@ -150,7 +156,7 @@ describe Notify do
subject { described_class.unapproved_merge_request_email(recipient.id, merge_request.id, last_unapprover.id) }
before do
merge_request.approvals.create(user: merge_request.assignee)
merge_request.approvals.create(user: assignee)
end
it_behaves_like 'a multiple recipients email'
......@@ -179,7 +185,15 @@ describe Notify do
end
it 'contains the names of all of the approvers' do
is_expected.to have_body_text /#{merge_request.assignee.name}/
merge_request.approvals.each do |approval|
is_expected.to have_body_text /#{approval.user.name}/
end
end
it 'contains the names of all assignees' do
merge_request.assignees.each do |assignee|
is_expected.to have_body_text /#{assignee.name}/
end
end
end
end
......@@ -190,7 +204,7 @@ describe Notify do
subject { described_class.unapproved_merge_request_email(recipient.id, merge_request_without_assignee.id, last_unapprover.id) }
before do
merge_request_without_assignee.approvals.create(user: merge_request_without_assignee.assignee)
merge_request_without_assignee.approvals.create(user: merge_request_without_assignee.assignees.first)
end
it 'contains the new status' do
......
......@@ -24,6 +24,33 @@ describe MergeRequest do
let(:set_mentionable_text) { ->(txt) { subject.description = txt } }
end
describe '#allows_multiple_assignees?' do
it 'does not allow multiple assignees without license' do
stub_licensed_features(multiple_merge_request_assignees: false)
merge_request = build_stubbed(:merge_request)
expect(merge_request.allows_multiple_assignees?).to be(false)
end
it 'allows multiple assignees when licensed' do
stub_licensed_features(multiple_merge_request_assignees: true)
merge_request = build(:merge_request)
expect(merge_request.allows_multiple_assignees?).to be(true)
end
it 'does not allows multiple assignees when licensed and feature flag is disabled' do
stub_licensed_features(multiple_merge_request_assignees: true)
stub_feature_flags(multiple_merge_request_assignees: false)
merge_request = build(:merge_request)
expect(merge_request.allows_multiple_assignees?).to be(false)
end
end
describe 'approval_rules' do
context 'when project contains approval_rules' do
let!(:project_rule1) { project.approval_rules.create(name: 'p1') }
......
......@@ -3,9 +3,10 @@ require 'spec_helper'
describe API::MergeRequestApprovals do
set(:user) { create(:user) }
set(:user2) { create(:user) }
set(:user3) { create(:user) }
set(:admin) { create(:user, :admin) }
set(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
set(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: Time.now) }
set(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user, user3], source_project: project, target_project: project, title: "Test", created_at: Time.now) }
before do
project.update_attribute(:approvals_before_merge, 2)
......@@ -394,7 +395,7 @@ describe "API::MergeRequestApprovals with approval_rule enabled" do
set(:user2) { create(:user) }
set(:admin) { create(:user, :admin) }
set(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
set(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: Time.now) }
set(:merge_request) { create(:merge_request, :simple, author: user, assignees: [user], source_project: project, target_project: project, title: "Test", created_at: Time.now) }
set(:approver) { create :user }
set(:group) { create :group }
......
......@@ -5,10 +5,11 @@ describe API::MergeRequests do
let(:base_time) { Time.now }
let(:user) { create(:user) }
let(:user2) { create(:user) }
let!(:project) { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
let(:milestone) { create(:milestone, title: '1.0.0', project: project) }
let(:milestone1) { create(:milestone, title: '0.9', project: project) }
let!(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
let!(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignees: [user, user2], source_project: project, target_project: project, title: "Test", created_at: base_time) }
let!(:label) do
create(:label, title: 'label', color: '#FFAABB', project: project)
end
......@@ -19,11 +20,49 @@ describe API::MergeRequests do
end
describe 'PUT /projects/:id/merge_requests' do
context 'when updating existing approval rules' do
def update_merge_request(params)
put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user), params: params
end
context 'multiple assignees' do
let(:other_user) { create(:user) }
let(:params) do
{ assignee_ids: [user.id, other_user.id] }
end
context 'when licensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: true)
end
it 'creates merge request with multiple assignees' do
update_merge_request(params)
expect(response).to have_gitlab_http_status(200)
expect(json_response['assignees'].size).to eq(2)
expect(json_response['assignees'].first['name']).to eq(user.name)
expect(json_response['assignees'].second['name']).to eq(other_user.name)
expect(json_response.dig('assignee', 'name')).to eq(user.name)
end
end
context 'when not licensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
it 'creates merge request with a single assignee' do
update_merge_request(params)
expect(response).to have_gitlab_http_status(200)
expect(json_response['assignees'].size).to eq(1)
expect(json_response['assignees'].first['name']).to eq(user.name)
expect(json_response.dig('assignee', 'name')).to eq(user.name)
end
end
end
context 'when updating existing approval rules' do
let!(:rule) { create(:approval_merge_request_rule, merge_request: merge_request, approvals_required: 1) }
it 'is successful' do
......@@ -57,6 +96,40 @@ describe API::MergeRequests do
defaults = defaults.merge(args)
post api("/projects/#{project.id}/merge_requests", user), params: defaults
end
context 'multiple assignees' do
context 'when licensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: true)
end
it 'creates merge request with multiple assignees' do
create_merge_request(assignee_ids: [user.id, user2.id])
expect(response).to have_gitlab_http_status(201)
expect(json_response['assignees'].size).to eq(2)
expect(json_response['assignees'].first['name']).to eq(user.name)
expect(json_response['assignees'].second['name']).to eq(user2.name)
expect(json_response.dig('assignee', 'name')).to eq(user.name)
end
end
context 'when not licensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
it 'creates merge request with a single assignee' do
create_merge_request(assignee_ids: [user.id, user2.id])
expect(response).to have_gitlab_http_status(201)
expect(json_response['assignees'].size).to eq(1)
expect(json_response['assignees'].first['name']).to eq(user.name)
expect(json_response.dig('assignee', 'name')).to eq(user.name)
end
end
end
context 'between branches projects' do
it "returns merge_request" do
create_merge_request(squash: true)
......@@ -142,9 +215,19 @@ describe API::MergeRequests do
create(:merge_request_with_approver, :simple, author: user, source_project: project, target_project: project, source_branch: 'other-branch')
end
let(:another_user) {}
context 'filter merge requests by assignee ID' do
let!(:merge_request2) do
create(:merge_request, :simple, assignees: [user2], source_project: project, target_project: project, source_branch: 'other-branch-2')
end
it 'returns merge requests with given assignee ID' do
get api('/merge_requests', user), params: { assignee_id: user2.id }
expect_response_contain_exactly(merge_request, merge_request2)
end
end
context 'request merge requests' do
context 'filter merge requests by approver IDs' do
before do
get api('/merge_requests', user), params: { approver_ids: approvers_param, scope: :all }
end
......
......@@ -215,11 +215,12 @@ describe API::V3::Github do
describe 'repo pulls' do
let(:assignee) { create(:user) }
let(:assignee2) { create(:user) }
let!(:merge_request) do
create(:merge_request, source_project: project, target_project: project, author: user, assignee: assignee)
create(:merge_request, source_project: project, target_project: project, author: user, assignees: [assignee])
end
let!(:merge_request_2) do
create(:merge_request, source_project: project2, target_project: project2, author: user, assignee: assignee)
create(:merge_request, source_project: project2, target_project: project2, author: user, assignees: [assignee, assignee2])
end
describe 'GET /-/jira/pulls' do
......
......@@ -168,10 +168,15 @@ describe DraftNotes::PublishService do
context 'with quick actions' do
it 'performs quick actions' do
create(:draft_note, merge_request: merge_request, author: user, note: "thanks\n/assign #{user.to_reference}")
other_user = create(:user)
project.add_developer(other_user)
create(:draft_note, merge_request: merge_request,
author: user,
note: "thanks\n/assign #{user.to_reference} #{other_user.to_reference}")
expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(2)
expect(merge_request.reload.assignee).to eq(user)
expect(merge_request.reload.assignees).to match_array([user, other_user])
expect(merge_request.notes.last).to be_system
end
......@@ -179,7 +184,7 @@ describe DraftNotes::PublishService do
create(:draft_note, merge_request: merge_request, author: user, note: "/assign #{user.to_reference}")
expect { publish }.to change { DraftNote.count }.by(-1).and change { Note.count }.by(1)
expect(merge_request.reload.assignee).to eq(user)
expect(merge_request.reload.assignees).to eq([user])
expect(merge_request.notes.last).to be_system
end
end
......
......@@ -124,8 +124,9 @@ describe EE::NotificationService, :mailer do
context 'new review' do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:reviewer) { create(:user) }
let(:merge_request) { create(:merge_request, source_project: project, assignee: user, author: create(:user)) }
let(:merge_request) { create(:merge_request, source_project: project, assignees: [user, user2], author: create(:user)) }
let(:review) { create(:review, merge_request: merge_request, project: project, author: reviewer) }
let(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, author: reviewer, review: review) }
......@@ -133,7 +134,7 @@ describe EE::NotificationService, :mailer do
build_team(review.project, merge_request)
project.add_maintainer(merge_request.author)
project.add_maintainer(reviewer)
project.add_maintainer(merge_request.assignee)
merge_request.assignees.each { |assignee| project.add_maintainer(assignee) }
create(:diff_note_on_merge_request,
project: project,
......@@ -146,7 +147,9 @@ describe EE::NotificationService, :mailer do
it 'sends emails' do
expect(Notify).not_to receive(:new_review_email).with(review.author.id, review.id)
expect(Notify).not_to receive(:new_review_email).with(@unsubscriber.id, review.id)
expect(Notify).to receive(:new_review_email).with(merge_request.assignee.id, review.id).and_call_original
merge_request.assignee_ids.each do |assignee_id|
expect(Notify).to receive(:new_review_email).with(assignee_id, review.id).and_call_original
end
expect(Notify).to receive(:new_review_email).with(merge_request.author.id, review.id).and_call_original
expect(Notify).to receive(:new_review_email).with(@u_watcher.id, review.id).and_call_original
expect(Notify).to receive(:new_review_email).with(@u_mentioned.id, review.id).and_call_original
......@@ -513,10 +516,11 @@ describe EE::NotificationService, :mailer do
context 'Merge Requests' do
let(:notification) { NotificationService.new }
let(:assignee) { create(:user) }
let(:assignee2) { create(:user) }
let(:group) { create(:group) }
let(:project) { create(:project, :public, :repository, namespace: group) }
let(:another_project) { create(:project, :public, namespace: group) }
let(:merge_request) { create :merge_request, source_project: project, assignee: create(:user), description: 'cc @participant' }
let(:merge_request) { create :merge_request, source_project: project, assignees: [assignee, assignee2], description: 'cc @participant' }
around do |example|
perform_enqueued_jobs do
......@@ -528,7 +532,7 @@ describe EE::NotificationService, :mailer do
stub_feature_flags(approval_rules: false)
project.add_maintainer(merge_request.author)
project.add_maintainer(merge_request.assignee)
merge_request.assignees.each { |assignee| project.add_maintainer(assignee) }
build_team(merge_request.target_project)
add_users_with_subscription(merge_request.target_project, merge_request)
update_custom_notification(:new_merge_request, @u_guest_custom, resource: project)
......@@ -537,6 +541,12 @@ describe EE::NotificationService, :mailer do
end
describe '#new_merge_request' do
it 'emails all assignees' do
notification.new_merge_request(merge_request, assignee)
merge_request.assignees.each { |assignee| should_email(assignee) }
end
context 'when the target project has approvers set' do
let(:project_approvers) { create_list(:user, 3) }
......
......@@ -11,7 +11,8 @@ describe QuickActions::InterpretService do
let(:service) { described_class.new(project, current_user) }
before do
stub_licensed_features(multiple_issue_assignees: true)
stub_licensed_features(multiple_issue_assignees: true,
multiple_merge_request_assignees: true)
project.add_developer(current_user)
end
......@@ -39,6 +40,42 @@ describe QuickActions::InterpretService do
end
end
end
context 'Merge Request' do
let(:merge_request) { create(:merge_request, source_project: project) }
it 'fetches assignees and populates them if content contains /assign' do
merge_request.update(assignee_ids: [user.id])
_, updates = service.execute("/assign @#{user2.username}", merge_request)
expect(updates[:assignee_ids]).to match_array([user.id, user2.id])
end
context 'assign command with multiple assignees' do
it 'fetches assignee and populates assignee_ids if content contains /assign' do
merge_request.update(assignee_ids: [user.id])
_, updates = service.execute("/assign @#{user.username}\n/assign @#{user2.username} @#{user3.username}", issue)
expect(updates[:assignee_ids]).to match_array([user.id, user2.id, user3.id])
end
context 'unlicensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
it 'does not recognize /assign with multiple user references' do
merge_request.update(assignee_ids: [user.id])
_, updates = service.execute("/assign @#{user2.username} @#{user3.username}", merge_request)
expect(updates[:assignee_ids]).to match_array([user2.id])
end
end
end
end
end
context 'unassign command' do
......@@ -69,6 +106,42 @@ describe QuickActions::InterpretService do
expect(updates[:assignee_ids]).to be_empty
end
end
context 'Merge Request' do
let(:merge_request) { create(:merge_request, source_project: project) }
it 'unassigns user if content contains /unassign @user' do
merge_request.update(assignee_ids: [user.id, user2.id])
_, updates = service.execute("/unassign @#{user2.username}", merge_request)
expect(updates[:assignee_ids]).to match_array([user.id])
end
context 'unassign command with multiple assignees' do
it 'unassigns both users if content contains /unassign @user @user1' do
merge_request.update(assignee_ids: [user.id, user2.id, user3.id])
_, updates = service.execute("/unassign @#{user.username} @#{user2.username}", merge_request)
expect(updates[:assignee_ids]).to match_array([user3.id])
end
context 'unlicensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
it 'does not recognize /unassign @user' do
merge_request.update(assignee_ids: [user.id, user2.id, user3.id])
_, updates = service.execute("/unassign @#{user.username}", merge_request)
expect(updates[:assignee_ids]).to be_empty
end
end
end
end
end
context 'reassign command' do
......@@ -77,6 +150,11 @@ describe QuickActions::InterpretService do
context 'Merge Request' do
let(:merge_request) { create(:merge_request, source_project: project) }
context 'unlicensed' do
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
it 'does not recognize /reassign @user' do
_, updates = service.execute(content, merge_request)
......@@ -84,6 +162,13 @@ describe QuickActions::InterpretService do
end
end
it 'reassigns user if content contains /reassign @user' do
_, updates = service.execute("/reassign @#{current_user.username}", merge_request)
expect(updates[:assignee_ids]).to match_array([current_user.id])
end
end
context 'Issue' do
let(:content) { "/reassign @#{current_user.username}" }
......
......@@ -663,7 +663,11 @@ module API
expose(:user_notes_count) { |merge_request, options| issuable_metadata(merge_request, options, :user_notes_count) }
expose(:upvotes) { |merge_request, options| issuable_metadata(merge_request, options, :upvotes) }
expose(:downvotes) { |merge_request, options| issuable_metadata(merge_request, options, :downvotes) }
expose :author, :assignee, using: Entities::UserBasic
expose :assignee, using: ::API::Entities::UserBasic do |merge_request|
merge_request.assignee
end
expose :author, :assignees, using: Entities::UserBasic
expose :source_project_id, :target_project_id
expose :labels do |merge_request|
# Avoids an N+1 query since labels are preloaded
......
......@@ -20,6 +20,7 @@ module API
def self.update_params_at_least_one_of
%i[
assignee_id
assignee_ids
description
labels
milestone_id
......@@ -186,6 +187,7 @@ module API
params :optional_params do
optional :description, type: String, desc: 'The description of the merge request'
optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request'
optional :assignee_ids, type: Array[Integer], desc: 'The array of user IDs to assign issue'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
optional :labels, type: Array[String], coerce_with: Validations::Types::LabelsList.coerce, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
......@@ -233,6 +235,7 @@ module API
mr_params = declared_params(include_missing: false)
mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch)
mr_params = convert_parameters_from_legacy_format(mr_params)
merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute
......@@ -336,6 +339,7 @@ module API
mr_params = declared_params(include_missing: false)
mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
mr_params = convert_parameters_from_legacy_format(mr_params)
merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
......
......@@ -10,7 +10,7 @@ module Banzai
nodes,
MergeRequest.includes(
:author,
:assignee,
:assignees,
{
# These associations are primarily used for checking permissions.
# Eager loading these ensures we don't end up running dozens of
......
......@@ -20,11 +20,7 @@ module Gitlab
repository: issuable.project.hook_attrs.slice(:name, :url, :description, :homepage)
}
if issuable.is_a?(Issue)
hook_data[:assignees] = issuable.assignees.map(&:hook_attrs) if issuable.assignees.any?
else
hook_data[:assignee] = issuable.assignee.hook_attrs if issuable.assignee
end
hook_data
end
......
......@@ -34,7 +34,6 @@ module Gitlab
end
SAFE_HOOK_RELATIONS = %i[
assignee
labels
total_time_spent
].freeze
......@@ -51,7 +50,9 @@ module Gitlab
work_in_progress: merge_request.work_in_progress?,
total_time_spent: merge_request.total_time_spent,
human_total_time_spent: merge_request.human_total_time_spent,
human_time_estimate: merge_request.human_time_estimate
human_time_estimate: merge_request.human_time_estimate,
assignee_ids: merge_request.assignee_ids,
assignee_id: merge_request.assignee_ids.first # This key is deprecated
}
merge_request.attributes.with_indifferent_access.slice(*self.class.safe_hook_attributes)
......
......@@ -1338,9 +1338,6 @@ msgstr ""
msgid "Assigned Merge Requests"
msgstr ""
msgid "Assigned to :name"
msgstr ""
msgid "Assigned to me"
msgstr ""
......@@ -7354,9 +7351,6 @@ msgstr ""
msgid "No activities found"
msgstr ""
msgid "No assignee"
msgstr ""
msgid "No branches found"
msgstr ""
......@@ -7468,9 +7462,6 @@ msgstr ""
msgid "None"
msgstr ""
msgid "Not allowed to merge"
msgstr ""
msgid "Not available"
msgstr ""
......@@ -9692,9 +9683,6 @@ msgstr ""
msgid "Select an existing Kubernetes cluster or create a new one"
msgstr ""
msgid "Select assignee"
msgstr ""
msgid "Select branch/tag"
msgstr ""
......@@ -12846,9 +12834,6 @@ msgstr ""
msgid "among other things"
msgstr ""
msgid "assign yourself"
msgstr ""
msgid "at"
msgstr ""
......
......@@ -26,7 +26,7 @@ module QA
element :issuable_label
end
view 'app/views/shared/issuable/form/_metadata_merge_request_assignee.html.haml' do
view 'app/views/shared/issuable/form/_metadata_issuable_assignee.html.haml' do
element :assign_to_me_link
end
......
......@@ -238,11 +238,11 @@ describe Projects::MergeRequestsController do
assignee = create(:user)
project.add_developer(assignee)
update_merge_request({ assignee_id: assignee.id }, format: :json)
update_merge_request({ assignee_ids: [assignee.id] }, format: :json)
body = JSON.parse(response.body)
expect(body['assignee'].keys)
.to match_array(%w(name username avatar_url id state web_url))
expect(body['assignees']).to all(include(*%w(name username avatar_url id state web_url)))
end
end
......
......@@ -8,7 +8,7 @@ describe 'Navigation bar counter', :use_clean_rails_memory_store_caching do
before do
issue.assignees = [user]
merge_request.update(assignee: user)
merge_request.update(assignees: [user])
sign_in(user)
end
......@@ -33,7 +33,7 @@ describe 'Navigation bar counter', :use_clean_rails_memory_store_caching do
expect_counters('merge_requests', '1')
merge_request.update(assignee: nil)
merge_request.update(assignees: [])
user.invalidate_cache_counts
......
......@@ -50,14 +50,14 @@ describe 'Dashboard Merge Requests' do
let!(:assigned_merge_request) do
create(:merge_request,
assignee: current_user,
assignees: [current_user],
source_project: project,
author: create(:user))
end
let!(:assigned_merge_request_from_fork) do
create(:merge_request,
source_branch: 'markdown', assignee: current_user,
source_branch: 'markdown', assignees: [current_user],
target_project: public_project, source_project: forked_project,
author: create(:user))
end
......
......@@ -38,7 +38,7 @@ describe 'Group merge requests page' do
context 'when merge request assignee to user' do
before do
issuable.update!(assignee: user)
issuable.update!(assignees: [user])
visit path
end
......
......@@ -32,8 +32,8 @@ describe 'New/edit issue', :js do
# the original method, resulting in infinite recursion when called.
# This is likely a bug with helper modules included into dynamically generated view classes.
# To work around this, we have to hold on to and call to the original implementation manually.
original_issue_dropdown_options = EE::FormHelper.instance_method(:issue_assignees_dropdown_options)
allow_any_instance_of(EE::FormHelper).to receive(:issue_assignees_dropdown_options).and_wrap_original do |original, *args|
original_issue_dropdown_options = FormHelper.instance_method(:assignees_dropdown_options)
allow_any_instance_of(FormHelper).to receive(:assignees_dropdown_options).and_wrap_original do |original, *args|
options = original_issue_dropdown_options.bind(original.receiver).call(*args)
options[:data][:per_page] = 2
......
......@@ -68,15 +68,15 @@ describe "User creates a merge request", :js do
fill_in("Title", with: title)
end
click_button("Assignee")
expect(find(".js-assignee-search")["data-project-id"]).to eq(project.id.to_s)
find('.js-assignee-search').click
page.within(".dropdown-menu-user") do
expect(page).to have_content("Unassigned")
.and have_content(user.name)
.and have_content(project.users.first.name)
end
find('.js-assignee-search').click
click_button("Submit merge request")
......
require 'rails_helper'
describe 'Merge request > User creates MR' do
include ProjectForksHelper
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
context 'non-fork merge request' do
include_context 'merge request create context'
it_behaves_like 'a creatable merge request'
end
context 'from a forked project' do
include ProjectForksHelper
let(:canonical_project) { create(:project, :public, :repository) }
let(:source_project) do
......@@ -15,6 +22,7 @@ describe 'Merge request > User creates MR' do
end
context 'to canonical project' do
include_context 'merge request create context'
it_behaves_like 'a creatable merge request'
end
......@@ -25,6 +33,7 @@ describe 'Merge request > User creates MR' do
namespace: user.namespace)
end
include_context 'merge request create context'
it_behaves_like 'a creatable merge request'
end
end
......
require 'rails_helper'
require 'spec_helper'
describe 'Merge request > User edits MR' do
include ProjectForksHelper
before do
stub_licensed_features(multiple_merge_request_assignees: false)
end
context 'non-fork merge request' do
include_context 'merge request edit context'
it_behaves_like 'an editable merge request'
end
context 'for a forked project' do
it_behaves_like 'an editable merge request' do
let(:source_project) { fork_project(target_project, nil, repository: true) }
end
include_context 'merge request edit context'
it_behaves_like 'an editable merge request'
end
end
......@@ -7,7 +7,7 @@ describe 'Merge Requests > User filters by assignees', :js do
let(:user) { project.creator }
before do
create(:merge_request, assignee: user, title: 'Bugfix1', source_project: project, target_project: project, source_branch: 'bugfix1')
create(:merge_request, assignees: [user], title: 'Bugfix1', source_project: project, target_project: project, source_branch: 'bugfix1')
create(:merge_request, title: 'Bugfix2', source_project: project, target_project: project, source_branch: 'bugfix2')
sign_in(user)
......
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
This diff is collapsed.
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