Merge branch 'master' into profile

Conflicts:
	lms/djangoapps/django_comment_client/forum/views.py
	lms/templates/course_navigation.html

doc/discussion.md

@@ -58,6 +58,32 @@ In the discussion service, notifications are handled asynchronously using a thir
 
     bundle exec rake jobs:work
 
+## Initialize roles and permissions
+
+To fully test the discussion forum, you might want to act as a moderator or an administrator. Currently, moderators can manage everything in the forum, and administrator can manage everything plus assigning and revoking moderator status of other users.
+
+First make sure that the database is up-to-date:
+
+    rake django-admin[syncdb]
+    rake django-admin[migrate]
+
+For convenience, add the following environment variables to the terminal (assuming that you're using configuration set lms.envs.dev):
+
+    export DJANGO_SETTINGS_MODULE=lms.envs.dev
+    export PYTHONPATH=.
+
+Now initialzie roles and permissions:
+
+    django-admin.py seed_permissions_roles
+
+To assign yourself as a moderator, use the following command (assuming your username is "test", and the course id is "MITx/6.002x/2012_Fall"):
+
+    django-admin.py assign_role test Moderator "MITx/6.002x/2012_Fall"
+
+To assign yourself as an administrator, use the following command
+
+    django-admin.py assign_role test Administrator "MITx/6.002x/2012_Fall"
+
 ## Some other useful commands
 
 ### generate seeds for a specific forum
@@ -104,18 +130,30 @@ We also have a command for generating comments within a forum with the specified
 
     bundle exec rake db:generate_comments[type_the_discussion_id_here]
 
-For instance, if you want to generate comments for the general discussion, for which the discussion id is the course id with slashes and dots replaced by underscores (you **should** do this before testing forum view) and you are in 6.002x, use the following command
+For instance, if you want to generate comments for a new discussion tab named "lab_3", then use the following command
 
-    bundle exec rake db:generate_comments[MITx_6_002x_2012_Fall]
+    bundle exec rake db:generate_comments[lab_3]
 
 ### Running tests for the service
 
     bundle exec rspec
 
-Warning: due to an unresolved bug in the test code, testing the service will "flush" the development database. So you need to generate seed again after testing.
+Warning: the development and test environments share the same elasticsearch index. After running tests, search may not work in the development environment. You simply need to reindex:
+
+    bundle exec rake db:reindex_search
 
 ### debugging the service
 
 You can use the following command to launch a console within the service environment:
 
     bundle exec rake console
+
+### show user roles and permissions
+
+Use the following command to see the roles and permissions of a user in a given course (assuming, again, that the username is "test"):
+
+    django-admin.py show_permissions moderator
+
+You need to make sure that the environment variables are exported. Otherwise you would need to do
+
+    django-admin.py show_permissions moderator --settings=lms.envs.dev --pythonpath=.

lms/djangoapps/django_comment_client/forum/urls.py

@@ -5,5 +5,5 @@ urlpatterns = patterns('django_comment_client.forum.views',
     url(r'users/(?P<user_id>\w+)$', 'user_profile', name='user_profile'),
     url(r'(?P<discussion_id>\w+)/threads/(?P<thread_id>\w+)$', 'single_thread', name='single_thread'),
     url(r'(?P<discussion_id>\w+)/inline$', 'inline_discussion', name='inline_discussion'),
-    url(r'(?P<discussion_id>\w+)$', 'forum_form_discussion', name='forum_form_discussion'),
+    url(r'', 'forum_form_discussion', name='forum_form_discussion'),
 )

lms/djangoapps/django_comment_client/forum/views.py

@@ -14,6 +14,7 @@ from datehelper import time_ago_in_words
 
 import django_comment_client.utils as utils
 from urllib import urlencode
+from django_comment_client.permissions import check_permissions_by_view
 
 import json
 import comment_client as cc
@@ -23,6 +24,10 @@ import dateutil
 THREADS_PER_PAGE = 5
 PAGES_NEARBY_DELTA = 2
 
+
+def _general_discussion_id(course_id):
+    return course_id.replace('/', '_').replace('.', '_')
+
 def _should_perform_search(request):
     return bool(request.GET.get('text', False) or \
             request.GET.get('tags', False))
@@ -56,7 +61,7 @@ def render_discussion(request, course_id, threads, *args, **kwargs):
 
     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])),
+        'forum': (lambda: reverse('django_comment_client.forum.views.forum_form_discussion', args=[course_id])),
         'user': (lambda: reverse('django_comment_client.forum.views.user_profile', args=[course_id, user_id])),
     }[discussion_type]()
 
