Merge branch 'master' into discussion2

Conflicts:
	lms/templates/discussion/_forum.html
	lms/templates/discussion/_inline.html

lms/djangoapps/django_comment_client/base/views.py

@@ -15,7 +15,7 @@ from django.core.files.storage import get_storage_class
 from django.utils.translation import ugettext as _
 from django.conf import settings
 
-from django_comment_client.utils import JsonResponse, JsonError
+from django_comment_client.utils import JsonResponse, JsonError, extract
 
 def thread_author_only(fn):
     def verified_fn(request, *args, **kwargs):
@@ -45,8 +45,7 @@ def instructor_only(fn): #TODO add instructor verification
             return fn(request, *args, **kwargs)
     return verified_fn
 
-def extract(dic, keys):
-    return {k: dic[k] for k in keys}
+
 
 @login_required
 @require_POST

lms/djangoapps/django_comment_client/forum/views.py

@@ -11,7 +11,7 @@ from courseware.courses import check_course
 from dateutil.tz import tzlocal
 from datehelper import time_ago_in_words
 
-from django_comment_client.utils import get_categorized_discussion_info
+from django_comment_client.utils import get_categorized_discussion_info, extract, strip_none
 from urllib import urlencode
 
 import json
@@ -39,57 +39,67 @@ def render_accordion(request, course, discussion_id):
 
     return render_to_string('discussion/_accordion.html', context)
 
-def render_discussion(request, course_id, threads, discussion_id=None, with_search_bar=True, 
-                      search_text='', discussion_type='inline', page=1, num_pages=None, 
-                      per_page=THREADS_PER_PAGE):
+def render_discussion(request, course_id, threads, discussion_id=None, with_search_bar=True, \
+                      discussion_type='inline', query_params={}):
+
     template = {
         'inline': 'discussion/_inline.html',
         'forum': 'discussion/_forum.html',
     }[discussion_type]
 
-    def _url_for_inline_page(page, per_page):
-        raw_url = reverse('django_comment_client.forum.views.inline_discussion', args=[course_id, discussion_id])
-        return raw_url + '?' + urlencode({'page': page, 'per_page': per_page})
-
-    def _url_for_forum_page(page, per_page):
-        raw_url = reverse('django_comment_client.forum.views.forum_form_discussion', args=[course_id, discussion_id])
-        return raw_url + '?' + urlencode({'page': page, 'per_page': per_page})
-
-    url_for_page = {
-        'inline': _url_for_inline_page,
-        'forum': _url_for_forum_page,
-    }[discussion_type]
-
+    base_url = {
+        'inline': (lambda: reverse('django_comment_client.forum.views.inline_discussion', args=[course_id, discussion_id])), 
+        'forum': (lambda: reverse('django_comment_client.forum.views.forum_form_discussion', args=[course_id, discussion_id])),
+    }[discussion_type]()
 
     context = {
         'threads': threads,
         'discussion_id': discussion_id,
         'search_bar': '' if not with_search_bar \
-                      else render_search_bar(request, course_id, discussion_id, text=search_text),
+                      else render_search_bar(request, course_id, discussion_id, text=query_params.get('text', '')),
         'user_info': comment_client.get_user_info(request.user.id, raw=True),
         'course_id': course_id,
         'request': request,
-        'page': page,
-        'per_page': per_page,
-        'num_pages': num_pages,
         'pages_nearby_delta': PAGES_NEARBY_DELTA,
         'discussion_type': discussion_type,
-        'url_for_page': url_for_page,
+        'base_url': base_url,
+        'query_params': strip_none(extract(query_params, ['page', 'sort_key', 'sort_order', 'tags', 'text'])),
     }
+    context = dict(context.items() + query_params.items())
     return render_to_string(template, context)
 
 def render_inline_discussion(*args, **kwargs):
-    
     return render_discussion(discussion_type='inline', *args, **kwargs)
 
 def render_forum_discussion(*args, **kwargs):
     return render_discussion(discussion_type='forum', *args, **kwargs)
 
