Commit ec66cf0a authored by Brett Walker's avatar Brett Walker Committed by Fatih Acet

Raise exception if we can't match the update_task

and some additional refactoring
parent 56506ff8
......@@ -238,6 +238,7 @@ class IssuableBaseService < BaseService
def update_task(issuable)
filter_params(issuable)
# old_associations = associations_before_update(issuable)
if issuable.changed? || params.present?
issuable.assign_attributes(params.merge(updated_by: current_user))
issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user)
......@@ -263,6 +264,36 @@ class IssuableBaseService < BaseService
issuable
end
# Handle the `update_task` event sent from UI. Attempts to update a specific
# line in the markdown and cached html, bypassing any unnecessary updates or checks.
def update_task_event(issue)
update_task_params = params.delete(:update_task)
return unless update_task_params
toggler = TaskListToggleService.new(issue.description, issue.description_html,
index: update_task_params[:index],
currently_checked: !update_task_params[:checked],
line_source: update_task_params[:line_source],
line_number: update_task_params[:line_number])
if toggler.execute
# by updating the description_html field at the same time,
# the markdown cache won't be considered invalid
params[:description] = toggler.updated_markdown
params[:description_html] = toggler.updated_markdown_html
# since we're updating a very specific line, we don't care whether
# the `lock_version` sent from the FE is the same or not. Just
# make sure the data hasn't changed since we queried it
params[:lock_version] = issue.lock_version
update_task(issue)
else
# if we make it here, the data is much newer than we thought it was - fail fast
raise ActiveRecord::StaleObjectError
end
end
def labels_changing?(old_label_ids, new_label_ids)
old_label_ids.sort != new_label_ids.sort
end
......
......@@ -8,7 +8,7 @@ module Issues
handle_move_between_ids(issue)
filter_spam_check_params
change_issue_duplicate(issue)
move_issue_to_new_project(issue) || task_change_event(issue) || update(issue)
move_issue_to_new_project(issue) || update_task_event(issue) || update(issue)
end
def update(issue)
......@@ -125,31 +125,6 @@ module Issues
end
end
def task_change_event(issue)
update_task_params = params.delete(:update_task)
return unless update_task_params
toggler = TaskListToggleService.new(issue.description, issue.description_html,
index: update_task_params[:index],
currently_checked: !update_task_params[:checked],
line_source: update_task_params[:line_source],
line_number: update_task_params[:line_number])
if toggler.execute
# by updating the description_html field at the same time,
# the markdown cache won't be considered invalid
params[:description] = toggler.updated_markdown
params[:description_html] = toggler.updated_markdown_html
# since we're updating a very specific line, we don't care whether
# the `lock_version` sent from the FE is the same or not. Just
# make sure the data hasn't changed since we queried it
params[:lock_version] = issue.lock_version
update_task(issue)
end
end
# rubocop: disable CodeReuse/ActiveRecord
def get_issue_if_allowed(id, board_group_id = nil)
return unless id
......
......@@ -2,7 +2,8 @@ require 'spec_helper'
describe TaskListToggleService do
context 'when ' do
let(:markdown) { <<-EOT.strip_heredoc
let(:markdown) do
<<-EOT.strip_heredoc
* [ ] Task 1
* [x] Task 2
......@@ -11,33 +12,35 @@ describe TaskListToggleService do
1. [X] Item 1
- [ ] Sub-item 1
EOT
}
end
let(:markdown_html) { <<-EOT.strip_heredoc
<ul class="task-list" dir="auto">
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
</li>
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
</li>
</ul>
<p dir="auto">A paragraph</p>
<ol class="task-list" dir="auto">
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
<ul class="task-list">
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
</li>
</ul>
</li>
</ol>
let(:markdown_html) do
<<-EOT.strip_heredoc
<ul class="task-list" dir="auto">
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled> Task 1
</li>
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled checked> Task 2
</li>
</ul>
<p dir="auto">A paragraph</p>
<ol class="task-list" dir="auto">
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled checked> Item 1
<ul class="task-list">
<li class="task-list-item">
<input type="checkbox" class="task-list-item-checkbox" disabled> Sub-item 1
</li>
</ul>
</li>
</ol>
EOT
}
end
it 'checks Task 1' do
toggler = described_class.new(markdown, markdown_html, index: 1, currently_checked: false,
toggler = described_class.new(markdown, markdown_html,
index: 1, currently_checked: false,
line_source: '* [ ] Task 1', line_number: 1)
expect(toggler.execute).to be_truthy
......@@ -46,7 +49,8 @@ describe TaskListToggleService do
end
it 'unchecks Item 1' do
toggler = described_class.new(markdown, markdown_html, index: 3, currently_checked: true,
toggler = described_class.new(markdown, markdown_html,
index: 3, currently_checked: true,
line_source: '1. [X] Item 1', line_number: 6)
expect(toggler.execute).to be_truthy
......@@ -55,21 +59,24 @@ describe TaskListToggleService do
end
it 'returns false if line_source does not match the text' do
toggler = described_class.new(markdown, markdown_html, index: 2, currently_checked: true,
toggler = described_class.new(markdown, markdown_html,
index: 2, currently_checked: true,
line_source: '* [x] Task Added', line_number: 2)
expect(toggler.execute).to be_falsey
end
it 'returns false if markdown is nil' do
toggler = described_class.new(nil, markdown_html, index: 2, currently_checked: true,
toggler = described_class.new(nil, markdown_html,
index: 2, currently_checked: true,
line_source: '* [x] Task Added', line_number: 2)
expect(toggler.execute).to be_falsey
end
it 'returns false if markdown_html is nil' do
toggler = described_class.new(markdown, nil, index: 2, currently_checked: true,
toggler = described_class.new(markdown, nil,
index: 2, currently_checked: true,
line_source: '* [x] Task Added', line_number: 2)
expect(toggler.execute).to be_falsey
......
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