@@ -92,11 +97,11 @@ def render_forum_discussion(*args, **kwargs):
 def render_user_discussion(*args, **kwargs):
     return render_discussion(discussion_type='user', *args, **kwargs)
 
-def get_threads(request, course_id, discussion_id):
+def get_threads(request, course_id, discussion_id=None):
     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', 'date'),
+        'sort_key': request.GET.get('sort_key', 'activity'),
         'sort_order': request.GET.get('sort_order', 'desc'),
         'text': request.GET.get('text', ''), 
         'tags': request.GET.get('tags', ''),
@@ -128,22 +133,20 @@ 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):
+def forum_form_discussion(request, course_id):
     course = get_course_with_access(request.user, course_id, 'load')
-    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)
+    threads, query_params = get_threads(request, course_id)
+    content = render_forum_discussion(request, course_id, threads, discussion_id=_general_discussion_id(course_id), query_params=query_params)
+
 
     recent_active_threads = cc.search_recent_active_threads(
         course_id,
         recursive=False,
-        query_params={'follower_id': request.user.id,
-                      'commentable_id': discussion_id},
+        query_params={'follower_id': request.user.id},
     )
 
     trending_tags = cc.search_trending_tags(
         course_id,
-        query_params={'commentable_id': discussion_id},
     )
 
     if request.is_ajax():
@@ -153,12 +156,31 @@ def forum_form_discussion(request, course_id, discussion_id):
             'csrf': csrf(request)['csrf_token'],
             'course': course,
             'content': content,
-            'accordion': render_accordion(request, course, discussion_id),
             'recent_active_threads': recent_active_threads,
             'trending_tags': trending_tags,
         }
         return render_to_response('discussion/index.html', context)
 
+def get_annotated_content_info(course_id, content, user, is_thread):
+    permissions = {
+        'editable': check_permissions_by_view(user, course_id, content, "update_thread" if is_thread else "update_comment"),
+        'can_reply': check_permissions_by_view(user, course_id, content, "create_comment" if is_thread else "create_sub_comment"),
+        'can_endorse': check_permissions_by_view(user, course_id, content, "endorse_comment") if not is_thread else False,
+        'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if is_thread else "delete_comment"),
+        'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if is_thread else False,
+        'can_vote': check_permissions_by_view(user, course_id, content, "vote_for_thread" if is_thread else "vote_for_comment"),
+    }
+    return permissions
+
+def get_annotated_content_infos(course_id, thread, user, is_thread=True):
+    infos = {}
+    def _annotate(content, is_thread=is_thread):
+        infos[str(content['id'])] = get_annotated_content_info(course_id, content, user, is_thread)
+        for child in content.get('children', []):
+            _annotate(child, is_thread=False)
+    _annotate(thread)
+    return infos
+
 def render_single_thread(request, discussion_id, course_id, thread_id):
     
     thread = cc.Thread.find(thread_id).retrieve(recursive=True)

lms/djangoapps/django_comment_client/permissions.py

@@ -5,6 +5,7 @@ from django.dispatch import receiver
 from student.models import CourseEnrollment
 
 import logging
+from util.cache import cache
 
 
 @receiver(post_save, sender=CourseEnrollment)
@@ -17,9 +18,20 @@ def assign_default_role(sender, instance, **kwargs):
     logging.info("assign_default_role: adding %s as %s" % (instance.user, role))
     instance.user.roles.add(role)
 
