Add specs and improve coffescript sortable binding functionissue_15588

5 files changed, 37 insertions, 41 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e7838e7a8f1..d32c1fd8492 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,6 +8,7 @@ v 8.10.0 (unreleased)
   - Fix pagination when sorting by columns with lots of ties (like priority)
   - Exclude email check from the standard health check
   - Implement Subresource Integrity for CSS and JavaScript assets. This prevents malicious assets from loading in the case of a CDN compromise.
+  - Fix changing issue state columns in milestone view
   - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt)
   - Add API endpoint for a group issues !4520 (mahcsig)
 
@@ -92,7 +93,6 @@ v 8.9.0
   - Links from a wiki page to other wiki pages should be rewritten as expected
   - Add option to project to only allow merge requests to be merged if the build succeeds (Rui Santos)
   - Added navigation shortcuts to the project pipelines, milestones, builds and forks page. !4393
-  - Fix changing issue state columns in milestone view
   - Fix issues filter when ordering by milestone
   - Disable SAML account unlink feature
   - Added artifacts:when to .gitlab-ci.yml - this requires GitLab Runner 1.3
diff --git a/app/assets/javascripts/milestone.js.coffee b/app/assets/javascripts/milestone.js.coffee
index 0fd64e9384e..a19e68b39e2 100644
--- a/app/assets/javascripts/milestone.js.coffee
+++ b/app/assets/javascripts/milestone.js.coffee
@@ -4,18 +4,10 @@ class @Milestone
       type: "PUT"
       url: issue_url
       data: data
-      success: (data) ->
-        if data.saved == true
-          if data.assignee_avatar_url
-            img_tag = $('<img/>')
-            img_tag.attr('src', data.assignee_avatar_url)
-            img_tag.addClass('avatar s16')
-            $(li).find('.assignee-icon').html(img_tag)
-          else
-            $(li).find('.assignee-icon').html('')
-          $(li).effect 'highlight'
-        else
-          new Flash("Issue update failed", 'alert')
+      success: (_data) =>
+        @successCallback(_data, li)
+      error: (data) ->
+        new Flash("Issue update failed", 'alert')
       dataType: "json"
 
   @sortIssues: (data) ->
@@ -25,9 +17,10 @@ class @Milestone
       type: "PUT"
       url: sort_issues_url
       data: data
-      success: (data) ->
-        if data.saved != true
-          new Flash("Issues update failed", 'alert')
+      success: (_data) =>
+        @successCallback(_data)
+      error: ->
+        new Flash("Issues update failed", 'alert')
       dataType: "json"
 
   @sortMergeRequests: (data) ->
@@ -37,9 +30,10 @@ class @Milestone
       type: "PUT"
       url: sort_mr_url
       data: data
-      success: (data) ->
-        if data.saved != true
-          new Flash("MR update failed", 'alert')
+      success: (_data) =>
+        @successCallback(_data)
+      error: (data) ->
+        new Flash("Issue update failed", 'alert')
       dataType: "json"
 
   @updateMergeRequest: (li, merge_request_url, data) ->
@@ -47,20 +41,23 @@ class @Milestone
       type: "PUT"
       url: merge_request_url
       data: data
-      success: (data) ->
-        if data.saved == true
-          if data.assignee_avatar_url
-            img_tag = $('<img/>')
-            img_tag.attr('src', data.assignee_avatar_url)
-            img_tag.addClass('avatar s16')
-            $(li).find('.assignee-icon').html(img_tag)
-          else
-            $(li).find('.assignee-icon').html('')
-          $(li).effect 'highlight'
-        else
-          new Flash("Issue update failed", 'alert')
+      success: (_data) =>
+        @successCallback(_data, li)
+      error: (data) ->
+        new Flash("Issue update failed", 'alert')
       dataType: "json"
 
+  @successCallback: (data, element) =>
+    if data.assignee
+      img_tag = $('<img/>')
+      img_tag.attr('src', data.assignee.avatar_url)
+      img_tag.addClass('avatar s16')
+      $(element).find('.assignee-icon').html(img_tag)
+    else
+      $(element).find('.assignee-icon').html('')
+
+    $(element).effect 'highlight'
+
   constructor: ->
     oldMouseStart = $.ui.sortable.prototype._mouseStart
     $.ui.sortable.prototype._mouseStart = (event, overrideHandle, noActivation) ->
@@ -81,7 +78,8 @@ class @Milestone
       stop: (event, ui) ->
         $(".issues-sortable-list").css "min-height", "0px"
       update: (event, ui) ->
-        if this == ui.item.parent()[0]
+        # Prevents sorting from container which element has been removed.
+        if $(this).find(ui.item).length > 0
           data = $(this).sortable("serialize")
           Milestone.sortIssues(data)
 
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 80dbabf51a5..8b8df680739 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -115,8 +115,9 @@ class Projects::IssuesController < Projects::ApplicationController
           render :edit
         end
       end
+
       format.json do
-        render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }, methods: :saved)
+        render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } })
       end
     end
   end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 421fd44dffc..3c5859194b4 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -190,8 +190,4 @@ class Issue < ActiveRecord::Base
   def overdue?
     due_date.try(:past?) || false
   end
-
-  def saved
-    valid?
-  end
 end
diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml
index 47b66d44e43..3c03c220ddd 100644
--- a/app/views/shared/milestones/_issuable.html.haml
+++ b/app/views/shared/milestones/_issuable.html.haml
@@ -21,7 +21,8 @@
       = link_to polymorphic_path(base_url_args, { milestone_title: @milestone.title, label_name: label.title, state: 'all' }) do
         - render_colored_label(label)
 
-    - if assignee
-      = link_to polymorphic_path(base_url_args, { milestone_title: @milestone.title, assignee_id: issuable.assignee_id, state: 'all' }),
-                class: 'has-tooltip', title: "Assigned to #{assignee.name}", data: { container: 'body' } do
-        - image_tag(avatar_icon(issuable.assignee, 16), class: "avatar s16", alt: '')
+    %span{ class: "assignee-icon" }
+      - if assignee
+        = link_to polymorphic_path(base_url_args, { milestone_title: @milestone.title, assignee_id: issuable.assignee_id, state: 'all' }),
+                  class: 'has-tooltip', title: "Assigned to #{assignee.name}", data: { container: 'body' } do
+          - image_tag(avatar_icon(issuable.assignee, 16), class: "avatar s16", alt: '')