Specs for Issue::UpdateService

app/services/issuable_base_service.rb

@@ -267,10 +267,11 @@ class IssuableBaseService < BaseService
     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])
+                                        line_number: update_task_params[:line_number],
+                                        currently_checked: !update_task_params[:checked],
+                                        index: update_task_params[:index],
+                                        sourcepos: false)
     if toggler.execute
       # by updating the description_html field at the same time,

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
+# 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
+# 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`
+#
+# Note: once we've removed RedCarpet support, we can remove the `index` and `sourcepos`
+# parameters
 class TaskListToggleService
   attr_reader :updated_markdown, :updated_markdown_html
-  def initialize(markdown, markdown_html, index:, currently_checked:, line_source:, line_number:)
+  def initialize(markdown, markdown_html, line_source:, line_number:, currently_checked:, index:, sourcepos: false)
     @markdown, @markdown_html  = markdown, markdown_html
-    @index, @currently_checked = index, currently_checked
     @line_source, @line_number = line_source, line_number
+    @currently_checked         = currently_checked
+    @index, @use_sourcepos     = index, sourcepos
     @updated_markdown, @updated_markdown_html = nil
   end
@@ -24,7 +28,8 @@ class TaskListToggleService
   private
-  attr_reader :markdown, :markdown_html, :index, :currently_checked, :line_source, :line_number
+  attr_reader :markdown, :markdown_html, :index, :currently_checked
+  attr_reader :line_source, :line_number, :use_sourcepos
   def toggle_markdown
     source_lines  = markdown.split("\n")
@@ -45,8 +50,7 @@ class TaskListToggleService
   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
+    html_checkbox = get_html_checkbox(html)
     return unless html_checkbox
     if currently_checked
@@ -57,4 +61,12 @@ class TaskListToggleService
     @updated_markdown_html = html.to_html
   end
+
+  def get_html_checkbox(html)
+    if use_sourcepos
+      html.css(".task-list-item[data-sourcepos^='#{line_number}:'] > input.task-list-item-checkbox").first
+    else
+      html.css('.task-list-item-checkbox')[index - 1]
+    end
+  end
 end

spec/services/issues/update_service_spec.rb

@@ -543,6 +543,76 @@ describe Issues::UpdateService, :mailer do
       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
       let(:label3) { create(:label, project: project) }
       let(:result) { described_class.new(project, user, params).execute(issue).reload }