Merge branch '208260-fix-mr-task-lists-starting-with-new-line' into 'master'

Fix errors when checking MR task lists

See merge request gitlab-org/gitlab!43125

app/assets/javascripts/task_list.js

@@ -31,6 +31,15 @@ export default class TaskList {
   init() {
     this.disable(); // Prevent duplicate event bindings
 
+    const taskListFields = document.querySelectorAll(
+      `${this.taskListContainerSelector} .js-task-list-field[data-value]`,
+    );
+
+    taskListFields.forEach(taskListField => {
+      // eslint-disable-next-line no-param-reassign
+      taskListField.value = taskListField.dataset.value;
+    });
+
     $(this.taskListContainerSelector).taskList('enable');
     $(document).on('tasklist:changed', this.taskListContainerSelector, this.updateHandler);
   }

app/views/projects/issues/show.html.haml

@@ -66,9 +66,8 @@
       .title-container
         %h2.title= markdown_field(@issue, :title)
       - if @issue.description.present?
-        .description{ class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '' }
+        .description
           .md= markdown_field(@issue, :description)
-          %textarea.hidden.js-task-list-field= @issue.description
 
       = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue-edited-ago js-issue-edited-ago')
 

app/views/projects/merge_requests/_description.html.haml

@@ -3,7 +3,6 @@
     .description.qa-description{ class: can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : '' }
       .md
         = markdown_field(@merge_request, :description)
-      %textarea.hidden.js-task-list-field
-        = @merge_request.description
+      %textarea.hidden.js-task-list-field{ data: { value: @merge_request.description } }
 
   = edited_time_ago_with_tooltip(@merge_request, placement: 'bottom')

app/views/shared/notes/_edit.html.haml

-%textarea.hidden.js-task-list-field.original-task-list{ data: { update_url: note_url(note) } }= note.note
+%textarea.hidden.js-task-list-field.original-task-list{ data: { update_url: note_url(note), value: note.note } }

app/views/shared/snippets/_header.html.haml

@@ -21,8 +21,6 @@
     .description
       .md
         = markdown_field(@snippet, :description)
-      %textarea.hidden.js-task-list-field
-        = @snippet.description
 
   - if @snippet.updated_at != @snippet.created_at
     = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', exclude_author: true)

changelogs/unreleased/208260-fix-mr-task-lists-starting-with-new-line.yml

+---
+title: Fix checking of task lists when MR description starts with a blank line
+merge_request: 43125
+author:
+type: fixed

spec/features/merge_request/user_views_open_merge_request_spec.rb

@@ -22,7 +22,7 @@ RSpec.describe 'User views an open merge request' do
       # returns the whole document, not the node's actual parent element
       expect(find(:xpath, "#{node.path}/..").text).to eq(merge_request.description[2..-1])
 
-      expect(page).to have_content(merge_request.title).and have_content(merge_request.description)
+      expect(page).to have_content(merge_request.title)
     end
   end
 

spec/features/projects/commit/user_comments_on_commit_spec.rb

@@ -6,19 +6,22 @@ RSpec.describe "User comments on commit", :js do
   include Spec::Support::Helpers::Features::NotesHelpers
   include RepoHelpers
 
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
   let(:comment_text) { "XML attached" }
 
-  before do
-    sign_in(user)
+  before_all do
     project.add_developer(user)
+  end
 
-    visit(project_commit_path(project, sample_commit.id))
+  before do
+    sign_in(user)
   end
 
   context "when adding new comment" do
     it "adds comment" do
+      visit(project_commit_path(project, sample_commit.id))
+
       emoji_code = ":+1:"
 
       page.within(".js-main-target-form") do
@@ -57,6 +60,8 @@ RSpec.describe "User comments on commit", :js do
 
   context "when editing comment" do
     before do
+      visit(project_commit_path(project, sample_commit.id))
+
       add_note(comment_text)
     end
 
@@ -87,6 +92,8 @@ RSpec.describe "User comments on commit", :js do
 
   context "when deleting comment" do
     before do
+      visit(project_commit_path(project, sample_commit.id))
+
       add_note(comment_text)
     end
 
@@ -108,4 +115,35 @@ RSpec.describe "User comments on commit", :js do
       expect(page).not_to have_css(".note")
     end
   end
+
+  context 'when checking task lists' do
+    let(:note_with_task) do
+      <<-EOT.strip_heredoc
+
+      - [ ] Task 1
+      EOT
+    end
+
+    before do
+      create(:note_on_commit, project: project, commit_id: sample_commit.id, note: note_with_task, author: user)
+      create(:note_on_commit, project: project, commit_id: sample_commit.id, note: note_with_task, author: user)
+
+      visit(project_commit_path(project, sample_commit.id))
+    end
+
+    it 'allows the tasks to be checked' do
+      expect(page).to have_selector('li.task-list-item', count: 2)
+      expect(page).to have_selector('li.task-list-item input[checked]', count: 0)
+
+      all('.task-list-item-checkbox').each do |checkbox|
+        checkbox.click
+      end
+      wait_for_requests
+
+      visit(project_commit_path(project, sample_commit.id))
+
+      expect(page).to have_selector('li.task-list-item', count: 2)
+      expect(page).to have_selector('li.task-list-item input[checked]', count: 2)
+    end
+  end
 end

spec/features/task_lists_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Task Lists' do
+RSpec.describe 'Task Lists', :js do
   include Warden::Test::Helpers
 
   let_it_be(:project) { create(:project, :public, :repository) }
@@ -38,41 +38,7 @@ RSpec.describe 'Task Lists' do
     MARKDOWN
   end
 
-  let(:nested_tasks_markdown) do
-    <<-EOT.strip_heredoc
-    - [ ] Task a
-      - [x] Task a.1
-      - [ ] Task a.2
-    - [ ] Task b
-
-    1. [ ] Task 1
-       1. [ ] Task 1.1
-       1. [x] Task 1.2
-    EOT
-  end
-
-  let(:commented_tasks_markdown) do
-    <<-EOT.strip_heredoc
-    <!--
-    - [ ] a
-    -->
-
-    - [ ] b
-    EOT
-  end
-
-  let(:summary_no_blank_line_markdown) do
-    <<-EOT.strip_heredoc
-    <details>
-    <summary>No blank line after summary element breaks task list</summary>
-    1. [ ] People Ops: do such and such
-    </details>
-
-    * [ ] Task 1
-    EOT
-  end
-
-  before(:all) do
+  before_all do
     project.add_maintainer(user)
     project.add_guest(user2)
   end
@@ -86,7 +52,7 @@ RSpec.describe 'Task Lists' do
   end
 
   describe 'for Issues' do
-    describe 'multiple tasks', :js do
+    describe 'multiple tasks' do
       let!(:issue) { create(:issue, description: markdown, author: user, project: project) }
 
       it 'renders' do
@@ -127,7 +93,7 @@ RSpec.describe 'Task Lists' do
       end
     end
 
-    describe 'single incomplete task', :js do
+    describe 'single incomplete task' do
       let!(:issue) { create(:issue, description: singleIncompleteMarkdown, author: user, project: project) }
 
       it 'renders' do
@@ -146,7 +112,7 @@ RSpec.describe 'Task Lists' do
       end
     end
 
-    describe 'single complete task', :js do
+    describe 'single complete task' do
       let!(:issue) { create(:issue, description: singleCompleteMarkdown, author: user, project: project) }
 
       it 'renders' do
@@ -175,7 +141,7 @@ RSpec.describe 'Task Lists' do
                       project: project, author: user)
       end
 
