Commit 091b879c authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce

parents c35d3c43 761bf638
...@@ -161,4 +161,9 @@ module Issuable ...@@ -161,4 +161,9 @@ module Issuable
def notes_with_associations def notes_with_associations
notes.includes(:author, :project) notes.includes(:author, :project)
end end
def updated_tasks
Taskable.get_updated_tasks(old_content: previous_changes['description'].first,
new_content: description)
end
end end
...@@ -7,14 +7,39 @@ require 'task_list/filter' ...@@ -7,14 +7,39 @@ require 'task_list/filter'
# #
# Used by MergeRequest and Issue # Used by MergeRequest and Issue
module Taskable module Taskable
COMPLETED = 'completed'.freeze
INCOMPLETE = 'incomplete'.freeze
ITEM_PATTERN = /
^
(?:\s*[-+*]|(?:\d+\.))? # optional list prefix
\s* # optional whitespace prefix
(\[\s\]|\[[xX]\]) # checkbox
(\s.+) # followed by whitespace and some text.
/x
def self.get_tasks(content)
content.to_s.scan(ITEM_PATTERN).map do |checkbox, label|
# ITEM_PATTERN strips out the hyphen, but Item requires it. Rabble rabble.
TaskList::Item.new("- #{checkbox}", label.strip)
end
end
def self.get_updated_tasks(old_content:, new_content:)
old_tasks, new_tasks = get_tasks(old_content), get_tasks(new_content)
new_tasks.select.with_index do |new_task, i|
old_task = old_tasks[i]
next unless old_task
new_task.source == old_task.source && new_task.complete? != old_task.complete?
end
end
# Called by `TaskList::Summary` # Called by `TaskList::Summary`
def task_list_items def task_list_items
return [] if description.blank? return [] if description.blank?
@task_list_items ||= description.scan(TaskList::Filter::ItemPattern).collect do |item| @task_list_items ||= Taskable.get_tasks(description)
# ItemPattern strips out the hyphen, but Item requires it. Rabble rabble.
TaskList::Item.new("- #{item}")
end
end end
def tasks def tasks
......
...@@ -27,6 +27,12 @@ class IssuableBaseService < BaseService ...@@ -27,6 +27,12 @@ class IssuableBaseService < BaseService
old_branch, new_branch) old_branch, new_branch)
end end
def create_task_status_note(issuable)
issuable.updated_tasks.each do |task|
SystemNoteService.change_task_status(issuable, issuable.project, current_user, task)
end
end
def filter_params(issuable_ability_name = :issue) def filter_params(issuable_ability_name = :issue)
params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE
params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE
...@@ -47,14 +53,7 @@ class IssuableBaseService < BaseService ...@@ -47,14 +53,7 @@ class IssuableBaseService < BaseService
if params.present? && issuable.update_attributes(params.merge(updated_by: current_user)) if params.present? && issuable.update_attributes(params.merge(updated_by: current_user))
issuable.reset_events_cache issuable.reset_events_cache
handle_common_system_notes(issuable, old_labels: old_labels)
if issuable.labels != old_labels
create_labels_note(
issuable,
issuable.labels - old_labels,
old_labels - issuable.labels)
end
handle_changes(issuable) handle_changes(issuable)
issuable.create_new_cross_references!(current_user) issuable.create_new_cross_references!(current_user)
execute_hooks(issuable, 'update') execute_hooks(issuable, 'update')
...@@ -71,4 +70,19 @@ class IssuableBaseService < BaseService ...@@ -71,4 +70,19 @@ class IssuableBaseService < BaseService
close_service.new(project, current_user, {}).execute(issuable) close_service.new(project, current_user, {}).execute(issuable)
end end
end end
def handle_common_system_notes(issuable, options = {})
if issuable.previous_changes.include?('title')
create_title_change_note(issuable, issuable.previous_changes['title'].first)
end
if issuable.previous_changes.include?('description') && issuable.tasks?
create_task_status_note(issuable)
end
old_labels = options[:old_labels]
if old_labels && (issuable.labels != old_labels)
create_labels_note(issuable, issuable.labels - old_labels, old_labels - issuable.labels)
end
end
end end
...@@ -13,10 +13,6 @@ module Issues ...@@ -13,10 +13,6 @@ module Issues
create_assignee_note(issue) create_assignee_note(issue)
notification_service.reassigned_issue(issue, current_user) notification_service.reassigned_issue(issue, current_user)
end end
if issue.previous_changes.include?('title')
create_title_change_note(issue, issue.previous_changes['title'].first)
end
end end
def reopen_service def reopen_service
......
...@@ -30,10 +30,6 @@ module MergeRequests ...@@ -30,10 +30,6 @@ module MergeRequests
notification_service.reassigned_merge_request(merge_request, current_user) notification_service.reassigned_merge_request(merge_request, current_user)
end end
if merge_request.previous_changes.include?('title')
create_title_change_note(merge_request, merge_request.previous_changes['title'].first)
end
if merge_request.previous_changes.include?('target_branch') || if merge_request.previous_changes.include?('target_branch') ||
merge_request.previous_changes.include?('source_branch') merge_request.previous_changes.include?('source_branch')
merge_request.mark_as_unchecked merge_request.mark_as_unchecked
......
...@@ -341,4 +341,22 @@ class SystemNoteService ...@@ -341,4 +341,22 @@ class SystemNoteService
"* #{commit_ids} - #{commits_text} from branch `#{branch}`\n" "* #{commit_ids} - #{commits_text} from branch `#{branch}`\n"
end end
# Called when the status of a Task has changed
#
# noteable - Noteable object.
# project - Project owning noteable
# author - User performing the change
# new_task - TaskList::Item object.
#
# Example Note text:
#
# "Soandso marked the task Whatever as completed."
#
# Returns the created Note object
def self.change_task_status(noteable, project, author, new_task)
status_label = new_task.complete? ? Taskable::COMPLETED : Taskable::INCOMPLETE
body = "Marked the task **#{new_task.source}** as #{status_label}"
create_note(noteable: noteable, project: project, author: author, note: body)
end
end end
...@@ -15,6 +15,17 @@ describe Issues::UpdateService do ...@@ -15,6 +15,17 @@ describe Issues::UpdateService do
end end
describe 'execute' do describe 'execute' do
def find_note(starting_with)
@issue.notes.find do |note|
note && note.note.start_with?(starting_with)
end
end
def update_issue(opts)
@issue = Issues::UpdateService.new(project, user, opts).execute(issue)
@issue.reload
end
context "valid params" do context "valid params" do
before do before do
opts = { opts = {
...@@ -44,12 +55,6 @@ describe Issues::UpdateService do ...@@ -44,12 +55,6 @@ describe Issues::UpdateService do
expect(email.subject).to include(issue.title) expect(email.subject).to include(issue.title)
end end
def find_note(starting_with)
@issue.notes.find do |note|
note && note.note.start_with?(starting_with)
end
end
it 'should create system note about issue reassign' do it 'should create system note about issue reassign' do
note = find_note('Reassigned to') note = find_note('Reassigned to')
...@@ -71,5 +76,71 @@ describe Issues::UpdateService do ...@@ -71,5 +76,71 @@ describe Issues::UpdateService do
expect(note.note).to eq 'Title changed from **Old title** to **New title**' expect(note.note).to eq 'Title changed from **Old title** to **New title**'
end end
end end
context 'when Issue has tasks' do
before { update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" }) }
it { expect(@issue.tasks?).to eq(true) }
context 'when tasks are marked as completed' do
before { update_issue({ description: "- [x] Task 1\n- [X] Task 2" }) }
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as completed')
note2 = find_note('Marked the task **Task 2** as completed')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
end
end
context 'when tasks are marked as incomplete' do
before do
update_issue({ description: "- [x] Task 1\n- [X] Task 2" })
update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" })
end
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as incomplete')
note2 = find_note('Marked the task **Task 2** as incomplete')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
end
end
context 'when tasks position has been modified' do
before do
update_issue({ description: "- [x] Task 1\n- [X] Task 2" })
update_issue({ description: "- [x] Task 1\n- [ ] Task 3\n- [ ] Task 2" })
end
it 'does not create a system note' do
note = find_note('Marked the task **Task 2** as incomplete')
expect(note).to be_nil
end
end
context 'when a Task list with a completed item is totally replaced' do
before do
update_issue({ description: "- [ ] Task 1\n- [X] Task 2" })
update_issue({ description: "- [ ] One\n- [ ] Two\n- [ ] Three" })
end
it 'does not create a system note referencing the position the old item' do
note = find_note('Marked the task **Two** as incomplete')
expect(note).to be_nil
end
it 'should not generate a new note at all' do
expect do
update_issue({ description: "- [ ] One\n- [ ] Two\n- [ ] Three" })
end.not_to change { Note.count }
end
end
end
end end
end end
...@@ -14,6 +14,17 @@ describe MergeRequests::UpdateService do ...@@ -14,6 +14,17 @@ describe MergeRequests::UpdateService do
end end
describe 'execute' do describe 'execute' do
def find_note(starting_with)
@merge_request.notes.find do |note|
note && note.note.start_with?(starting_with)
end
end
def update_merge_request(opts)
@merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
@merge_request.reload
end
context 'valid params' do context 'valid params' do
let(:opts) do let(:opts) do
{ {
...@@ -56,12 +67,6 @@ describe MergeRequests::UpdateService do ...@@ -56,12 +67,6 @@ describe MergeRequests::UpdateService do
expect(email.subject).to include(merge_request.title) expect(email.subject).to include(merge_request.title)
end end
def find_note(starting_with)
@merge_request.notes.find do |note|
note && note.note.start_with?(starting_with)
end
end
it 'should create system note about merge_request reassign' do it 'should create system note about merge_request reassign' do
note = find_note('Reassigned to') note = find_note('Reassigned to')
...@@ -90,5 +95,39 @@ describe MergeRequests::UpdateService do ...@@ -90,5 +95,39 @@ describe MergeRequests::UpdateService do
expect(note.note).to eq 'Target branch changed from `master` to `target`' expect(note.note).to eq 'Target branch changed from `master` to `target`'
end end
end end
context 'when MergeRequest has tasks' do
before { update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" }) }
it { expect(@merge_request.tasks?).to eq(true) }
context 'when tasks are marked as completed' do
before { update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" }) }
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as completed')
note2 = find_note('Marked the task **Task 2** as completed')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
end
end
context 'when tasks are marked as incomplete' do
before do
update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" })
update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" })
end
it 'creates system note about task status change' do
note1 = find_note('Marked the task **Task 1** as incomplete')
note2 = find_note('Marked the task **Task 2** as incomplete')
expect(note1).not_to be_nil
expect(note2).not_to be_nil
end
end
end
end end
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