Merge pull request #7979 from mr-vinn/refactor-tasks

Refactor task list tests and coffeescript

8 files changed, 61 insertions, 82 deletions

diff --git a/app/assets/javascripts/behaviors/taskable.js.coffee b/app/assets/javascripts/behaviors/taskable.js.coffee
new file mode 100644
index 00000000000..ddce71c1886
--- /dev/null
+++ b/app/assets/javascripts/behaviors/taskable.js.coffee
@@ -0,0 +1,21 @@
+window.updateTaskState = (taskableType) ->
+  objType = taskableType.data
+  isChecked = $(this).prop("checked")
+  if $(this).is(":checked")
+    stateEvent = "task_check"
+  else
+    stateEvent = "task_uncheck"
+
+  taskableUrl = $("form.edit-" + objType).first().attr("action")
+  taskableNum = taskableUrl.match(/\d+$/)
+  taskNum = 0
+  $("li.task-list-item input:checkbox").each( (index, e) =>
+    if e == this
+      taskNum = index + 1
+  )
+
+  $.ajax
+    type: "PATCH"
+    url: taskableUrl
+    data: objType + "[state_event]=" + stateEvent +
+      "&" + objType + "[task_num]=" + taskNum
diff --git a/app/assets/javascripts/issue.js.coffee b/app/assets/javascripts/issue.js.coffee
index f2b531fb2b1..0e2a2fa792a 100644
--- a/app/assets/javascripts/issue.js.coffee
+++ b/app/assets/javascripts/issue.js.coffee
@@ -9,25 +9,11 @@ class Issue
     if $("a.btn-close").length
       $("li.task-list-item input:checkbox").prop("disabled", false)
 
-    $(".task-list-item input:checkbox").on "click", ->
-      is_checked = $(this).prop("checked")
-      if $(this).is(":checked")
-        state_event = "task_check"
-      else
-        state_event = "task_uncheck"
-
-      mr_url = $("form.edit-issue").first().attr("action")
-      mr_num = mr_url.match(/\d+$/)
-      task_num = 0
-      $("li.task-list-item input:checkbox").each( (index, e) =>
-        if e == this
-          task_num = index + 1
-      )
-
-      $.ajax
-        type: "PATCH"
-        url: mr_url
-        data: "issue[state_event]=" + state_event +
-          "&issue[task_num]=" + task_num
+    $(".task-list-item input:checkbox").on(
+      "click"
+      null
+      "issue"
+      updateTaskState
+    )
 
 @Issue = Issue
diff --git a/app/assets/javascripts/merge_request.js.coffee b/app/assets/javascripts/merge_request.js.coffee
index c0460a7ec4e..9f99ff403f8 100644
--- a/app/assets/javascripts/merge_request.js.coffee
+++ b/app/assets/javascripts/merge_request.js.coffee
@@ -17,7 +17,7 @@ class MergeRequest
 
     disableButtonIfEmptyField '#commit_message', '.accept_merge_request'
 
-    if $("a.close-mr-link").length
+    if $("a.btn-close").length
       $("li.task-list-item input:checkbox").prop("disabled", false)
 
   # Local jQuery finder
@@ -74,26 +74,12 @@ class MergeRequest
       this.$('.remove_source_branch_in_progress').hide()
       this.$('.remove_source_branch_widget.failed').show()
 
-    this.$(".task-list-item input:checkbox").on "click", ->
-      is_checked = $(this).prop("checked")
-      if $(this).is(":checked")
-        state_event = "task_check"
-      else
-        state_event = "task_uncheck"
-
-      mr_url = $("form.edit-merge_request").first().attr("action")
-      mr_num = mr_url.match(/\d+$/)
-      task_num = 0
-      $("li.task-list-item input:checkbox").each( (index, e) =>
-        if e == this
-          task_num = index + 1
-      )
-
-      $.ajax
-        type: "PATCH"
-        url: mr_url
-        data: "merge_request[state_event]=" + state_event +
-          "&merge_request[task_num]=" + task_num
+    $(".task-list-item input:checkbox").on(
+      "click"
+      null
+      "merge_request"
+      updateTaskState
+    )
 
   activateTab: (action) ->
     this.$('.merge-request-tabs li').removeClass 'active'
