Merge pull request #5403 from mlkwaqas/waqas/tnl90-ordering-of-pinned-thread-ignored

Pinned thread should come first when sorting

common/static/coffee/spec/discussion/view/discussion_thread_list_view_spec.coffee

@@ -103,6 +103,7 @@ describe "DiscussionThreadListView", ->
             id: "1",
             title: "Thread1",
             votes: {up_count: '20'},
+            pinned: true,
             comments_count: 1,
             created_at: '2013-04-03T20:08:39Z',
           }),
@@ -120,6 +121,14 @@ describe "DiscussionThreadListView", ->
             comments_count: 3,
             created_at: '2013-04-03T20:06:39Z',
           }),
+          DiscussionViewSpecHelper.makeThreadWithProps({
+            id: "4",
+            title: "Thread4",
+            votes: {up_count: '25'},
+            comments_count: 0,
+            pinned: true,
+            created_at: '2013-04-03T20:05:39Z',
+          }),
         ]
 
         spyOn($, "ajax")
@@ -199,10 +208,11 @@ describe "DiscussionThreadListView", ->
         expect(@view.$(".forum-nav-filter-main-control").val()).toEqual("all")
 
     checkThreadsOrdering =  (view, sort_order, type) ->
-      expect(view.$el.find(".forum-nav-thread").children().length).toEqual(3)
+      expect(view.$el.find(".forum-nav-thread").children().length).toEqual(4)
       expect(view.$el.find(".forum-nav-thread:nth-child(1) .forum-nav-thread-title").text()).toEqual(sort_order[0])
       expect(view.$el.find(".forum-nav-thread:nth-child(2) .forum-nav-thread-title").text()).toEqual(sort_order[1])
       expect(view.$el.find(".forum-nav-thread:nth-child(3) .forum-nav-thread-title").text()).toEqual(sort_order[2])
+      expect(view.$el.find(".forum-nav-thread:nth-child(4) .forum-nav-thread-title").text()).toEqual(sort_order[3])
       expect(view.$el.find(".forum-nav-sort-control").val()).toEqual(type)
 
     describe "thread rendering should be correct", ->
@@ -211,17 +221,17 @@ describe "DiscussionThreadListView", ->
             view = makeView(discussion)
             view.render()
             checkThreadsOrdering(view, sort_order, type)
-            expect(view.$el.find(".forum-nav-thread-comments-count:visible").length).toEqual(if type == "votes" then 0 else 3)
-            expect(view.$el.find(".forum-nav-thread-votes-count:visible").length).toEqual(if type == "votes" then 3 else 0)
+            expect(view.$el.find(".forum-nav-thread-comments-count:visible").length).toEqual(if type == "votes" then 0 else 4)
+            expect(view.$el.find(".forum-nav-thread-votes-count:visible").length).toEqual(if type == "votes" then 4 else 0)
 
         it "with sort preference date", ->
-            checkRender(@threads, "date", [ "Thread1", "Thread2", "Thread3"])
+            checkRender(@threads, "date", ["Thread1", "Thread4", "Thread2", "Thread3"])
 
         it "with sort preference votes", ->
-            checkRender(@threads, "votes", [ "Thread2", "Thread1", "Thread3"])
+            checkRender(@threads, "votes", ["Thread4", "Thread1", "Thread2", "Thread3"])
 
         it "with sort preference comments", ->
-            checkRender(@threads, "comments", [ "Thread3", "Thread2", "Thread1"])
+            checkRender(@threads, "comments", ["Thread1", "Thread4", "Thread3", "Thread2"])
 
     describe "Sort change should be correct", ->
       changeSorting = (threads, selected_type, new_type, sort_order) ->
@@ -232,11 +242,11 @@ describe "DiscussionThreadListView", ->
         expect(sortControl.val()).toEqual(selected_type)
         sorted_threads = []
         if new_type == 'date'
-          sorted_threads = [threads[0], threads[1], threads[2]]
+          sorted_threads = [threads[0], threads[3], threads[1], threads[2]]
         else if new_type == 'comments'
-          sorted_threads = [threads[2], threads[1], threads[0]]
+          sorted_threads = [threads[0], threads[3], threads[2], threads[1]]
         else if new_type == 'votes'
