Merge branch 'rs-task_list' into 'master'

Use task_list gem for task lists

Task Lists can now be used in comments, and they'll render in previews. :clap:

Closes internal https://dev.gitlab.org/gitlab/gitlabhq/issues/2271

See merge request !599

10 files changed, 287 insertions, 139 deletions

diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb
index 57d01236505..1746ce128e4 100644
--- a/spec/features/markdown_spec.rb
+++ b/spec/features/markdown_spec.rb
@@ -24,6 +24,7 @@ require 'erb'
 #               -> Rinku (http, https, ftp)
 #               -> Other schemes
 #             -> References
+#             -> TaskList
 #           -> `html_safe`
 #           -> Template
 #
@@ -279,6 +280,15 @@ describe 'GitLab Markdown' do
         expect(body).to have_selector('a.gfm.gfm-label', count: 3)
       end
     end
+
+    describe 'Task Lists' do
+      it 'generates task lists' do
+        body = get_section('task-lists')
+        expect(body).to have_selector('ul.task-list', count: 2)
+        expect(body).to have_selector('li.task-list-item', count: 7)
+        expect(body).to have_selector('input[checked]', count: 3)
+      end
+    end
   end
 end
 
@@ -289,9 +299,8 @@ end
 # once. Unfortunately RSpec will not let you access `let`s in a `before(:all)`
 # block, so we fake it by encapsulating all the shared setup in this class.
 #
-# The class contains the raw Markup used in the test, dynamically substituting
-# real objects, created from factories and setup on-demand, when referenced in
-# the Markdown.
+# The class renders `spec/fixtures/markdown.md.erb` using ERB, allowing for
+# reference to the factory-created objects.
 class MarkdownFeature
   include FactoryGirl::Syntax::Methods
 
diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb
new file mode 100644
index 00000000000..2099fc40cca
--- /dev/null
+++ b/spec/features/task_lists_spec.rb
@@ -0,0 +1,151 @@
+require 'spec_helper'
+
+feature 'Task Lists' do
+  include Warden::Test::Helpers
+
+  let(:project) { create(:project) }
+  let(:user)    { create(:user) }
+  let(:user2)   { create(:user) }
+
+  let(:markdown) do
+    <<-MARKDOWN.strip_heredoc
+    This is a task list:
+
+    - [ ] Incomplete entry 1
+    - [x] Complete entry 1
+    - [ ] Incomplete entry 2
+    - [x] Complete entry 2
+    - [ ] Incomplete entry 3
+    - [ ] Incomplete entry 4
+    MARKDOWN
+  end
+
+  before do
+    Warden.test_mode!
+
+    project.team << [user, :master]
+    project.team << [user2, :guest]
+
+    login_as(user)
+  end
+
+  def visit_issue(project, issue)
+    visit namespace_project_issue_path(project.namespace, project, issue)
+  end
+
+  describe 'for Issues' do
+    let!(:issue) { create(:issue, description: markdown, author: user, project: project) }
+
+    it 'renders' do
+      visit_issue(project, issue)
+
+      expect(page).to have_selector('ul.task-list',      count: 1)
+      expect(page).to have_selector('li.task-list-item', count: 6)
+      expect(page).to have_selector('ul input[checked]', count: 2)
+    end
+
+    it 'contains the required selectors' do
+      visit_issue(project, issue)
+
+      container = '.issue-details .description.js-task-list-container'
+
+      expect(page).to have_selector(container)
+      expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox")
+      expect(page).to have_selector("#{container} .js-task-list-field")
+      expect(page).to have_selector('form.js-issue-update')
+      expect(page).to have_selector('a.btn-close')
+    end
+
+    it 'is only editable by author' do
+      visit_issue(project, issue)
+      expect(page).to have_selector('.js-task-list-container')
+
+      logout(:user)
+
+      login_as(user2)
+      visit current_path
+      expect(page).not_to have_selector('.js-task-list-container')
+    end
+
+    it 'provides a summary on Issues#index' do
+      visit namespace_project_issues_path(project.namespace, project)
+      expect(page).to have_content("6 tasks (2 completed, 4 remaining)")
+    end
+  end
+
+  describe 'for Notes' do
+    let!(:issue) { create(:issue, author: user, project: project) }
+    let!(:note)  { create(:note, note: markdown, noteable: issue, author: user) }
+
+    it 'renders for note body' do
+      visit_issue(project, issue)
+
+      expect(page).to have_selector('.note ul.task-list',      count: 1)
+      expect(page).to have_selector('.note li.task-list-item', count: 6)
+      expect(page).to have_selector('.note ul input[checked]', count: 2)
+    end
+
+    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')
+      expect(page).to have_selector('.note .js-task-list-container .js-task-list-field')
+    end
+
+    it 'is only editable by author' do
+      visit_issue(project, issue)
+      expect(page).to have_selector('.js-task-list-container')
+
+      logout(:user)
+
+      login_as(user2)
+      visit current_path
+      expect(page).not_to have_selector('.js-task-list-container')
+    end
+  end
+
+  describe 'for Merge Requests' do
+    def visit_merge_request(project, merge)
+      visit namespace_project_merge_request_path(project.namespace, project, merge)
+    end
+
+    let!(:merge) { create(:merge_request, :simple, description: markdown, author: user, source_project: project) }
+
+    it 'renders for description' do
+      visit_merge_request(project, merge)
+
+      expect(page).to have_selector('ul.task-list',      count: 1)
+      expect(page).to have_selector('li.task-list-item', count: 6)
+      expect(page).to have_selector('ul input[checked]', count: 2)
+    end
+
+    it 'contains the required selectors' do
+      visit_merge_request(project, merge)
+
+      container = '.merge-request-details .description.js-task-list-container'
+
+      expect(page).to have_selector(container)
+      expect(page).to have_selector("#{container} .wiki .task-list .task-list-item .task-list-item-checkbox")
+      expect(page).to have_selector("#{container} .js-task-list-field")
+      expect(page).to have_selector('form.js-merge-request-update')
+      expect(page).to have_selector('a.btn-close')
+    end
+
+    it 'is only editable by author' do
+      visit_merge_request(project, merge)
+      expect(page).to have_selector('.js-task-list-container')
+
+      logout(:user)
+
+      login_as(user2)
+      visit current_path
+      expect(page).not_to have_selector('.js-task-list-container')
+    end
+
+    it 'provides a summary on MergeRequests#index' do
+      visit namespace_project_merge_requests_path(project.namespace, project)
+      expect(page).to have_content("6 tasks (2 completed, 4 remaining)")
+    end
+  end
+end
diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb
index 6b79aad8f86..bc023ecf793 100644
--- a/spec/fixtures/markdown.md.erb
+++ b/spec/fixtures/markdown.md.erb
@@ -184,3 +184,13 @@ References should be parseable even inside _!<%= merge_request.iid %>_ emphasis.
 - Label by name in quotes: ~"<%= label.name %>"
 - Ignored in code: `~<%= simple_label.name %>`
 - Ignored in links: [Link to ~<%= simple_label.id %>](#label-link)
+
+### Task Lists
+
+- [ ] Incomplete task 1
+- [x] Complete task 1
+- [ ] Incomplete task 2
+  - [ ] Incomplete sub-task 1
+  - [ ] Incomplete sub-task 2
+  - [x] Complete sub-task 1
+- [X] Complete task 2
diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb
index b6be82e4109..2f67879efdc 100644
--- a/spec/helpers/gitlab_markdown_helper_spec.rb
+++ b/spec/helpers/gitlab_markdown_helper_spec.rb
@@ -43,115 +43,6 @@ describe GitlabMarkdownHelper do
         expect(gfm(actual)).to match(expected)
       end
     end
-
-    context 'parse_tasks: true' do
-      before(:all) do
-        @source_text_asterisk = <<-EOT.strip_heredoc
-          * [ ] valid unchecked task
-          * [x] valid lowercase checked task
-          * [X] valid uppercase checked task
-              * [ ] valid unchecked nested task
-              * [x] valid checked nested task
-
-          [ ] not an unchecked task - no list item
-          [x] not a checked task - no list item
-
-          * [  ] not an unchecked task - too many spaces
-          * [x ] not a checked task - too many spaces
-          * [] not an unchecked task - no spaces
-          * Not a task [ ] - not at beginning
-        EOT
-
-        @source_text_dash = <<-EOT.strip_heredoc
-          - [ ] valid unchecked task
-          - [x] valid lowercase checked task
-          - [X] valid uppercase checked task
-              - [ ] valid unchecked nested task
-              - [x] valid checked nested task
-        EOT
-      end
-
-      it 'should render checkboxes at beginning of asterisk list items' do
-        rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
-
-        expect(rendered_text).to match(/<input.*checkbox.*valid unchecked task/)
-        expect(rendered_text).to match(
-          /<input.*checkbox.*valid lowercase checked task/
-        )
-        expect(rendered_text).to match(
-          /<input.*checkbox.*valid uppercase checked task/
-        )
-      end
-
-      it 'should render checkboxes at beginning of dash list items' do
-        rendered_text = markdown(@source_text_dash, parse_tasks: true)
-
-        expect(rendered_text).to match(/<input.*checkbox.*valid unchecked task/)
-        expect(rendered_text).to match(
-          /<input.*checkbox.*valid lowercase checked task/
-        )
-        expect(rendered_text).to match(
-          /<input.*checkbox.*valid uppercase checked task/
-        )
-      end
-
-      it 'should render checkboxes for nested tasks' do
-        rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
-
-        expect(rendered_text).to match(
-          /<input.*checkbox.*valid unchecked nested task/
-        )
-        expect(rendered_text).to match(
-          /<input.*checkbox.*valid checked nested task/
-        )
-      end
-
-      it 'should not be confused by whitespace before bullets' do
-        rendered_text_asterisk = markdown(@source_text_asterisk, parse_tasks: true)
-        rendered_text_dash = markdown(@source_text_dash, parse_tasks: true)
-
-        expect(rendered_text_asterisk).to match(
-          /<input.*checkbox.*valid unchecked nested task/
-        )
-        expect(rendered_text_asterisk).to match(
-          /<input.*checkbox.*valid checked nested task/
-        )
-        expect(rendered_text_dash).to match(
-          /<input.*checkbox.*valid unchecked nested task/
-        )
-        expect(rendered_text_dash).to match(
-          /<input.*checkbox.*valid checked nested task/
-        )
-      end
-
-      it 'should not render checkboxes outside of list items' do
-        rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
-
-        expect(rendered_text).not_to match(
-          /<input.*checkbox.*not an unchecked task - no list item/
-        )
-        expect(rendered_text).not_to match(
-          /<input.*checkbox.*not a checked task - no list item/
-        )
-      end
-
-      it 'should not render checkboxes with invalid formatting' do
-        rendered_text = markdown(@source_text_asterisk, parse_tasks: true)
-
-        expect(rendered_text).not_to match(
-          /<input.*checkbox.*not an unchecked task - too many spaces/
-        )
-        expect(rendered_text).not_to match(
-          /<input.*checkbox.*not a checked task - too many spaces/
-        )
-        expect(rendered_text).not_to match(
-          /<input.*checkbox.*not an unchecked task - no spaces/
-        )
-        expect(rendered_text).not_to match(
-          /Not a task.*<input.*checkbox.*not at beginning/
-        )
-      end
-    end
   end
 
   describe '#link_to_gfm' do
diff --git a/spec/javascripts/helpers/.gitkeep b/spec/javascripts/helpers/.gitkeep
deleted file mode 100644
index e69de29bb2d..00000000000
--- a/spec/javascripts/helpers/.gitkeep
+++ /dev/null
diff --git a/spec/javascripts/issue_spec.js.coffee b/spec/javascripts/issue_spec.js.coffee
new file mode 100644
index 00000000000..13b25862f57
--- /dev/null
+++ b/spec/javascripts/issue_spec.js.coffee
@@ -0,0 +1,36 @@
+#= require jquery
+#= require jasmine-fixture
+#= require issue
+
+describe 'Issue', ->
+  describe 'task lists', ->
+    selectors = {
+      container: '.issue-details .description.js-task-list-container'
+      item:      '.wiki ul.task-list li.task-list-item input.task-list-item-checkbox[type=checkbox] {Task List Item}'
+      textarea:  '.wiki textarea.js-task-list-field{- [ ] Task List Item}'
+      form:      'form.js-issue-update[action="/foo"]'
+      close:     'a.btn-close'
+    }
+
+    beforeEach ->
+      $container = affix(selectors.container)
+
+      # # These two elements are siblings inside the container
+      $container.find('.js-task-list-container').append(affix(selectors.item))
+      $container.find('.js-task-list-container').append(affix(selectors.textarea))
+
+      # Task lists don't get initialized unless this button exists. Not ideal.
+      $container.append(affix(selectors.close))
+
+      # This form is used to get the `update` URL. Not ideal.
+      $container.append(affix(selectors.form))
+
+      @issue = new Issue()
+
+    it 'submits an ajax request on tasklist:changed', ->
+      spyOn($, 'ajax').and.callFake (req) ->
+        expect(req.type).toBe('PATCH')
+        expect(req.url).toBe('/foo')
+        expect(req.data.issue.description).not.toBe(null)
+
+      $('.js-task-list-field').trigger('tasklist:changed')
diff --git a/spec/javascripts/merge_request_spec.js.coffee b/spec/javascripts/merge_request_spec.js.coffee
new file mode 100644
index 00000000000..3ebc4a4eed5
--- /dev/null
+++ b/spec/javascripts/merge_request_spec.js.coffee
@@ -0,0 +1,36 @@
+#= require jquery
+#= require jasmine-fixture
+#= require merge_request
+
+describe 'MergeRequest', ->
+  describe 'task lists', ->
+    selectors = {
+      container: '.merge-request-details .description.js-task-list-container'
+      item:      '.wiki ul.task-list li.task-list-item input.task-list-item-checkbox[type=checkbox] {Task List Item}'
+      textarea:  '.wiki textarea.js-task-list-field{- [ ] Task List Item}'
+      form:      'form.js-merge-request-update[action="/foo"]'
+      close:     'a.btn-close'
+    }
+
+    beforeEach ->
+      $container = affix(selectors.container)
+
+      # # These two elements are siblings inside the container
+      $container.find('.js-task-list-container').append(affix(selectors.item))
+      $container.find('.js-task-list-container').append(affix(selectors.textarea))
+
+      # Task lists don't get initialized unless this button exists. Not ideal.
+      $container.append(affix(selectors.close))
+
+      # This form is used to get the `update` URL. Not ideal.
+      $container.append(affix(selectors.form))
+
+      @merge = new MergeRequest({})
+
+    it 'submits an ajax request on tasklist:changed', ->
+      spyOn($, 'ajax').and.callFake (req) ->
+        expect(req.type).toBe('PATCH')
+        expect(req.url).toBe('/foo')
+        expect(req.data.merge_request.description).not.toBe(null)
+
+      $('.js-task-list-field').trigger('tasklist:changed')
diff --git a/spec/javascripts/notes_spec.js.coffee b/spec/javascripts/notes_spec.js.coffee
new file mode 100644
index 00000000000..de2e8e7f6c8
--- /dev/null
+++ b/spec/javascripts/notes_spec.js.coffee
@@ -0,0 +1,30 @@
+#= require jquery
+#= require jasmine-fixture
+#= require notes
+
+window.gon = {}
+window.disableButtonIfEmptyField = -> null
+
+describe 'Notes', ->
+  describe 'task lists', ->
+    selectors = {
+      container: 'li.note .js-task-list-container'
+      item:      '.note-text ul.task-list li.task-list-item input.task-list-item-checkbox[type=checkbox] {Task List Item}'
+      textarea:  '.note-edit-form form textarea.js-task-list-field{- [ ] Task List Item}'
+    }
+
+    beforeEach ->
+      $container = affix(selectors.container)
+
+      # These two elements are siblings inside the container
+      $container.find('.js-task-list-container').append(affix(selectors.item))
+      $container.find('.js-task-list-container').append(affix(selectors.textarea))
+
+      @notes = new Notes()
+
+    it 'submits the form on tasklist:changed', ->
+      submitted = false
+      $('form').on 'submit', (e) -> submitted = true; e.preventDefault()
+
+      $('.js-task-list-field').trigger('tasklist:changed')
+      expect(submitted).toBe(true)
diff --git a/spec/javascripts/support/jasmine.yml b/spec/javascripts/support/jasmine.yml
index f4b01c9f27a..168c9618643 100644
--- a/spec/javascripts/support/jasmine.yml
+++ b/spec/javascripts/support/jasmine.yml
@@ -9,11 +9,6 @@
 # defaults to spec/javascripts
 spec_dir: spec/javascripts
 
-# list of file expressions to include as helpers into spec runner
-# relative path from spec_dir
-helpers:
-  - "helpers/**/*.{js.coffee,js,coffee}"
-
 # list of file expressions to include as specs into spec runner
 # relative path from spec_dir
 spec_files:
diff --git a/spec/support/taskable_shared_examples.rb b/spec/support/taskable_shared_examples.rb
index 8e5e3a8aafc..927c72c7409 100644
--- a/spec/support/taskable_shared_examples.rb
+++ b/spec/support/taskable_shared_examples.rb
@@ -4,39 +4,29 @@
 #   subject { Issue or MergeRequest }
 shared_examples 'a Taskable' do
   before do
-    subject.description = <<EOT.gsub(/ {6}/, '')
+    subject.description = <<-EOT.strip_heredoc
       * [ ] Task 1
       * [x] Task 2
       * [x] Task 3
       * [ ] Task 4
       * [ ] Task 5
-EOT
-  end
-
-  it 'updates the Nth task correctly' do
-    subject.update_nth_task(1, true)
-    expect(subject.description).to match(/\[x\] Task 1/)
-
-    subject.update_nth_task(2, true)
-    expect(subject.description).to match('\[x\] Task 2')
-
-    subject.update_nth_task(3, false)
-    expect(subject.description).to match('\[ \] Task 3')
-
-    subject.update_nth_task(4, false)
-    expect(subject.description).to match('\[ \] Task 4')
+    EOT
   end
 
   it 'returns the correct task status' do
     expect(subject.task_status).to match('5 tasks')
-    expect(subject.task_status).to match('2 done')
-    expect(subject.task_status).to match('3 unfinished')
+    expect(subject.task_status).to match('2 completed')
+    expect(subject.task_status).to match('3 remaining')
   end
 
-  it 'knows if it has tasks' do
-    expect(subject.tasks?).to be_truthy
+  describe '#tasks?' do
+    it 'returns true when object has tasks' do
+      expect(subject.tasks?).to eq true
+    end
 
-    subject.description = 'Now I have no tasks'
-    expect(subject.tasks?).to be_falsey
+    it 'returns false when object has no tasks' do
+      subject.description = 'Now I have no tasks'
+      expect(subject.tasks?).to eq false
+    end
   end
 end