From e49226e9e147d41d9c2ccf29c587ef42f5f8431f Mon Sep 17 00:00:00 2001 From: Rocky Duan Date: Sun, 19 Aug 2012 00:16:04 -0700 Subject: [PATCH] some refactor --- .../django_comment_client/base/views.py | 4 +- .../django_comment_client/forum/views.py | 38 +++++++------ lms/djangoapps/django_comment_client/utils.py | 9 ++- lms/envs/common.py | 2 +- lms/lib/comment_client/thread.py | 4 +- lms/lib/comment_client/utils.py | 9 ++- .../coffee/src/discussion/content.coffee | 4 +- lms/templates/discussion/_forum.html | 15 +---- lms/templates/discussion/_inline.html | 10 +--- lms/templates/discussion/_js_data.html | 10 ++++ .../discussion/_js_dependencies.html | 57 +++++++++---------- lms/templates/discussion/_single_thread.html | 10 +--- lms/templates/discussion/_thread.html | 48 ++++++---------- .../discussion/_user_active_threads.html | 10 +--- lms/templates/discussion/_user_profile.html | 7 ++- 15 files changed, 102 insertions(+), 135 deletions(-) create mode 100644 lms/templates/discussion/_js_data.html diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index e20202538e..e619140993 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -64,7 +64,7 @@ def ajax_content_response(request, course_id, content, template_name): def create_thread(request, course_id, commentable_id): post = request.POST thread = cc.Thread(**extract(post, ['body', 'title', 'tags'])) - thread.update_attributes({ + thread.update_attributes(**{ 'anonymous' : post.get('anonymous', 'false').lower() == 'true', 'commentable_id' : commentable_id, 'course_id' : course_id, @@ -94,7 +94,7 @@ def update_thread(request, course_id, thread_id): def _create_comment(request, course_id, thread_id=None, parent_id=None): post = request.POST comment = cc.Comment(**extract(post, ['body'])) - comment.update_attributes({ + comment.update_attributes(**{ 'anonymous' : post.get('anonymous', 'false').lower() == 'true', 'user_id' : request.user.id, 'course_id' : course_id, diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index b33b67aaee..bf263ffaa1 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -12,13 +12,14 @@ from courseware.courses import get_course_with_access from dateutil.tz import tzlocal 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 +from django_comment_client.utils import merge_dict, extract, strip_none import json -import comment_client as cc import dateutil +import django_comment_client.utils as utils +import comment_client as cc THREADS_PER_PAGE = 5 @@ -65,11 +66,8 @@ def render_discussion(request, course_id, threads, *args, **kwargs): 'user': (lambda: reverse('django_comment_client.forum.views.user_profile', args=[course_id, user_id])), }[discussion_type]() - print "start annotating" annotated_content_infos = map(lambda x: utils.get_annotated_content_infos(course_id, x, request.user), threads) - print "start merging annotations" - annotated_content_info = reduce(utils.merge_dict, annotated_content_infos, {}) - print "finished annotating" + annotated_content_info = reduce(merge_dict, annotated_content_infos, {}) context = { 'threads': threads, @@ -82,7 +80,7 @@ def render_discussion(request, course_id, threads, *args, **kwargs): 'pages_nearby_delta': PAGES_NEARBY_DELTA, 'discussion_type': discussion_type, 'base_url': base_url, - 'query_params': utils.strip_none(utils.extract(query_params, ['page', 'sort_key', 'sort_order', 'tags', 'text'])), + 'query_params': strip_none(extract(query_params, ['page', 'sort_key', 'sort_order', 'tags', 'text'])), 'annotated_content_info': json.dumps(annotated_content_info), } context = dict(context.items() + query_params.items()) @@ -98,17 +96,21 @@ def render_user_discussion(*args, **kwargs): return render_discussion(discussion_type='user', *args, **kwargs) 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', 'activity'), - 'sort_order': request.GET.get('sort_order', 'desc'), - 'text': request.GET.get('text', ''), - 'tags': request.GET.get('tags', ''), + + default_query_params = { + 'page': 1, + 'per_page': THREADS_PER_PAGE, + 'sort_key': 'activity', + 'sort_order': 'desc', + 'text': '', + 'tags': '', 'commentable_id': discussion_id, 'course_id': course_id, } + query_params = merge_dict(default_query_params, + strip_none(extract(request.GET, ['page', 'sort_key', 'sort_order', 'text', 'tags']))) + threads, page, num_pages = cc.Thread.search(query_params) query_params['page'] = page @@ -138,7 +140,6 @@ def forum_form_discussion(request, course_id): 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, @@ -159,6 +160,7 @@ def forum_form_discussion(request, course_id): 'recent_active_threads': recent_active_threads, 'trending_tags': trending_tags, } + print "start rendering.." return render_to_response('discussion/index.html', context) def render_single_thread(request, discussion_id, course_id, thread_id): @@ -209,14 +211,14 @@ def single_thread(request, course_id, discussion_id, thread_id): def user_profile(request, course_id, user_id): course = get_course_with_access(request.user, course_id, 'load') - discussion_user = cc.User(id=user_id, course_id=course_id) + profiled_user = cc.User(id=user_id, course_id=course_id) query_params = { 'page': request.GET.get('page', 1), 'per_page': THREADS_PER_PAGE, # more than threads_per_page to show more activities } - threads, page, num_pages = discussion_user.active_threads(query_params) + threads, page, num_pages = profiled_user.active_threads(query_params) query_params['page'] = page query_params['num_pages'] = num_pages @@ -230,7 +232,7 @@ def user_profile(request, course_id, user_id): 'course': course, 'user': request.user, 'django_user': User.objects.get(id=user_id), - 'discussion_user': discussion_user.to_dict(), + 'profiled_user': profiled_user.to_dict(), 'content': content, } diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 5075bff295..3ef33afbbf 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -20,9 +20,12 @@ def extract(dic, keys): return {k: dic.get(k) for k in keys} def strip_none(dic): - def _is_none(v): - return v is None or (isinstance(v, str) and len(v.strip()) == 0) - return dict([(k, v) for k, v in dic.iteritems() if not _is_none(v)]) + return dict([(k, v) for k, v in dic.iteritems() if v is not None]) + +def strip_blank(dic): + def _is_blank(v): + return isinstance(v, str) and len(v.strip()) == 0 + return dict([(k, v) for k, v in dic.iteritems() if not _is_blank(v)]) def merge_dict(dic1, dic2): return dict(dic1.items() + dic2.items()) diff --git a/lms/envs/common.py b/lms/envs/common.py index c6ed8a0c0c..139fb09226 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -41,7 +41,7 @@ PERFSTATS = False # Features MITX_FEATURES = { 'SAMPLE' : False, - 'USE_DJANGO_PIPELINE' : True, + 'USE_DJANGO_PIPELINE' : False, 'DISPLAY_HISTOGRAMS_TO_STAFF' : True, 'REROUTE_ACTIVATION_EMAIL' : False, # nonempty string = address for all activation emails 'DEBUG_LEVEL' : 0, # 0 = lowest level, least verbose, 255 = max level, most verbose diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index e5fccb81d0..c203e22d22 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -30,8 +30,8 @@ class Thread(models.Model): 'per_page': 20, 'course_id': query_params['course_id'], 'recursive': False} - params = merge_dict(default_params, strip_none(query_params)) - if query_params['text'] or query_params['tags']: + params = merge_dict(default_params, strip_blank(strip_none(query_params))) + if query_params.get('text') or query_params.get('tags'): url = cls.url(action='search') else: url = cls.url(action='get_all', params=extract(params, 'commentable_id')) diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py index 7e696bb99d..16b1952303 100644 --- a/lms/lib/comment_client/utils.py +++ b/lms/lib/comment_client/utils.py @@ -2,9 +2,12 @@ import requests import json def strip_none(dic): - def _is_none(v): - return v is None or (isinstance(v, str) and len(v.strip()) == 0) - return dict([(k, v) for k, v in dic.iteritems() if not _is_none(v)]) + return dict([(k, v) for k, v in dic.iteritems() if v is not None]) + +def strip_blank(dic): + def _is_blank(v): + return isinstance(v, str) and len(v.strip()) == 0 + return dict([(k, v) for k, v in dic.iteritems() if not _is_blank(v)]) def extract(dic, keys): if isinstance(keys, str): diff --git a/lms/static/coffee/src/discussion/content.coffee b/lms/static/coffee/src/discussion/content.coffee index 2e26e191b0..a55322b9f5 100644 --- a/lms/static/coffee/src/discussion/content.coffee +++ b/lms/static/coffee/src/discussion/content.coffee @@ -402,5 +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" + #if not Discussion.getContentInfo id, 'can_vote' + # $local(".discussion-vote").css "visibility", "hidden" diff --git a/lms/templates/discussion/_forum.html b/lms/templates/discussion/_forum.html index 6ef21b6de7..f0c6ad4be9 100644 --- a/lms/templates/discussion/_forum.html +++ b/lms/templates/discussion/_forum.html @@ -1,5 +1,4 @@ <%namespace name="renderer" file="_thread.html"/> -<%! from django.template.defaultfilters import escapejs %>
@@ -24,16 +23,4 @@ % endif
-<%! - def escape_quotes(text): - return text.replace('\"', '\\\"').replace("\'", "\\\'") -%> - - +<%include file="_js_data.html" /> diff --git a/lms/templates/discussion/_inline.html b/lms/templates/discussion/_inline.html index 85482e420a..9e16c49d58 100644 --- a/lms/templates/discussion/_inline.html +++ b/lms/templates/discussion/_inline.html @@ -1,5 +1,4 @@ <%namespace name="renderer" file="_thread.html"/> -<%! from django.template.defaultfilters import escapejs %>
@@ -14,11 +13,4 @@ <%include file="_paginator.html" />
- +<%include file="_js_data.html" /> diff --git a/lms/templates/discussion/_js_data.html b/lms/templates/discussion/_js_data.html new file mode 100644 index 0000000000..3e1dc4c280 --- /dev/null +++ b/lms/templates/discussion/_js_data.html @@ -0,0 +1,10 @@ +<%! from django.template.defaultfilters import escapejs %> + + diff --git a/lms/templates/discussion/_js_dependencies.html b/lms/templates/discussion/_js_dependencies.html index 0e5e6aa19f..474f1abfe3 100644 --- a/lms/templates/discussion/_js_dependencies.html +++ b/lms/templates/discussion/_js_dependencies.html @@ -1,33 +1,30 @@ - <%namespace name='static' file='../static_content.html'/> - - + - ## This must appear after all mathjax-config blocks, so it is after the imports from the other templates. - ## It can't be run through static.url because MathJax uses crazy url introspection to do lazy loading of MathJax extension libraries - - - - - - - - - - - - - +## This must appear after all mathjax-config blocks, so it is after the imports from the other templates. +## It can't be run through static.url because MathJax uses crazy url introspection to do lazy loading of MathJax extension libraries + + + + + + + + + + + + + diff --git a/lms/templates/discussion/_single_thread.html b/lms/templates/discussion/_single_thread.html index 3c92fd37b6..3f380bcc7a 100644 --- a/lms/templates/discussion/_single_thread.html +++ b/lms/templates/discussion/_single_thread.html @@ -1,16 +1,8 @@ <%namespace name="renderer" file="_thread.html"/> -<%! from django.template.defaultfilters import escapejs %>
Discussion ${renderer.render_thread(course_id, thread, show_comments=True)}
- +<%include file="_js_data.html" /> diff --git a/lms/templates/discussion/_thread.html b/lms/templates/discussion/_thread.html index 7975173396..8d4c36ea77 100644 --- a/lms/templates/discussion/_thread.html +++ b/lms/templates/discussion/_thread.html @@ -5,15 +5,23 @@ <%! import urllib %> <%! - def user_id_with_anonymity(content): - if content.get('anonymous', False): - return '' + def show_if(text, condition): + if condition: + return text else: - return content['user_id'] + return '' +%> + +<%! + def close_thread_text(content): + if content.get('closed'): + return 'Re-open thread' + else: + return 'Close thread' %> <%def name="render_thread(course_id, thread, show_comments=False)"> -
+
${render_content(thread, "thread", show_comments=show_comments)} % if show_comments: ${render_comments(thread.get('children', []))} @@ -22,11 +30,7 @@ <%def name="render_comment(comment)"> - % if comment['endorsed']: -
- % else: -
- % endif +
${render_content(comment, "comment")}
${render_comments(comment.get('children', []))} @@ -45,7 +49,6 @@ <%def name="render_content(content, type, **kwargs)">
- ${render_vote(content)}
- ${render_title(content, type, **kwargs)}
- % if content.get('highlighted_body', None): -
${content['highlighted_body'] | h}
- % else: -
${content['body'] | h}
- % endif +
${(content.get('highlighted_body') or content['body']) | h}
${render_tags(content, type, **kwargs)} ${render_bottom_bar(content, type, **kwargs)} @@ -84,11 +76,7 @@ <%def name="render_title(content, type, **kwargs)"> % if type == "thread": - % if content.get('highlighted_title', None): - ${content['highlighted_title'] | h} - % else: - ${content['title'] | h} - % endif + ${(content.get('highlighted_title') or content['title']) | h} % endif @@ -110,7 +98,7 @@ <%def name="render_bottom_bar(content, type, **kwargs)">
- ${render_info(content, type, **kwargs)} + ${render_info(content, type, **kwargs)}
  • ${render_link("discussion-link discussion-reply discussion-reply-" + type, "Reply")}
    • diff --git a/lms/templates/discussion/_user_active_threads.html b/lms/templates/discussion/_user_active_threads.html index 6a4de78fdc..c3f5d0c403 100644 --- a/lms/templates/discussion/_user_active_threads.html +++ b/lms/templates/discussion/_user_active_threads.html @@ -1,5 +1,4 @@ <%namespace name="renderer" file="_thread.html"/> -<%! from django.template.defaultfilters import escapejs %>
    @@ -14,11 +13,4 @@ <%include file="_paginator.html" />
    - +<%include file="_js_data.html" /> diff --git a/lms/templates/discussion/_user_profile.html b/lms/templates/discussion/_user_profile.html index 94fe86ceaf..6d7bc13e8c 100644 --- a/lms/templates/discussion/_user_profile.html +++ b/lms/templates/discussion/_user_profile.html @@ -1,17 +1,18 @@ <%! from django_comment_client.utils import pluralize %> <%! from django_comment_client.permissions import has_permission, check_permissions_by_view %> +<%! from operator import attrgetter %>