Refactor and added specs

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?

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,
+      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

app/services/task_list_toggle_service.rb

+# 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

spec/services/task_list_toggle_service_spec.rb

+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