diff --git a/features/project/issues/issues.feature b/features/project/issues/issues.feature
index e989569ccd4..4db8551559b 100644
--- a/features/project/issues/issues.feature
+++ b/features/project/issues/issues.feature
@@ -144,7 +144,7 @@ Feature: Project Issues
   Scenario: Issues list should display task status
     Given project "Shop" has "Tasks-open" open issue with task markdown
     When I visit project "Shop" issues page
-    Then I should see the task status for issue "Tasks-open"
+    Then I should see the task status for the Taskable
 
   # Toggling task items
 
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index e1e0edd0545..d20358a7dc6 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -168,11 +168,10 @@ Feature: Project Merge Requests
 
   # Task status in issues list
 
-  @now
   Scenario: Merge requests list should display task status
     Given project "Shop" has "MR-task-open" open MR with task markdown
     When I visit project "Shop" merge requests page
-    Then I should see the task status for merge request "MR-task-open"
+    Then I should see the task status for the Taskable
 
   # Toggling task items
 
diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb
index 26c3c7c14bc..640603562dd 100644
--- a/features/steps/project/issues/issues.rb
+++ b/features/steps/project/issues/issues.rb
@@ -154,29 +154,11 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps
   end
 
   step 'project "Shop" has "Tasks-open" open issue with task markdown' do
-    desc_text = <<EOT.gsub(/^ {6}/, '')
-      * [ ] Task 1
-      * [x] Task 2
-EOT
-    create(:issue,
-           title: 'Tasks-open',
-           project: project,
-           author: project.users.first,
-           description: desc_text
-          )
+    create_taskable(:issue, 'Tasks-open')
   end
 
   step 'project "Shop" has "Tasks-closed" closed issue with task markdown' do
-    desc_text = <<EOT.gsub(/^ {6}/, '')
-      * [ ] Task 1
-      * [x] Task 2
-EOT
-    create(:closed_issue,
-           title: 'Tasks-closed',
-           project: project,
-           author: project.users.first,
-           description: desc_text
-          )
+    create_taskable(:closed_issue, 'Tasks-closed')
   end
 
   step 'empty project "Empty Project"' do
diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb
index 0fec2604915..c009568977d 100644
--- a/features/steps/project/merge_requests.rb
+++ b/features/steps/project/merge_requests.rb
@@ -98,17 +98,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
   end
 
   step 'project "Shop" has "MR-task-open" open MR with task markdown' do
-    desc_text = <<EOT.gsub(/^ {6}/, '')
-      * [ ] Task 1
-      * [x] Task 2
-EOT
-    create(:merge_request,
-           title: 'MR-task-open',
-           source_project: project,
-           target_project: project,
-           author: project.users.first,
-           description: desc_text
-          )
+    create_taskable(:merge_request, 'MR-task-open')
   end
 
   step 'I switch to the diff tab' do
diff --git a/features/steps/shared/markdown.rb b/features/steps/shared/markdown.rb
index 1d9058cf256..8bf138065b0 100644
--- a/features/steps/shared/markdown.rb
+++ b/features/steps/shared/markdown.rb
@@ -6,6 +6,27 @@ module SharedMarkdown
     find(:css, "#{parent} h#{level}##{id} > :last-child")[:href].should =~ /##{id}$/
   end
 
+  def create_taskable(type, title)
+    desc_text = <<EOT.gsub(/^ {6}/, '')
+      * [ ] Task 1
+      * [x] Task 2
+EOT
+
+    case type
+    when :issue, :closed_issue
+      options = { project: project }
+    when :merge_request
+      options = { source_project: project, target_project: project }
+    end
+
+    create(
+      type,
+      options.merge(title: title,
+                    author: project.users.first,
+                    description: desc_text)
+    )
+  end
+
   step 'Header "Description header" should have correct id and link' do
     header_should_have_correct_id_and_link(1, 'Description header', 'description-header')
   end
@@ -16,13 +37,7 @@ module SharedMarkdown
     )
   end
 
-  step 'I should see the task status for issue "Tasks-open"' do
-    expect(find(:css, 'span.task-status').text).to eq(
-      '2 tasks (1 done, 1 unfinished)'
-    )
-  end
-
-  step 'I should see the task status for merge request "MR-task-open"' do
+  step 'I should see the task status for the Taskable' do
     expect(find(:css, 'span.task-status').text).to eq(
       '2 tasks (1 done, 1 unfinished)'
     )