Commit 0d3ecf58 authored by Clement Ho's avatar Clement Ho

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

parents a0f6bf9d d525c9a8
......@@ -20,6 +20,7 @@ import eventHub from '../eventhub';
list: {
type: Object,
required: false,
default: () => ({}),
},
rootPath: {
type: String,
......@@ -31,7 +32,67 @@ import eventHub from '../eventhub';
default: false,
},
},
data() {
return {
limitBeforeCounter: 3,
maxRender: 4,
maxCounter: 99,
};
},
computed: {
numberOverLimit() {
return this.issue.assignees.length - this.limitBeforeCounter;
},
assigneeCounterTooltip() {
return `${this.assigneeCounterLabel} more`;
},
assigneeCounterLabel() {
if (this.numberOverLimit > this.maxCounter) {
return `${this.maxCounter}+`;
}
return `+${this.numberOverLimit}`;
},
shouldRenderCounter() {
if (this.issue.assignees.length <= this.maxRender) {
return false;
}
return this.issue.assignees.length > this.numberOverLimit;
},
cardUrl() {
return `${this.issueLinkBase}/${this.issue.id}`;
},
issueId() {
return `#${this.issue.id}`;
},
showLabelFooter() {
return this.issue.labels.find(l => this.showLabel(l)) !== undefined;
},
},
methods: {
isIndexLessThanlimit(index) {
return index < this.limitBeforeCounter;
},
shouldRenderAssignee(index) {
// Eg. maxRender is 4,
// Render up to all 4 assignees if there are only 4 assigness
// Otherwise render up to the limitBeforeCounter
if (this.issue.assignees.length <= this.maxRender) {
return index < this.maxRender;
}
return index < this.limitBeforeCounter;
},
assigneeUrl(assignee) {
return `${this.rootPath}${assignee.username}`;
},
assigneeUrlTitle(assignee) {
return `Assigned to ${assignee.name}`;
},
avatarUrlTitle(assignee) {
return `Avatar for ${assignee.name}`;
},
showLabel(label) {
if (!this.list) return true;
......@@ -67,35 +128,55 @@ import eventHub from '../eventhub';
},
template: `
<div>
<div class="card-header">
<h4 class="card-title">
<i
class="fa fa-eye-slash confidential-icon"
v-if="issue.confidential"></i>
v-if="issue.confidential"
aria-hidden="true"
/>
<a
:href="issueLinkBase + '/' + issue.id"
:title="issue.title">
{{ issue.title }}
</a>
</h4>
<div class="card-footer">
class="js-no-trigger"
:href="cardUrl"
:title="issue.title">{{ issue.title }}</a>
<span
class="card-number"
v-if="issue.id">
#{{ issue.id }}
v-if="issue.id"
>
{{ issueId }}
</span>
</h4>
<div class="card-assignee">
<a
class="card-assignee has-tooltip"
:href="rootPath + issue.assignee.username"
:title="'Assigned to ' + issue.assignee.name"
v-if="issue.assignee"
data-container="body">
class="has-tooltip js-no-trigger"
:href="assigneeUrl(assignee)"
:title="assigneeUrlTitle(assignee)"
v-for="(assignee, index) in issue.assignees"
v-if="shouldRenderAssignee(index)"
data-container="body"
data-placement="bottom"
>
<img
class="avatar avatar-inline s20"
:src="issue.assignee.avatar"
:src="assignee.avatar"
width="20"
height="20"
:alt="'Avatar for ' + issue.assignee.name" />
:alt="avatarUrlTitle(assignee)"
/>
</a>
<span
class="avatar-counter has-tooltip"
:title="assigneeCounterTooltip"
v-if="shouldRenderCounter"
>
{{ assigneeCounterLabel }}
</span>
</div>
</div>
<div
class="card-footer"
v-if="showLabelFooter"
>
<button
class="label color-label has-tooltip"
v-for="label in issue.labels"
......
......@@ -15,14 +15,9 @@ class ListIssue {
this.subscribed = obj.subscribed;
this.labels = [];
this.selected = false;
this.assignee = false;
this.position = obj.relative_position || Infinity;
this.milestone_id = obj.milestone_id;
if (obj.assignee) {
this.assignee = new ListUser(obj.assignee);
}
if (obj.milestone) {
this.milestone = new ListMilestone(obj.milestone);
}
......@@ -30,6 +25,8 @@ class ListIssue {
obj.labels.forEach((label) => {
this.labels.push(new ListLabel(label));
});
this.assignees = obj.assignees.map(a => new ListUser(a));
}
addLabel (label) {
......
......@@ -226,7 +226,7 @@
.card {
position: relative;
padding: 10px $gl-padding;
padding: 11px 10px 11px $gl-padding;
background: $white-light;
border-radius: $border-radius-default;
box-shadow: 0 1px 2px $issue-boards-card-shadow;
......@@ -236,8 +236,13 @@
margin-bottom: 5px;
}
&.is-active {
&.is-active,
&.is-active .card-assignee:hover a {
background-color: $row-hover;
&:first-child:not(:only-child) {
box-shadow: -10px 0 10px 1px $row-hover;
}
}
.label {
......@@ -246,36 +251,111 @@
}
.confidential-icon {
position: relative;
top: 1px;
margin-right: 5px;
}
}
.card-title {
margin: 0;
margin: 0 30px 0 0;
font-size: 1em;
line-height: inherit;
a {
color: inherit;
color: $gl-text-color;
word-wrap: break-word;
margin-right: 2px;
}
}
.card-footer {
margin-top: 5px;
line-height: 25px;
.card-header {
display: flex;
min-height: 20px;
.label {
margin-right: 5px;
font-size: (14px / $issue-boards-font-size) * 1em;
.card-assignee {
display: flex;
justify-content: flex-end;
position: absolute;
right: 15px;
height: 20px;
width: 20px;
.avatar-counter {
display: none;
vertical-align: middle;
min-width: 20px;
line-height: 19px;
height: 20px;
padding-left: 2px;
padding-right: 2px;
border-radius: 2em;
}
img {
vertical-align: top;
}
a {
position: relative;
margin-left: -15px;
}
a:nth-child(1) {
z-index: 3;
}
a:nth-child(2) {
z-index: 2;
}
a:nth-child(3) {
z-index: 1;
}
a:nth-child(4) {
display: none;
}
&:hover {
.avatar-counter {
display: inline-block;
}
a {
position: static;
background-color: $white-light;
transition: background-color 0s;
margin-left: auto;
&:nth-child(4) {
display: block;
}
&:first-child:not(:only-child) {
box-shadow: -10px 0 10px 1px $white-light;
}
}
}
}
.avatar {
margin-left: 0;
margin: 0;
}
}
.card-footer {
margin: 0 0 5px;
.label {
margin-top: 5px;
margin-right: 6px;
}
}
.card-number {
margin-right: 5px;
font-size: 12px;
color: $gl-text-color-secondary;
}
.issue-boards-search {
......
......@@ -231,6 +231,17 @@ class IssuableFinder
klass.all
end
def by_scope(items)
case params[:scope]
when 'created-by-me', 'authored'
items.where(author_id: current_user.id)
when 'assigned-to-me'
items.assigned_to(current_user)
else
items
end
end
def by_state(items)
case params[:state].to_s
when 'closed'
......
......@@ -28,40 +28,26 @@ class IssuesFinder < IssuableFinder
def by_assignee(items)
if assignee
items = items.where("issue_assignees.user_id = ?", assignee.id)
items.assigned_to(assignee)
elsif no_assignee?
items = items.where("issue_assignees.user_id is NULL")
items.unassigned
elsif assignee_id? || assignee_username? # assignee not found
items = items.none
end
items
end
def by_scope(items)
case params[:scope]
when 'created-by-me', 'authored'
items.where(author_id: current_user.id)
when 'assigned-to-me'
items.where("issue_assignees.user_id = ?", current_user.id)
items.none
else
items
end
end
def self.not_restricted_by_confidentiality(user)
issues = Issue.with_assignees
return issues.where('issues.confidential IS NULL OR issues.confidential IS FALSE') if user.blank?
return Issue.where('issues.confidential IS NOT TRUE') if user.blank?
return issues.all if user.admin_or_auditor?
return Issue.all if user.admin_or_auditor?
issues.where('
issues.confidential IS NULL
OR issues.confidential IS FALSE
Issue.where('
issues.confidential IS NOT TRUE
OR (issues.confidential = TRUE
AND (issues.author_id = :user_id
OR issue_assignees.user_id = :user_id
OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id)
OR issues.project_id IN(:project_ids)))',
user_id: user.id,
project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
......
......@@ -23,17 +23,6 @@ class MergeRequestsFinder < IssuableFinder
private
def by_scope(items)
case params[:scope]
when 'created-by-me', 'authored'
items.where(author_id: current_user.id)
when 'assigned-to-me'
items.where(assignee_id: current_user.id)
else
items
end
end
def item_project_ids(items)
items&.reorder(nil)&.select(:target_project_id)
end
......
......@@ -40,7 +40,7 @@ module Milestoneish
def issues_visible_to_user(user)
memoize_per_user(user, :issues_visible_to_user) do
IssuesFinder.new(user, issues_finder_params)
.execute.where(milestone_id: milestoneish_ids)
.execute.includes(:assignees).where(milestone_id: milestoneish_ids)
end
end
......
......@@ -94,7 +94,7 @@ class GlobalMilestone
end
def participants
@participants ||= milestones.includes(:participants).map(&:participants).flatten.compact.uniq
@participants ||= milestones.map(&:participants).flatten.uniq
end
def labels
......
......@@ -34,13 +34,11 @@ class Issue < ActiveRecord::Base
validates :project, presence: true
scope :cared, ->(user) { with_assignees.where("issue_assignees.user_id IN(?)", user.id) }
scope :open_for, ->(user) { opened.assigned_to(user) }
scope :in_projects, ->(project_ids) { where(project_id: project_ids) }
scope :with_assignees, -> { joins("LEFT JOIN issue_assignees ON issue_id = issues.id") }
scope :assigned, -> { with_assignees.where('issue_assignees.user_id IS NOT NULL') }
scope :unassigned, -> { with_assignees.where('issue_assignees.user_id IS NULL') }
scope :assigned_to, ->(u) { with_assignees.where('issue_assignees.user_id = ?', u.id)}
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 :without_due_date, -> { where(due_date: nil) }
scope :due_before, ->(date) { where('issues.due_date < ?', date) }
......
......@@ -106,7 +106,7 @@ class User < ActiveRecord::Base
has_many :protected_branch_push_access_levels, dependent: :destroy, class_name: ProtectedBranch::PushAccessLevel
has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id
has_many :issue_assignees, dependent: :destroy
has_many :issue_assignees
has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue
has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest"
......
......@@ -22,9 +22,9 @@ module Issues
end
def filter_assignee(issuable)
return if params[:assignee_ids].to_a.empty?
return if params[:assignee_ids].blank?
assignee_ids = params[:assignee_ids].select{ |assignee_id| assignee_can_read?(issuable, assignee_id)}
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] = []
......
......@@ -21,7 +21,7 @@ module Issues
def csv_builder
@csv_builder ||=
CsvBuilder.new(@issues.includes(:author), header_to_value_hash)
CsvBuilder.new(@issues.includes(:author, :assignees), header_to_value_hash)
end
private
......@@ -35,8 +35,8 @@ module Issues
'Description' => 'description',
'Author' => 'author_name',
'Author Username' => -> (issue) { issue.author&.username },
'Assignee' => -> (issue) { issue.assignees.pluck(:name).join(', ') },
'Assignee Username' => -> (issue) { issue.assignees.pluck(:username).join(', ') },
'Assignee' => -> (issue) { issue.assignees.map(&:name).join(', ') },
'Assignee Username' => -> (issue) { issue.assignees.map(&:username).join(', ') },
'Confidential' => -> (issue) { issue.confidential? ? 'Yes' : 'No' },
'Due Date' => -> (issue) { issue.due_date&.to_s(:csv) },
'Created At (UTC)' => -> (issue) { issue.created_at&.to_s(:csv) },
......
......@@ -4,7 +4,7 @@ module MergeRequests
@assignable_issues ||= begin
if current_user == merge_request.author
closes_issues.select do |issue|
!issue.is_a?(ExternalIssue) && !issue.assignees.any? && can?(current_user, :admin_issue, issue)
!issue.is_a?(ExternalIssue) && !issue.assignees.present? && can?(current_user, :admin_issue, issue)
end
else
[]
......
......@@ -29,7 +29,7 @@ class NotificationRecipientService
recipients << target.assignee
when :reassign_issue
previous_assignees = Array(previous_assignee)
recipients.concat(previous_assignees) if previous_assignees.any?
recipients.concat(previous_assignees)
recipients.concat(target.assignees)
end
......
......@@ -86,15 +86,18 @@ module SlashCommands
current_user.can?(:"admin_#{issuable.to_ability_name}", project)
end
command :assign do |assignee_param|
user = extract_references(assignee_param, :user).first
user ||= User.find_by(username: assignee_param)
user_ids = extract_references(assignee_param, :user).map(&:id)
next unless user
if user_ids.empty?
user_ids = User.where(username: assignee_param.split(' ').map(&:strip)).pluck(:id)
end
next if user_ids.empty?
if issuable.is_a?(Issue)
@updates[:assignee_ids] = [user.id]
@updates[:assignee_ids] = user_ids
else
@updates[:assignee_id] = user.id
@updates[:assignee_id] = user_ids.last
end
end
......
......@@ -54,7 +54,7 @@ module SystemNoteService
# issue - Issue object
# project - Project owning noteable
# author - User performing the change
# assignees - User being assigned, or nil
# assignees - Users being assigned, or nil
#
# Example Note text:
#
......
Reassigned Issue <%= @issue.iid %>
<%= url_for([@issue.project.namespace.becomes(Namespace), @issue.project, @issue, { only_path: false }]) %>
Assignee changed <%= "from #{@previous_assignees.map(&:name).to_sentence}" if @previous_assignees.any? -%>
to <%= "#{@issue.assignees.any? ? @issue.assignee_list : 'Unassigned'}" %>
......@@ -7,6 +7,6 @@
- if @issue.description
= markdown(@issue.description, pipeline: :email, author: @issue.author)
- if @issue.assignees.any?
- if @issue.assignees.present?
%p
Assignee: #{@issue.assignee_list}
......@@ -8,4 +8,3 @@
%strong= @issue.assignee_list
- else
%strong Unassigned
<%= render 'reassigned_issuable_email', issuable: @issue %>
Reassigned Issue <%= @issue.iid %>
<%= url_for([@issue.project.namespace.becomes(Namespace), @issue.project, @issue, { only_path: false }]) %>
Assignee changed <%= "from #{@previous_assignees.map(&:name).to_sentence}" if @previous_assignees.any? -%>
to <%= "#{@issue.assignees.any? ? @issue.assignee_list : 'Unassigned'}" %>
= render 'reassigned_issuable_email', issuable: @merge_request
Reassigned Merge Request #{ @merge_request.iid }
= url_for([@merge_request.project.namespace.becomes(Namespace), @merge_request.project, @merge_request, { only_path: false }])
Assignee changed
- if @previous_assignee
from #{@previous_assignee.name}
to
= @merge_request.assignee_id ? @merge_request.assignee_name : 'Unassigned'
---
title: Update issue board cards design
merge_request: 10353
author:
......@@ -26,7 +26,7 @@ class CreateIssueAssigneesTable < ActiveRecord::Migration
# disable_ddl_transaction!
def up
create_table :issue_assignees do |t|
create_table :issue_assignees, id: false 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
......@@ -35,10 +35,6 @@ class CreateIssueAssigneesTable < ActiveRecord::Migration
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
......@@ -21,9 +21,23 @@ class MigrateAssignees < ActiveRecord::Migration
#
# To disable transactions uncomment the following line and remove these
# comments:
# disable_ddl_transaction!
disable_ddl_transaction!
def up
# Optimisation: this accounts for most of the invalid assignee IDs on GitLab.com
update_column_in_batches(:issues, :assignee_id, nil) do |table, query|
query.where(table[:assignee_id].eq(0))
end
users = Arel::Table.new(:users)
update_column_in_batches(:issues, :assignee_id, nil) do |table, query|
query.where(table[:assignee_id].not_eq(nil)\
.and(
users.project("true").where(users[:id].eq(table[:assignee_id])).exists.not
))
end
execute <<-EOF
INSERT INTO issue_assignees(issue_id, user_id)
SELECT id, assignee_id FROM issues WHERE assignee_id IS NOT NULL
......
......@@ -495,7 +495,7 @@ ActiveRecord::Schema.define(version: 20170320173259) do
add_index "index_statuses", ["project_id"], name: "index_index_statuses_on_project_id", unique: true, using: :btree
create_table "issue_assignees", force: :cascade do |t|
create_table "issue_assignees", id: false, force: :cascade do |t|
t.integer "user_id", null: false
t.integer "issue_id", null: false
end
......
......@@ -68,14 +68,14 @@ Example response:
"updated_at" : "2016-01-04T15:31:39.996Z"
},
"project_id" : 1,
"assignee" : {
"assignees" : [{
"state" : "active",
"id" : 1,
"name" : "Administrator",
"web_url" : "https://gitlab.example.com/root",
"avatar_url" : null,
"username" : "root"
},
}],
"updated_at" : "2016-01-04T15:31:51.081Z",
"id" : 76,
"title" : "Consequatur vero maxime deserunt laboriosam est voluptas dolorem.",
......@@ -150,14 +150,14 @@ Example response:
"description" : "Omnis vero earum sunt corporis dolor et placeat.",
"state" : "closed",
"iid" : 1,
"assignee" : {
"assignees" : [{
"avatar_url" : null,
"web_url" : "https://gitlab.example.com/lennie",
"state" : "active",
"username" : "lennie",
"id" : 9,
"name" : "Dr. Luella Kovacek"
},
}],
"labels" : [],
"id" : 41,
"title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
......@@ -231,14 +231,14 @@ Example response:
"description" : "Omnis vero earum sunt corporis dolor et placeat.",
"state" : "closed",
"iid" : 1,
"assignee" : {
"assignees" : [{
"avatar_url" : null,
"web_url" : "https://gitlab.example.com/lennie",
"state" : "active",
"username" : "lennie",
"id" : 9,
"name" : "Dr. Luella Kovacek"
},
}],
"labels" : [],
"id" : 41,
"title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
......@@ -297,14 +297,14 @@ Example response:
"description" : "Omnis vero earum sunt corporis dolor et placeat.",
"state" : "closed",
"iid" : 1,
"assignee" : {
"assignees" : [{
"avatar_url" : null,
"web_url" : "https://gitlab.example.com/lennie",
"state" : "active",
"username" : "lennie",
"id" : 9,
"name" : "Dr. Luella Kovacek"
},
}],
"labels" : [],
"id" : 41,
"title" : "Ut commodi ullam eos dolores perferendis nihil sunt.",
......@@ -333,7 +333,7 @@ POST /projects/:id/issues
| `title` | string | yes | The title of an issue |
| `description` | string | no | The description of an issue |
| `confidential` | boolean | no | Set an issue to be confidential. Default is `false`. |
| `assignee_id` | integer | no | The ID of a user to assign issue |
| `assignee_ids` | Array[integer] | no | The ID of a user to assign issue |
| `milestone_id` | integer | no | The ID of a milestone to assign issue |
| `labels` | string | no | Comma-separated label names for an issue |
| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) |
......@@ -356,7 +356,7 @@ Example response:
"iid" : 14,
"title" : "Issues with auth",
"state" : "opened",
"assignee" : null,
"assignees" : [],
"labels" : [
"bug"
],
......@@ -396,7 +396,7 @@ PUT /projects/:id/issues/:issue_iid
| `title` | string | no | The title of an issue |
| `description` | string | no | The description of an issue |
| `confidential` | boolean | no | Updates an issue to be confidential |
| `assignee_id` | integer | no | The ID of a user to assign the issue to |
| `assignee_ids` | Array[integer] | no | The ID of a user to assign the issue to |
| `milestone_id` | integer | no | The ID of a milestone to assign the issue to |
| `labels` | string | no | Comma-separated label names for an issue |
| `state_event` | string | no | The state event of an issue. Set `close` to close the issue and `reopen` to reopen it |
......@@ -431,7 +431,7 @@ Example response:
"bug"
],
"id" : 85,
"assignee" : null,
"assignees" : [],
"milestone" : null,
"subscribed" : true,
"user_notes_count": 0,
......@@ -496,14 +496,14 @@ Example response:
"updated_at": "2016-04-07T12:20:17.596Z",
"labels": [],
"milestone": null,
"assignee": {
"assignees": [{
"name": "Miss Monserrate Beier",
"username": "axel.block",
"id": 12,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon",
"web_url": "https://gitlab.example.com/axel.block"
},
}],
"author": {
"name": "Kris Steuber",
"username": "solon.cremin",
......@@ -552,14 +552,14 @@ Example response:
"updated_at": "2016-04-07T12:20:17.596Z",
"labels": [],
"milestone": null,
"assignee": {
"assignees": [{
"name": "Miss Monserrate Beier",
"username": "axel.block",
"id": 12,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/46f6f7dc858ada7be1853f7fb96e81da?s=80&d=identicon",
"web_url": "https://gitlab.example.com/axel.block"
},
}],
"author": {
"name": "Kris Steuber",
"username": "solon.cremin",
......@@ -656,14 +656,14 @@ Example response:
"updated_at": "2016-06-17T07:47:33.832Z",
"due_date": null
},
"assignee": {
"assignees": [{
"name": "Jarret O'Keefe",
"username": "francisca",
"id": 14,
"state": "active",
"avatar_url": "http://www.gravatar.com/avatar/a7fa515d53450023c83d62986d0658a8?s=80&d=identicon",
"web_url": "https://gitlab.example.com/francisca"
},
}],
"author": {
"name": "Maxie Medhurst",
"username": "craig_rutherford",
......
......@@ -232,7 +232,7 @@ X-Gitlab-Event: Issue Hook
"object_attributes": {
"id": 301,
"title": "New API: create/update/delete file",
"assignee_id": 51,
"assignee_ids": [51],
"author_id": 51,
"project_id": 14,
"created_at": "2013-12-03T17:15:43Z",
......@@ -246,11 +246,11 @@ X-Gitlab-Event: Issue Hook
"url": "http://example.com/diaspora/issues/23",
"action": "open"
},
"assignee": {
"assignees": [{
"name": "User1",
"username": "user1",
"avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=40\u0026d=identicon"
},
}],
"labels": [{
"id": 206,
"title": "API",
......@@ -544,7 +544,7 @@ X-Gitlab-Event: Note Hook
"issue": {
"id": 92,
"title": "test",
"assignee_id": null,
"assignee_ids": [],
"author_id": 1,
"project_id": 5,
"created_at": "2015-04-12 14:53:17 UTC",
......
......@@ -295,6 +295,13 @@ module API
expose :project_id, :issues_events, :merge_requests_events
expose :note_events, :build_events, :pipeline_events, :wiki_page_events
end
class Issue < ::API::Entities::Issue
unexpose :assignees
expose :assignee do |issue, options|
::API::Entities::UserBasic.represent(issue.assignees.first, options)
end
end
end
end
end
......@@ -14,6 +14,14 @@ module API
authorize! access_level, merge_request
merge_request
end
def convert_parameters_from_legacy_format(params)
if params[:assignee_id].present?
params[:assignee_ids] = [params.delete(:assignee_id)]
end
params
end
end
end
end
......@@ -8,6 +8,7 @@ module API
helpers do
def find_issues(args = {})
args = params.merge(args)
args = convert_parameters_from_legacy_format(args)
args.delete(:id)
args[:milestone_title] = args.delete(:milestone)
......@@ -53,7 +54,7 @@ module API
resource :issues do
desc "Get currently authenticated user's issues" do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
optional :state, type: String, values: %w[opened closed all], default: 'all',
......@@ -62,7 +63,7 @@ module API
end
get do
issues = find_issues(scope: 'authored')
present paginate(issues), with: ::API::Entities::Issue, current_user: current_user
present paginate(issues), with: ::API::V3::Entities::Issue, current_user: current_user
end
end
......@@ -71,7 +72,7 @@ module API
end
resource :groups, requirements: { id: %r{[^/]+} } do
desc 'Get a list of group issues' do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
optional :state, type: String, values: %w[opened closed all], default: 'opened',
......@@ -83,7 +84,7 @@ module API
issues = find_issues(group_id: group.id, state: params[:state] || 'opened', match_all_labels: true)
present paginate(issues), with: ::API::Entities::Issue, current_user: current_user
present paginate(issues), with: ::API::V3::Entities::Issue, current_user: current_user
end
end
......@@ -95,7 +96,7 @@ module API
desc 'Get a list of project issues' do
detail 'iid filter is deprecated have been removed on V4'
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
optional :state, type: String, values: %w[opened closed all], default: 'all',
......@@ -108,22 +109,22 @@ module API
issues = find_issues(project_id: project.id)
present paginate(issues), with: ::API::Entities::Issue, current_user: current_user, project: user_project
present paginate(issues), with: ::API::V3::Entities::Issue, current_user: current_user, project: user_project
end
desc 'Get a single project issue' do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
end
get ":id/issues/:issue_id" do
issue = find_project_issue(params[:issue_id])
present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project
present issue, with: ::API::V3::Entities::Issue, current_user: current_user, project: user_project
end
desc 'Create a new project issue' do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
requires :title, type: String, desc: 'The title of an issue'
......@@ -141,6 +142,7 @@ module API
issue_params = declared_params(include_missing: false)
issue_params = issue_params.merge(merge_request_to_resolve_discussions_of: issue_params.delete(:merge_request_for_resolving_discussions))
issue_params = convert_parameters_from_legacy_format(issue_params)
issue = ::Issues::CreateService.new(user_project,
current_user,
......@@ -148,14 +150,14 @@ module API
render_spam_error! if issue.spam?
if issue.valid?
present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project
present issue, with: ::API::V3::Entities::Issue, current_user: current_user, project: user_project
else
render_validation_error!(issue)
end
end
desc 'Update an existing issue' do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
......@@ -178,6 +180,7 @@ module API
end
update_params = declared_params(include_missing: false).merge(request: request, api: true)
update_params = convert_parameters_from_legacy_format(update_params)
issue = ::Issues::UpdateService.new(user_project,
current_user,
......@@ -186,14 +189,14 @@ module API
render_spam_error! if issue.spam?
if issue.valid?
present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project
present issue, with: ::API::V3::Entities::Issue, current_user: current_user, project: user_project
else
render_validation_error!(issue)
end
end
desc 'Move an existing issue' do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
requires :issue_id, type: Integer, desc: 'The ID of a project issue'
......@@ -208,7 +211,7 @@ module API
begin
issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project)
present issue, with: ::API::Entities::Issue, current_user: current_user, project: user_project
present issue, with: ::API::V3::Entities::Issue, current_user: current_user, project: user_project
rescue ::Issues::MoveService::MoveError => error
render_api_error!(error.message, 400)
end
......
......@@ -32,7 +32,7 @@ module API
if project.has_external_issue_tracker?
::API::Entities::ExternalIssue
else
::API::Entities::Issue
::API::V3::Entities::Issue
end
end
......
......@@ -39,7 +39,7 @@ module API
end
desc 'Get all issues for a single project milestone' do
success ::API::Entities::Issue
success ::API::V3::Entities::Issue
end
params do
requires :milestone_id, type: Integer, desc: 'The ID of a project milestone'
......@@ -56,7 +56,7 @@ module API
}
issues = IssuesFinder.new(current_user, finder_params).execute
present paginate(issues), with: ::API::Entities::Issue, current_user: current_user, project: user_project
present paginate(issues), with: ::API::V3::Entities::Issue, current_user: current_user, project: user_project
end
end
end
......
......@@ -1329,7 +1329,7 @@ describe Projects::MergeRequestsController do
end
it 'correctly pluralizes flash message on success' do
issue2.update!(assignees: [user])
issue2.assignees = [user]
post_assign_issues
......
......@@ -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('assignees email', text: issue2.assignees.first.public_email)
expect(entry).to have_selector('assignees email', text: assignee.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('assignees email', text: issue1.assignees.first.public_email)
expect(entry).to have_selector('assignees email', text: assignee.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')
......
......@@ -22,7 +22,7 @@ describe 'Issues Feed', feature: true do
to have_content('application/atom+xml')
expect(body).to have_selector('title', text: "#{project.name} issues")
expect(body).to have_selector('author email', text: issue.author_public_email)
expect(body).to have_selector('assignee email', text: issue.author_public_email)
expect(body).to have_selector('assignees email', text: issue.author_public_email)
expect(body).to have_selector('entry summary', text: issue.title)
end
end
......@@ -36,7 +36,7 @@ describe 'Issues Feed', feature: true do
to have_content('application/atom+xml')
expect(body).to have_selector('title', text: "#{project.name} issues")
expect(body).to have_selector('author email', text: issue.author_public_email)
expect(body).to have_selector('assignee email', text: issue.author_public_email)
expect(body).to have_selector('assignees email', text: issue.author_public_email)
expect(body).to have_selector('entry summary', text: issue.title)
end
end
......
......@@ -131,7 +131,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
context 'selecing issues' do
it 'selects single issue' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
page.within('.nav-links') do
expect(page).to have_content('Selected issues 1')
......@@ -141,7 +141,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
it 'changes button text' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
expect(first('.add-issues-footer .btn')).to have_content('Add 1 issue')
end
......@@ -149,7 +149,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
it 'changes button text with plural' do
page.within('.add-issues-modal') do
all('.card').each do |el|
all('.card .card-number').each do |el|
el.click
end
......@@ -159,7 +159,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
it 'shows only selected issues on selected tab' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
click_link 'Selected issues'
......@@ -189,7 +189,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
it 'selects all that arent already selected' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
expect(page).to have_selector('.is-active', count: 1)
......@@ -201,11 +201,11 @@ describe 'Issue Boards add issue modal', :feature, :js do
it 'unselects from selected tab' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
click_link 'Selected issues'
first('.card').click
first('.card .card-number').click
expect(page).not_to have_selector('.is-active')
end
......@@ -215,7 +215,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
context 'adding issues' do
it 'adds to board' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
click_button 'Add 1 issue'
end
......@@ -227,7 +227,7 @@ describe 'Issue Boards add issue modal', :feature, :js do
it 'adds to second list' do
page.within('.add-issues-modal') do
first('.card').click
first('.card .card-number').click
click_button planning.title
......
......@@ -80,6 +80,6 @@ describe 'Issues csv', feature: true do
author: user,
milestone: milestone,
labels: [feature_label, idea_label])
expect{ request_csv }.not_to exceed_query_limit(control_count + 23)
expect{ request_csv }.not_to exceed_query_limit(control_count + 5)
end
end
......@@ -6,7 +6,7 @@ describe 'Unsubscribe links', feature: true do
let(:recipient) { create(:user) }
let(:author) { create(:user) }
let(:project) { create(:empty_project, :public) }
let(:params) { { title: 'A bug!', description: 'Fix it!', assignee: recipient } }
let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } }
let(:issue) { Issues::CreateService.new(project, author, params).execute }
let(:mail) { ActionMailer::Base.deliveries.last }
......
......@@ -31,13 +31,14 @@ describe Issue, elastic: true do
end
it "returns json with all needed elements" do
issue = create :issue, project: project
assignee = create(:user)
issue = create :issue, project: project, assignees: [assignee]
expected_hash = issue.attributes.extract!('id', 'iid', 'title', 'description', 'created_at',
'updated_at', 'state', 'project_id', 'author_id',
'confidential')
expected_hash['assignee_id'] = []
expected_hash['assignee_id'] = [assignee.id]
expect(issue.as_indexed_json).to eq(expected_hash)
end
......
......@@ -56,84 +56,6 @@ describe Issue, "Issuable" do
end
end
describe "before_save" 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.assignees << assignee
end
it "updates cache counts for new assignee" do
user = create(:user)
expect(user).to receive(:update_cache_counts)
issue.assignees << user
end
it "updates cache counts for previous assignee" do
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)
merge_request.update(assignee: nil)
end
end
context "when previous assignee does not exist" do
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)
merge_request.update(assignee: user)
end
end
end
end
describe ".search" do
let!(:searchable_issue) { create(:issue, title: "Searchable issue") }
......
......@@ -38,6 +38,46 @@ describe Issue, models: true do
end
end
describe "before_save" do
describe "#update_cache_counts when an issue is reassigned" do
let(:issue) { create(:issue) }
let(:assignee) { create(:user) }
context "when previous assignee exists" do
before do
issue.project.team << [assignee, :developer]
issue.assignees << assignee
end
it "updates cache counts for new assignee" do
user = create(:user)
expect(user).to receive(:update_cache_counts)
issue.assignees << user
end
it "updates cache counts for previous assignee" do
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
it "updates cache count for the new assignee" do
issue.assignees = []
expect_any_instance_of(User).to receive(:update_cache_counts)
issue.assignees << assignee
end
end
end
end
describe '#card_attributes' do
it 'includes the author name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert'))
......
......@@ -88,6 +88,48 @@ describe MergeRequest, models: true do
end
end
describe "before_save" do
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) }
let(:assignee) { create :user }
context "when previous assignee exists" do
before do
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)
merge_request.update(assignee: nil)
end
end
context "when previous assignee does not exist" do
it "updates cache count for the new assignee" do
merge_request.update(assignee: nil)
expect_any_instance_of(User).to receive(:update_cache_counts)
merge_request.update(assignee: assignee)
end
end
end
end
describe '#card_attributes' do
it 'includes the author name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert'))
......
......@@ -15,14 +15,14 @@ describe MergeRequests::AssignIssuesService, services: true do
expect(service.assignable_issues.map(&:id)).to include(issue.id)
end
it 'ignores issues already assigned to any user' do
issue.assignees = [create(:user)]
it 'ignores issues the user cannot update assignee on' do
project.team.truncate
expect(service.assignable_issues).to be_empty
end
it 'ignores issues the user cannot update assignee on' do
project.team.truncate
it 'ignores issues already assigned to any user' do
issue.assignees = [create(:user)]
expect(service.assignable_issues).to be_empty
end
......
......@@ -460,7 +460,7 @@ describe NotificationService, services: true do
it do
notification.new_issue(issue, @u_disabled)
should_email(issue.assignees.first)
should_email(assignee)
should_email(@u_watcher)
should_email(@u_guest_watcher)
should_email(@u_guest_custom)
......
......@@ -3,6 +3,7 @@ require 'spec_helper'
describe SlashCommands::InterpretService, services: true do
let(:project) { create(:project, :public) }
let(:developer) { create(:user) }
let(:developer2) { create(:user) }
let(:issue) { create(:issue, project: project) }
let(:milestone) { create(:milestone, project: project, title: '9.10') }
let(:inprogress) { create(:label, project: project, title: 'In Progress') }
......@@ -388,6 +389,28 @@ describe SlashCommands::InterpretService, services: true do
end
end
context 'assign command with multiple assignees' do
let(:content) { "/assign @#{developer.username} @#{developer2.username}" }
before{ project.team << [developer2, :developer] }
context 'Issue' do
it 'fetches assignee and populates assignee_id if content contains /assign' do
_, updates = service.execute(content, issue)
expect(updates[:assignee_ids]).to match_array([developer.id, developer2.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
let(:content) { '/assign @abcd1234' }
let(:issuable) { issue }
......
......@@ -63,7 +63,7 @@ shared_examples 'issuable record that supports slash commands in its description
note = issuable.notes.user.first
expect(note.note).to eq "Awesome!"
expect(issuable.assignee).to eq assignee
expect(issuable.assignees).to eq [assignee]
expect(issuable.labels).to eq [label_bug]
expect(issuable.milestone).to eq milestone
end
......@@ -81,7 +81,7 @@ shared_examples 'issuable record that supports slash commands in its description
issuable.reload
expect(issuable.notes.user).to be_empty
expect(issuable.assignee).to eq assignee
expect(issuable.assignees).to eq [assignee]
expect(issuable.labels).to eq [label_bug]
expect(issuable.milestone).to eq milestone
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