Make pagination respect any filters or searches or sort keys. Searches and filters will

query the comment service to get their results rather than filtering threads that have
already been loaded.

lms/djangoapps/django_comment_client/forum/views.py

@@ -22,7 +22,7 @@ import django_comment_client.utils as utils
 import comment_client as cc
 import xml.sax.saxutils as saxutils
 
-THREADS_PER_PAGE = 2
+THREADS_PER_PAGE = 5
 INLINE_THREADS_PER_PAGE = 5
 PAGES_NEARBY_DELTA = 2
 escapedict = {'"': '"'}
@@ -78,8 +78,8 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG
         user.save()
 
     query_params = merge_dict(default_query_params,
-                              strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags'])))
-
+                              strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags', 'commentable_ids'])))
+    
     threads, page, num_pages = cc.Thread.search(query_params)
 
     query_params['page'] = page
@@ -143,6 +143,8 @@ def forum_form_discussion(request, course_id):
         return utils.JsonResponse({
             'discussion_data': threads, # TODO: Standardize on 'discussion_data' vs 'threads'
             'annotated_content_info': annotated_content_info,
+            'num_pages': query_params['num_pages'],
+            'page': query_params['page'],
         })
     else:
         #recent_active_threads = cc.search_recent_active_threads(

lms/djangoapps/django_comment_client/permissions.py

@@ -11,7 +11,7 @@ cache = cache.get_cache('default')
 def cached_has_permission(user, permission, course_id=None):
     """
     Call has_permission if it's not cached. A change in a user's role or
-    a role's permissions will only become effective after CACHE_LIFESPAN seconds. 
+    a role's permissions will only become effective after CACHE_LIFESPAN seconds.
     """
     CACHE_LIFESPAN = 60
     key = "permission_%d_%s_%s" % (user.id, str(course_id), permission)
@@ -54,8 +54,8 @@ def check_conditions_permissions(user, permissions, course_id, **kwargs):
     """
     Accepts a list of permissions and proceed if any of the permission is valid.
     Note that ["can_view", "can_edit"] will proceed if the user has either
-    "can_view" or "can_edit" permission. To use AND operator in between, wrap them in 
-    a list. 
+    "can_view" or "can_edit" permission. To use AND operator in between, wrap them in
+    a list.
     """
 
     def test(user, per, operator="or"):
@@ -87,7 +87,7 @@ VIEW_PERMISSIONS = {
     'vote_for_thread'   :       [['vote', 'is_open']],
     'undo_vote_for_thread':     [['unvote', 'is_open']],
     'follow_thread'     :       ['follow_thread'],
-    'follow_commentable':       ['follow_commentable'], 
+    'follow_commentable':       ['follow_commentable'],
     'follow_user'       :       ['follow_user'],
     'unfollow_thread'   :       ['unfollow_thread'],
     'unfollow_commentable':     ['unfollow_commentable'],

lms/lib/comment_client/thread.py

@@ -16,7 +16,7 @@ class Thread(models.Model):
     ]
 
     updatable_fields = [
-        'title', 'body', 'anonymous', 'course_id', 
+        'title', 'body', 'anonymous', 'course_id',
         'closed', 'tags', 'user_id', 'commentable_id',
     ]
 
@@ -33,7 +33,7 @@ class Thread(models.Model):
                           'course_id': query_params['course_id'],
                           'recursive': False}
         params = merge_dict(default_params, strip_blank(strip_none(query_params)))
-        if query_params.get('text') or query_params.get('tags'):
+        if query_params.get('text') or query_params.get('tags') or query_params.get('commentable_ids'):
             url = cls.url(action='search')
         else:
             url = cls.url(action='get_all', params=extract(params, 'commentable_id'))
@@ -41,7 +41,7 @@ class Thread(models.Model):
                 del params['commentable_id']
         response = perform_request('get', url, params, *args, **kwargs)
         return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1)
-        
+
     @classmethod
     def url_for_threads(cls, params={}):
         if params.get('commentable_id'):

lms/static/coffee/src/discussion/discussion.coffee

@@ -18,18 +18,23 @@ if Backbone?
       @current_page < @pages
 
     addThread: (thread, options) ->