+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. 
+    """
+    CACHE_LIFESPAN = 60
+    key = "permission_%d_%s_%s" % (user.id, str(course_id), permission)
+    val = cache.get(key, None)
+    if val not in [True, False]:
+        val = has_permission(user, permission, course_id=course_id)
+        cache.set(key, val, CACHE_LIFESPAN)
+    return val
+
 def has_permission(user, permission, course_id=None):
-    # if user.permissions.filter(name=permission).exists():
-    #     return True
     for role in user.roles.filter(course_id=course_id):
         if role.has_permission(permission):
             return True
@@ -60,7 +72,7 @@ def check_conditions_permissions(user, permissions, course_id, **kwargs):
         if isinstance(per, basestring):
             if per in CONDITIONS:
                 return check_condition(user, per, course_id, kwargs)
-            return has_permission(user, per, course_id=course_id)
+            return cached_has_permission(user, per, course_id=course_id)
         elif isinstance(per, list) and operator in ["and", "or"]:
             results = [test(user, x, operator="and") for x in per]
             if operator == "or":

lms/djangoapps/django_comment_client/utils.py

@@ -5,7 +5,8 @@ from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from django.http import HttpResponse
 from django.utils import simplejson
-
+from django.db import connection
+import logging
 from django.conf import settings
 import operator
 import itertools
@@ -128,6 +129,31 @@ class ViewNameMiddleware(object):
     def process_view(self, request, view_func, view_args, view_kwargs):  
         request.view_name = view_func.__name__
 
+class QueryCountDebugMiddleware(object):
+    """
+    This middleware will log the number of queries run
+    and the total time taken for each request (with a
+    status code of 200). It does not currently support
+    multi-db setups.
+    """
+    def process_response(self, request, response):
+        if response.status_code == 200:
+            total_time = 0
+
+            for query in connection.queries:
+                query_time = query.get('time')
+                if query_time is None:
+                    # django-debug-toolbar monkeypatches the connection
+                    # cursor wrapper and adds extra information in each
+                    # item in connection.queries. The query time is stored
+                    # under the key "duration" rather than "time" and is
+                    # in milliseconds, not seconds.
+                    query_time = query.get('duration', 0) / 1000
+                total_time += float(query_time)
+
+            logging.info('%s queries run, total %s seconds' % (len(connection.queries), total_time))
+        return response
+
 def get_annotated_content_info(course_id, content, user, type):
     return {
         'editable': check_permissions_by_view(user, course_id, content, "update_thread" if type == 'thread' else "update_comment"),
@@ -135,6 +161,7 @@ def get_annotated_content_info(course_id, content, user, type):
         'can_endorse': check_permissions_by_view(user, course_id, content, "endorse_comment") if type == 'comment' else False,
         'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if type == 'thread' else "delete_comment"),
         'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if type == 'thread' else False,
+        'can_vote': check_permissions_by_view(user, course_id, content, "vote_for_thread" if type == 'thread' else "vote_for_comment"),
     }
 
 def get_annotated_content_infos(course_id, thread, user, type='thread'):

lms/envs/common.py

@@ -34,10 +34,9 @@ from .discussionsettings import *
 
 ################################### FEATURES ###################################
 COURSEWARE_ENABLED = True
-ASKBOT_ENABLED = True
+ASKBOT_ENABLED = False
 GENERATE_RANDOM_USER_CREDENTIALS = False
 PERFSTATS = False
-DISCUSSION_SERVICE_ENABLED = True
 
 # Features
 MITX_FEATURES = {
@@ -57,7 +56,7 @@ MITX_FEATURES = {
     'SUBDOMAIN_COURSE_LISTINGS' : False,
 
     'ENABLE_TEXTBOOK' : True,
-    'ENABLE_DISCUSSION' : True,
+    'ENABLE_DISCUSSION' : False,
     'ENABLE_DISCUSSION_SERVICE': True,
 
     'ENABLE_SQL_TRACKING_LOGS': False,
@@ -350,6 +349,7 @@ MIDDLEWARE_CLASSES = (
     # 'debug_toolbar.middleware.DebugToolbarMiddleware',
 
     'django_comment_client.utils.ViewNameMiddleware',
+    'django_comment_client.utils.QueryCountDebugMiddleware',
 )
 
 ############################### Pipeline #######################################

lms/lib/comment_client/models.py

@@ -78,7 +78,16 @@ class Model(object):
     def initializable_attributes(self):
         return extract(self.attributes, self.initializable_fields)
         
+    @classmethod
+    def before_save(cls, instance):
+        pass
+
+    @classmethod
+    def after_save(cls, instance):
+        pass
+
     def save(self):
+        self.__class__.before_save(self)
         if self.id: # if we have id already, treat this as an update
             url = self.url(action='put', params=self.attributes)
             response = perform_request('put', url, self.updatable_attributes())
@@ -87,6 +96,7 @@ class Model(object):
             response = perform_request('post', url, self.initializable_attributes())
         self.retrieved = True
         self.update_attributes(**response)
+        self.__class__.after_save(self)
 
     def delete(self):
         url = self.url(action='delete', params=self.attributes)

lms/lib/comment_client/thread.py

@@ -35,13 +35,17 @@ class Thread(models.Model):
             url = cls.url(action='search')
         else:
             url = cls.url(action='get_all', params=extract(params, 'commentable_id'))
-            del params['commentable_id']
+            if params.get('commentable_id'):
+                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={}):
-        return "{prefix}/{commentable_id}/threads".format(prefix=settings.PREFIX, commentable_id=params['commentable_id'])
+        if params.get('commentable_id'):
+            return "{prefix}/{commentable_id}/threads".format(prefix=settings.PREFIX, commentable_id=params['commentable_id'])
+        else:
+            return "{prefix}/threads".format(prefix=settings.PREFIX)
 
     @classmethod
     def url_for_search_threads(cls, params={}):

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

@@ -402,3 +402,5 @@ initializeFollowThread = (thread) ->
       $local(".admin-delete").remove()
     if not Discussion.getContentInfo id, 'can_openclose'
       $local(".discussion-openclose").remove()
+    if not Discussion.getContentInfo id, 'can_vote'
+      $local(".discussion-vote").css "visibility", "hidden"

lms/static/sass/_discussion.scss

@@ -102,10 +102,12 @@ $tag-text-color: #5b614f;
       li {
         @include clearfix;
         margin-bottom: 8px;
+        border: none;
       }
 
       a {        
         @include standard-discussion-link;
+        background: none;
       }
     }
 

lms/templates/course_navigation.html

@@ -20,19 +20,19 @@ def url_class(url):
       <li class="courseware"><a href="${reverse('courseware', args=[course.id])}" class="${url_class('courseware')}">Courseware</a></li>
       <li class="info"><a href="${reverse('info', args=[course.id])}" class="${url_class('info')}">Course Info</a></li>
 % if user.is_authenticated():
-% if settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'):
-      % for index, textbook in enumerate(course.textbooks):
-          <li class="book"><a href="${reverse('book', args=[course.id, index])}" class="${url_class('book')}">${textbook.title}</a></li>
-      % endfor
-% endif
-% if settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'):
-      <li class="discussion"><a href="${reverse('django_comment_client.forum.views.forum_form_discussion', args=[course.id, course.id.replace('/', '_').replace('.', '_')])}" class="${url_class('discussion')}">Discussion</a></li>
-      <li class="news"><a href="${reverse('news', args=[course.id])}" class="${url_class('news')}">News</a></li>
-% endif 
-      
-% if settings.MITX_FEATURES.get('ENABLE_DISCUSSION'):
-      <li class="discussion"><a href="${reverse('questions')}">Discussion</a></li>
-% endif
+  % if settings.MITX_FEATURES.get('ENABLE_TEXTBOOK'):
+        % for index, textbook in enumerate(course.textbooks):
+            <li class="book"><a href="${reverse('book', args=[course.id, index])}" class="${url_class('book')}">${textbook.title}</a></li>
+        % endfor
+  % endif
+  % if settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'):
+        <li class="discussion"><a href="${reverse('django_comment_client.forum.views.forum_form_discussion', args=[course.id])}" class="${url_class('discussion')}">Discussion</a></li>
+        <li class="news"><a href="${reverse('news', args=[course.id])}" class="${url_class('news')}">News</a></li>
+  % endif 
+        
+  % if settings.MITX_FEATURES.get('ENABLE_DISCUSSION'):
+        <li class="discussion"><a href="${reverse('questions')}">Discussion</a></li>
+  % endif
 % endif
 % if settings.WIKI_ENABLED:
       <li class="wiki"><a href="${reverse('course_wiki', args=[course.id])}" class="${url_class('wiki')}">Wiki</a></li>

lms/templates/discussion/_thread.html

@@ -44,10 +44,10 @@
 
 <%def name="render_content(content, type, **kwargs)">
   <div class="discussion-content">
-    <div class="discussion-content-wrapper clearfix">
+    <div class="discussion-content-wrapper">
       
       ${render_vote(content)}
-      <div class="discussion-right-wrapper clearfix">
+      <div class="discussion-right-wrapper">
         <ul class="admin-actions">
           % if type == 'comment':
             <li><a href="javascript:void(0)" class="admin-endorse">Endorse</a></li>
@@ -96,7 +96,7 @@
 <%def name="render_tags(content, type, **kwargs)">
   <%
     def url_for_tags(tags):
-      return reverse('django_comment_client.forum.views.forum_form_discussion', args=[course_id, content['commentable_id']]) + '?' + urllib.urlencode({'tags': ",".join(tags)})
+      return reverse('django_comment_client.forum.views.forum_form_discussion', args=[course_id]) + '?' + urllib.urlencode({'tags': ",".join(tags)})
   %>
   % if type == "thread":
     <div class="thread-tags">

lms/templates/discussion/index.html

@@ -16,10 +16,6 @@
 <section class="container">
   <div class="course-wrapper">
     <section aria-label="Course Navigation" class="course-index">
-      <header id="open_close_accordion">
-        <h2>Discussion Boards</h2>
-        <a href="#">close</a>
-      </header>
       <nav>
         
         <article class="sidebar-module discussion-sidebar">

lms/urls.py

@@ -156,7 +156,7 @@ if settings.COURSEWARE_ENABLED:
     )
 
     # discussion forums live within courseware, so courseware must be enabled first
-    if settings.DISCUSSION_SERVICE_ENABLED:
+    if settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'):
 
         urlpatterns += (
             url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/news$',