root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Ensure that we only have one task per issue/mr

This commit is contained in:
Douglas Barbosa Alexandre 2016-02-18 19:12:52 -02:00
parent 4465613647
commit fc3f8a8ff7
4 changed files with 82 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
app/services/issues/update_service.rb
View File

@ -6,6 +6,11 @@ module Issues
def handle_changes(issue, options = {})
if have_changes?(issue, options)
task_service.mark_pending_tasks_as_done(issue, current_user)
end
if issue.previous_changes.include?('title') ||
issue.previous_changes.include?('description')
task_service.update_issue(issue, current_user)
end

5
app/services/merge_requests/update_service.rb
View File

@ -16,6 +16,11 @@ module MergeRequests
def handle_changes(merge_request, options = {})
if have_changes?(merge_request, options)
task_service.mark_pending_tasks_as_done(merge_request, current_user)
end
if merge_request.previous_changes.include?('title') ||
merge_request.previous_changes.include?('description')
task_service.update_merge_request(merge_request, current_user)
end

148
app/services/task_service.rb
View File

@ -18,10 +18,9 @@ class TaskService
# When update an issue we should:
#
# * mark all pending tasks related to the issue for the current user as done
# * create a task for each new user mentioned on issue
#
def update_issue(issue, current_user)
update_issuable(issue, current_user)
create_mention_tasks(issue.project, issue, current_user)
end
# When close an issue we should:
@ -37,7 +36,7 @@ class TaskService
# * create a pending task for new assignee if issue is assigned
#
def reassigned_issue(issue, current_user)
reassigned_issuable(issue, current_user)
create_assignment_task(issue, current_user)
end
# When create a merge request we should:
@ -51,11 +50,10 @@ class TaskService
# When update a merge request we should:
#
# * mark all pending tasks related to the merge request for the current user as done
# * create a task for each new user mentioned on merge request
# * create a task for each mentioned user on merge request
#
def update_merge_request(merge_request, current_user)
update_issuable(merge_request, current_user)
create_mention_tasks(merge_request.project, merge_request, current_user)
end
# When close a merge request we should:
@ -71,7 +69,7 @@ class TaskService
# * creates a pending task for new assignee if merge request is assigned
#
def reassigned_merge_request(merge_request, current_user)
reassigned_issuable(merge_request, current_user)
create_assignment_task(merge_request, current_user)
end
# When merge a merge request we should:
@ -87,21 +85,8 @@ class TaskService
# * mark all pending tasks related to the noteable for the note author as done
# * create a task for each mentioned user on note
#
def new_note(note)
# Skip system notes, like status changes and cross-references
return if note.system
project = note.project
target = note.noteable
author = note.author
mark_pending_tasks_as_done(target, author)
mentioned_users = build_mentioned_users(project, note, author)
mentioned_users.each do |user|
create_task(project, target, author, user, Task::MENTIONED, note)
end
def new_note(note, current_user)
handle_note(note, current_user)
end
# When update a note we should:
@ -110,41 +95,63 @@ class TaskService
# * create a task for each new user mentioned on note
#
def update_note(note, current_user)
handle_note(note, current_user)
end
# When marking pending tasks as done we should:
#
# * mark all pending tasks related to the target for the current user as done
#
def mark_pending_tasks_as_done(target, user)
pending_tasks(user, target.project, target).update_all(state: :done)
end
private
def create_tasks(project, target, author, users, action, note = nil)
Array(users).each do |user|
next if pending_tasks(user, project, target).exists?
Task.create(
project: project,
user_id: user.id,
author_id: author.id,
target_id: target.id,
target_type: target.class.name,
action: action,
note: note
)
end
end
def new_issuable(issuable, author)
create_assignment_task(issuable, author)
create_mention_tasks(issuable.project, issuable, author)
end
def handle_note(note, author)
# Skip system notes, like status changes and cross-references
return if note.system
project = note.project
target = note.noteable
author = current_user
mark_pending_tasks_as_done(target, author)
create_mention_tasks(project, target, author, note)
end
mentioned_users = build_mentioned_users(project, note, author)
mentioned_users.each do |user|
unless pending_tasks(mentioned_user, project, target, note: note, action: Task::MENTIONED).exists?
create_task(project, target, author, user, Task::MENTIONED, note)
end
def create_assignment_task(issuable, author)
if issuable.assignee && issuable.assignee != author
create_tasks(issuable.project, issuable, author, issuable.assignee, Task::ASSIGNED)
end
end
private
def create_task(project, target, author, user, action, note = nil)
attributes = {
project: project,
user_id: user.id,
author_id: author.id,
target_id: target.id,
target_type: target.class.name,
action: action,
note: note
}
Task.create(attributes)
def create_mention_tasks(project, issuable, author, note = nil)
mentioned_users = filter_mentioned_users(project, note || issuable, author)
create_tasks(project, issuable, author, mentioned_users, Task::MENTIONED, note)
end
def build_mentioned_users(project, target, author)
def filter_mentioned_users(project, target, author)
mentioned_users = target.mentioned_users.select do |user|
user.can?(:read_project, project)
end
@ -153,54 +160,11 @@ class TaskService
mentioned_users.uniq
end
def mark_pending_tasks_as_done(target, user)
pending_tasks(user, target.project, target).update_all(state: :done)
end
def pending_tasks(user, project, target, options = {})
options.reverse_merge(
project: project,
target: target
def pending_tasks(user, project, target)
user.tasks.pending.where(
project_id: project.id,
target_id: target.id,
target_type: target.class.name
)
user.tasks.pending.where(options)
end
def new_issuable(issuable, current_user)
project = issuable.project
target = issuable
author = issuable.author
if target.is_assigned? && target.assignee != current_user
create_task(project, target, author, target.assignee, Task::ASSIGNED)
end
mentioned_users = build_mentioned_users(project, target, author)
mentioned_users.delete(issuable.assignee)
mentioned_users.each do |mentioned_user|
create_task(project, target, author, mentioned_user, Task::MENTIONED)
end
end
def update_issuable(issuable, current_user)
project = issuable.project
author = current_user
mark_pending_tasks_as_done(issuable, author)
mentioned_users = build_mentioned_users(project, issuable, author)
mentioned_users.each do |mentioned_user|
unless pending_tasks(mentioned_user, project, issuable, action: Task::MENTIONED).exists?
create_task(project, issuable, author, mentioned_user, Task::MENTIONED)
end
end
end
def reassigned_issuable(issuable, current_user)
if issuable.assignee && issuable.assignee != current_user
create_task(issuable.project, issuable, current_user, issuable.assignee, Task::ASSIGNED)
end
end
end

34
spec/services/task_service_spec.rb
View File

@ -45,13 +45,6 @@ describe TaskService, services: true do
end
describe '#update_issue' do
it 'marks pending tasks to the issue for the user as done' do
pending_task = create(:task, :assigned, user: john_doe, project: project, target: issue, author: author)
service.update_issue(issue, john_doe)
expect(pending_task.reload).to be_done
end
it 'creates a task for each valid mentioned user' do
service.update_issue(issue, author)
@ -101,6 +94,18 @@ describe TaskService, services: true do
end
end
describe '#mark_pending_tasks_as_done' do
it 'marks related pending tasks to the target for the user as done' do
first_task = create(:task, :assigned, user: john_doe, project: project, target: issue, author: author)
second_task = create(:task, :assigned, user: john_doe, project: project, target: issue, author: author)
service.mark_pending_tasks_as_done(issue, john_doe)
expect(first_task.reload).to be_done
expect(second_task.reload).to be_done
end
end
describe '#new_note' do
let!(:first_task) { create(:task, :assigned, user: john_doe, project: project, target: issue, author: author) }
let!(:second_task) { create(:task, :assigned, user: john_doe, project: project, target: issue, author: author) }
@ -112,28 +117,28 @@ describe TaskService, services: true do
first_task = create(:task, :assigned, user: john_doe, project: project, target: issue, author: author)
second_task = create(:task, :assigned, user: john_doe, project: project, target: issue, author: author)
service.new_note(note)
service.new_note(note, john_doe)
expect(first_task.reload).to be_done
expect(second_task.reload).to be_done
end
it 'mark related pending tasks to the noteable for the award note author as done' do
service.new_note(award_note)
service.new_note(award_note, john_doe)
expect(first_task.reload).to be_done
expect(second_task.reload).to be_done
end
it 'does not mark related pending tasks it is a system note' do
service.new_note(system_note)
service.new_note(system_note, john_doe)
expect(first_task.reload).to be_pending
expect(second_task.reload).to be_pending
end
it 'creates a task for each valid mentioned user' do
service.new_note(note)
service.new_note(note, john_doe)
should_create_task(user: michael, target: issue, author: john_doe, action: Task::MENTIONED, note: note)
should_create_task(user: author, target: issue, author: john_doe, action: Task::MENTIONED, note: note)
@ -173,13 +178,6 @@ describe TaskService, services: true do
end
describe '#update_merge_request' do
it 'marks pending tasks to the merge request for the user as done' do
pending_task = create(:task, :assigned, user: john_doe, project: project, target: mr_assigned, author: author)
service.update_merge_request(mr_assigned, john_doe)
expect(pending_task.reload).to be_done
end
it 'creates a task for each valid mentioned user' do
service.update_merge_request(mr_assigned, author)