diff --git a/app/models/concerns/taskable.rb b/app/models/concerns/taskable.rb index 547afd5ee064f521666225848d19400df80d524d..f147ce8ad6b72eeb03e53cf7af39ec07d776cf8f 100644 --- a/app/models/concerns/taskable.rb +++ b/app/models/concerns/taskable.rb @@ -39,44 +39,6 @@ module Taskable end end - def self.toggle_task(content, content_html, index:, currently_checked:, line_source:, line_number:) - source_lines = content.split("\n") - markdown_task = source_lines[line_number - 1] - output = {} - - if markdown_task == line_source - html = Nokogiri::HTML.fragment(content_html) - html_checkbox = html.css('.task-list-item-checkbox')[index - 1] - # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first - updated_task = toggle_task_source(line_source, currently_checked: currently_checked) - - if html_checkbox && updated_task - source_lines[line_number - 1] = updated_task - - if currently_checked - html_checkbox.remove_attribute('checked') - else - html_checkbox[:checked] = 'checked' - end - - output[:content] = source_lines.join("\n") - output[:content_html] = html.to_html - end - end - - output - end - - def self.toggle_task_source(markdown_line, currently_checked:) - if source_checkbox = ITEM_PATTERN.match(markdown_line) - if TaskList::Item.new(source_checkbox[1]).complete? - markdown_line.sub(COMPLETE_PATTERN, '[ ]') if currently_checked - else - markdown_line.sub(INCOMPLETE_PATTERN, '[x]') unless currently_checked - end - end - end - # Called by `TaskList::Summary` def task_list_items return [] if description.blank? diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index cf6173da8d410ad8c2513538a19f06fa44d5b9d8..9c7b8fcc6abecdd74198b3a8f0a80d08423eb976 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -129,17 +129,17 @@ module Issues update_task_params = params.delete(:update_task) return unless update_task_params - updated_content = Taskable.toggle_task(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]) + 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]) - unless updated_content.empty? + if toggler.execute # by updating the description_html field at the same time, # the markdown cache won't be considered invalid - params[:description] = updated_content[:content] - params[:description_html] = updated_content[:content_html] + 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 diff --git a/app/services/task_list_toggle_service.rb b/app/services/task_list_toggle_service.rb new file mode 100644 index 0000000000000000000000000000000000000000..84a875dbaf75fe43d5fa28d97b58e79706704a0a --- /dev/null +++ b/app/services/task_list_toggle_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +# Finds the correct checkbox in the passed in markdown/html and toggles it's state, +# returning the updated markdown/html +# We don't care if the text has changed above or below the specific checkbox, as long +# the checkbox still exists at exactly the same line number and the text is equal +# If successful, new values are available in `updated_markdown` and `updated_markdown_html` +class TaskListToggleService + attr_reader :updated_markdown, :updated_markdown_html + + def initialize(markdown, markdown_html, index:, currently_checked:, line_source:, line_number:) + @markdown, @markdown_html = markdown, markdown_html + @index, @currently_checked = index, currently_checked + @line_source, @line_number = line_source, line_number + + @updated_markdown, @updated_markdown_html = nil + end + + def execute + return false unless markdown && markdown_html + + !!(toggle_markdown && toggle_html) + end + + private + + attr_reader :markdown, :markdown_html, :index, :currently_checked, :line_source, :line_number + + def toggle_markdown + source_lines = markdown.split("\n") + markdown_task = source_lines[line_number - 1] + + return unless markdown_task == line_source + return unless source_checkbox = Taskable::ITEM_PATTERN.match(markdown_task) + + if TaskList::Item.new(source_checkbox[1]).complete? + markdown_task.sub!(Taskable::COMPLETE_PATTERN, '[ ]') if currently_checked + else + markdown_task.sub!(Taskable::INCOMPLETE_PATTERN, '[x]') unless currently_checked + end + + source_lines[line_number - 1] = markdown_task + @updated_markdown = source_lines.join("\n") + end + + def toggle_html + html = Nokogiri::HTML.fragment(markdown_html) + html_checkbox = html.css('.task-list-item-checkbox')[index - 1] + # html_checkbox = html.css(".task-list-item[data-sourcepos^='#{changed_line_number}:'] > input.task-list-item-checkbox").first + return unless html_checkbox + + if currently_checked + html_checkbox.remove_attribute('checked') + else + html_checkbox[:checked] = 'checked' + end + + @updated_markdown_html = html.to_html + end +end diff --git a/spec/services/task_list_toggle_service_spec.rb b/spec/services/task_list_toggle_service_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..abc2701245204f928fc3110ef091c32cfc7a3f61 --- /dev/null +++ b/spec/services/task_list_toggle_service_spec.rb @@ -0,0 +1,78 @@ +require 'spec_helper' + +describe TaskListToggleService do + context 'when ' do + let(:markdown) { <<-EOT.strip_heredoc + * [ ] Task 1 + * [x] Task 2 + + A paragraph + + 1. [X] Item 1 + - [ ] Sub-item 1 + EOT + } + + 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> + EOT + } + + it 'checks Task 1' do + 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 + expect(toggler.updated_markdown.lines[0]).to eq "* [x] Task 1\n" + expect(toggler.updated_markdown_html).to include('disabled checked> Task 1') + end + + it 'unchecks Item 1' do + 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 + expect(toggler.updated_markdown.lines[5]).to eq "1. [ ] Item 1\n" + expect(toggler.updated_markdown_html).to include('disabled> Item 1') + 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, + 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, + 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, + line_source: '* [x] Task Added', line_number: 2) + + expect(toggler.execute).to be_falsey + end + end +end