-      it 'renders for note body', :js do
+      it 'renders for note body' do
         visit_issue(project, issue)
 
         expect(page).to have_selector('.note ul.task-list',      count: 1)
@@ -183,14 +149,14 @@ RSpec.describe 'Task Lists' do
         expect(page).to have_selector('.note ul input[checked]', count: 2)
       end
 
-      it 'contains the required selectors', :js do
+      it 'contains the required selectors' do
         visit_issue(project, issue)
 
         expect(page).to have_selector('.note .js-task-list-container')
         expect(page).to have_selector('.note .js-task-list-container .task-list .task-list-item .task-list-item-checkbox')
       end
 
-      it 'is only editable by author', :js do
+      it 'is only editable by author' do
         visit_issue(project, issue)
 
         expect(page).to have_selector('.js-task-list-container')
@@ -209,7 +175,7 @@ RSpec.describe 'Task Lists' do
                       project: project, author: user)
       end
 
-      it 'renders for note body', :js do
+      it 'renders for note body' do
         visit_issue(project, issue)
 
         expect(page).to have_selector('.note ul.task-list',      count: 1)
@@ -224,7 +190,7 @@ RSpec.describe 'Task Lists' do
                       project: project, author: user)
       end
 
-      it 'renders for note body', :js do
+      it 'renders for note body' do
         visit_issue(project, issue)
 
         expect(page).to have_selector('.note ul.task-list',      count: 1)
