Uses update URL to update the status of a todo

4 files changed, 51 insertions, 41 deletions

diff --git a/app/assets/javascripts/right_sidebar.js.coffee b/app/assets/javascripts/right_sidebar.js.coffee
index 18abec4f51e..8eb005b0a22 100644
--- a/app/assets/javascripts/right_sidebar.js.coffee
+++ b/app/assets/javascripts/right_sidebar.js.coffee
@@ -47,44 +47,51 @@ class @Sidebar
       .off 'click', '.js-issuable-todo'
       .on 'click', '.js-issuable-todo', @toggleTodo
 
-  toggleTodo: ->
-    $this = $(@)
+  toggleTodo: (e) =>
+    $this = $(e.currentTarget)
     $todoLoading = $('.js-issuable-todo-loading')
     $btnText = $('.js-issuable-todo-text', $this)
+    ajaxType = if $this.attr('data-id') then 'PATCH' else 'POST'
+    ajaxUrlExtra = if $this.attr('data-id') then "/#{$this.attr('data-id')}" else ''
 
     $.ajax(
-      url: $this.data('url')
-      type: 'POST'
+      url: "#{$this.data('url')}#{ajaxUrlExtra}"
+      type: ajaxType
       dataType: 'json'
       data:
-        todo_id: $this.attr('data-id')
         issuable_id: $this.data('issuable')
         issuable_type: $this.data('issuable-type')
-      beforeSend: ->
-        $this.disable()
-        $todoLoading.removeClass 'hidden'
-    ).done (data) ->
-      $todoPendingCount = $('.todos-pending-count')
-      $todoPendingCount.text data.count
-
-      $this.enable()
-      $todoLoading.addClass 'hidden'
-
-      if data.count is 0
-        $todoPendingCount.addClass 'hidden'
-      else
-        $todoPendingCount.removeClass 'hidden'
-
-      if data.todo?
-        $this
-          .attr 'aria-label', $this.data('mark-text')
-          .attr 'data-id', data.todo.id
-        $btnText.text $this.data('mark-text')
-      else
-        $this
-          .attr 'aria-label', $this.data('todo-text')
-          .removeAttr 'data-id'
-        $btnText.text $this.data('todo-text')
+      beforeSend: =>
+        @beforeTodoSend($this, $todoLoading)
+    ).done (data) =>
+      @todoUpdateDone(data, $this, $btnText, $todoLoading)
+
+  beforeTodoSend: ($btn, $todoLoading) ->
+    $btn.disable()
+    $todoLoading.removeClass 'hidden'
+
+  todoUpdateDone: (data, $btn, $btnText, $todoLoading) ->
+    $todoPendingCount = $('.todos-pending-count')
+    $todoPendingCount.text data.count
+
+    $btn.enable()
+    $todoLoading.addClass 'hidden'
+
+    if data.count is 0
+      $todoPendingCount.addClass 'hidden'
+    else
+      $todoPendingCount.removeClass 'hidden'
+
+    if data.todo?
+      $btn
+        .attr 'aria-label', $btn.data('mark-text')
+        .attr 'data-id', data.todo.id
+      $btnText.text $btn.data('mark-text')
+    else
+      $btn
+        .attr 'aria-label', $btn.data('todo-text')
+        .removeAttr 'data-id'
+      $btnText.text $btn.data('todo-text')
 
   sidebarDropdownLoading: (e) ->
     $sidebarCollapsedIcon = $(@).closest('.block').find('.sidebar-collapsed-icon')
diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb
index 21745977860..64e70a5bcc6 100644
--- a/app/controllers/projects/todos_controller.rb
+++ b/app/controllers/projects/todos_controller.rb
@@ -1,20 +1,23 @@
 class Projects::TodosController < Projects::ApplicationController
   def create
-    json_data = Hash.new
+    TodoService.new.mark_todo(issuable, current_user)
 
-    if params[:todo_id].nil?
-      TodoService.new.mark_todo(issuable, current_user)
+    render json: {
+      todo: current_user.todos.find_by(state: :pending, action: Todo::MARKED, target_id: issuable.id),
+      count: current_user.todos.pending.count,
+    }
+  end
 
-      json_data[:todo] = current_user.todos.find_by(state: :pending, action: Todo::MARKED, target_id: issuable.id)
-    else
-      current_user.todos.find_by_id(params[:todo_id]).update(state: :done)
-    end
+  def update
+    current_user.todos.find_by_id(params[:id]).update(state: :done)
 
-    render json: json_data.merge({ count: current_user.todos.pending.count })
+    render json: {
+      count: current_user.todos.pending.count,
+    }
   end
 
   private
-  
+
   def issuable
     @issuable ||= begin
       case params[:issuable_type]
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml
index 17f623b3461..539c4f3630a 100644
--- a/app/views/shared/issuable/_sidebar.html.haml
+++ b/app/views/shared/issuable/_sidebar.html.haml
@@ -9,7 +9,7 @@
       %a.gutter-toggle.pull-right.js-sidebar-toggle{ role: "button", href: "#", aria: { label: "Toggle sidebar" } }
         = sidebar_gutter_toggle_icon
       - if current_user
-        %button.btn.btn-default.issuable-header-btn.pull-right.js-issuable-todo{ type: "button", aria: { label: (todo.nil? ? "Add Todo" : "Mark Done") }, data: { todo_text: "Add Todo", mark_text: "Mark Done", id: (todo.id unless todo.nil?), issuable: issuable.id, issuable_type: issuable.class.name.underscore, url: namespace_project_todos_path(@project.namespace, @project, :json) } }
+        %button.btn.btn-default.issuable-header-btn.pull-right.js-issuable-todo{ type: "button", aria: { label: (todo.nil? ? "Add Todo" : "Mark Done") }, data: { todo_text: "Add Todo", mark_text: "Mark Done", id: (todo.id unless todo.nil?), issuable: issuable.id, issuable_type: issuable.class.name.underscore, url: namespace_project_todos_path(@project.namespace, @project) } }
           %span.js-issuable-todo-text
             - if todo.nil?
               Add Todo
diff --git a/config/routes.rb b/config/routes.rb
index ef198a5e87a..93dd3c938d0 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -789,7 +789,7 @@ Rails.application.routes.draw do
           end
         end
 
-        resources :todos, only: [:create], constraints: { id: /\d+/ }
+        resources :todos, only: [:create, :update], constraints: { id: /\d+/ }
 
         resources :uploads, only: [:create] do
           collection do