Enable fast task lists for merge requests
Allow single tasks to be updated quickly
This commit is contained in:
parent
ca154f0ff4
commit
79bd1b8717
|
@ -34,7 +34,8 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
|
||||||
:task_num,
|
:task_num,
|
||||||
:title,
|
:title,
|
||||||
:discussion_locked,
|
:discussion_locked,
|
||||||
label_ids: []
|
label_ids: [],
|
||||||
|
update_task: [:index, :checked, :line_number, :line_source]
|
||||||
]
|
]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -21,7 +21,7 @@ module MergeRequests
|
||||||
end
|
end
|
||||||
|
|
||||||
handle_wip_event(merge_request)
|
handle_wip_event(merge_request)
|
||||||
update(merge_request)
|
update_task_event(merge_request) || update(merge_request)
|
||||||
end
|
end
|
||||||
|
|
||||||
# rubocop:disable Metrics/AbcSize
|
# rubocop:disable Metrics/AbcSize
|
||||||
|
@ -83,6 +83,11 @@ module MergeRequests
|
||||||
end
|
end
|
||||||
# rubocop:enable Metrics/AbcSize
|
# rubocop:enable Metrics/AbcSize
|
||||||
|
|
||||||
|
def handle_task_changes(merge_request)
|
||||||
|
todo_service.mark_pending_todos_as_done(merge_request, current_user)
|
||||||
|
todo_service.update_merge_request(merge_request, current_user)
|
||||||
|
end
|
||||||
|
|
||||||
def merge_from_quick_action(merge_request)
|
def merge_from_quick_action(merge_request)
|
||||||
last_diff_sha = params.delete(:merge)
|
last_diff_sha = params.delete(:merge)
|
||||||
return unless merge_request.mergeable_with_quick_action?(current_user, last_diff_sha: last_diff_sha)
|
return unless merge_request.mergeable_with_quick_action?(current_user, last_diff_sha: last_diff_sha)
|
||||||
|
|
|
@ -32,7 +32,8 @@ class TaskListToggleService
|
||||||
source_line_index = line_number - 1
|
source_line_index = line_number - 1
|
||||||
markdown_task = source_lines[source_line_index]
|
markdown_task = source_lines[source_line_index]
|
||||||
|
|
||||||
return unless markdown_task == line_source
|
# The source in the DB could be using either \n or \r\n line endings
|
||||||
|
return unless markdown_task == line_source || markdown_task == line_source + "\r"
|
||||||
return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task)
|
return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task)
|
||||||
|
|
||||||
currently_checked = TaskList::Item.new(source_checkbox[1]).complete?
|
currently_checked = TaskList::Item.new(source_checkbox[1]).complete?
|
||||||
|
|
|
@ -471,6 +471,8 @@ describe Issues::UpdateService, :mailer do
|
||||||
|
|
||||||
it { expect(issue.tasks?).to eq(true) }
|
it { expect(issue.tasks?).to eq(true) }
|
||||||
|
|
||||||
|
it_behaves_like 'updating a single task'
|
||||||
|
|
||||||
context 'when tasks are marked as completed' do
|
context 'when tasks are marked as completed' do
|
||||||
before do
|
before do
|
||||||
update_issue(description: "- [x] Task 1\n- [X] Task 2")
|
update_issue(description: "- [x] Task 1\n- [X] Task 2")
|
||||||
|
@ -543,76 +545,6 @@ describe Issues::UpdateService, :mailer do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when updating a single task' do
|
|
||||||
before do
|
|
||||||
update_issue(description: "- [ ] Task 1\n- [ ] Task 2")
|
|
||||||
end
|
|
||||||
|
|
||||||
it { expect(issue.tasks?).to eq(true) }
|
|
||||||
|
|
||||||
context 'when a task is marked as completed' do
|
|
||||||
before do
|
|
||||||
update_issue(update_task: { index: 1, checked: true, line_source: '- [ ] Task 1', line_number: 1 })
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'creates system note about task status change' do
|
|
||||||
note1 = find_note('marked the task **Task 1** as completed')
|
|
||||||
|
|
||||||
expect(note1).not_to be_nil
|
|
||||||
|
|
||||||
description_notes = find_notes('description')
|
|
||||||
expect(description_notes.length).to eq(1)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when a task is marked as incomplete' do
|
|
||||||
before do
|
|
||||||
update_issue(description: "- [x] Task 1\n- [X] Task 2")
|
|
||||||
update_issue(update_task: { index: 2, checked: false, line_source: '- [X] Task 2', line_number: 2 })
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'creates system note about task status change' do
|
|
||||||
note1 = find_note('marked the task **Task 2** as incomplete')
|
|
||||||
|
|
||||||
expect(note1).not_to be_nil
|
|
||||||
|
|
||||||
description_notes = find_notes('description')
|
|
||||||
expect(description_notes.length).to eq(1)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when the task position has been modified' do
|
|
||||||
before do
|
|
||||||
update_issue(description: "- [ ] Task 1\n- [ ] Task 3\n- [ ] Task 2")
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'raises an exception' do
|
|
||||||
expect(Note.count).to eq(2)
|
|
||||||
expect do
|
|
||||||
update_issue(update_task: { index: 2, checked: true, line_source: '- [ ] Task 2', line_number: 2 })
|
|
||||||
end.to raise_error(ActiveRecord::StaleObjectError)
|
|
||||||
expect(Note.count).to eq(2)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'when the content changes but not task line number' do
|
|
||||||
before do
|
|
||||||
update_issue(description: "Paragraph\n\n- [ ] Task 1\n- [x] Task 2")
|
|
||||||
update_issue(description: "Paragraph with more words\n\n- [ ] Task 1\n- [x] Task 2")
|
|
||||||
update_issue(update_task: { index: 2, checked: false, line_source: '- [x] Task 2', line_number: 4 })
|
|
||||||
end
|
|
||||||
|
|
||||||
it 'creates system note about task status change' do
|
|
||||||
note1 = find_note('marked the task **Task 2** as incomplete')
|
|
||||||
|
|
||||||
expect(note1).not_to be_nil
|
|
||||||
|
|
||||||
description_notes = find_notes('description')
|
|
||||||
expect(description_notes.length).to eq(2)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
context 'updating labels' do
|
context 'updating labels' do
|
||||||
let(:label3) { create(:label, project: project) }
|
let(:label3) { create(:label, project: project) }
|
||||||
let(:result) { described_class.new(project, user, params).execute(issue).reload }
|
let(:result) { described_class.new(project, user, params).execute(issue).reload }
|
||||||
|
|
|
@ -466,6 +466,8 @@ describe MergeRequests::UpdateService, :mailer do
|
||||||
|
|
||||||
it { expect(@merge_request.tasks?).to eq(true) }
|
it { expect(@merge_request.tasks?).to eq(true) }
|
||||||
|
|
||||||
|
it_behaves_like 'updating a single task'
|
||||||
|
|
||||||
context 'when tasks are marked as completed' do
|
context 'when tasks are marked as completed' do
|
||||||
before do
|
before do
|
||||||
update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" })
|
update_merge_request({ description: "- [x] Task 1\n- [X] Task 2" })
|
||||||
|
|
|
@ -67,6 +67,17 @@ describe TaskListToggleService do
|
||||||
expect(toggler.execute).to be_falsey
|
expect(toggler.execute).to be_falsey
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'tolerates \r\n line endings' do
|
||||||
|
rn_markdown = markdown.gsub("\n", "\r\n")
|
||||||
|
toggler = described_class.new(rn_markdown, markdown_html,
|
||||||
|
toggle_as_checked: true,
|
||||||
|
line_source: '* [ ] Task 1', line_number: 1)
|
||||||
|
|
||||||
|
expect(toggler.execute).to be_truthy
|
||||||
|
expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\r\n"
|
||||||
|
expect(toggler.updated_markdown_html).to include('disabled checked> Task 1')
|
||||||
|
end
|
||||||
|
|
||||||
it 'returns false if markdown is nil' do
|
it 'returns false if markdown is nil' do
|
||||||
toggler = described_class.new(nil, markdown_html,
|
toggler = described_class.new(nil, markdown_html,
|
||||||
toggle_as_checked: false,
|
toggle_as_checked: false,
|
||||||
|
|
|
@ -36,3 +36,76 @@ shared_examples 'system notes for milestones' do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
shared_examples 'updating a single task' do
|
||||||
|
def update_issuable(opts)
|
||||||
|
issuable = try(:issue) || try(:merge_request)
|
||||||
|
described_class.new(project, user, opts).execute(issuable)
|
||||||
|
end
|
||||||
|
|
||||||
|
before do
|
||||||
|
update_issuable(description: "- [ ] Task 1\n- [ ] Task 2")
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when a task is marked as completed' do
|
||||||
|
before do
|
||||||
|
update_issuable(update_task: { index: 1, checked: true, line_source: '- [ ] Task 1', line_number: 1 })
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates system note about task status change' do
|
||||||
|
note1 = find_note('marked the task **Task 1** as completed')
|
||||||
|
|
||||||
|
expect(note1).not_to be_nil
|
||||||
|
|
||||||
|
description_notes = find_notes('description')
|
||||||
|
expect(description_notes.length).to eq(1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when a task is marked as incomplete' do
|
||||||
|
before do
|
||||||
|
update_issuable(description: "- [x] Task 1\n- [X] Task 2")
|
||||||
|
update_issuable(update_task: { index: 2, checked: false, line_source: '- [X] Task 2', line_number: 2 })
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates system note about task status change' do
|
||||||
|
note1 = find_note('marked the task **Task 2** as incomplete')
|
||||||
|
|
||||||
|
expect(note1).not_to be_nil
|
||||||
|
|
||||||
|
description_notes = find_notes('description')
|
||||||
|
expect(description_notes.length).to eq(1)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the task position has been modified' do
|
||||||
|
before do
|
||||||
|
update_issuable(description: "- [ ] Task 1\n- [ ] Task 3\n- [ ] Task 2")
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'raises an exception' do
|
||||||
|
expect(Note.count).to eq(2)
|
||||||
|
expect do
|
||||||
|
update_issuable(update_task: { index: 2, checked: true, line_source: '- [ ] Task 2', line_number: 2 })
|
||||||
|
end.to raise_error(ActiveRecord::StaleObjectError)
|
||||||
|
expect(Note.count).to eq(2)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when the content changes but not task line number' do
|
||||||
|
before do
|
||||||
|
update_issuable(description: "Paragraph\n\n- [ ] Task 1\n- [x] Task 2")
|
||||||
|
update_issuable(description: "Paragraph with more words\n\n- [ ] Task 1\n- [x] Task 2")
|
||||||
|
update_issuable(update_task: { index: 2, checked: false, line_source: '- [x] Task 2', line_number: 4 })
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates system note about task status change' do
|
||||||
|
note1 = find_note('marked the task **Task 2** as incomplete')
|
||||||
|
|
||||||
|
expect(note1).not_to be_nil
|
||||||
|
|
||||||
|
description_notes = find_notes('description')
|
||||||
|
expect(description_notes.length).to eq(2)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
Loading…
Reference in New Issue