+      # TODO: Check for existing thread with same ID?
       options ||= {}
       model = new Thread thread
       @add model
       model
 
-    retrieveAnotherPage: (search_text="", commentable_id="", sort_key="")->
-      # TODO: Obey dropdown filter (commentable_id)
+    retrieveAnotherPage: (search_text="", commentable_ids="", sort_key="")->
+      # TODO: I really feel that this belongs in DiscussionThreadListView
       @current_page += 1
       url = DiscussionUtil.urlFor 'threads'
-      data = { page: @current_page, text: search_text }
+      data = { page: @current_page }
+      if search_text
+        data['text'] = search_text
       if sort_key
         data['sort_key'] = sort_key
+      if commentable_ids
+        data['commentable_ids'] = commentable_ids
       DiscussionUtil.safeAjax
         $elem: @$el
         url: url

lms/static/coffee/src/discussion/views/discussion_thread_list_view.coffee

@@ -14,6 +14,7 @@ if Backbone?
       @displayedCollection = new Discussion(@collection.models, pages: @collection.pages)
       @collection.on "change", @reloadDisplayedCollection
       @sortBy = "date"
+      @discussionIds=""
       @collection.on "reset", (discussion) =>
         board = $(".current-board").html()
         @displayedCollection.current_page = discussion.current_page
@@ -117,10 +118,12 @@ if Backbone?
         @$(".post-list").append("<li class='more-pages'><a href='#'>Load more</a></li>")
 
     loadMorePages: ->
-      # TODO: Obey dropdown filter
       @$(".more-pages").html('<div class="loading-animation"></div>')
       @$(".more-pages").addClass("loading")
-      @collection.retrieveAnotherPage(@current_search, "", @sortBy)
+      @collection.retrieveAnotherPage(@current_search, @discussionIds, @sortBy)
+      if not @collection.hasMorePages()
+        $(".more-pages").hide()
+
 
     renderThread: (thread) =>
       content = $(_.template($("#thread-list-item-template").html())(thread.toJSON()))
@@ -232,14 +235,27 @@ if Backbone?
         @setTopic(event)
         @clearSearch @filterTopic, event
       else
-        @setTopic(event)
+        @setTopic(event)  # just sets the title for the dropdown
         item = $(event.target).closest('li')
         if item.find("span.board-name").data("discussion_id") == "#all"
           item = item.parent()
+          @discussionIds = ""
         discussionIds = _.map item.find(".board-name[data-discussion_id]"), (board) -> $(board).data("discussion_id").id
-        filtered = @collection.filter (thread) =>
-          _.include(discussionIds, thread.get('commentable_id'))
-        @displayedCollection.reset filtered
+        @retrieveDiscussions(discussionIds)
+
+    retrieveDiscussions: (discussion_ids) ->
+      @discussionIds = discussion_ids.join(',')
+      url = DiscussionUtil.urlFor("search")
+      DiscussionUtil.safeAjax
+        data: { 'commentable_ids': @discussionIds }
+        url: url
+        type: "GET"
+        success: (response, textStatus) =>
+          @collection.current_page = response.page
+          @collection.pages = response.num_pages
+          @collection.reset(response.discussion_data)
+          Content.loadContentInfos(response.content_info)
+          @displayedCollection.reset(@collection.models)
 
     sortThreads: (event) ->
       @$(".sort-bar a").removeClass("active")
@@ -283,6 +299,8 @@ if Backbone?
             # TODO: Augment existing collection?
             @collection.reset(response.discussion_data)
             Content.loadContentInfos(response.content_info)
+            @collection.current_page = response.page
+            @collection.pages = response.num_pages
             # TODO: Perhaps reload user info so that votes can be updated.
             # In the future we might not load all of a user's votes at once
             # so this would probably be necessary anyway

lms/static/js/jquery.tagsinput.js

@@ -105,7 +105,7 @@
                         $('<span>').text(value).append('&nbsp;&nbsp;'),
                         $('<a>', {
                             href  : '#',
-                            title : 'Removing tag',
+                            title : 'Remove tag',
                             text  : 'x'
                         }).click(function () {
                             return $('#' + id).removeTag(escape(value));