From 46a7c51df5687aa2ea4678e4eae59d032a549cda Mon Sep 17 00:00:00 2001 From: Mike Chen Date: Thu, 9 Aug 2012 12:12:43 -0400 Subject: [PATCH 1/6] remove voting arrows if one cannot vote --- lms/djangoapps/django_comment_client/forum/views.py | 1 + lms/static/coffee/src/discussion/content.coffee | 2 ++ 2 files changed, 3 insertions(+) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index d4f6fda3c0..d0be18b06e 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -155,6 +155,7 @@ def get_annotated_content_info(course_id, content, user, is_thread): '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 diff --git a/lms/static/coffee/src/discussion/content.coffee b/lms/static/coffee/src/discussion/content.coffee index 16c224d165..bf389f80c3 100644 --- a/lms/static/coffee/src/discussion/content.coffee +++ b/lms/static/coffee/src/discussion/content.coffee @@ -386,3 +386,5 @@ initializeFollowThread = (thread) -> $local(".discussion-delete").remove() if not Discussion.getContentInfo id, 'can_openclose' $local(".discussion-openclose").remove() + if not Discussion.getContentInfo id, 'can_vote' + $local(".discussion-vote").remove() From 1bea419237c5822a0a7e5161b7d0d92418573de3 Mon Sep 17 00:00:00 2001 From: Mike Chen Date: Thu, 9 Aug 2012 13:02:43 -0400 Subject: [PATCH 2/6] fix discussion_module after merging from MITx/mitx --- common/lib/xmodule/xmodule/discussion_module.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index c3388a968c..694b0673be 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -17,9 +17,10 @@ class DiscussionModule(XModule): } return self.system.render_template('discussion/_discussion_module.html', context) - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) - + def __init__(self, system, location, definition, descriptor, instance_state=None, + shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, descriptor, + instance_state, shared_state, **kwargs) if isinstance(instance_state, str): instance_state = json.loads(instance_state) xml_data = etree.fromstring(definition['data']) From efad5632d878e3989ce6b239eb72b103233b6ee1 Mon Sep 17 00:00:00 2001 From: Mike Chen Date: Thu, 9 Aug 2012 13:03:28 -0400 Subject: [PATCH 3/6] set invisible instead of remove to preserve space of voting arrows when voting is disabled --- lms/static/coffee/src/discussion/content.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/static/coffee/src/discussion/content.coffee b/lms/static/coffee/src/discussion/content.coffee index bf389f80c3..51d144c5a1 100644 --- a/lms/static/coffee/src/discussion/content.coffee +++ b/lms/static/coffee/src/discussion/content.coffee @@ -387,4 +387,4 @@ initializeFollowThread = (thread) -> if not Discussion.getContentInfo id, 'can_openclose' $local(".discussion-openclose").remove() if not Discussion.getContentInfo id, 'can_vote' - $local(".discussion-vote").remove() + $local(".discussion-vote").css "visibility", "hidden" From b4621d056236b762a7a7ed2f167a989018bd011e Mon Sep 17 00:00:00 2001 From: Mike Chen Date: Wed, 15 Aug 2012 11:21:11 -0400 Subject: [PATCH 4/6] added QueryCountDebugMiddleware --- lms/djangoapps/django_comment_client/utils.py | 28 ++++++++++++++++++- lms/envs/common.py | 1 + 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index aac409435a..1aeef06ec7 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/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"), diff --git a/lms/envs/common.py b/lms/envs/common.py index 931a88e0c7..eaa05f4a70 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -329,6 +329,7 @@ MIDDLEWARE_CLASSES = ( # 'debug_toolbar.middleware.DebugToolbarMiddleware', 'django_comment_client.utils.ViewNameMiddleware', + 'django_comment_client.utils.QueryCountDebugMiddleware', ) ############################### Pipeline ####################################### From 158e978a7ec677b79fc61a6c5a0394bc7226c888 Mon Sep 17 00:00:00 2001 From: Mike Chen Date: Wed, 15 Aug 2012 17:15:08 -0400 Subject: [PATCH 5/6] added naive caching. not sure how to expire. --- lms/djangoapps/django_comment_client/permissions.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index 9ca2b46af0..4d79369708 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/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,17 @@ 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 has_permission(user, permission, course_id=None): +def cached_has_permission(user, permission, course_id=None): # if user.permissions.filter(name=permission).exists(): # return True + 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, 3600) + return val + +def has_permission(user, permission, course_id=None): for role in user.roles.filter(course_id=course_id): if role.has_permission(permission): return True @@ -60,7 +69,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": From d56ac9a71b4f8a3756e9863abd32befe35c23e3d Mon Sep 17 00:00:00 2001 From: Mike Chen Date: Wed, 15 Aug 2012 17:22:09 -0400 Subject: [PATCH 6/6] added comment --- lms/djangoapps/django_comment_client/permissions.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index 4d79369708..f9d9580412 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -19,13 +19,16 @@ def assign_default_role(sender, instance, **kwargs): instance.user.roles.add(role) def cached_has_permission(user, permission, course_id=None): - # if user.permissions.filter(name=permission).exists(): - # return True + """ + 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, 3600) + cache.set(key, val, CACHE_LIFESPAN) return val def has_permission(user, permission, course_id=None):