+def get_threads(request, course_id, discussion_id):
+    query_params = {
+        'page': request.GET.get('page', 1),
+        'per_page': THREADS_PER_PAGE, #TODO maybe change this later
+        'sort_key': request.GET.get('sort_key', None),
+        'sort_order': request.GET.get('sort_order', None),
+        'text': request.GET.get('text', None), 
+        'tags': request.GET.get('tags', None),
+    }
+
+    if query_params['text'] or query_params['tags']: #TODO do tags search without sunspot
+        query_params['commentable_id'] = discussion_id
+        threads, page, num_pages = comment_client.search_threads(course_id, recursive=False, query_params=query_params)
+    else:
+        threads, page, num_pages = comment_client.get_threads(discussion_id, recursive=False, query_params=query_params)
+
+    query_params['page'] = page
+    query_params['num_pages'] = num_pages
+
+    return threads, query_params
+
 # discussion per page is fixed for now
 def inline_discussion(request, course_id, discussion_id):
-    page = request.GET.get('page', 1)
-    threads, page, per_page, num_pages = comment_client.get_threads(discussion_id, recursive=False, page=page, per_page=THREADS_PER_PAGE)
-    html = render_inline_discussion(request, course_id, threads, discussion_id=discussion_id, num_pages=num_pages, page=page, per_page=per_page)
+    threads, query_params = get_threads(request, course_id, discussion_id)
+    html = render_inline_discussion(request, course_id, threads, discussion_id=discussion_id,  \
+                                                                 query_params=query_params)
     return HtmlResponse(html)
 
 def render_search_bar(request, course_id, discussion_id=None, text=''):
@@ -103,38 +113,19 @@ def render_search_bar(request, course_id, discussion_id=None, text=''):
     return render_to_string('discussion/_search_bar.html', context)
 
 def forum_form_discussion(request, course_id, discussion_id):
-
     course = check_course(course_id)
-    search_text = request.GET.get('text', '')
-    page = request.GET.get('page', 1)
-
-    if len(search_text) > 0:
-        threads, page, per_page, num_pages = comment_client.search_threads({
-            'text': search_text,
-            'commentable_id': discussion_id,
-            'course_id': course_id,
-            'page': page,
-            'per_page': THREADS_PER_PAGE,
-        })
-    else:
-        threads, page, per_page, num_pages = comment_client.get_threads(discussion_id, recursive=False, page=page, per_page=THREADS_PER_PAGE)
-
+    threads, query_params = get_threads(request, course_id, discussion_id)
+    content = render_forum_discussion(request, course_id, threads, discussion_id=discussion_id, \
+                                                                   query_params=query_params)
     context = {
         'csrf': csrf(request)['csrf_token'],
         'course': course,
-        'content': render_forum_discussion(request, course_id, threads, 
-                                           discussion_id=discussion_id,
-                                           search_text=search_text,
-                                           num_pages=num_pages,
-                                           per_page=per_page,
-                                           page=page),
+        'content': content,
         'accordion': render_accordion(request, course, discussion_id),
     }
-
     return render_to_response('discussion/index.html', context)
 
 def render_single_thread(request, course_id, thread_id):
-    
     def get_annotated_content_info(thread, user_id):
         infos = {}
         def _annotate(content):

lms/djangoapps/django_comment_client/utils.py

@@ -13,6 +13,12 @@ import itertools
 _FULLMODULES = None
 _DISCUSSIONINFO = None
 
+def extract(dic, keys):
+    return {k: dic[k] for k in keys}
+
+def strip_none(dic):
+    return dict([(k, v) for k, v in dic.iteritems() if v is not None])
+
 def get_full_modules():
     global _FULLMODULES
     if not _FULLMODULES:

lms/lib/comment_client.py

@@ -15,9 +15,19 @@ class CommentClientUnknownError(CommentClientError):
 def delete_threads(commentable_id, *args, **kwargs):
     return _perform_request('delete', _url_for_commentable_threads(commentable_id), *args, **kwargs)
 
-def get_threads(commentable_id, recursive=False, page=1, per_page=20, *args, **kwargs):
-    response = _perform_request('get', _url_for_threads(commentable_id), {'recursive': recursive, 'page': page, 'per_page': per_page}, *args, **kwargs)
-    return response['collection'], response['page'], response['per_page'], response['num_pages']
+def get_threads(commentable_id, recursive=False, query_params={}, *args, **kwargs):
+    default_params = {'page': 1, 'per_page': 20}
+    attributes = dict(default_params.items() + query_params.items())
+    response = _perform_request('get', _url_for_threads(commentable_id), \
+            attributes, *args, **kwargs)
+    return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1)
+
+def search_threads(course_id, recursive=False, query_params={}, *args, **kwargs):
+    default_params = {'page': 1, 'per_page': 20, 'course_id': course_id}
+    attributes = dict(default_params.items() + query_params.items())
+    response = _perform_request('get', _url_for_search_threads(), \
+            attributes, *args, **kwargs)
+    return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1)
 
 def get_threads_tags(*args, **kwargs):
     return _perform_request('get', _url_for_threads_tags(), {}, *args, **kwargs)