-          sorted_threads = [threads[1], threads[0], threads[2]]
+          sorted_threads = [threads[3], threads[0], threads[1], threads[2]]
         $.ajax.andCallFake((params) =>
           params.success(
                 {"discussion_data":sorted_threads, page:1, num_pages:1}
@@ -248,13 +258,13 @@ describe "DiscussionThreadListView", ->
         checkThreadsOrdering(view, sort_order, new_type)
 
       it "with sort preference date", ->
-          changeSorting(@threads, "comments", "date", ["Thread1", "Thread2", "Thread3"])
+          changeSorting(@threads, "comments", "date", ["Thread1", "Thread4", "Thread2", "Thread3"])
 
       it "with sort preference votes", ->
-          changeSorting(@threads, "date", "votes", ["Thread2", "Thread1", "Thread3"])
+          changeSorting(@threads, "date", "votes", ["Thread4", "Thread1", "Thread2", "Thread3"])
 
       it "with sort preference comments", ->
-          changeSorting(@threads, "votes", "comments", ["Thread3", "Thread2", "Thread1"])
+          changeSorting(@threads, "votes", "comments", ["Thread1", "Thread4", "Thread3", "Thread2"])
 
     describe "search alerts", ->
 

common/static/coffee/src/discussion/discussion.coffee

@@ -67,31 +67,21 @@ if Backbone?
         error: error
 
     sortByDate: (thread) ->
-        #   
-        #The comment client asks each thread for a value by which to sort the collection
-        #and calls this sort routine regardless of the order returned from the LMS/comments service
-        #so, this takes advantage of this per-thread value and returns tomorrow's date
-        #for pinned threads, ensuring that they appear first, (which is the intent of pinned threads)
-        #
-        if thread.get('pinned')
-          #use tomorrow's date
-          today = new Date();
-          new Date(today.getTime() + (24 * 60 * 60 * 1000));
-        else
-          thread.get("created_at")
+      #
+      # The comment client asks each thread for a value by which to sort the collection
+      # and calls this sort routine regardless of the order returned from the LMS/comments service
+      # so, this takes advantage of this per-thread value and returns tomorrow's date
+      # for pinned threads, ensuring that they appear first, (which is the intent of pinned threads)
+      #
+      @pinnedThreadsSortComparatorWithDate(thread, true)
 
 
     sortByDateRecentFirst: (thread) ->
-        #
-        #Same as above
-        #but negative to flip the order (newest first)
-        #
-        if thread.get('pinned')
-          #use tomorrow's date
-          today = new Date();
-          -(new Date(today.getTime() + (24 * 60 * 60 * 1000)));
-        else
-          -(new Date(thread.get("created_at")).getTime())
+      #
+      # Same as above
+      # but negative to flip the order (newest first)
+      #
+      @pinnedThreadsSortComparatorWithDate(thread, false)
       #return String.fromCharCode.apply(String,
       #  _.map(thread.get("created_at").split(""),
       #        ((c) -> return 0xffff - c.charChodeAt()))
@@ -100,15 +90,42 @@ if Backbone?
     sortByVotes: (thread1, thread2) ->
       thread1_count = parseInt(thread1.get("votes")['up_count'])
       thread2_count = parseInt(thread2.get("votes")['up_count'])
-      if thread2_count != thread1_count
-        thread2_count - thread1_count
-      else
-        thread2.created_at_time() - thread1.created_at_time()
+      @pinnedThreadsSortComparatorWithCount(thread1, thread2, thread1_count, thread2_count)
 
     sortByComments: (thread1, thread2) ->
       thread1_count = parseInt(thread1.get("comments_count"))
       thread2_count = parseInt(thread2.get("comments_count"))
-      if thread2_count != thread1_count
-        thread2_count - thread1_count
+      @pinnedThreadsSortComparatorWithCount(thread1, thread2, thread1_count, thread2_count)
+
+    pinnedThreadsSortComparatorWithCount: (thread1, thread2, thread1_count, thread2_count) ->
+      # if threads are pinned they should be displayed on top.
+      # Unpinned will be sorted by their property count
+      if thread1.get('pinned') and not thread2.get('pinned')
+        -1
+      else if thread2.get('pinned') and not thread1.get('pinned')
+        1
+      else
+        if thread1_count > thread2_count
+          -1
+        else if thread2_count > thread1_count
+          1
+        else
+          if thread1.created_at_time() > thread2.created_at_time()
+            -1
+          else
+            1
+
+    pinnedThreadsSortComparatorWithDate: (thread, ascending)->
+      # if threads are pinned they should be displayed on top.
+      # Unpinned will be sorted by their date
+      threadCreatedTime = new Date(thread.get("created_at")).getTime()
+      if thread.get('pinned')
+        #use tomorrow's date
+        today = new Date();
+        preferredDate = new Date(today.getTime() + (24 * 60 * 60 * 1000) + threadCreatedTime);
+      else
+        preferredDate = threadCreatedTime
+      if ascending
+        preferredDate
       else
-        thread2.created_at_time() - thread1.created_at_time()
+        -(preferredDate)