@@ -240,7 +206,7 @@ RSpec.describe 'Task Lists' do
     end
 
     shared_examples 'multiple tasks' do
-      it 'renders for description', :js do
+      it 'renders for description' do
         visit_merge_request(project, merge)
         wait_for_requests
 
@@ -249,7 +215,7 @@ RSpec.describe 'Task Lists' do
         expect(page).to have_selector('ul input[checked]', count: 2)
       end
 
-      it 'contains the required selectors', :js do
+      it 'contains the required selectors' do
         visit_merge_request(project, merge)
         wait_for_requests
 
@@ -261,7 +227,7 @@ RSpec.describe 'Task Lists' do
         expect(page).to have_selector('form.js-issuable-update')
       end
 
-      it 'is only editable by author', :js do
+      it 'is only editable by author' do
         visit_merge_request(project, merge)
         wait_for_requests
 
@@ -300,7 +266,7 @@ RSpec.describe 'Task Lists' do
     describe 'single incomplete task' do
       let!(:merge) { create(:merge_request, :simple, description: singleIncompleteMarkdown, author: user, source_project: project) }
 
-      it 'renders for description', :js do
+      it 'renders for description' do
         visit_merge_request(project, merge)
         wait_for_requests
 
@@ -319,7 +285,7 @@ RSpec.describe 'Task Lists' do
     describe 'single complete task' do
       let!(:merge) { create(:merge_request, :simple, description: singleCompleteMarkdown, author: user, source_project: project) }
 
-      it 'renders for description', :js do
+      it 'renders for description' do
         visit_merge_request(project, merge)
         wait_for_requests
 
@@ -337,7 +303,17 @@ RSpec.describe 'Task Lists' do
   end
 
   describe 'markdown task edge cases' do
-    describe 'commented tasks', :js do
+    describe 'commented tasks' do
+      let(:commented_tasks_markdown) do
+        <<-EOT.strip_heredoc
+        <!--
+        - [ ] a
+        -->
+
+        - [ ] b
+        EOT
+      end
+
       let!(:issue) { create(:issue, description: commented_tasks_markdown, author: user, project: project) }
 
       it 'renders' do
@@ -360,7 +336,18 @@ RSpec.describe 'Task Lists' do
       end
     end
 
-    describe 'summary with no blank line', :js do
+    describe 'summary with no blank line' do
+      let(:summary_no_blank_line_markdown) do
+        <<-EOT.strip_heredoc
+        <details>
+        <summary>No blank line after summary element breaks task list</summary>
+        1. [ ] People Ops: do such and such
+        </details>
+
+        * [ ] Task 1
+        EOT
+      end
+
       let!(:issue) { create(:issue, description: summary_no_blank_line_markdown, author: user, project: project) }
 
       it 'renders' do
@@ -382,5 +369,31 @@ RSpec.describe 'Task Lists' do
         expect(page).to have_selector('ul input[checked]', count: 1)
       end
     end
+
+    describe 'markdown starting with new line character' do
+      let(:markdown_starting_with_new_line) do
+        <<-EOT.strip_heredoc
+
+        - [ ] Task 1
+        EOT
+      end
+
+      let(:merge_request) { create(:merge_request, description: markdown_starting_with_new_line, author: user, source_project: project) }
+
+      it 'allows the task to be checked' do
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page).to have_selector('ul input[checked]', count: 0)
+
+        find('.task-list-item-checkbox').click
+        wait_for_requests
+
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page).to have_selector('ul input[checked]', count: 1)
+      end
+    end
   end
 end