@@ -98,10 +108,7 @@ def unsubscribe_thread(user_id, thread_id, *args, **kwargs):
 def unsubscribe_commentable(user_id, commentable_id, *args, **kwargs):
     return unsubscribe(user_id, {'source_type': 'other', 'source_id': commentable_id})
 
-def search_threads(attributes, *args, **kwargs):
-    default_attributes = {'page': 1, 'per_page': 20}
-    attributes = dict(default_attributes.items() + attributes.items())
-    return _perform_request('get', _url_for_search_threads(), attributes, *args, **kwargs)
+
 
 def _perform_request(method, url, data_or_params=None, *args, **kwargs):
     if method in ['post', 'put', 'patch']:

lms/templates/discussion/_forum.html

@@ -13,6 +13,7 @@
 
     <div class="discussion-new-post control-button" href="javascript:void(0)">New Post</div>
   </div>
+  <%include file="_sort.html" />
   % for thread in threads:
     ${renderer.render_thread(course_id, thread, edit_thread=False, show_comments=False)}
   % endfor

lms/templates/discussion/_inline.html

@@ -7,6 +7,7 @@
     </div>
     <div class="discussion-new-post control-button" href="javascript:void(0)">New Post</div>
   </div>
+  <%include file="_sort.html" />
   % for thread in threads:
     ${renderer.render_thread(course_id, thread, edit_thread=False, show_comments=False)}
   % endfor

lms/templates/discussion/_paginator.html

+<%! from urllib import urlencode %>
+
+<% 
+  def merge(dic1, dic2):
+    return dict(dic1.items() + dic2.items())
+
+  def url_for_page(_page):
+    return base_url + '?' + urlencode(merge(query_params, {'page': _page}))
+%>
+
 <%def name="link_to_page(_page)">
+  
   % if _page != page:
     <div class="page-link">
-      <a href="${url_for_page(_page, per_page)}">${_page}</a>
+      <a href="${url_for_page(_page)}">${_page}</a>
     </div>
   % else:
     <div class="page-link">${_page}</div>
@@ -25,7 +36,7 @@
 <div class="discussion-${discussion_type}-paginator discussion-paginator">
   <div class="prev-page">
     % if page > 1:
-      <a href="${url_for_page(page - 1, per_page)}">&lt; Previous page</a>
+      <a href="${url_for_page(page - 1)}">&lt; Previous page</a>
     % else:
       &lt; Previous page
     % endif
@@ -44,7 +55,7 @@
   % endif
   <div class="next-page">
     % if page < num_pages:
-      <a href="${url_for_page(page + 1, per_page)}">Next page &gt;</a>
+      <a href="${url_for_page(page + 1)}">Next page &gt;</a>
     % else:
       Next page &gt;
     % endif

lms/templates/discussion/_sort.html

+<%! from urllib import urlencode %>
+
+<%def name="link_to_sort(key, title)">
+  % if key == sort_key:
+    % if sort_order.lower() == 'desc':
+      ${_link_to_sort(key, 'asc', title + ' (desc)')}
+    % else:
+      ${_link_to_sort(key, 'desc', title + '(asc)')}
+    % endif
+  % else:
+    ${_link_to_sort(key, 'desc', title)}
+  % endif
+</%def>
+
+<%def name="_link_to_sort(key, order, title)">
+  <% 
+  def merge(dic1, dic2):
+    return dict(dic1.items() + dic2.items())
+
+  def url_for_sort(key, order):
+    return base_url + '?' + urlencode(merge(query_params, {'page': 1, 'sort_key': key, 'sort_order': order}))
+  %>
+  <a href="${url_for_sort(key, order)}">${title}</a>
+</%def>
+    
+<div class="discussion-sort">
+  Sort by: 
+  ${link_to_sort('date', 'Date')}
+  ${link_to_sort('votes', 'Votes')}
+  ${link_to_sort('comments', 'Comments')}
+</div>