Commit 12ca2cf0 authored by Clement Ho's avatar Clement Ho

Merge branch 'multiple_assignees_review' into 'multiple-assignees-fe-sidebar'

# Conflicts:
#   app/assets/stylesheets/framework/avatar.scss
parents afbda11c f7674c73
......@@ -101,6 +101,6 @@
border-radius: 1em;
font-family: $regular_font;
font-size: 9px;
line-height: 17px;
line-height: 16px;
text-align: center;
}
}
\ No newline at end of file
......@@ -261,6 +261,7 @@ ul.controls {
.avatar-inline {
margin-left: 0;
margin-right: 0;
margin-bottom: 0;
}
}
}
......
......@@ -575,6 +575,16 @@
vertical-align: text-top;
}
}
.avatar-counter {
display: inline-block;
vertical-align: middle;
min-width: 16px;
line-height: 14px;
height: 16px;
padding-left: 2px;
padding-right: 2px;
}
}
}
......
......@@ -40,7 +40,7 @@ module IssuableCollections
end
def issues_collection
issues_finder.execute.preload(:project, :author, :labels, :milestone, project: :namespace)
issues_finder.execute.preload(:project, :author, :assignees, :labels, :milestone, project: :namespace)
end
def merge_requests_collection
......
......@@ -17,7 +17,14 @@ module Elastic
indexes :state, type: :text
indexes :project_id, type: :integer
indexes :author_id, type: :integer
# The field assignee_id does not exist in issues table anymore.
# Nevertheless we'll keep this field as is because we don't want users to rebuild index
# + the ES treats arrays transparently so
# to any integer field you can write any array of integers and you don't have to change mapping.
# More over you can query those items just like a single integer value.
indexes :assignee_id, type: :integer
indexes :confidential, type: :boolean
end
......@@ -26,10 +33,12 @@ module Elastic
# We don't use as_json(only: ...) because it calls all virtual and serialized attributtes
# https://gitlab.com/gitlab-org/gitlab-ee/issues/349
[:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id, :assignee_id, :confidential].each do |attr|
[:id, :iid, :title, :description, :created_at, :updated_at, :state, :project_id, :author_id, :confidential].each do |attr|
data[attr.to_s] = safely_read_attribute_for_elasticsearch(attr)
end
data['assignee_id'] = safely_read_attribute_for_elasticsearch(:assignee_ids)
data
end
......
......@@ -34,7 +34,7 @@ module Elastic
if noteable.is_a?(Issue)
data['issue'] = {
assignee_id: noteable.assignee_id,
assignee_id: noteable.assignee_ids,
author_id: noteable.author_id,
confidential: noteable.confidential
}
......
......@@ -136,14 +136,6 @@ class Issue < ActiveRecord::Base
"id DESC")
end
def update_assignee_cache_counts
return true # TODO implement it properly
# make sure we flush the cache for both the old *and* new assignees(if they exist)
previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was
previous_assignee&.update_cache_counts
assignee&.update_cache_counts
end
# Returns a Hash of attributes to be used for Twitter card metadata
def card_attributes
{
......
class IssueAssignee < ActiveRecord::Base
belongs_to :issue
belongs_to :assignee, class_name: "User", foreign_key: :user_id
end
\ No newline at end of file
extend Gitlab::CurrentSettings
belongs_to :issue
belongs_to :assignee, class_name: "User", foreign_key: :user_id
after_create :update_assignee_cache_counts
after_destroy :update_assignee_cache_counts
# EE-specific
after_create :update_elasticsearch_index
after_destroy :update_elasticsearch_index
# EE-specific
def update_assignee_cache_counts
assignee&.update_cache_counts
end
def update_elasticsearch_index
if current_application_settings.elasticsearch_indexing?
ElasticIndexerWorker.perform_async(
:update,
'Issue',
issue.id,
changed_fields: ['assignee_ids']
)
end
end
end
......@@ -189,10 +189,6 @@ class MergeRequest < ActiveRecord::Base
work_in_progress?(title) ? title : "WIP: #{title}"
end
def is_being_reassigned?
assignee_id_changed?
end
def update_assignee_cache_counts
# make sure we flush the cache for both the old *and* new assignees(if they exist)
previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was
......@@ -210,7 +206,7 @@ class MergeRequest < ActiveRecord::Base
# This method is needed for compatibility with issues to not mess view and other code
def assignees
assignee ? [assignee] : []
Array(assignee)
end
def assignee_or_author?(user)
......
......@@ -7,10 +7,14 @@ module Issuable
ids = params.delete(:issuable_ids).split(",")
items = model_class.where(id: ids)
%i(state_event milestone_id assignee_id add_label_ids remove_label_ids subscription_event).each do |key|
%i(state_event milestone_id assignee_id assignee_ids add_label_ids remove_label_ids subscription_event).each do |key|
params.delete(key) unless params[key].present?
end
if params[:assignee_ids] == [IssuableFinder::NONE.to_s]
params[:assignee_ids] = []
end
items.each do |issuable|
next unless can?(current_user, :"update_#{type}", issuable)
......
......@@ -14,7 +14,7 @@ module MergeRequests
def execute
assignable_issues.each do |issue|
Issues::UpdateService.new(issue.project, current_user, assignee_ids: current_user.id.to_s).execute(issue)
Issues::UpdateService.new(issue.project, current_user, assignee_ids: [current_user.id]).execute(issue)
end
{
......
......@@ -8,7 +8,6 @@ class NotificationRecipientService
@project = project
end
# TODO: refactor this: previous_assignee argument can be a user object or an array which is not really nice
def build_recipients(target, current_user, action: nil, previous_assignee: nil, skip_current_user: true)
custom_action = build_custom_key(action, target)
......@@ -29,7 +28,8 @@ class NotificationRecipientService
recipients << previous_assignee if previous_assignee
recipients << target.assignee
when :reassign_issue
recipients.concat(previous_assignee) if previous_assignee.any?
previous_assignees = Array(previous_assignee)
recipients.concat(previous_assignees) if previous_assignees.any?
recipients.concat(target.assignees)
end
......
......@@ -89,17 +89,27 @@ module SlashCommands
user = extract_references(assignee_param, :user).first
user ||= User.find_by(username: assignee_param)
@updates[:assignee_id] = user.id if user
next unless user
if issuable.is_a?(Issue)
@updates[:assignee_ids] = [user.id]
else
@updates[:assignee_id] = user.id
end
end
desc 'Remove assignee'
condition do
issuable.persisted? &&
issuable.assignee_id? &&
issuable.assignees.any? &&
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
end
command :unassign do
@updates[:assignee_id] = nil
if issuable.is_a?(Issue)
@updates[:assignee_ids] = []
else
@updates[:assignee_id] = nil
end
end
desc 'Set milestone'
......
......@@ -68,13 +68,16 @@ module SystemNoteService
#
# Returns the created Note object
def change_issue_assignees(issue, project, author, old_assignees)
# TODO: basic implementation, should be improved before merging the MR
body =
if issue.assignees.any? && old_assignees.any?
unassigned_users = old_assignees - issue.assignees
added_users = issue.assignees.to_a - old_assignees
"assigned to #{added_users.map(&:to_reference).to_sentence} and unassigned #{unassigned_users.map(&:to_reference).to_sentence}"
text_parts = []
text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any?
text_parts << "unassigned #{unassigned_users.map(&:to_reference).to_sentence}" if unassigned_users.any?
text_parts.join(' and ')
elsif old_assignees.any?
"removed all assignees"
elsif issue.assignees.any?
......@@ -84,7 +87,6 @@ module SystemNoteService
create_note(noteable: issue, project: project, author: author, note: body)
end
# Called when one or more labels on a Noteable are added and/or removed
#
# noteable - Noteable object
......
......@@ -270,7 +270,7 @@ class TodoService
end
end
def create_mention_todos(project, target, author, note = nil)
def create_mention_todos(project, target, author, note = nil)
# Create Todos for directly addressed users
directly_addressed_users = filter_directly_addressed_users(project, note || target, author)
attributes = attributes_for_todo(project, target, author, Todo::DIRECTLY_ADDRESSED, note)
......
......@@ -26,8 +26,8 @@ xml.entry do
if issue.assignees.any?
xml.assignees do
issue.assignees.each do |assignee|
xml.name ssignee.name
xml.email assignee.assignee_public_email
xml.name assignee.name
xml.email assignee.public_email
end
end
end
......
......@@ -15,8 +15,7 @@
- if issue.assignees.any?
%li
- issue.assignees.each do |assignee|
= link_to_member(@project, assignee, name: false, title: "Assigned to :name")
= render 'shared/issuable/assignees', project: @project, issue: issue
= render 'shared/issuable_meta_data', issuable: issue
......
- max_render = 3
- max = [max_render, issue.assignees.length].min
- issue.assignees.each_with_index do |assignee, index|
- if index < max
= link_to_member(@project, assignee, name: false, title: "Assigned to :name")
- if issue.assignees.length > max_render
- counter = issue.assignees.length - max_render
%span{ class: 'avatar-counter has-tooltip', data: { container: 'body', placement: 'bottom', 'line-type' => 'old', 'original-title' => "+#{counter} more assignees" } }
- if counter < 99
= "+#{counter}"
- else
99+
......@@ -5,7 +5,7 @@ class ElasticIndexerWorker
sidekiq_options queue: :elasticsearch, retry: 2
ISSUE_TRACKED_FIELDS = %w(assignee_id author_id confidential).freeze
ISSUE_TRACKED_FIELDS = %w(assignee_ids author_id confidential).freeze
def perform(operation, class_name, record_id, options = {})
return true unless current_application_settings.elasticsearch_indexing?
......
......@@ -4,6 +4,8 @@
class CreateIssueAssigneesTable < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
INDEX_NAME = 'index_issue_assignees_on_issue_id_and_user_id'
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
......@@ -23,12 +25,20 @@ class CreateIssueAssigneesTable < ActiveRecord::Migration
# comments:
# disable_ddl_transaction!
def change
def up
create_table :issue_assignees do |t|
t.references :user, foreign_key: { on_delete: :cascade }, index: true, null: false
t.references :issue, foreign_key: { on_delete: :cascade }, null: false
end
add_index :issue_assignees, [:issue_id, :user_id], unique: true
add_index :issue_assignees, [:issue_id, :user_id], unique: true, name: INDEX_NAME
end
def down
if index_exists?(:issue_assignees, name: INDEX_NAME)
remove_index :issue_assignees, name: INDEX_NAME
end
drop_table :issue_assignees
end
end
......@@ -279,7 +279,7 @@ module API
class IssueBasic < ProjectEntity
expose :label_names, as: :labels
expose :milestone, using: Entities::Milestone
expose :assignee, :author, using: Entities::UserBasic
expose :assignees, :author, using: Entities::UserBasic
expose :user_notes_count
expose :upvotes, :downvotes
......
......@@ -31,7 +31,7 @@ module API
params :issue_params do
optional :description, type: String, desc: 'The description of an issue'
optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue'
optional :assignee_ids, type: Array[Integer], desc: 'The ID of a user to assign issue'
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue'
optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :due_date, type: String, desc: 'Date string in the format YEAR-MONTH-DAY'
......@@ -156,7 +156,7 @@ module API
desc: 'Date time when the issue was updated. Available only for admins and project owners.'
optional :state_event, type: String, values: %w[reopen close], desc: 'State of the issue'
use :issue_params
at_least_one_of :title, :description, :assignee_id, :milestone_id,
at_least_one_of :title, :description, :assignee_ids, :milestone_id,
:labels, :created_at, :due_date, :confidential, :state_event,
:weight
end
......
......@@ -22,7 +22,7 @@ module Gitlab
[
{
title: "Assignee",
value: @resource.assignee ? @resource.assignee.name : "_None_",
value: @resource.assignees.any? ? @resource.assignees.first.name : "_None_",
short: true
},
{
......
......@@ -122,15 +122,15 @@ module Gitlab
author_id = user_info(bug['ixPersonOpenedBy'])[:gitlab_id] || project.creator_id
issue = Issue.create!(
iid: bug['ixBug'],
project_id: project.id,
title: bug['sTitle'],
description: body,
author_id: author_id,
assignee_id: assignee_id,
state: bug['fOpen'] == 'true' ? 'opened' : 'closed',
created_at: date,
updated_at: DateTime.parse(bug['dtLastUpdated'])
iid: bug['ixBug'],
project_id: project.id,
title: bug['sTitle'],
description: body,
author_id: author_id,
assignee_ids: [assignee_id],
state: bug['fOpen'] == 'true' ? 'opened' : 'closed',
created_at: date,
updated_at: DateTime.parse(bug['dtLastUpdated'])
)
issue_labels = ::LabelsFinder.new(nil, project_id: project.id, title: labels).execute(skip_authorization: true)
......
......@@ -10,7 +10,7 @@ module Gitlab
description: description,
state: state,
author_id: author_id,
assignee_id: assignee_id,
assignee_ids: Array(assignee_id),
created_at: raw_data.created_at,
updated_at: raw_data.updated_at
}
......
......@@ -92,13 +92,13 @@ module Gitlab
end
issue = Issue.create!(
iid: raw_issue['id'],
project_id: project.id,
title: raw_issue['title'],
description: body,
author_id: project.creator_id,
assignee_id: assignee_id,
state: raw_issue['state'] == 'closed' ? 'closed' : 'opened'
iid: raw_issue['id'],
project_id: project.id,
title: raw_issue['title'],
description: body,
author_id: project.creator_id,
assignee_ids: [assignee_id],
state: raw_issue['state'] == 'closed' ? 'closed' : 'opened'
)
issue_labels = ::LabelsFinder.new(nil, project_id: project.id, title: labels).execute(skip_authorization: true)
......
......@@ -41,7 +41,7 @@ describe "Dashboard Issues Feed", feature: true do
expect(entry).to be_present
expect(entry).to have_selector('author email', text: issue2.author_public_email)
expect(entry).to have_selector('assignee email', text: issue2.assignee_public_email)
expect(entry).to have_selector('assignees email', text: issue2.assignees.first.public_email)
expect(entry).not_to have_selector('labels')
expect(entry).not_to have_selector('milestone')
expect(entry).to have_selector('description', text: issue2.description)
......@@ -64,7 +64,7 @@ describe "Dashboard Issues Feed", feature: true do
expect(entry).to be_present
expect(entry).to have_selector('author email', text: issue1.author_public_email)
expect(entry).to have_selector('assignee email', text: issue1.assignee_public_email)
expect(entry).to have_selector('assignees email', text: issue1.assignees.first.public_email)
expect(entry).to have_selector('labels label', text: label1.title)
expect(entry).to have_selector('milestone', text: milestone1.title)
expect(entry).not_to have_selector('description')
......
......@@ -7,7 +7,7 @@ describe 'Awards Emoji', feature: true do
let!(:user) { create(:user) }
let(:issue) do
create(:issue,
assignees: [@user],
assignees: [user],
project: project)
end
......
......@@ -62,7 +62,7 @@ describe 'Issues', feature: true do
expect(page).to have_content 'No assignee - assign yourself'
end
expect(issue.reload.assignee).to be_nil
expect(issue.reload.assignees).to be_empty
end
end
......@@ -363,9 +363,9 @@ describe 'Issues', feature: true do
let(:user2) { create(:user) }
before do
foo.assignee = user2
foo.assignees << user2
foo.save
bar.assignee = user2
bar.assignees << user2
bar.save
end
......@@ -440,7 +440,7 @@ describe 'Issues', feature: true do
expect(page).to have_content 'No assignee'
end
expect(issue.reload.assignee).to be_nil
expect(issue.reload.assignees).to be_empty
end
it 'allows user to select an assignee', js: true do
......@@ -498,7 +498,7 @@ describe 'Issues', feature: true do
login_with guest
visit namespace_project_issue_path(project.namespace, project, issue)
expect(page).to have_content issue.assignee.name
expect(page).to have_content issue.assignees.first.name
end
end
end
......@@ -590,7 +590,7 @@ describe 'Issues', feature: true do
let(:user2) { create(:user) }
before do
issue.assignee = user2
issue.assignees << user2
issue.save
end
end
......
......@@ -40,21 +40,23 @@
"additionalProperties": false
}
},
"assignee": {
"type": ["object", "null"],
"required": [
"id",
"name",
"username",
"avatar_url"
],
"properties": {
"id": { "type": "integer" },
"name": { "type": "string" },
"username": { "type": "string" },
"avatar_url": { "type": "uri" }
},
"additionalProperties": false
"assignees": {
"type": "array",
"items": {
"type": ["object", "null"],
"required": [
"id",
"name",
"username",
"avatar_url"
],
"properties": {
"id": { "type": "integer" },
"name": { "type": "string" },
"username": { "type": "string" },
"avatar_url": { "type": "uri" }
}
}
},
"subscribed": { "type": ["boolean", "null"] }
},
......
......@@ -33,17 +33,20 @@
},
"additionalProperties": false
},
"assignee": {
"type": ["object", "null"],
"properties": {
"name": { "type": "string" },
"username": { "type": "string" },
"id": { "type": "integer" },
"state": { "type": "string" },
"avatar_url": { "type": "uri" },
"web_url": { "type": "uri" }
},
"additionalProperties": false
"assignees": {
"type": "array",
"items": {
"type": ["object", "null"],
"properties": {
"name": { "type": "string" },
"username": { "type": "string" },
"id": { "type": "integer" },
"state": { "type": "string" },
"avatar_url": { "type": "uri" },
"web_url": { "type": "uri" }
},
"additionalProperties": false
}
},
"author": {
"type": "object",
......@@ -68,7 +71,7 @@
"required": [
"id", "iid", "project_id", "title", "description",
"state", "created_at", "updated_at", "labels",
"milestone", "assignee", "author", "user_notes_count",
"milestone", "assignees", "author", "user_notes_count",
"upvotes", "downvotes", "due_date", "confidential",
"web_url", "weight"
],
......
......@@ -103,7 +103,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do
it 'allows references for assignee' do
assignee = create(:user)
project = create(:empty_project, :public)
issue = create(:issue, :confidential, project: project, assignee: assignee)
issue = create(:issue, :confidential, project: project, assignees: [assignee])
link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue')
doc = filter(link, current_user: assignee)
......
......@@ -43,7 +43,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
description: "*Created by: octocat*\n\nI'm having a problem with this.",
state: 'opened',
author_id: project.creator_id,
assignee_id: nil,
assignee_ids: [],
created_at: created_at,
updated_at: updated_at
}
......@@ -64,7 +64,7 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
description: "*Created by: octocat*\n\nI'm having a problem with this.",
state: 'closed',
author_id: project.creator_id,
assignee_id: nil,
assignee_ids: [],
created_at: created_at,
updated_at: updated_at
}
......@@ -77,19 +77,19 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do
let(:raw_data) { double(base_data.merge(assignee: octocat)) }
it 'returns nil as assignee_id when is not a GitLab user' do
expect(issue.attributes.fetch(:assignee_id)).to be_nil
expect(issue.attributes.fetch(:assignee_ids)).to be_empty
end
it 'returns GitLab user id associated with GitHub id as assignee_id' do
gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github')
expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id
expect(issue.attributes.fetch(:assignee_ids)).to eq [gl_user.id]
end
it 'returns GitLab user id associated with GitHub email as assignee_id' do
gl_user = create(:user, email: octocat.email)
expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id
expect(issue.attributes.fetch(:assignee_ids)).to eq [gl_user.id]
end
end
......
......@@ -49,7 +49,7 @@ describe Gitlab::GoogleCodeImport::Importer, lib: true do
expect(issue).not_to be_nil
expect(issue.iid).to eq(169)
expect(issue.author).to eq(project.creator)
expect(issue.assignee).to eq(mapped_user)
expect(issue.assignees).to eq([mapped_user])
expect(issue.state).to eq("closed")
expect(issue.label_names).to include("Priority: Medium")
expect(issue.label_names).to include("Status: Fixed")
......
......@@ -3,7 +3,7 @@ issues:
- subscriptions
- award_emoji
- author
- assignee
- assignees
- updated_by
- milestone
- notes
......@@ -16,6 +16,7 @@ issues:
- merge_requests_closing_issues
- metrics
- timelogs
- issue_assignees
events:
- author
- project
......
......@@ -35,7 +35,9 @@ describe Issue, elastic: true do
expected_hash = issue.attributes.extract!('id', 'iid', 'title', 'description', 'created_at',
'updated_at', 'state', 'project_id', 'author_id',
'assignee_id', 'confidential')
'confidential')
expected_hash['assignee_id'] = []
expect(issue.as_indexed_json).to eq(expected_hash)
end
......
......@@ -7,7 +7,6 @@ describe Issue, "Issuable" do
describe "Associations" do
it { is_expected.to belong_to(:project) }
it { is_expected.to belong_to(:author) }
it { is_expected.to belong_to(:assignee) }
it { is_expected.to have_many(:notes).dependent(:destroy) }
it { is_expected.to have_many(:todos).dependent(:destroy) }
......@@ -58,12 +57,12 @@ describe Issue, "Issuable" do
end
describe "before_save" do
describe "#update_cache_counts" do
describe "#update_cache_counts when an issue is reassigned" do
context "when previous assignee exists" do
before do
assignee = create(:user)
issue.project.team << [assignee, :developer]
issue.update(assignee: assignee)
issue.assignees << assignee
end
it "updates cache counts for new assignee" do
......@@ -71,26 +70,65 @@ describe Issue, "Issuable" do
expect(user).to receive(:update_cache_counts)
issue.update(assignee: user)
issue.assignees << user
end
it "updates cache counts for previous assignee" do
old_assignee = issue.assignee
old_assignee = issue.assignees.first
expect_any_instance_of(User).to receive(:update_cache_counts)
issue.assignees.destroy_all
end
end
context "when previous assignee does not exist" do
before{ issue.assignees = [] }
it "updates cache count for the new assignee" do
expect_any_instance_of(User).to receive(:update_cache_counts)
issue.assignees << user
end
end
end
describe "#update_cache_counts when a merge request is reassigned" do
let(:project) { create :project }
let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
context "when previous assignee exists" do
before do
assignee = create(:user)
project.team << [assignee, :developer]
merge_request.update(assignee: assignee)
end
it "updates cache counts for new assignee" do
user = create(:user)
expect(user).to receive(:update_cache_counts)
merge_request.update(assignee: user)
end
it "updates cache counts for previous assignee" do
old_assignee = merge_request.assignee
allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee)
expect(old_assignee).to receive(:update_cache_counts)
issue.update(assignee: nil)
merge_request.update(assignee: nil)
end
end
context "when previous assignee does not exist" do
before{ issue.update(assignee: nil) }
before { merge_request.update(assignee: nil) }
it "updates cache count for the new assignee" do
expect_any_instance_of(User).to receive(:update_cache_counts)
issue.update(assignee: user)
merge_request.update(assignee: user)
end
end
end
......@@ -300,7 +338,20 @@ describe Issue, "Issuable" do
end
context "issue is assigned" do
before { issue.update_attribute(:assignee, user) }
before { issue.assignees << user }
it "returns correct hook data" do
expect(data[:assignees].first).to eq(user.hook_attrs)
end
end
context "merge_request is assigned" do
let(:merge_request) { create(:merge_request) }
let(:data) { merge_request.to_hook_data(user) }
before do
merge_request.update_attribute(:assignee, user)
end
it "returns correct hook data" do
expect(data[:object_attributes]['assignee_id']).to eq(user.id)
......@@ -322,24 +373,6 @@ describe Issue, "Issuable" do
include_examples 'deprecated repository hook data'
end
describe '#card_attributes' do
it 'includes the author name' do
allow(issue).to receive(:author).and_return(double(name: 'Robert'))
allow(issue).to receive(:assignee).and_return(nil)
expect(issue.card_attributes).
to eq({ 'Author' => 'Robert', 'Assignee' => nil })
end
it 'includes the assignee name' do
allow(issue).to receive(:author).and_return(double(name: 'Robert'))
allow(issue).to receive(:assignee).and_return(double(name: 'Douwe'))
expect(issue.card_attributes).
to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' })
end
end
describe '#labels_array' do
let(:project) { create(:empty_project) }
let(:bug) { create(:label, project: project, title: 'bug') }
......
......@@ -28,7 +28,7 @@ describe IssueCollection do
end
it 'returns the issues the user is assigned to' do
issue1.assignee = user
issue1.assignees << user
expect(collection.updatable_by_user(user)).to eq([issue1])
end
......
......@@ -3,6 +3,7 @@ require 'spec_helper'
describe Issue, models: true do
describe "Associations" do
it { is_expected.to belong_to(:milestone) }
it { is_expected.to have_many(:assignees) }
end
describe 'modules' do
......@@ -37,6 +38,24 @@ describe Issue, models: true do
end
end
describe '#card_attributes' do
it 'includes the author name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert'))
allow(subject).to receive(:assignees).and_return([])
expect(subject.card_attributes).
to eq({ 'Author' => 'Robert', 'Assignee' => '' })
end
it 'includes the assignee name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert'))
allow(subject).to receive(:assignees).and_return([double(name: 'Douwe')])
expect(subject.card_attributes).
to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' })
end
end
describe '#closed_at' do
after do
Timecop.return
......@@ -153,25 +172,6 @@ describe Issue, models: true do
end
end
describe '#is_being_reassigned?' do
it 'returns true if the issue assignee has changed' do
subject.assignee = create(:user)
expect(subject.is_being_reassigned?).to be_truthy
end
it 'returns false if the issue assignee has not changed' do
expect(subject.is_being_reassigned?).to be_falsey
end
end
describe '#is_being_reassigned?' do
it 'returns issues assigned to user' do
user = create(:user)
create_list(:issue, 2, assignees: [user])
expect(Issue.open_for(user).count).to eq 2
end
end
describe '#closed_by_merge_requests' do
let(:project) { create(:project, :repository) }
let(:issue) { create(:issue, project: project)}
......@@ -404,7 +404,7 @@ describe Issue, models: true do
expect(user1.assigned_open_issues_count).to eq(1)
expect(user2.assigned_open_issues_count).to eq(0)
issue.assignee = user2
issue.assignees = [user2]
issue.save
expect(user1.assigned_open_issues_count).to eq(0)
......
......@@ -9,6 +9,7 @@ describe MergeRequest, models: true do
it { is_expected.to belong_to(:target_project).class_name('Project') }
it { is_expected.to belong_to(:source_project).class_name('Project') }
it { is_expected.to belong_to(:merge_user).class_name("User") }
it { is_expected.to belong_to(:assignee) }
it { is_expected.to have_many(:merge_request_diffs).dependent(:destroy) }
it { is_expected.to have_many(:approver_groups).dependent(:destroy) }
end
......@@ -87,8 +88,26 @@ describe MergeRequest, models: true do
end
end
describe '#card_attributes' do
it 'includes the author name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert'))
allow(subject).to receive(:assignee).and_return(nil)
expect(subject.card_attributes).
to eq({ 'Author' => 'Robert', 'Assignee' => nil })
end
it 'includes the assignee name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert'))
allow(subject).to receive(:assignee).and_return(double(name: 'Douwe'))
expect(subject.card_attributes).
to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' })
end
end
describe '#assignee_or_author?' do
let(:user) { build(:user) }
let(:user) { create(:user) }
it 'returns true for a user that is assigned to a merge request' do
subject.assignee = user
......@@ -298,16 +317,6 @@ describe MergeRequest, models: true do
end
end
describe '#is_being_reassigned?' do
it 'returns true if the merge_request assignee has changed' do
subject.assignee = create(:user)
expect(subject.is_being_reassigned?).to be_truthy
end
it 'returns false if the merge request assignee has not changed' do
expect(subject.is_being_reassigned?).to be_falsey
end
end
describe '#for_fork?' do
it 'returns true if the merge request is for a fork' do
subject.source_project = build_stubbed(:empty_project, namespace: create(:group))
......
......@@ -24,7 +24,6 @@ describe User, models: true do
it { is_expected.to have_many(:recent_events).class_name('Event') }
it { is_expected.to have_many(:issues).dependent(:restrict_with_exception) }
it { is_expected.to have_many(:notes).dependent(:destroy) }
it { is_expected.to have_many(:assigned_issues).dependent(:nullify) }
it { is_expected.to have_many(:merge_requests).dependent(:destroy) }
it { is_expected.to have_many(:assigned_merge_requests).dependent(:nullify) }
it { is_expected.to have_many(:identities).dependent(:destroy) }
......
......@@ -788,7 +788,7 @@ describe API::Issues, api: true do
expect(json_response['updated_at']).to be_present
expect(json_response['labels']).to eq(issue.label_names)
expect(json_response['milestone']).to be_a Hash
expect(json_response['assignee']).to be_a Hash
expect(json_response['assignees']).to be_a Array
expect(json_response['author']).to be_a Hash
expect(json_response['confidential']).to be_falsy
expect(json_response['weight']).to be_nil
......
......@@ -4,11 +4,12 @@ describe Issuable::BulkUpdateService, services: true do
let(:user) { create(:user) }
let(:project) { create(:empty_project, namespace: user.namespace) }
def bulk_update(issues, extra_params = {})
def bulk_update(issuables, extra_params = {})
bulk_update_params = extra_params
.reverse_merge(issuable_ids: Array(issues).map(&:id).join(','))
.reverse_merge(issuable_ids: Array(issuables).map(&:id).join(','))
Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute('issue')
type = Array(issuables).first.model_name.param_key
Issuable::BulkUpdateService.new(project, user, bulk_update_params).execute(type)
end
describe 'close issues' do
......@@ -47,15 +48,15 @@ describe Issuable::BulkUpdateService, services: true do
end
end
describe 'updating assignee' do
let(:issue) { create(:issue, project: project, assignee: user) }
describe 'updating merge request assignee' do
let(:merge_request) { create(:merge_request, target_project: project, source_project: project, assignee: user) }
context 'when the new assignee ID is a valid user' do
it 'succeeds' do
new_assignee = create(:user)
project.team << [new_assignee, :developer]
result = bulk_update(issue, assignee_id: new_assignee.id)
result = bulk_update(merge_request, assignee_id: new_assignee.id)
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1)
......@@ -65,22 +66,59 @@ describe Issuable::BulkUpdateService, services: true do
assignee = create(:user)
project.team << [assignee, :developer]
expect { bulk_update(issue, assignee_id: assignee.id) }
.to change { issue.reload.assignee }.from(user).to(assignee)
expect { bulk_update(merge_request, assignee_id: assignee.id) }
.to change { merge_request.reload.assignee }.from(user).to(assignee)
end
end
context "when the new assignee ID is #{IssuableFinder::NONE}" do
it "unassigns the issues" do
expect { bulk_update(issue, assignee_id: IssuableFinder::NONE) }
.to change { issue.reload.assignee }.to(nil)
expect { bulk_update(merge_request, assignee_id: IssuableFinder::NONE) }
.to change { merge_request.reload.assignee }.to(nil)
end
end
context 'when the new assignee ID is not present' do
it 'does not unassign' do
expect { bulk_update(issue, assignee_id: nil) }
.not_to change { issue.reload.assignee }
expect { bulk_update(merge_request, assignee_id: nil) }
.not_to change { merge_request.reload.assignee }
end
end
end
describe 'updating issue assignee' do
let(:issue) { create(:issue, project: project, assignees: [user]) }
context 'when the new assignee ID is a valid user' do
it 'succeeds' do
new_assignee = create(:user)
project.team << [new_assignee, :developer]
result = bulk_update(issue, assignee_ids: [new_assignee.id])
expect(result[:success]).to be_truthy
expect(result[:count]).to eq(1)
end
it 'updates the assignee to the use ID passed' do
assignee = create(:user)
project.team << [assignee, :developer]
expect { bulk_update(issue, assignee_ids: [assignee.id]) }
.to change { issue.reload.assignees.first }.from(user).to(assignee)
end
end
context "when the new assignee ID is #{IssuableFinder::NONE}" do
it "unassigns the issues" do
expect { bulk_update(issue, assignee_ids: [IssuableFinder::NONE.to_s]) }
.to change { issue.reload.assignees.count }.from(1).to(0)
end
end
context 'when the new assignee ID is not present' do
it 'does not unassign' do
expect { bulk_update(issue, assignee_ids: []) }
.not_to change{ issue.reload.assignees }
end
end
end
......
......@@ -6,10 +6,10 @@ describe Issues::CreateService, services: true do
describe '#execute' do
let(:issue) { described_class.new(project, user, opts).execute }
let(:assignee) { create(:user) }
let(:milestone) { create(:milestone, project: project) }
context 'when params are valid' do
let(:assignee) { create(:user) }
let(:milestone) { create(:milestone, project: project) }
let(:labels) { create_pair(:label, project: project) }
before do
......@@ -20,7 +20,7 @@ describe Issues::CreateService, services: true do
let(:opts) do
{ title: 'Awesome issue',
description: 'please fix',
assignee_ids: assignee.id.to_s,
assignee_ids: [assignee.id],
label_ids: labels.map(&:id),
milestone_id: milestone.id,
due_date: Date.tomorrow }
......@@ -37,7 +37,7 @@ describe Issues::CreateService, services: true do
context 'when current user cannot admin issues in the project' do
let(:guest) { create(:user) }
before do
project.team << [guest, :guest]
end
......@@ -137,10 +137,83 @@ describe Issues::CreateService, services: true do
end
end
it_behaves_like 'issuable create service'
context 'issue create service' do
context 'assignees' do
before { project.team << [user, :master] }
it 'removes assignee when user id is invalid' do
opts = { title: 'Title', description: 'Description', assignee_ids: [-1] }
issue = described_class.new(project, user, opts).execute
expect(issue.assignees).to be_empty
end
it 'removes assignee when user id is 0' do
opts = { title: 'Title', description: 'Description', assignee_ids: [0] }
issue = described_class.new(project, user, opts).execute
expect(issue.assignees).to be_empty
end
it 'saves assignee when user id is valid' do
project.team << [assignee, :master]
opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] }
issue = described_class.new(project, user, opts).execute
expect(issue.assignees).to eq([assignee])
end
context "when issuable feature is private" do
before do
project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE,
merge_requests_access_level: ProjectFeature::PRIVATE)
end
levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC]
levels.each do |level|
it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
project.update(visibility_level: level)
opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] }
issue = described_class.new(project, user, opts).execute
expect(issue.assignees).to be_empty
end
end
end
end
end
it_behaves_like 'new issuable record that supports slash commands'
context 'Slash commands' do
context 'with assignee and milestone in params and command' do
let(:opts) do
{
assignee_ids: [create(:user).id],
milestone_id: 1,
title: 'Title',
description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}")
}
end
before do
project.team << [user, :master]
project.team << [assignee, :master]
end
it 'assigns and sets milestone to issuable from command' do
expect(issue).to be_persisted
expect(issue.assignees).to eq([assignee])
expect(issue.milestone).to eq(milestone)
end
end
end
context 'resolving discussions' do
let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
let(:merge_request) { discussion.noteable }
......
......@@ -162,7 +162,7 @@ describe Issues::UpdateService, services: true do
it 'does not update assignee_id with unauthorized users' do
project.update(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
update_issue(confidential: true)
non_member = create(:user)
non_member = create(:user)
original_assignees = issue.assignees
update_issue(assignee_ids: [non_member.id])
......
......@@ -84,7 +84,87 @@ describe MergeRequests::CreateService, services: true do
end
end
it_behaves_like 'issuable create service'
context 'Slash commands' do
context 'with assignee and milestone in params and command' do
let(:merge_request) { described_class.new(project, user, opts).execute }
let(:milestone) { create(:milestone, project: project) }
let(:opts) do
{
assignee_id: create(:user).id,
milestone_id: 1,
title: 'Title',
description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}"),
source_branch: 'feature',
target_branch: 'master'
}
end
before do
project.team << [user, :master]
project.team << [assignee, :master]
end
it 'assigns and sets milestone to issuable from command' do
expect(merge_request).to be_persisted
expect(merge_request.assignee).to eq(assignee)
expect(merge_request.milestone).to eq(milestone)
end
end
end
context 'merge request create service' do
context 'asssignee_id' do
let(:assignee) { create(:user) }
before { project.team << [user, :master] }
it 'removes assignee_id when user id is invalid' do
opts = { title: 'Title', description: 'Description', assignee_id: -1 }
merge_request = described_class.new(project, user, opts).execute
expect(merge_request.assignee_id).to be_nil
end
it 'removes assignee_id when user id is 0' do
opts = { title: 'Title', description: 'Description', assignee_id: 0 }
merge_request = described_class.new(project, user, opts).execute
expect(merge_request.assignee_id).to be_nil
end
it 'saves assignee when user id is valid' do
project.team << [assignee, :master]
opts = { title: 'Title', description: 'Description', assignee_id: assignee.id }
merge_request = described_class.new(project, user, opts).execute
expect(merge_request.assignee).to eq(assignee)
end
context "when issuable feature is private" do
before do
project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE,
merge_requests_access_level: ProjectFeature::PRIVATE)
end
levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC]
levels.each do |level|
it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
project.update(visibility_level: level)
opts = { title: 'Title', description: 'Description', assignee_id: assignee.id }
merge_request = described_class.new(project, user, opts).execute
expect(merge_request.assignee_id).to be_nil
end
end
end
end
end
context 'while saving references to issues that the created merge request closes' do
let(:first_issue) { create(:issue, project: project) }
......
......@@ -66,7 +66,7 @@ describe Notes::SlashCommandsService, services: true do
expect(content).to eq ''
expect(note.noteable).to be_closed
expect(note.noteable.labels).to match_array(labels)
expect(note.noteable.assignee).to eq(assignee)
expect(note.noteable.assignees).to eq([assignee])
expect(note.noteable.milestone).to eq(milestone)
end
end
......@@ -113,7 +113,7 @@ describe Notes::SlashCommandsService, services: true do
expect(content).to eq "HELLO\nWORLD"
expect(note.noteable).to be_closed
expect(note.noteable.labels).to match_array(labels)
expect(note.noteable.assignee).to eq(assignee)
expect(note.noteable.assignees).to eq([assignee])
expect(note.noteable.milestone).to eq(milestone)
end
end
......
......@@ -569,7 +569,7 @@ describe NotificationService, services: true do
end
it 'emails previous assignee even if he has the "on mention" notif level' do
issue.assignees = [@u_mentioned]
issue.assignees = [@u_mentioned]
notification.reassigned_issue(issue, @u_disabled, [@u_watcher])
should_email(@u_mentioned)
......
......@@ -42,23 +42,6 @@ describe SlashCommands::InterpretService, services: true do
end
end
shared_examples 'assign command' do
it 'fetches assignee and populates assignee_id if content contains /assign' do
_, updates = service.execute(content, issuable)
expect(updates).to eq(assignee_id: developer.id)
end
end
shared_examples 'unassign command' do
it 'populates assignee_id: nil if content contains /unassign' do
issuable.update(assignee_id: developer.id)
_, updates = service.execute(content, issuable)
expect(updates).to eq(assignee_id: nil)
end
end
shared_examples 'milestone command' do
it 'fetches milestone and populates milestone_id if content contains /milestone' do
milestone # populate the milestone
......@@ -385,14 +368,24 @@ describe SlashCommands::InterpretService, services: true do
let(:issuable) { issue }
end
it_behaves_like 'assign command' do
context 'assign command' do
let(:content) { "/assign @#{developer.username}" }
let(:issuable) { issue }
end
it_behaves_like 'assign command' do
let(:content) { "/assign @#{developer.username}" }
let(:issuable) { merge_request }
context 'Issue' do
it 'fetches assignee and populates assignee_id if content contains /assign' do
_, updates = service.execute(content, issue)
expect(updates).to eq(assignee_ids: [developer.id])
end
end
context 'Merge Request' do
it 'fetches assignee and populates assignee_id if content contains /assign' do
_, updates = service.execute(content, merge_request)
expect(updates).to eq(assignee_id: developer.id)
end
end
end
it_behaves_like 'empty command' do
......@@ -405,14 +398,26 @@ describe SlashCommands::InterpretService, services: true do
let(:issuable) { issue }
end
it_behaves_like 'unassign command' do
context 'unassign command' do
let(:content) { '/unassign' }
let(:issuable) { issue }
end
it_behaves_like 'unassign command' do
let(:content) { '/unassign' }
let(:issuable) { merge_request }
context 'Issue' do
it 'populates assignee_ids: [] if content contains /unassign' do
issue.update(assignee_ids: [developer.id])
_, updates = service.execute(content, issue)
expect(updates).to eq(assignee_ids: [])
end
end
context 'Merge Request' do
it 'populates assignee_id: nil if content contains /unassign' do
merge_request.update(assignee_id: developer.id)
_, updates = service.execute(content, merge_request)
expect(updates).to eq(assignee_id: nil)
end
end
end
it_behaves_like 'milestone command' do
......
......@@ -6,6 +6,7 @@ describe SystemNoteService, services: true do
let(:project) { create(:project) }
let(:author) { create(:user) }
let(:noteable) { create(:issue, project: project) }
let(:issue) { noteable }
shared_examples_for 'a system note' do
it 'is valid' do
......@@ -133,6 +134,50 @@ describe SystemNoteService, services: true do
end
end
describe '.change_issue_assignees' do
subject { described_class.change_issue_assignees(noteable, project, author, [assignee]) }
let(:assignee) { create(:user) }
let(:assignee1) { create(:user) }
let(:assignee2) { create(:user) }
let(:assignee3) { create(:user) }
it_behaves_like 'a system note'
def build_note(old_assignees, new_assignees)
issue.assignees = new_assignees
described_class.change_issue_assignees(issue, project, author, old_assignees).note
end
it 'builds a correct phrase when an assignee is added to a non-assigned issue' do
expect(build_note([], [assignee1])).to eq "assigned to @#{assignee1.username}"
end
it 'builds a correct phrase when assignee removed' do
expect(build_note([assignee1], [])).to eq 'removed all assignees'
end
it 'builds a correct phrase when assignees changed' do
expect(build_note([assignee1], [assignee2])).to eq \
"assigned to @#{assignee2.username} and unassigned @#{assignee1.username}"
end
it 'builds a correct phrase when three assignees removed and one added' do
expect(build_note([assignee, assignee1, assignee2], [assignee3])).to eq \
"assigned to @#{assignee3.username} and unassigned @#{assignee.username}, @#{assignee1.username}, and @#{assignee2.username}"
end
it 'builds a correct phrase when one assignee changed from a set' do
expect(build_note([assignee, assignee1], [assignee, assignee2])).to eq \
"assigned to @#{assignee2.username} and unassigned @#{assignee1.username}"
end
it 'builds a correct phrase when one assignee removed from a set' do
expect(build_note([assignee, assignee1, assignee2], [assignee, assignee1])).to eq \
"unassigned @#{assignee2.username}"
end
end
describe '.change_label' do
subject { described_class.change_label(noteable, project, author, added, removed) }
......
......@@ -240,20 +240,20 @@ describe TodoService, services: true do
describe '#reassigned_issue' do
it 'creates a pending todo for new assignee' do
unassigned_issue.update_attribute(:assignee, john_doe)
unassigned_issue.assignees << john_doe
service.reassigned_issue(unassigned_issue, author)
should_create_todo(user: john_doe, target: unassigned_issue, action: Todo::ASSIGNED)
end
it 'does not create a todo if unassigned' do
issue.update_attribute(:assignee, nil)
issue.assignees.destroy_all
should_not_create_any_todo { service.reassigned_issue(issue, author) }
end
it 'creates a todo if new assignee is the current user' do
unassigned_issue.update_attribute(:assignee, john_doe)
unassigned_issue.assignees << john_doe
service.reassigned_issue(unassigned_issue, john_doe)
should_create_todo(user: john_doe, target: unassigned_issue, author: john_doe, action: Todo::ASSIGNED)
......
shared_examples 'issuable create service' do
context 'asssignee_id' do
let(:assignee) { create(:user) }
before { project.team << [user, :master] }
it 'removes assignee_id when user id is invalid' do
opts = { title: 'Title', description: 'Description', assignee_id: -1 }
issuable = described_class.new(project, user, opts).execute
expect(issuable.assignee_id).to be_nil
end
it 'removes assignee_id when user id is 0' do
opts = { title: 'Title', description: 'Description', assignee_id: 0 }
issuable = described_class.new(project, user, opts).execute
expect(issuable.assignee_id).to be_nil
end
it 'saves assignee when user id is valid' do
project.team << [assignee, :master]
opts = { title: 'Title', description: 'Description', assignee_id: assignee.id }
issuable = described_class.new(project, user, opts).execute
expect(issuable.assignee_id).to eq(assignee.id)
end
context "when issuable feature is private" do
before do
project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE,
merge_requests_access_level: ProjectFeature::PRIVATE)
end
levels = [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PUBLIC]
levels.each do |level|
it "removes not authorized assignee when project is #{Gitlab::VisibilityLevel.level_name(level)}" do
project.update(visibility_level: level)
opts = { title: 'Title', description: 'Description', assignee_id: assignee.id }
issuable = described_class.new(project, user, opts).execute
expect(issuable.assignee_id).to be_nil
end
end
end
end
end
......@@ -49,23 +49,7 @@ shared_examples 'new issuable record that supports slash commands' do
it 'assigns and sets milestone to issuable' do
expect(issuable).to be_persisted
expect(issuable.assignee).to eq(assignee)
expect(issuable.milestone).to eq(milestone)
end
end
context 'with assignee and milestone in params and command' do
let(:example_params) do
{
assignee: create(:user),
milestone_id: 1,
description: %(/assign @#{assignee.username}\n/milestone %"#{milestone.name}")
}
end
it 'assigns and sets milestone to issuable from command' do
expect(issuable).to be_persisted
expect(issuable.assignee).to eq(assignee)
expect(issuable.assignees).to eq([assignee])
expect(issuable.milestone).to eq(milestone)
end
end
......
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