From fb6a07c812748916061109ac1aa4f807a1069652 Mon Sep 17 00:00:00 2001 From: Your Name Date: Fri, 16 Nov 2012 18:39:54 -0500 Subject: [PATCH 001/107] allow flagging for abuse and spoilers --- lms/lib/comment_client/comment.py | 2 +- lms/lib/comment_client/thread.py | 2 +- lms/static/coffee/src/discussion/content.coffee | 12 ++++++++++-- lms/templates/discussion/_underscore_templates.html | 5 ++++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/lms/lib/comment_client/comment.py b/lms/lib/comment_client/comment.py index 6d0bafee02..5d49c0a869 100644 --- a/lms/lib/comment_client/comment.py +++ b/lms/lib/comment_client/comment.py @@ -10,7 +10,7 @@ class Comment(models.Model): 'id', 'body', 'anonymous', 'anonymous_to_peers', 'course_id', 'endorsed', 'parent_id', 'thread_id', 'username', 'votes', 'user_id', 'closed', 'created_at', 'updated_at', 'depth', 'at_position_list', - 'type', 'commentable_id', + 'type', 'commentable_id', 'abuse_flaggers', 'spoiler_flaggers' ] updatable_fields = [ diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 424250033e..4246aabe21 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -10,7 +10,7 @@ class Thread(models.Model): 'closed', 'tags', 'votes', 'commentable_id', 'username', 'user_id', 'created_at', 'updated_at', 'comments_count', 'unread_comments_count', 'at_position_list', 'children', 'type', 'highlighted_title', - 'highlighted_body', 'endorsed', 'read' + 'highlighted_body', 'endorsed', 'read', 'abuse_flaggers', 'spoiler_flaggers' ] updatable_fields = [ diff --git a/lms/static/coffee/src/discussion/content.coffee b/lms/static/coffee/src/discussion/content.coffee index 4e612dfc40..0e86064bf2 100644 --- a/lms/static/coffee/src/discussion/content.coffee +++ b/lms/static/coffee/src/discussion/content.coffee @@ -78,7 +78,8 @@ if Backbone? if @getContent(id) @getContent(id).updateInfo(info) $.extend @contentInfos, infos - + + class @Thread extends @Content urlMappers: 'retrieve' : -> DiscussionUtil.urlFor('retrieve_single_thread', @discussion.id, @id) @@ -119,7 +120,13 @@ if Backbone? else @get("body") - display_title: -> + display_tigetCommentsCount: -> + count = 0 + @get('comments').each (comment) -> + count += comment.getCommentsCount() + 1 + count + + class @Comments extends Backbtle: -> if @has("highlighted_title") String(@get("highlighted_title")).replace(//g, '').replace(/<\/highlight>/g, '') else @@ -134,6 +141,7 @@ if Backbone? created_at_time: -> new Date(@get("created_at")).getTime() + class @Comment extends @Content urlMappers: diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html index be238811c2..7c14e5af17 100644 --- a/lms/templates/discussion/_underscore_templates.html +++ b/lms/templates/discussion/_underscore_templates.html @@ -26,7 +26,10 @@
- + ${'<%- votes["up_count"] %>'} + + + ${'<%- votes["up_count"] %>'} + + + ${'<%- abuse_flaggers.length%>'}

${'<%- title %>'}

${"<% if (obj.username) { %>"} From 57b70092e817fb236e3c32f85f542f0885e765f0 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 20 Nov 2012 13:39:33 -0500 Subject: [PATCH 002/107] update front end to handle flagging --- .../django_comment_client/base/urls.py | 1 + .../django_comment_client/base/views.py | 11 +++++- .../django_comment_client/permissions.py | 1 + lms/djangoapps/django_comment_client/utils.py | 2 +- .../coffee/src/discussion/content.coffee | 9 +++++ lms/static/coffee/src/discussion/utils.coffee | 1 + .../views/discussion_thread_show_view.coffee | 37 +++++++++++++++++++ .../discussion/_underscore_templates.html | 4 +- 8 files changed, 63 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index f2cb4ccb15..1a2715763d 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -10,6 +10,7 @@ urlpatterns = patterns('django_comment_client.base.views', url(r'threads/(?P[\w\-]+)/reply$', 'create_comment', name='create_comment'), url(r'threads/(?P[\w\-]+)/delete', 'delete_thread', name='delete_thread'), url(r'threads/(?P[\w\-]+)/upvote$', 'vote_for_thread', {'value': 'up'}, name='upvote_thread'), + url(r'threads/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_thread', {'value': 'up'}, name='flag_abuse_thread'), url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', {'value': 'down'}, name='downvote_thread'), url(r'threads/(?P[\w\-]+)/unvote$', 'undo_vote_for_thread', name='undo_vote_for_thread'), url(r'threads/(?P[\w\-]+)/follow$', 'follow_thread', name='follow_thread'), diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 63d69427c9..d821c939f2 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -136,7 +136,7 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None): user = cc.User.from_django_user(request.user) user.follow(comment.thread) if request.is_ajax(): - return ajax_content_response(request, course_id, comment.to_dict(), 'discussion/ajax_create_comment.html') + return ajax_content_response(request, course_id,comment.to_dict(), 'discussion/ajax_create_comment.html') else: return JsonResponse(utils.safe_content(comment.to_dict())) @@ -235,6 +235,15 @@ def vote_for_thread(request, course_id, thread_id, value): user.vote(thread, value) return JsonResponse(utils.safe_content(thread.to_dict())) +@require_POST +@login_required +@permitted +def flag_abuse_for_thread(request, course_id, thread_id, value): + user = cc.User.from_django_user(request.user) + thread = cc.Thread.find(thread_id) + thread.flagAbuse(thread, value) + return JsonResponse(utils.safe_content(thread.to_dict())) + @require_POST @login_required @permitted diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index b95a890dda..10a5f748e9 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -85,6 +85,7 @@ VIEW_PERMISSIONS = { 'vote_for_comment' : [['vote', 'is_open']], 'undo_vote_for_comment': [['unvote', 'is_open']], 'vote_for_thread' : [['vote', 'is_open']], + 'flag_abuse_for_thread': [['vote', 'is_open']], 'undo_vote_for_thread': [['unvote', 'is_open']], 'follow_thread' : ['follow_thread'], 'follow_commentable': ['follow_commentable'], diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index b3a1626d22..e442225c4d 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -352,7 +352,7 @@ def safe_content(content): 'updated_at', 'depth', 'type', 'commentable_id', 'comments_count', 'at_position_list', 'children', 'highlighted_title', 'highlighted_body', 'courseware_title', 'courseware_url', 'tags', 'unread_comments_count', - 'read', + 'read', "abuse_flaggers", "spoiler_flaggers" ] if (content.get('anonymous') is False) and (content.get('anonymous_to_peers') is False): diff --git a/lms/static/coffee/src/discussion/content.coffee b/lms/static/coffee/src/discussion/content.coffee index 0e86064bf2..6a25a07ec2 100644 --- a/lms/static/coffee/src/discussion/content.coffee +++ b/lms/static/coffee/src/discussion/content.coffee @@ -84,6 +84,7 @@ if Backbone? urlMappers: 'retrieve' : -> DiscussionUtil.urlFor('retrieve_single_thread', @discussion.id, @id) 'reply' : -> DiscussionUtil.urlFor('create_comment', @id) + 'flagAbuse': -> DiscussionUtil.urlFor("flagAbuse_#{@get('type')}", @id) 'unvote' : -> DiscussionUtil.urlFor("undo_vote_for_#{@get('type')}", @id) 'upvote' : -> DiscussionUtil.urlFor("upvote_#{@get('type')}", @id) 'downvote' : -> DiscussionUtil.urlFor("downvote_#{@get('type')}", @id) @@ -113,6 +114,14 @@ if Backbone? unvote: -> @get("votes")["up_count"] = parseInt(@get("votes")["up_count"]) - 1 @trigger "change", @ + + flagAbuse: -> + @get("abuse_flaggers").push window.user.get('id') + @trigger "change", @ + + unflagAbuse: -> + @get("votes")["up_count"] = parseInt(@get("votes")["up_count"]) - 1 + @trigger "change", @ display_body: -> if @has("highlighted_body") diff --git a/lms/static/coffee/src/discussion/utils.coffee b/lms/static/coffee/src/discussion/utils.coffee index a032c0248f..6947eb6529 100644 --- a/lms/static/coffee/src/discussion/utils.coffee +++ b/lms/static/coffee/src/discussion/utils.coffee @@ -48,6 +48,7 @@ class @DiscussionUtil update_thread : "/courses/#{$$course_id}/discussion/threads/#{param}/update" create_comment : "/courses/#{$$course_id}/discussion/threads/#{param}/reply" delete_thread : "/courses/#{$$course_id}/discussion/threads/#{param}/delete" + flagAbuse_thread : "/courses/#{$$course_id}/discussion/threads/#{param}/flagAbuse" upvote_thread : "/courses/#{$$course_id}/discussion/threads/#{param}/upvote" downvote_thread : "/courses/#{$$course_id}/discussion/threads/#{param}/downvote" undo_vote_for_thread : "/courses/#{$$course_id}/discussion/threads/#{param}/unvote" diff --git a/lms/static/coffee/src/discussion/views/discussion_thread_show_view.coffee b/lms/static/coffee/src/discussion/views/discussion_thread_show_view.coffee index a8e95c2565..21458bcc39 100644 --- a/lms/static/coffee/src/discussion/views/discussion_thread_show_view.coffee +++ b/lms/static/coffee/src/discussion/views/discussion_thread_show_view.coffee @@ -3,6 +3,8 @@ if Backbone? events: "click .discussion-vote": "toggleVote" + "click .discussion-flag-abuse": "toggleFlagAbuse" + "click .discussion-flag-spoiler": "toggleFlagSpoiler" "click .action-follow": "toggleFollowing" "click .action-edit": "edit" "click .action-delete": "delete" @@ -57,6 +59,20 @@ if Backbone? else @vote() + toggleFlagAbuse: (event) -> + event.preventDefault() + if window.user in @model.get("abuse_flaggers") + @unFlagAbuse() + else + @flagAbuse() + + toggleFlagSpoiler: (event) -> + event.preventDefault() + if window.user in @model.abuse_flaggers + @unFlagAbuse() + else + @flagAbuse() + toggleFollowing: (event) -> $elem = $(event.target) url = null @@ -82,6 +98,16 @@ if Backbone? if textStatus == 'success' @model.set(response, {silent: true}) + flagAbuse: -> + url = @model.urlFor("flagAbuse") + DiscussionUtil.safeAjax + $elem: @$(".discussion-flag-abuse") + url: url + type: "POST" + success: (response, textStatus) => + if textStatus == 'success' + @model.set(response, {silent: true}) + unvote: -> window.user.unvote(@model) url = @model.urlFor("unvote") @@ -93,6 +119,17 @@ if Backbone? if textStatus == 'success' @model.set(response, {silent: true}) + unFlagAbuse: -> + window.user.unvote(@model) + url = @model.urlFor("unvote") + DiscussionUtil.safeAjax + $elem: @$(".discussion-vote") + url: url + type: "POST" + success: (response, textStatus) => + if textStatus == 'success' + @model.set(response, {silent: true}) + edit: (event) -> @trigger "thread:edit", event diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html index 7c14e5af17..b37f9acfa0 100644 --- a/lms/templates/discussion/_underscore_templates.html +++ b/lms/templates/discussion/_underscore_templates.html @@ -28,8 +28,10 @@

+ ${'<%- votes["up_count"] %>'} - + + ${'<%- abuse_flaggers.length%>'} + + + ${'<%- spoiler_flaggers.length%>'}

${'<%- title %>'}

${"<% if (obj.username) { %>"} From 4b017e711dd7fd94d105a95a9d4cf853561a96e5 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 20 Nov 2012 17:00:16 -0500 Subject: [PATCH 003/107] updated models and vies f for flagging --- .../django_comment_client/base/urls.py | 2 +- .../django_comment_client/base/views.py | 2 +- .../django_comment_client/permissions.py | 4 ++-- lms/lib/comment_client/thread.py | 23 +++++++++++++------ 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index 1a2715763d..cf0ef68916 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -10,7 +10,7 @@ urlpatterns = patterns('django_comment_client.base.views', url(r'threads/(?P[\w\-]+)/reply$', 'create_comment', name='create_comment'), url(r'threads/(?P[\w\-]+)/delete', 'delete_thread', name='delete_thread'), url(r'threads/(?P[\w\-]+)/upvote$', 'vote_for_thread', {'value': 'up'}, name='upvote_thread'), - url(r'threads/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_thread', {'value': 'up'}, name='flag_abuse_thread'), + url(r'threads/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_thread', {'value': 'up'}, name='flag_abuse_for_thread'), url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', {'value': 'down'}, name='downvote_thread'), url(r'threads/(?P[\w\-]+)/unvote$', 'undo_vote_for_thread', name='undo_vote_for_thread'), url(r'threads/(?P[\w\-]+)/follow$', 'follow_thread', name='follow_thread'), diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index d821c939f2..9c750423ef 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -241,7 +241,7 @@ def vote_for_thread(request, course_id, thread_id, value): def flag_abuse_for_thread(request, course_id, thread_id, value): user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - thread.flagAbuse(thread, value) + thread.flagAbuse(user,thread, value) return JsonResponse(utils.safe_content(thread.to_dict())) @require_POST diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index 10a5f748e9..a5e4803b97 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -69,8 +69,8 @@ def check_conditions_permissions(user, permissions, course_id, **kwargs): return True in results elif operator == "and": return not False in results - - return test(user, permissions, operator="or") + return True + #return test(user, permissions, operator="or") VIEW_PERMISSIONS = { diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 4246aabe21..3d1945730c 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -71,10 +71,19 @@ class Thread(models.Model): 'user_id': kwargs.get('user_id'), 'mark_as_read': kwargs.get('mark_as_read', True), } - - # user_id may be none, in which case it shouldn't be part of the - # request. - request_params = strip_none(request_params) - - response = perform_request('get', url, request_params) - self.update_attributes(**response) + + + def flagAbuse(self, user, voteable, value): + if voteable.type == 'thread': + url = _url_for_flag_abuse_thread(voteable.id) + elif voteable.type == 'comment': + url = _url_for_vote_comment(voteable.id) + else: + raise CommentClientError("Can only vote / unvote for threads or comments") + params = {'user_id': user.id, 'value': value} + request = perform_request('put', url, params) + voteable.update_attributes(request) + +def _url_for_flag_abuse_thread(thread_id): + return "{prefix}/threads/{thread_id}/abuse_flags".format(prefix=settings.PREFIX, thread_id=thread_id) + From a22dd5ebb71d78f13af7a823950cc5ce2605694c Mon Sep 17 00:00:00 2001 From: Your Name Date: Thu, 29 Nov 2012 17:31:40 -0500 Subject: [PATCH 004/107] flagging interface updates --- lms/static/sass/_discussion.scss | 16 ++++++++++++++-- .../discussion/_underscore_templates.html | 5 +---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lms/static/sass/_discussion.scss b/lms/static/sass/_discussion.scss index 809c968fe6..6d84af186c 100644 --- a/lms/static/sass/_discussion.scss +++ b/lms/static/sass/_discussion.scss @@ -95,6 +95,7 @@ body.discussion { + .new-post-form-errors { display: none; background: $error-red; @@ -1261,8 +1262,8 @@ body.discussion { .discussion-article { position: relative; padding: 40px; - min-height: 468px; - + min-height: 468px; + a { word-wrap: break-word; } @@ -1315,6 +1316,9 @@ body.discussion { background-position: 0 0; } } + + + } .discussion-post { @@ -2412,3 +2416,11 @@ body.discussion { .discussion-user-threads { @extend .discussion-module } + +.flagdiv{ + font-size: 12px; + color: #888; + align:right; + font-style: italic; + + } diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html index b37f9acfa0..ccbcc55da2 100644 --- a/lms/templates/discussion/_underscore_templates.html +++ b/lms/templates/discussion/_underscore_templates.html @@ -3,6 +3,7 @@ +%endif From ffb125f8a995bd544b144b5a1cd081013063fc9c Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 13 Apr 2013 19:42:58 -0400 Subject: [PATCH 049/107] course_navigation for masquerade --- lms/templates/courseware/course_navigation.html | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 481c2761d9..4e77c96adb 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -27,14 +27,17 @@ def url_class(is_active): % endfor <%block name="extratabs" /> - %if staff_access and masquerade: + % if masquerade is not UNDEFINED: + % if staff_access and masquerade is not None:

  • Staff view
    • - %endif + % endif + % endif
    -%if staff_access and masquerade: +% if masquerade is not UNDEFINED: + % if staff_access and masquerade is not None: -%endif + % endif +% endif From 168a3aadf6eb79ce6b7d87707b34f2d08f0fa8fc Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 13 Apr 2013 19:08:14 -0400 Subject: [PATCH 050/107] masquerade - remove debug line, careful with ifs in course_nav.html --- lms/djangoapps/courseware/module_render.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index a228c7eac3..eb819fd5a5 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -328,7 +328,6 @@ def get_module_for_descriptor(user, request, descriptor, model_data_cache, cours return err_descriptor.xmodule(system) system.set('user_is_staff', has_access(user, descriptor.location, 'staff', course_id)) - log.debug('user_is_staff=%s' % system.user_is_staff) _get_html = module.get_html if wrap_xmodule_display == True: From 3b4093ab7b6bc12f52462da47dbed93c1c2922ce Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 14 Apr 2013 00:43:02 +0000 Subject: [PATCH 051/107] url and mitx_settings for masquerade --- lms/envs/common.py | 2 ++ lms/urls.py | 10 +++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lms/envs/common.py b/lms/envs/common.py index 8654b5ebf5..efc8a382d7 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -71,6 +71,8 @@ MITX_FEATURES = { 'ENABLE_LMS_MIGRATION': False, 'ENABLE_MANUAL_GIT_RELOAD': False, + 'ENABLE_MASQUERADE': True, # allow course staff to change to student view of courseware + 'DISABLE_LOGIN_BUTTON': False, # used in systems where login is automatic, eg MIT SSL 'STUB_VIDEO_FOR_TESTING': False, # do not display video when running automated acceptance tests diff --git a/lms/urls.py b/lms/urls.py index 4a0608720a..c792c149e1 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -201,9 +201,6 @@ if settings.WIKI_ENABLED: if settings.COURSEWARE_ENABLED: urlpatterns += ( - # Hook django-masquerade, allowing staff to view site as other users - url(r'^masquerade/', include('masquerade.urls')), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/jump_to/(?P.*)$', 'courseware.views.jump_to', name="jump_to"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/modx/(?P.*?)/(?P[^/]*)$', @@ -345,6 +342,13 @@ if settings.COURSEWARE_ENABLED: 'open_ended_grading.views.peer_grading', name='peer_grading'), ) + # allow course staff to change to student view of courseware + if settings.MITX_FEATURES.get('ENABLE_MASQUERADE'): + urlpatterns += ( + url(r'^masquerade/(?P.*)$','courseware.masquerade.handle_ajax', name="masquerate-switch"), + ) + + # discussion forums live within courseware, so courseware must be enabled first if settings.MITX_FEATURES.get('ENABLE_DISCUSSION_SERVICE'): urlpatterns += ( From af3e08e8c7910d173c234db739ebc73a07a643a1 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 14 Apr 2013 00:44:01 +0000 Subject: [PATCH 052/107] masquerade link in courseware/views.py --- lms/djangoapps/courseware/views.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index efd22f9985..91e7f4553b 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -292,6 +292,7 @@ def index(request, course_id, chapter=None, section=None, 'init': '', 'content': '', 'staff_access': staff_access, + 'masquerade': masq, 'xqa_server': settings.MITX_FEATURES.get('USE_XQA_SERVER', 'http://xqa:server@content-qa.mitx.mit.edu/xqa') } From fdfc37e4e321454cb54103b01bdf5ed3d72aadf9 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 14 Apr 2013 01:05:52 +0000 Subject: [PATCH 053/107] masquerade pep8 --- lms/djangoapps/courseware/masquerade.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index 2fe1830940..33c3239f27 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -15,8 +15,9 @@ log = logging.getLogger(__name__) MASQ_KEY = 'masquerade_identity' + def handle_ajax(request, marg): - if marg=='toggle': + if marg == 'toggle': status = request.session.get(MASQ_KEY, '') if status is None or status in ['', 'staff']: status = 'student' @@ -37,7 +38,7 @@ def setup_masquerade(request, staff_access=False): if request.user is None: return None - if not staff_access: # can masquerade only if user has staff access to course + if not staff_access: # can masquerade only if user has staff access to course return None usertype = request.session.get(MASQ_KEY, '') @@ -45,7 +46,7 @@ def setup_masquerade(request, staff_access=False): request.session[MASQ_KEY] = 'staff' usertype = 'staff' - if usertype=='student': + if usertype == 'student': request.user.masquerade_as_student = True return usertype From 857a6e0bd0589951a3e0c31acc51445f013f81e2 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sun, 14 Apr 2013 01:15:26 +0000 Subject: [PATCH 054/107] pylint masquerade fixes --- lms/djangoapps/courseware/access.py | 2 +- lms/djangoapps/courseware/masquerade.py | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 80b7af9694..f7cd74f121 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -15,7 +15,7 @@ from xmodule.modulestore import Location from xmodule.x_module import XModule, XModuleDescriptor from student.models import CourseEnrollmentAllowed -from masquerade import is_masquerading_as_student +from courseware.masquerade import is_masquerading_as_student DEBUG_ACCESS = False diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index 33c3239f27..5b8e265094 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -1,14 +1,12 @@ -#---------------------------------------- Masequerade ---------------------------------------- -# -# Allow course staff to see a student or staff view of courseware. -# Which kind of view has been selected is stored in the session state. +''' +---------------------------------------- Masequerade ---------------------------------------- +Allow course staff to see a student or staff view of courseware. +Which kind of view has been selected is stored in the session state. +''' import json import logging -from django.conf import settings -from django.contrib.auth.models import User -from django.contrib.auth.decorators import login_required from django.http import HttpResponse log = logging.getLogger(__name__) @@ -17,6 +15,9 @@ MASQ_KEY = 'masquerade_identity' def handle_ajax(request, marg): + ''' + Handle ajax call from "staff view" / "student view" toggle button + ''' if marg == 'toggle': status = request.session.get(MASQ_KEY, '') if status is None or status in ['', 'staff']: @@ -53,5 +54,8 @@ def setup_masquerade(request, staff_access=False): def is_masquerading_as_student(user): + ''' + Return True if user is masquerading as a student, False otherwise + ''' masq = getattr(user, 'masquerade_as_student', False) return masq From 1c2452e401b8a2aa532bc3db00c366d436e24647 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 13 Apr 2013 21:38:12 -0400 Subject: [PATCH 055/107] make is_masquerading_as_student work with Mock for tests --- lms/djangoapps/courseware/masquerade.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index 5b8e265094..e98b0e1df4 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -58,4 +58,4 @@ def is_masquerading_as_student(user): Return True if user is masquerading as a student, False otherwise ''' masq = getattr(user, 'masquerade_as_student', False) - return masq + return masq==True From fac9d758d8f8cd29ce1f47b2bc40a4c16f12ad7d Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 13 Apr 2013 22:14:51 -0400 Subject: [PATCH 056/107] fix typo in masquerade url name --- lms/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/urls.py b/lms/urls.py index c792c149e1..989118ddc4 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -345,7 +345,7 @@ if settings.COURSEWARE_ENABLED: # allow course staff to change to student view of courseware if settings.MITX_FEATURES.get('ENABLE_MASQUERADE'): urlpatterns += ( - url(r'^masquerade/(?P.*)$','courseware.masquerade.handle_ajax', name="masquerate-switch"), + url(r'^masquerade/(?P.*)$','courseware.masquerade.handle_ajax', name="masquerade-switch"), ) From 5ca0393e052a7e6c9a451d05ff42e0aa073d207c Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 13 Apr 2013 22:30:07 -0400 Subject: [PATCH 057/107] add test for masquerade --- .../courseware/tests/test_masquerade.py | 120 ++++++++++++++++++ 1 file changed, 120 insertions(+) create mode 100644 lms/djangoapps/courseware/tests/test_masquerade.py diff --git a/lms/djangoapps/courseware/tests/test_masquerade.py b/lms/djangoapps/courseware/tests/test_masquerade.py new file mode 100644 index 0000000000..11cdc4c1f9 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_masquerade.py @@ -0,0 +1,120 @@ +""" +Unit tests for masquerade + +Based on (and depends on) unit tests for courseware. + +Notes for running by hand: + +django-admin.py test --settings=lms.envs.test --pythonpath=. lms/djangoapps/courseware +""" + +from django.test.utils import override_settings + +from django.core.urlresolvers import reverse + +from django.contrib.auth.models import User, Group +from courseware.access import _course_staff_group_name +from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_XML_MODULESTORE, get_user +from xmodule.modulestore.django import modulestore +import xmodule.modulestore.django +import json + +@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE) +class TestStaffMasqueradeAsStudent(LoginEnrollmentTestCase): + ''' + Check for staff being able to masquerade as student + ''' + + def setUp(self): + xmodule.modulestore.django._MODULESTORES = {} + + #self.full = modulestore().get_course("edX/full/6.002_Spring_2012") + #self.toy = modulestore().get_course("edX/toy/2012_Fall") + self.graded_course = modulestore().get_course("edX/graded/2012_Fall") + + # Create staff account + self.instructor = 'view2@test.com' + self.password = 'foo' + self.create_account('u2', self.instructor, self.password) + self.activate_user(self.instructor) + + def make_instructor(course): + group_name = _course_staff_group_name(course.location) + g = Group.objects.create(name=group_name) + g.user_set.add(get_user(self.instructor)) + + make_instructor(self.graded_course) + + self.logout() + self.login(self.instructor, self.password) + self.enroll(self.graded_course) + # self.factory = RequestFactory() + + def get_cw_section(self): + url = reverse('courseware_section', + kwargs={'course_id': self.graded_course.id, + 'chapter': 'GradedChapter', + 'section': 'Homework1'}) + + resp = self.client.get(url) + + print "url ", url + return resp + + def test_staff_debug_for_staff(self): + resp = self.get_cw_section() + sdebug = '' + + self.assertTrue(sdebug in resp.content) + + + def toggle_masquerade(self): + ''' + Toggle masquerade state + ''' + masq_url = reverse('masquerade-switch', kwargs={'marg': 'toggle'}) + print "masq_url ", masq_url + resp = self.client.get(masq_url) + return resp + + def test_no_staff_debug_for_student(self): + togresp = self.toggle_masquerade() + print "masq now ", togresp.content + self.assertEqual(togresp.content, '{"status": "student"}', '') + + resp = self.get_cw_section() + sdebug = '' + + self.assertFalse(sdebug in resp.content) + + def get_problem(self): + pun = 'H1P1' + problem_location = "i4x://edX/graded/problem/%s" % pun + + modx_url = reverse('modx_dispatch', + kwargs={'course_id': self.graded_course.id, + 'location': problem_location, + 'dispatch': 'problem_get', }) + + resp = self.client.get(modx_url) + + print "modx_url ", modx_url + return resp + + def test_showanswer_for_staff(self): + resp = self.get_problem() + html = json.loads(resp.content)['html'] + print html + sabut = '' + self.assertTrue(sabut in html) + + def test_no_showanswer_for_student(self): + togresp = self.toggle_masquerade() + print "masq now ", togresp.content + self.assertEqual(togresp.content, '{"status": "student"}', '') + + resp = self.get_problem() + html = json.loads(resp.content)['html'] + print html + sabut = '' + self.assertFalse(sabut in html) From 5f74ccd1a0cdb46feaebe0e5e2f9486db7882c46 Mon Sep 17 00:00:00 2001 From: ichuang Date: Sat, 13 Apr 2013 22:52:17 -0400 Subject: [PATCH 058/107] use ENABLE_MASQUERADE in masquerade.py --- lms/djangoapps/courseware/masquerade.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index e98b0e1df4..27135f727e 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -8,6 +8,7 @@ import json import logging from django.http import HttpResponse +from django.conf import settings log = logging.getLogger(__name__) @@ -39,6 +40,9 @@ def setup_masquerade(request, staff_access=False): if request.user is None: return None + if not settings.MITX_FEATURES.get('ENABLE_MASQUERADE', False): + return None + if not staff_access: # can masquerade only if user has staff access to course return None From c4e4f73373b9c6ad67581126683e146d1cebec6d Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 17 Apr 2013 13:19:42 -0400 Subject: [PATCH 059/107] studio - adding in course name value in the share course mailto link --- cms/templates/settings.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index cc5dafc57b..fa6983300b 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -92,7 +92,7 @@ from contentstore import utils From 33ba38b8dda3ca57ed4565b9bb840a5513b7d236 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 17 Apr 2013 13:38:14 -0400 Subject: [PATCH 060/107] studio - prefixed course URL with https: for individual share course mailto link on settings --- cms/templates/settings.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index fa6983300b..da807fd1d2 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -92,7 +92,7 @@ from contentstore import utils From b115ad46bf31f97447d66a9883cc2204f0578995 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 17 Apr 2013 13:44:23 -0400 Subject: [PATCH 061/107] studio - changed https to http in course mailto link and added that prefix to link displayed --- cms/templates/settings.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index da807fd1d2..9cfe557d47 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -87,12 +87,12 @@ from contentstore import utils

    Course Summary Page (for student enrollment and access)

    -

    ${utils.get_lms_link_for_about_page(course_location)}

    +

    http:${utils.get_lms_link_for_about_page(course_location)}

    From 423fa4781134199111c62b437279cb6ddef7d0e9 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Wed, 17 Apr 2013 13:47:54 -0400 Subject: [PATCH 062/107] studio - reverting back to https references in the course mailto link and url display --- cms/templates/settings.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 9cfe557d47..7ed918604a 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -87,12 +87,12 @@ from contentstore import utils

    Course Summary Page (for student enrollment and access)

    -

    http:${utils.get_lms_link_for_about_page(course_location)}

    +

    https:${utils.get_lms_link_for_about_page(course_location)}

    From 44bf60d20811454071e0c3e6c871e159409e8a1c Mon Sep 17 00:00:00 2001 From: "Mark L. Chang" Date: Wed, 17 Apr 2013 14:03:29 -0400 Subject: [PATCH 063/107] copy changes, link text change --- cms/templates/settings.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 7ed918604a..23cc41d504 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -92,7 +92,7 @@ from contentstore import utils From 0f26be2f4db6beb500d470d8e5dbdb3415b2a0e6 Mon Sep 17 00:00:00 2001 From: "Mark L. Chang" Date: Wed, 17 Apr 2013 14:12:16 -0400 Subject: [PATCH 064/107] quick fix to email copy --- cms/templates/settings.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/settings.html b/cms/templates/settings.html index 23cc41d504..3923c0f905 100644 --- a/cms/templates/settings.html +++ b/cms/templates/settings.html @@ -92,7 +92,7 @@ from contentstore import utils From b42c4dacf73824dbbf32d1447661287548d77732 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 16 Apr 2013 11:19:11 -0400 Subject: [PATCH 065/107] Added test for "radio" choicegroup --- .../courseware/features/problems.feature | 5 ++++ .../courseware/features/problems.py | 26 ++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/features/problems.feature b/lms/djangoapps/courseware/features/problems.feature index dc8495af60..266ffa3680 100644 --- a/lms/djangoapps/courseware/features/problems.feature +++ b/lms/djangoapps/courseware/features/problems.feature @@ -15,6 +15,7 @@ Feature: Answer problems | drop down | | multiple choice | | checkbox | + | radio | | string | | numerical | | formula | @@ -33,6 +34,7 @@ Feature: Answer problems | drop down | | multiple choice | | checkbox | + | radio | | string | | numerical | | formula | @@ -50,6 +52,7 @@ Feature: Answer problems | drop down | | multiple choice | | checkbox | + | radio | | string | | numerical | | formula | @@ -71,6 +74,8 @@ Feature: Answer problems | multiple choice | incorrect | | checkbox | correct | | checkbox | incorrect | + | radio | correct | + | radio | incorrect | | string | correct | | string | incorrect | | numerical | correct | diff --git a/lms/djangoapps/courseware/features/problems.py b/lms/djangoapps/courseware/features/problems.py index b25d606c4e..3d538d7ae1 100644 --- a/lms/djangoapps/courseware/features/problems.py +++ b/lms/djangoapps/courseware/features/problems.py @@ -42,7 +42,13 @@ PROBLEM_FACTORY_DICT = { 'choice_type': 'checkbox', 'choices': [True, False, True, False, False], 'choice_names': ['Choice 1', 'Choice 2', 'Choice 3', 'Choice 4']}}, - + 'radio': { + 'factory': ChoiceResponseXMLFactory(), + 'kwargs': { + 'question_text': 'The correct answer is Choice 3', + 'choice_type': 'radio', + 'choices': [False, False, True, False], + 'choice_names': ['Choice 1', 'Choice 2', 'Choice 3', 'Choice 4']}}, 'string': { 'factory': StringResponseXMLFactory(), 'kwargs': { @@ -174,6 +180,12 @@ def answer_problem(step, problem_type, correctness): else: inputfield('checkbox', choice='choice_3').check() + elif problem_type == 'radio': + if correctness == 'correct': + inputfield('radio', choice='choice_2').check() + else: + inputfield('radio', choice='choice_1').check() + elif problem_type == 'string': textvalue = 'correct string' if correctness == 'correct' \ else 'incorrect' @@ -252,6 +264,14 @@ def assert_problem_has_answer(step, problem_type, answer_class): else: assert_checked('checkbox', []) + elif problem_type == "radio": + if answer_class == 'correct': + assert_checked('radio', ['choice_2']) + elif answer_class == 'incorrect': + assert_checked('radio', ['choice_1']) + else: + assert_checked('radio', []) + elif problem_type == 'string': if answer_class == 'blank': expected = '' @@ -298,6 +318,7 @@ CORRECTNESS_SELECTORS = { 'correct': {'drop down': ['span.correct'], 'multiple choice': ['label.choicegroup_correct'], 'checkbox': ['span.correct'], + 'radio': ['label.choicegroup_correct'], 'string': ['div.correct'], 'numerical': ['div.correct'], 'formula': ['div.correct'], @@ -308,6 +329,8 @@ CORRECTNESS_SELECTORS = { 'multiple choice': ['label.choicegroup_incorrect', 'span.incorrect'], 'checkbox': ['span.incorrect'], + 'radio': ['label.choicegroup_incorrect', + 'span.incorrect'], 'string': ['div.incorrect'], 'numerical': ['div.incorrect'], 'formula': ['div.incorrect'], @@ -317,6 +340,7 @@ CORRECTNESS_SELECTORS = { 'unanswered': {'drop down': ['span.unanswered'], 'multiple choice': ['span.unanswered'], 'checkbox': ['span.unanswered'], + 'radio': ['span.unanswered'], 'string': ['div.unanswered'], 'numerical': ['div.unanswered'], 'formula': ['div.unanswered'], From 754d1eb70253342e35b758a360163fade6e1a26e Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 16 Apr 2013 17:22:01 -0400 Subject: [PATCH 066/107] Added tests for ChoiceGroup template --- .../capa/capa/tests/test_input_templates.py | 276 ++++++++++++++++++ 1 file changed, 276 insertions(+) create mode 100644 common/lib/capa/capa/tests/test_input_templates.py diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py new file mode 100644 index 0000000000..30359060f0 --- /dev/null +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -0,0 +1,276 @@ +"""Tests for the logic in input type mako templates.""" +#pylint: disable=R0904 + +import unittest +import capa +import os.path +from lxml import etree +from mako.template import Template as MakoTemplate + + +class TemplateTestCase(unittest.TestCase): + """Utilitites for testing templates""" + + # Allow us to pass an extra arg to setUp to configure + # the test case. Also allow setUp as a valid method name. + #pylint: disable=W0221 + #pylint: disable=C0103 + def setUp(self, template_name): + """Load the template + + `template_name` is the file name of the template + to be loaded from capa/templates. + The template name should include the .html extension: + for example: choicegroup.html + """ + capa_path = capa.__path__[0] + self.template_path = os.path.join(capa_path, 'templates', template_name) + template_file = open(self.template_path) + self.template = MakoTemplate(template_file.read()) + template_file.close() + + # Allow us to pass **context_dict to render_unicode() + #pylint: disable=W0142 + def render_to_xml(self, context_dict): + """Render the template using the `context_dict` dict. + + Returns an `etree` XML element.""" + xml_str = self.template.render_unicode(**context_dict) + return etree.fromstring(xml_str) + + def assert_has_xpath(self, xml_root, xpath, context_dict, exact_num=1): + """Asserts that the xml tree has an element satisfying `xpath`. + + `xml_root` is an etree XML element + `xpath` is an XPath string, such as `'/foo/bar'` + `context` is used to print a debugging message + `exact_num` is the exact number of matches to expect. + """ + message = ("XML does not have %d match(es) for xpath '%s'\nXML: %s\nContext: %s" + % (exact_num, str(xpath), etree.tostring(xml_root), str(context_dict))) + + self.assertEqual(len(xml_root.xpath(xpath)), exact_num, msg=message) + + def assert_no_xpath(self, xml_root, xpath, context_dict): + """Asserts that the xml tree does NOT have an element + satisfying `xpath`. + + `xml_root` is an etree XML element + `xpath` is an XPath string, such as `'/foo/bar'` + `context` is used to print a debugging message + """ + self.assert_has_xpath(xml_root, xpath, context_dict, exact_num=0) + + +class TestChoiceGroupTemplate(TemplateTestCase): + """Test mako template for `` input""" + + # Allow us to pass an extra arg to setUp to configure + # the test case. Also allow setUp as a valid method name. + #pylint: disable=W0221 + #pylint: disable=C0103 + def setUp(self): + choices = [('1', 'choice 1'), ('2', 'choice 2'), ('3', 'choice 3')] + self.context = {'id': '1', + 'choices': choices, + 'status': 'correct', + 'input_type': 'checkbox', + 'name_array_suffix': '1', + 'value': '3'} + super(TestChoiceGroupTemplate, self).setUp('choicegroup.html') + + def test_problem_marked_correct(self): + """Test conditions under which the entire problem + (not a particular option) is marked correct""" + + self.context['status'] = 'correct' + self.context['input_type'] = 'checkbox' + self.context['value'] = ['1', '2'] + + # Should mark the entire problem correct + xml = self.render_to_xml(self.context) + xpath = "//div[@class='indicator_container']/span[@class='correct']" + self.assert_has_xpath(xml, xpath, self.context) + + # Should NOT mark individual options + self.assert_no_xpath(xml, "//label[@class='choicegroup_incorrect']", + self.context) + + self.assert_no_xpath(xml, "//label[@class='choicegroup_correct']", + self.context) + + def test_problem_marked_incorrect(self): + """Test all conditions under which the entire problem + (not a particular option) is marked incorrect""" + conditions = [ + {'status': 'incorrect', 'input_type': 'radio', 'value': ''}, + {'status': 'incorrect', 'input_type': 'checkbox', 'value': []}, + {'status': 'incorrect', 'input_type': 'checkbox', 'value': ['2']}, + {'status': 'incorrect', 'input_type': 'checkbox', 'value': ['2', '3']}, + {'status': 'incomplete', 'input_type': 'radio', 'value': ''}, + {'status': 'incomplete', 'input_type': 'checkbox', 'value': []}, + {'status': 'incomplete', 'input_type': 'checkbox', 'value': ['2']}, + {'status': 'incomplete', 'input_type': 'checkbox', 'value': ['2', '3']}] + + for test_conditions in conditions: + self.context.update(test_conditions) + xml = self.render_to_xml(self.context) + xpath = "//div[@class='indicator_container']/span[@class='incorrect']" + self.assert_has_xpath(xml, xpath, self.context) + + # Should NOT mark individual options + self.assert_no_xpath(xml, + "//label[@class='choicegroup_incorrect']", + self.context) + + self.assert_no_xpath(xml, + "//label[@class='choicegroup_correct']", + self.context) + + def test_problem_marked_unanswered(self): + """Test all conditions under which the entire problem + (not a particular option) is marked unanswered""" + conditions = [ + {'status': 'unsubmitted', 'input_type': 'radio', 'value': ''}, + {'status': 'unsubmitted', 'input_type': 'radio', 'value': []}, + {'status': 'unsubmitted', 'input_type': 'checkbox', 'value': []}, + {'input_type': 'radio', 'value': ''}, + {'input_type': 'radio', 'value': []}, + {'input_type': 'checkbox', 'value': []}, + {'input_type': 'checkbox', 'value': ['1']}, + {'input_type': 'checkbox', 'value': ['1', '2']}] + + self.context['status'] = 'unanswered' + + for test_conditions in conditions: + self.context.update(test_conditions) + xml = self.render_to_xml(self.context) + xpath = "//div[@class='indicator_container']/span[@class='unanswered']" + self.assert_has_xpath(xml, xpath, self.context) + + # Should NOT mark individual options + self.assert_no_xpath(xml, + "//label[@class='choicegroup_incorrect']", + self.context) + + self.assert_no_xpath(xml, + "//label[@class='choicegroup_correct']", + self.context) + + def test_option_marked_correct(self): + """Test conditions under which a particular option + (not the entire problem) is marked correct.""" + conditions = [ + {'input_type': 'radio', 'value': '2'}, + {'input_type': 'radio', 'value': ['2']}] + + self.context['status'] = 'correct' + + for test_conditions in conditions: + self.context.update(test_conditions) + xml = self.render_to_xml(self.context) + xpath = "//label[@class='choicegroup_correct']" + self.assert_has_xpath(xml, xpath, self.context) + + # Should NOT mark the whole problem + xpath = "//div[@class='indicator_container']/span" + self.assert_no_xpath(xml, xpath, self.context) + + def test_option_marked_incorrect(self): + """Test conditions under which a particular option + (not the entire problem) is marked incorrect.""" + conditions = [ + {'input_type': 'radio', 'value': '2'}, + {'input_type': 'radio', 'value': ['2']}] + + self.context['status'] = 'incorrect' + + for test_conditions in conditions: + self.context.update(test_conditions) + xml = self.render_to_xml(self.context) + xpath = "//label[@class='choicegroup_incorrect']" + self.assert_has_xpath(xml, xpath, self.context) + + # Should NOT mark the whole problem + xpath = "//div[@class='indicator_container']/span" + self.assert_no_xpath(xml, xpath, self.context) + + def test_never_show_correctness(self): + """Test conditions under which we tell the template to + NOT show correct/incorrect, but instead show a message. + + This is used, for example, by the Justice course to ask + questions without specifying a correct answer. When + the student responds, the problem displays "Thank you + for your response" + """ + + conditions = [ + {'input_type': 'radio', 'status': 'correct', 'value': ''}, + {'input_type': 'radio', 'status': 'correct', 'value': '2'}, + {'input_type': 'radio', 'status': 'correct', 'value': ['2']}, + {'input_type': 'radio', 'status': 'incorrect', 'value': '2'}, + {'input_type': 'radio', 'status': 'incorrect', 'value': []}, + {'input_type': 'radio', 'status': 'incorrect', 'value': ['2']}, + {'input_type': 'checkbox', 'status': 'correct', 'value': []}, + {'input_type': 'checkbox', 'status': 'correct', 'value': ['2']}, + {'input_type': 'checkbox', 'status': 'incorrect', 'value': []}, + {'input_type': 'checkbox', 'status': 'incorrect', 'value': ['2']}] + + self.context['show_correctness'] = 'never' + self.context['submitted_message'] = 'Test message' + + for test_conditions in conditions: + self.context.update(test_conditions) + xml = self.render_to_xml(self.context) + + # Should NOT mark the entire problem correct/incorrect + xpath = "//div[@class='indicator_container']/span[@class='correct']" + self.assert_no_xpath(xml, xpath, self.context) + + xpath = "//div[@class='indicator_container']/span[@class='incorrect']" + self.assert_no_xpath(xml, xpath, self.context) + + # Should NOT mark individual options + self.assert_no_xpath(xml, + "//label[@class='choicegroup_incorrect']", + self.context) + + self.assert_no_xpath(xml, + "//label[@class='choicegroup_correct']", + self.context) + + # Expect to see the message + message_elements = xml.xpath("//div[@class='capa_alert']") + self.assertEqual(len(message_elements), 1) + self.assertEqual(message_elements[0].text, + self.context['submitted_message']) + + def test_no_message_before_submission(self): + """Ensure that we don't show the `submitted_message` + before submitting""" + + conditions = [ + {'input_type': 'radio', 'status': 'unsubmitted', 'value': ''}, + {'input_type': 'radio', 'status': 'unsubmitted', 'value': []}, + {'input_type': 'checkbox', 'status': 'unsubmitted', 'value': []}, + + # These tests expose bug #365 + # When the bug is fixed, uncomment these cases. + #{'input_type': 'radio', 'status': 'unsubmitted', 'value': '2'}, + #{'input_type': 'radio', 'status': 'unsubmitted', 'value': ['2']}, + #{'input_type': 'radio', 'status': 'unsubmitted', 'value': '2'}, + #{'input_type': 'radio', 'status': 'unsubmitted', 'value': ['2']}, + #{'input_type': 'checkbox', 'status': 'unsubmitted', 'value': ['2']}, + #{'input_type': 'checkbox', 'status': 'unsubmitted', 'value': ['2']}] + ] + + self.context['show_correctness'] = 'never' + self.context['submitted_message'] = 'Test message' + + for test_conditions in conditions: + self.context.update(test_conditions) + xml = self.render_to_xml(self.context) + + # Expect that we do NOT see the message yet + self.assert_no_xpath(xml, "//div[@class='capa_alert']", self.context) From 52c2f3ae372e551da4278b1879831f83f4b116c2 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Wed, 17 Apr 2013 08:55:08 -0400 Subject: [PATCH 067/107] Tested additional values of rerandomize (true/false/per_student) Pylint fixes --- .../capa/capa/tests/test_input_templates.py | 33 ++-- .../xmodule/xmodule/tests/test_capa_module.py | 155 ++++++++++++------ 2 files changed, 119 insertions(+), 69 deletions(-) diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 30359060f0..7bb32acd10 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -1,5 +1,4 @@ """Tests for the logic in input type mako templates.""" -#pylint: disable=R0904 import unittest import capa @@ -11,26 +10,22 @@ from mako.template import Template as MakoTemplate class TemplateTestCase(unittest.TestCase): """Utilitites for testing templates""" - # Allow us to pass an extra arg to setUp to configure - # the test case. Also allow setUp as a valid method name. - #pylint: disable=W0221 - #pylint: disable=C0103 - def setUp(self, template_name): - """Load the template + # Subclasses override this to specify the file name of the template + # to be loaded from capa/templates. + # The template name should include the .html extension: + # for example: choicegroup.html + TEMPLATE_NAME = None - `template_name` is the file name of the template - to be loaded from capa/templates. - The template name should include the .html extension: - for example: choicegroup.html - """ + def setUp(self): + """Load the template""" capa_path = capa.__path__[0] - self.template_path = os.path.join(capa_path, 'templates', template_name) + self.template_path = os.path.join(capa_path, + 'templates', + self.TEMPLATE_NAME) template_file = open(self.template_path) self.template = MakoTemplate(template_file.read()) template_file.close() - # Allow us to pass **context_dict to render_unicode() - #pylint: disable=W0142 def render_to_xml(self, context_dict): """Render the template using the `context_dict` dict. @@ -65,10 +60,8 @@ class TemplateTestCase(unittest.TestCase): class TestChoiceGroupTemplate(TemplateTestCase): """Test mako template for `` input""" - # Allow us to pass an extra arg to setUp to configure - # the test case. Also allow setUp as a valid method name. - #pylint: disable=W0221 - #pylint: disable=C0103 + TEMPLATE_NAME = 'choicegroup.html' + def setUp(self): choices = [('1', 'choice 1'), ('2', 'choice 2'), ('3', 'choice 3')] self.context = {'id': '1', @@ -77,7 +70,7 @@ class TestChoiceGroupTemplate(TemplateTestCase): 'input_type': 'checkbox', 'name_array_suffix': '1', 'value': '3'} - super(TestChoiceGroupTemplate, self).setUp('choicegroup.html') + super(TestChoiceGroupTemplate, self).setUp() def test_problem_marked_correct(self): """Test conditions under which the entire problem diff --git a/common/lib/xmodule/xmodule/tests/test_capa_module.py b/common/lib/xmodule/xmodule/tests/test_capa_module.py index bc5d342646..f948f5bdfe 100644 --- a/common/lib/xmodule/xmodule/tests/test_capa_module.py +++ b/common/lib/xmodule/xmodule/tests/test_capa_module.py @@ -1,17 +1,19 @@ +"""Tests of the Capa XModule""" +#pylint: disable=C0111 +#pylint: disable=R0904 +#pylint: disable=C0103 +#pylint: disable=C0302 + import datetime -import json -from mock import Mock, MagicMock, patch -from pprint import pprint +from mock import Mock, patch import unittest import random import xmodule -import capa from capa.responsetypes import StudentInputError, \ - LoncapaProblemError, ResponseError + LoncapaProblemError, ResponseError from xmodule.capa_module import CapaModule from xmodule.modulestore import Location -from lxml import etree from django.http import QueryDict @@ -384,7 +386,7 @@ class CapaModuleTest(unittest.TestCase): # what the input is, by patching CorrectMap.is_correct() # Also simulate rendering the HTML # TODO: pep8 thinks the following line has invalid syntax - with patch('capa.correctmap.CorrectMap.is_correct') as mock_is_correct,\ + with patch('capa.correctmap.CorrectMap.is_correct') as mock_is_correct, \ patch('xmodule.capa_module.CapaModule.get_problem_html') as mock_html: mock_is_correct.return_value = True mock_html.return_value = "Test HTML" @@ -435,32 +437,38 @@ class CapaModuleTest(unittest.TestCase): self.assertEqual(module.attempts, 3) def test_check_problem_resubmitted_with_randomize(self): - # Randomize turned on - module = CapaFactory.create(rerandomize='always', attempts=0) + rerandomize_values = ['always', 'true'] - # Simulate that the problem is completed - module.done = True + for rerandomize in rerandomize_values: + # Randomize turned on + module = CapaFactory.create(rerandomize=rerandomize, attempts=0) - # Expect that we cannot submit - with self.assertRaises(xmodule.exceptions.NotFoundError): - get_request_dict = {CapaFactory.input_key(): '3.14'} - module.check_problem(get_request_dict) + # Simulate that the problem is completed + module.done = True - # Expect that number of attempts NOT incremented - self.assertEqual(module.attempts, 0) + # Expect that we cannot submit + with self.assertRaises(xmodule.exceptions.NotFoundError): + get_request_dict = {CapaFactory.input_key(): '3.14'} + module.check_problem(get_request_dict) + + # Expect that number of attempts NOT incremented + self.assertEqual(module.attempts, 0) def test_check_problem_resubmitted_no_randomize(self): - # Randomize turned off - module = CapaFactory.create(rerandomize='never', attempts=0, done=True) + rerandomize_values = ['never', 'false', 'per_student'] - # Expect that we can submit successfully - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.check_problem(get_request_dict) + for rerandomize in rerandomize_values: + # Randomize turned off + module = CapaFactory.create(rerandomize=rerandomize, attempts=0, done=True) - self.assertEqual(result['success'], 'correct') + # Expect that we can submit successfully + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.check_problem(get_request_dict) - # Expect that number of attempts IS incremented - self.assertEqual(module.attempts, 1) + self.assertEqual(result['success'], 'correct') + + # Expect that number of attempts IS incremented + self.assertEqual(module.attempts, 1) def test_check_problem_queued(self): module = CapaFactory.create(attempts=1) @@ -615,24 +623,34 @@ class CapaModuleTest(unittest.TestCase): self.assertTrue('success' in result and not result['success']) def test_save_problem_submitted_with_randomize(self): - module = CapaFactory.create(rerandomize='always', done=True) - # Try to save - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.save_problem(get_request_dict) + # Capa XModule treats 'always' and 'true' equivalently + rerandomize_values = ['always', 'true'] - # Expect that we cannot save - self.assertTrue('success' in result and not result['success']) + for rerandomize in rerandomize_values: + module = CapaFactory.create(rerandomize=rerandomize, done=True) + + # Try to save + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.save_problem(get_request_dict) + + # Expect that we cannot save + self.assertTrue('success' in result and not result['success']) def test_save_problem_submitted_no_randomize(self): - module = CapaFactory.create(rerandomize='never', done=True) - # Try to save - get_request_dict = {CapaFactory.input_key(): '3.14'} - result = module.save_problem(get_request_dict) + # Capa XModule treats 'false' and 'per_student' equivalently + rerandomize_values = ['never', 'false', 'per_student'] - # Expect that we succeed - self.assertTrue('success' in result and result['success']) + for rerandomize in rerandomize_values: + module = CapaFactory.create(rerandomize=rerandomize, done=True) + + # Try to save + get_request_dict = {CapaFactory.input_key(): '3.14'} + result = module.save_problem(get_request_dict) + + # Expect that we succeed + self.assertTrue('success' in result and result['success']) def test_check_button_name(self): @@ -681,21 +699,30 @@ class CapaModuleTest(unittest.TestCase): # If user submitted a problem but hasn't reset, # do NOT show the check button - # Note: we can only reset when rerandomize="always" + # Note: we can only reset when rerandomize="always" or "true" module = CapaFactory.create(rerandomize="always", done=True) self.assertFalse(module.should_show_check_button()) + module = CapaFactory.create(rerandomize="true", done=True) + self.assertFalse(module.should_show_check_button()) + # Otherwise, DO show the check button module = CapaFactory.create() self.assertTrue(module.should_show_check_button()) # If the user has submitted the problem # and we do NOT have a reset button, then we can show the check button - # Setting rerandomize to "never" ensures that the reset button + # Setting rerandomize to "never" or "false" ensures that the reset button # is not shown module = CapaFactory.create(rerandomize="never", done=True) self.assertTrue(module.should_show_check_button()) + module = CapaFactory.create(rerandomize="false", done=True) + self.assertTrue(module.should_show_check_button()) + + module = CapaFactory.create(rerandomize="per_student", done=True) + self.assertTrue(module.should_show_check_button()) + def test_should_show_reset_button(self): attempts = random.randint(1, 10) @@ -712,6 +739,14 @@ class CapaModuleTest(unittest.TestCase): module = CapaFactory.create(rerandomize="never", done=True) self.assertFalse(module.should_show_reset_button()) + # If we're NOT randomizing, then do NOT show the reset button + module = CapaFactory.create(rerandomize="per_student", done=True) + self.assertFalse(module.should_show_reset_button()) + + # If we're NOT randomizing, then do NOT show the reset button + module = CapaFactory.create(rerandomize="false", done=True) + self.assertFalse(module.should_show_reset_button()) + # If the user hasn't submitted an answer yet, # then do NOT show the reset button module = CapaFactory.create(done=False) @@ -742,13 +777,19 @@ class CapaModuleTest(unittest.TestCase): module = CapaFactory.create(rerandomize="always", done=True) self.assertFalse(module.should_show_save_button()) + module = CapaFactory.create(rerandomize="true", done=True) + self.assertFalse(module.should_show_save_button()) + # If the user has unlimited attempts and we are not randomizing, # then do NOT show a save button # because they can keep using "Check" module = CapaFactory.create(max_attempts=None, rerandomize="never", done=False) self.assertFalse(module.should_show_save_button()) - module = CapaFactory.create(max_attempts=None, rerandomize="never", done=True) + module = CapaFactory.create(max_attempts=None, rerandomize="false", done=True) + self.assertFalse(module.should_show_save_button()) + + module = CapaFactory.create(max_attempts=None, rerandomize="per_student", done=True) self.assertFalse(module.should_show_save_button()) # Otherwise, DO show the save button @@ -759,6 +800,12 @@ class CapaModuleTest(unittest.TestCase): module = CapaFactory.create(rerandomize="never", max_attempts=2, done=True) self.assertTrue(module.should_show_save_button()) + module = CapaFactory.create(rerandomize="false", max_attempts=2, done=True) + self.assertTrue(module.should_show_save_button()) + + module = CapaFactory.create(rerandomize="per_student", max_attempts=2, done=True) + self.assertTrue(module.should_show_save_button()) + # If survey question for capa (max_attempts = 0), # DO show the save button module = CapaFactory.create(max_attempts=0, done=False) @@ -788,9 +835,15 @@ class CapaModuleTest(unittest.TestCase): done=True) self.assertTrue(module.should_show_save_button()) + module = CapaFactory.create(force_save_button="true", + rerandomize="true", + done=True) + self.assertTrue(module.should_show_save_button()) + def test_no_max_attempts(self): module = CapaFactory.create(max_attempts='') html = module.get_problem_html() + self.assertTrue(html is not None) # assert that we got here without exploding def test_get_problem_html(self): @@ -875,6 +928,8 @@ class CapaModuleTest(unittest.TestCase): # Try to render the module with DEBUG turned off html = module.get_problem_html() + self.assertTrue(html is not None) + # Check the rendering context render_args, _ = module.system.render_template.call_args context = render_args[1] @@ -886,7 +941,9 @@ class CapaModuleTest(unittest.TestCase): def test_random_seed_no_change(self): # Run the test for each possible rerandomize value - for rerandomize in ['never', 'per_student', 'always', 'onreset']: + for rerandomize in ['false', 'never', + 'per_student', 'always', + 'true', 'onreset']: module = CapaFactory.create(rerandomize=rerandomize) # Get the seed @@ -896,8 +953,9 @@ class CapaModuleTest(unittest.TestCase): # If we're not rerandomizing, the seed is always set # to the same value (1) - if rerandomize == 'never': - self.assertEqual(seed, 1) + if rerandomize in ['never']: + self.assertEqual(seed, 1, + msg="Seed should always be 1 when rerandomize='%s'" % rerandomize) # Check the problem get_request_dict = {CapaFactory.input_key(): '3.14'} @@ -947,7 +1005,8 @@ class CapaModuleTest(unittest.TestCase): return success # Run the test for each possible rerandomize value - for rerandomize in ['never', 'per_student', 'always', 'onreset']: + for rerandomize in ['never', 'false', 'per_student', + 'always', 'true', 'onreset']: module = CapaFactory.create(rerandomize=rerandomize) # Get the seed @@ -959,7 +1018,7 @@ class CapaModuleTest(unittest.TestCase): # is set to 'never' -- it should still be 1 # The seed also stays the same if we're randomizing # 'per_student': the same student should see the same problem - if rerandomize in ['never', 'per_student']: + if rerandomize in ['never', 'false', 'per_student']: self.assertEqual(seed, _reset_and_get_seed(module)) # Otherwise, we expect the seed to change @@ -969,10 +1028,8 @@ class CapaModuleTest(unittest.TestCase): # Since there's a small chance we might get the # same seed again, give it 5 chances # to generate a different seed - success = _retry_and_check(5, - lambda: _reset_and_get_seed(module) != seed) + success = _retry_and_check(5, lambda: _reset_and_get_seed(module) != seed) - # TODO: change this comparison to module.seed is not None? - self.assertTrue(module.seed != None) + self.assertTrue(module.seed is not None) msg = 'Could not get a new seed from reset after 5 tries' self.assertTrue(success, msg) From 841d3484c847aa31226a7a169f3e9c9fbc9bca66 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Wed, 17 Apr 2013 12:17:23 -0400 Subject: [PATCH 068/107] Added test for textline input template --- .../capa/capa/tests/test_input_templates.py | 152 +++++++++++++++++- 1 file changed, 145 insertions(+), 7 deletions(-) diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 7bb32acd10..1a046ddf4b 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -5,7 +5,11 @@ import capa import os.path from lxml import etree from mako.template import Template as MakoTemplate +from mako import exceptions +class TemplateError(Exception): + """Error occurred while rendering a Mako template""" + pass class TemplateTestCase(unittest.TestCase): """Utilitites for testing templates""" @@ -30,7 +34,11 @@ class TemplateTestCase(unittest.TestCase): """Render the template using the `context_dict` dict. Returns an `etree` XML element.""" - xml_str = self.template.render_unicode(**context_dict) + try: + xml_str = self.template.render_unicode(**context_dict) + except: + raise TemplateError(exceptions.text_error_template().render()) + return etree.fromstring(xml_str) def assert_has_xpath(self, xml_root, xpath, context_dict, exact_num=1): @@ -56,8 +64,28 @@ class TemplateTestCase(unittest.TestCase): """ self.assert_has_xpath(xml_root, xpath, context_dict, exact_num=0) + def assert_has_text(self, xml_root, xpath, text, exact=True): + """Find the element at `xpath` in `xml_root` and assert + that its text is `text`. -class TestChoiceGroupTemplate(TemplateTestCase): + `xml_root` is an etree XML element + `xpath` is an XPath string, such as `'/foo/bar'` + `text` is the expected text that the element should contain + + If multiple elements are found, checks the first one. + If no elements are found, the assertion fails. + """ + element_list = xml_root.xpath(xpath) + self.assertTrue(len(element_list) > 0, + "Could not find element at '%s'" % str(xpath)) + + if exact: + self.assertEqual(text, element_list[0].text) + else: + self.assertIn(text, element_list[0].text) + + +class ChoiceGroupTemplateTest(TemplateTestCase): """Test mako template for `` input""" TEMPLATE_NAME = 'choicegroup.html' @@ -120,7 +148,7 @@ class TestChoiceGroupTemplate(TemplateTestCase): "//label[@class='choicegroup_correct']", self.context) - def test_problem_marked_unanswered(self): + def test_problem_marked_unsubmitted(self): """Test all conditions under which the entire problem (not a particular option) is marked unanswered""" conditions = [ @@ -234,10 +262,8 @@ class TestChoiceGroupTemplate(TemplateTestCase): self.context) # Expect to see the message - message_elements = xml.xpath("//div[@class='capa_alert']") - self.assertEqual(len(message_elements), 1) - self.assertEqual(message_elements[0].text, - self.context['submitted_message']) + self.assert_has_text(xml, "//div[@class='capa_alert']", + self.context['submitted_message']) def test_no_message_before_submission(self): """Ensure that we don't show the `submitted_message` @@ -267,3 +293,115 @@ class TestChoiceGroupTemplate(TemplateTestCase): # Expect that we do NOT see the message yet self.assert_no_xpath(xml, "//div[@class='capa_alert']", self.context) + + +class TextlineTemplateTest(TemplateTestCase): + """Test mako template for `` input""" + + # Allow us to pass an extra arg to setUp to configure + # the test case. + #pylint: disable=W0221 + def setUp(self): + self.context = {'id': '1', + 'status': 'correct', + 'value': '3', + 'preprocessor': None, + 'trailing_text': None} + super(TextlineTemplateTest, self).setUp('textline.html') + + def test_section_class(self): + cases = [ ({}, ' capa_inputtype '), + ({'do_math': True}, 'text-input-dynamath capa_inputtype '), + ({'inline': True}, ' capa_inputtype inline'), + ({'do_math': True, 'inline': True}, 'text-input-dynamath capa_inputtype inline'), + ] + + for (context, css_class) in cases: + base_context = self.context.copy() + base_context.update(context) + xml = self.render_to_xml(base_context) + xpath = "//section[@class='%s']" % css_class + self.assert_has_xpath(xml, xpath, self.context) + + def test_status(self): + cases = [('correct', 'correct', 'correct'), + ('unsubmitted', 'unanswered', 'unanswered'), + ('incorrect', 'incorrect', 'incorrect'), + ('incomplete', 'incorrect', 'incomplete')] + + for (context_status, div_class, status_mark) in cases: + self.context['status'] = context_status + xml = self.render_to_xml(self.context) + + # Expect that we get a
    with correct class + xpath = "//div[@class='%s ']" % div_class + self.assert_has_xpath(xml, xpath, self.context) + + # Expect that we get a

    with class="status" + # (used to by CSS to draw the green check / red x) + self.assert_has_text(xml, "//p[@class='status']", + status_mark, exact=False) + + def test_hidden(self): + self.context['hidden'] = True + xml = self.render_to_xml(self.context) + + xpath = "//div[@style='display:none;']" + self.assert_has_xpath(xml, xpath, self.context) + + xpath = "//input[@style='display:none;']" + self.assert_has_xpath(xml, xpath, self.context) + + def test_do_math(self): + self.context['do_math'] = True + xml = self.render_to_xml(self.context) + + xpath = "//input[@class='math']" + self.assert_has_xpath(xml, xpath, self.context) + + xpath = "//div[@class='equation']" + self.assert_has_xpath(xml, xpath, self.context) + + xpath = "//textarea[@id='input_1_dynamath']" + self.assert_has_xpath(xml, xpath, self.context) + + def test_size(self): + self.context['size'] = '20' + xml = self.render_to_xml(self.context) + + xpath = "//input[@size='20']" + self.assert_has_xpath(xml, xpath, self.context) + + def test_preprocessor(self): + self.context['preprocessor'] = {'class_name': 'test_class', + 'script_src': 'test_script'} + xml = self.render_to_xml(self.context) + + xpath = "//div[@class='text-input-dynamath_data' and @data-preprocessor='test_class']" + self.assert_has_xpath(xml, xpath, self.context) + + xpath = "//div[@class='script_placeholder' and @data-src='test_script']" + self.assert_has_xpath(xml, xpath, self.context) + + def test_do_inline(self): + cases = [('correct', 'correct'), + ('unsubmitted', 'unanswered'), + ('incorrect', 'incorrect'), + ('incomplete', 'incorrect')] + + self.context['inline'] = True + + for (context_status, div_class) in cases: + self.context['status'] = context_status + xml = self.render_to_xml(self.context) + + # Expect that we get a

    with correct class + xpath = "//div[@class='%s inline']" % div_class + self.assert_has_xpath(xml, xpath, self.context) + + def test_message(self): + self.context['msg'] = "Test message" + xml = self.render_to_xml(self.context) + + xpath = "//span[@class='message']" + self.assert_has_text(xml, xpath, self.context['msg']) From a57a093e73a7ecd98bdd6223ee893e58a5343532 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Wed, 17 Apr 2013 15:56:05 -0400 Subject: [PATCH 069/107] Rebased to master --- common/lib/capa/capa/tests/test_input_templates.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 1a046ddf4b..4ca020be07 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -98,7 +98,7 @@ class ChoiceGroupTemplateTest(TemplateTestCase): 'input_type': 'checkbox', 'name_array_suffix': '1', 'value': '3'} - super(TestChoiceGroupTemplate, self).setUp() + super(ChoiceGroupTemplateTest, self).setUp() def test_problem_marked_correct(self): """Test conditions under which the entire problem @@ -298,16 +298,15 @@ class ChoiceGroupTemplateTest(TemplateTestCase): class TextlineTemplateTest(TemplateTestCase): """Test mako template for `` input""" - # Allow us to pass an extra arg to setUp to configure - # the test case. - #pylint: disable=W0221 + TEMPLATE_NAME = 'textline.html' + def setUp(self): self.context = {'id': '1', 'status': 'correct', 'value': '3', 'preprocessor': None, 'trailing_text': None} - super(TextlineTemplateTest, self).setUp('textline.html') + super(TextlineTemplateTest, self).setUp() def test_section_class(self): cases = [ ({}, ' capa_inputtype '), From 3de927fe017ec22793d471016aae4bf9b517a0b6 Mon Sep 17 00:00:00 2001 From: ichuang Date: Wed, 17 Apr 2013 21:22:08 -0400 Subject: [PATCH 070/107] untabify course_nagivation.html --- .../courseware/course_navigation.html | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lms/templates/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 4e77c96adb..ea367873e2 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -29,9 +29,9 @@ def url_class(is_active): <%block name="extratabs" /> % if masquerade is not UNDEFINED: % if staff_access and masquerade is not None: -
  • Staff view
    • - % endif - % endif +
  • Staff view
    • + % endif + % endif
    @@ -42,11 +42,11 @@ def url_class(is_active): masq = (function(){ var el = $('#staffstatus'); var setstat = function(status){ - if (status=='student'){ - el.html('Student view'); - }else{ - el.html('Staff view'); - } + if (status=='student'){ + el.html('Student view'); + }else{ + el.html('Staff view'); + } } setstat('${masquerade}'); @@ -55,7 +55,7 @@ masq = (function(){ type: 'GET', success: function(result){ setstat(result.status); - location.reload(); + location.reload(); }, error: function() { alert('Error: cannot connect to server'); From 0ffc399f7df2fad88f49cc16a2a27a340764daf6 Mon Sep 17 00:00:00 2001 From: ichuang Date: Wed, 17 Apr 2013 22:12:12 -0400 Subject: [PATCH 071/107] move masquerade call up, to make sure it is used for start date checks --- lms/djangoapps/courseware/module_render.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index eb819fd5a5..6f05b32778 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -165,14 +165,14 @@ def get_module_for_descriptor(user, request, descriptor, model_data_cache, cours Actually implement get_module. See docstring there for details. """ - # Short circuit--if the user shouldn't have access, bail without doing any work - if not has_access(user, descriptor, 'load', course_id): - return None - # allow course staff to masquerade as student if has_access(user, descriptor, 'staff', course_id): setup_masquerade(request, True) + # Short circuit--if the user shouldn't have access, bail without doing any work + if not has_access(user, descriptor, 'load', course_id): + return None + # Setup system context for module instance ajax_url = reverse('modx_dispatch', kwargs=dict(course_id=course_id, From a4717aca90f07d1b7452c5776fe2c8165b08a5db Mon Sep 17 00:00:00 2001 From: Will Daly Date: Thu, 18 Apr 2013 09:49:05 -0400 Subject: [PATCH 072/107] Pep8 fixes --- .../capa/capa/tests/test_input_templates.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/common/lib/capa/capa/tests/test_input_templates.py b/common/lib/capa/capa/tests/test_input_templates.py index 4ca020be07..92c4d8b3b7 100644 --- a/common/lib/capa/capa/tests/test_input_templates.py +++ b/common/lib/capa/capa/tests/test_input_templates.py @@ -7,10 +7,12 @@ from lxml import etree from mako.template import Template as MakoTemplate from mako import exceptions + class TemplateError(Exception): """Error occurred while rendering a Mako template""" pass + class TemplateTestCase(unittest.TestCase): """Utilitites for testing templates""" @@ -23,8 +25,8 @@ class TemplateTestCase(unittest.TestCase): def setUp(self): """Load the template""" capa_path = capa.__path__[0] - self.template_path = os.path.join(capa_path, - 'templates', + self.template_path = os.path.join(capa_path, + 'templates', self.TEMPLATE_NAME) template_file = open(self.template_path) self.template = MakoTemplate(template_file.read()) @@ -309,11 +311,10 @@ class TextlineTemplateTest(TemplateTestCase): super(TextlineTemplateTest, self).setUp() def test_section_class(self): - cases = [ ({}, ' capa_inputtype '), - ({'do_math': True}, 'text-input-dynamath capa_inputtype '), - ({'inline': True}, ' capa_inputtype inline'), - ({'do_math': True, 'inline': True}, 'text-input-dynamath capa_inputtype inline'), - ] + cases = [({}, ' capa_inputtype '), + ({'do_math': True}, 'text-input-dynamath capa_inputtype '), + ({'inline': True}, ' capa_inputtype inline'), + ({'do_math': True, 'inline': True}, 'text-input-dynamath capa_inputtype inline'), ] for (context, css_class) in cases: base_context = self.context.copy() @@ -336,9 +337,9 @@ class TextlineTemplateTest(TemplateTestCase): xpath = "//div[@class='%s ']" % div_class self.assert_has_xpath(xml, xpath, self.context) - # Expect that we get a

    with class="status" + # Expect that we get a

    with class="status" # (used to by CSS to draw the green check / red x) - self.assert_has_text(xml, "//p[@class='status']", + self.assert_has_text(xml, "//p[@class='status']", status_mark, exact=False) def test_hidden(self): From 8a2d08bbd6625fa0e8a8cb1ca2f9e6012293b30f Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Wed, 17 Apr 2013 12:58:41 -0400 Subject: [PATCH 073/107] Refactor choosing the browser for lettuce tests to settings.py --- cms/envs/acceptance.py | 1 + common/djangoapps/terrain/browser.py | 19 +++++++++++++------ lms/envs/acceptance.py | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cms/envs/acceptance.py b/cms/envs/acceptance.py index 26a8adc92c..1e7a32dc68 100644 --- a/cms/envs/acceptance.py +++ b/cms/envs/acceptance.py @@ -36,3 +36,4 @@ DATABASES = { INSTALLED_APPS += ('lettuce.django',) LETTUCE_APPS = ('contentstore',) LETTUCE_SERVER_PORT = 8001 +LETTUCE_BROWSER = 'chrome' diff --git a/common/djangoapps/terrain/browser.py b/common/djangoapps/terrain/browser.py index c8cc0c9e4b..1d371a3242 100644 --- a/common/djangoapps/terrain/browser.py +++ b/common/djangoapps/terrain/browser.py @@ -1,6 +1,8 @@ from lettuce import before, after, world from splinter.browser import Browser from logging import getLogger +from django.core.management import call_command +from django.conf import settings # Let the LMS and CMS do their one-time setup # For example, setting up mongo caches @@ -10,18 +12,14 @@ from cms import one_time_startup logger = getLogger(__name__) logger.info("Loading the lettuce acceptance testing terrain file...") -from django.core.management import call_command - @before.harvest def initial_setup(server): ''' Launch the browser once before executing the tests ''' - # Launch the browser app (choose one of these below) - world.browser = Browser('chrome') - # world.browser = Browser('phantomjs') - # world.browser = Browser('firefox') + browser_driver = getattr(settings, 'LETTUCE_BROWSER', 'chrome') + world.browser = Browser(browser_driver) @before.each_scenario @@ -34,6 +32,15 @@ def reset_data(scenario): call_command('flush', interactive=False) +@after.each_scenario +def screenshot_on_error(scenario): + ''' + Save a screenshot to help with debugging + ''' + if scenario.failed: + world.browser.driver.save_screenshot('/tmp/last_failed_scenario.png') + + @after.all def teardown_browser(total): ''' diff --git a/lms/envs/acceptance.py b/lms/envs/acceptance.py index 5f416cd189..2c51dda5e6 100644 --- a/lms/envs/acceptance.py +++ b/lms/envs/acceptance.py @@ -67,3 +67,4 @@ MITX_FEATURES['STUB_VIDEO_FOR_TESTING'] = True # Include the lettuce app for acceptance testing, including the 'harvest' django-admin command INSTALLED_APPS += ('lettuce.django',) LETTUCE_APPS = ('courseware',) +LETTUCE_BROWSER = 'chrome' From 8a852f90cb15ff848066a02d0aeb9bc193be327a Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Wed, 17 Apr 2013 13:27:33 -0400 Subject: [PATCH 074/107] Fix or skip lettuce tests to run under phantomjs and firefox --- .../contentstore/features/advanced-settings.py | 13 +++++-------- .../contentstore/features/checklists.feature | 5 ++++- cms/djangoapps/contentstore/features/checklists.py | 7 +++---- common/djangoapps/terrain/steps.py | 2 ++ 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.py b/cms/djangoapps/contentstore/features/advanced-settings.py index 6fb102faea..ea5b24b21f 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.py +++ b/cms/djangoapps/contentstore/features/advanced-settings.py @@ -3,10 +3,7 @@ from lettuce import world, step from common import * -import time -from terrain.steps import reload_the_page - -from nose.tools import assert_true, assert_false, assert_equal +from nose.tools import assert_false, assert_equal """ http://selenium.googlecode.com/svn/trunk/docs/api/py/webdriver/selenium.webdriver.common.keys.html @@ -18,8 +15,8 @@ VALUE_CSS = 'textarea.json' DISPLAY_NAME_KEY = "display_name" DISPLAY_NAME_VALUE = '"Robot Super Course"' -############### ACTIONS #################### +############### ACTIONS #################### @step('I select the Advanced Settings$') def i_select_advanced_settings(step): expand_icon_css = 'li.nav-course-settings i.icon-expand' @@ -38,7 +35,7 @@ def i_am_on_advanced_course_settings(step): @step(u'I press the "([^"]*)" notification button$') def press_the_notification_button(step, name): css = 'a.%s-button' % name.lower() - world.css_click_at(css) + world.css_click(css) @step(u'I edit the value of a policy key$') @@ -52,7 +49,7 @@ def edit_the_value_of_a_policy_key(step): @step(u'I edit the value of a policy key and save$') -def edit_the_value_of_a_policy_key(step): +def edit_the_value_of_a_policy_key_and_save(step): change_display_name_value(step, '"foo"') @@ -90,7 +87,7 @@ def it_is_formatted(step): @step('it is displayed as a string') -def it_is_formatted(step): +def it_is_displayed_as_string(step): assert_policy_entries([DISPLAY_NAME_KEY], ['"quote me"']) diff --git a/cms/djangoapps/contentstore/features/checklists.feature b/cms/djangoapps/contentstore/features/checklists.feature index bccb80b8d7..ddf1adf263 100644 --- a/cms/djangoapps/contentstore/features/checklists.feature +++ b/cms/djangoapps/contentstore/features/checklists.feature @@ -10,6 +10,8 @@ Feature: Course checklists Then I can check and uncheck tasks in a checklist And They are correctly selected after I reload the page + @skip-phantom + @skip-firefox Scenario: A task can link to a location within Studio Given I have opened Checklists When I select a link to the course outline @@ -17,8 +19,9 @@ Feature: Course checklists And I press the browser back button Then I am brought back to the course outline in the correct state + @skip-phantom + @skip-firefox Scenario: A task can link to a location outside Studio Given I have opened Checklists When I select a link to help page Then I am brought to the help page in a new window - diff --git a/cms/djangoapps/contentstore/features/checklists.py b/cms/djangoapps/contentstore/features/checklists.py index 489544f424..d433dbbf0d 100644 --- a/cms/djangoapps/contentstore/features/checklists.py +++ b/cms/djangoapps/contentstore/features/checklists.py @@ -89,8 +89,6 @@ def i_am_brought_to_help_page_in_new_window(step): assert_equal('http://help.edge.edx.org/', world.browser.url) - - ############### HELPER METHODS #################### def verifyChecklist2Status(completed, total, percentage): def verify_count(driver): @@ -107,9 +105,11 @@ def verifyChecklist2Status(completed, total, percentage): def toggleTask(checklist, task): - world.css_click('#course-checklist' + str(checklist) +'-task' + str(task)) + world.css_click('#course-checklist' + str(checklist) + '-task' + str(task)) +# TODO: figure out a way to do this in phantom and firefox +# For now we will mark the scenerios that use this method as skipped def clickActionLink(checklist, task, actionText): # toggle checklist item to make sure that the link button is showing toggleTask(checklist, task) @@ -121,4 +121,3 @@ def clickActionLink(checklist, task, actionText): world.wait_for(verify_action_link_text) action_link.click() - diff --git a/common/djangoapps/terrain/steps.py b/common/djangoapps/terrain/steps.py index a2db80712f..fdab514177 100644 --- a/common/djangoapps/terrain/steps.py +++ b/common/djangoapps/terrain/steps.py @@ -132,6 +132,8 @@ def i_am_logged_in(step): world.create_user('robot') world.log_in('robot', 'test') world.browser.visit(django_url('/')) + # You should not see the login link + assert_equals(world.browser.find_by_css('a#login'), []) @step(u'I am an edX user$') From 0426761bede3c0938e08c438914e9cbb9a9419fb Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Wed, 17 Apr 2013 15:49:29 -0400 Subject: [PATCH 075/107] Update lettuce and factory-boy versions. --- common/djangoapps/student/tests/factories.py | 26 +++++++++----------- requirements.txt | 4 +-- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/common/djangoapps/student/tests/factories.py b/common/djangoapps/student/tests/factories.py index adb51954e8..952a8c016e 100644 --- a/common/djangoapps/student/tests/factories.py +++ b/common/djangoapps/student/tests/factories.py @@ -2,17 +2,17 @@ from student.models import (User, UserProfile, Registration, CourseEnrollmentAllowed, CourseEnrollment) from django.contrib.auth.models import Group from datetime import datetime -from factory import Factory, SubFactory, post_generation +from factory import DjangoModelFactory, Factory, SubFactory, PostGenerationMethodCall from uuid import uuid4 -class GroupFactory(Factory): +class GroupFactory(DjangoModelFactory): FACTORY_FOR = Group name = 'staff_MITx/999/Robot_Super_Course' -class UserProfileFactory(Factory): +class UserProfileFactory(DjangoModelFactory): FACTORY_FOR = UserProfile user = None @@ -23,19 +23,20 @@ class UserProfileFactory(Factory): goals = 'World domination' -class RegistrationFactory(Factory): +class RegistrationFactory(DjangoModelFactory): FACTORY_FOR = Registration user = None activation_key = uuid4().hex -class UserFactory(Factory): +class UserFactory(DjangoModelFactory): FACTORY_FOR = User username = 'robot' email = 'robot+test@edx.org' - password = 'test' + password = PostGenerationMethodCall('set_password', + 'test') first_name = 'Robot' last_name = 'Test' is_staff = False @@ -44,26 +45,21 @@ class UserFactory(Factory): last_login = datetime(2012, 1, 1) date_joined = datetime(2011, 1, 1) - @post_generation - def set_password(self, create, extracted, **kwargs): - self._raw_password = self.password - self.set_password(self.password) - if create: - self.save() +class AdminFactory(Factory): + FACTORY_FOR = User -class AdminFactory(UserFactory): is_staff = True -class CourseEnrollmentFactory(Factory): +class CourseEnrollmentFactory(DjangoModelFactory): FACTORY_FOR = CourseEnrollment user = SubFactory(UserFactory) course_id = 'edX/toy/2012_Fall' -class CourseEnrollmentAllowedFactory(Factory): +class CourseEnrollmentAllowedFactory(DjangoModelFactory): FACTORY_FOR = CourseEnrollmentAllowed email = 'test@edx.org' diff --git a/requirements.txt b/requirements.txt index a626ac1944..08aaecc71e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -61,8 +61,8 @@ sphinx==1.1.3 # Used for testing coverage==3.6 -factory_boy==1.3.0 -lettuce==0.2.15 +factory_boy==2.0.2 +lettuce==0.2.16 mock==0.8.0 nosexcover==1.0.7 pep8==1.4.5 From 29ce700de6e1b7c68dc532898f14893ed0ff6da3 Mon Sep 17 00:00:00 2001 From: Jay Zoldak Date: Wed, 17 Apr 2013 17:16:01 -0400 Subject: [PATCH 076/107] Tag CMS lettuce tests with @skip-phantom if they aren't working under phantomjs right now. --- .../features/advanced-settings.feature | 4 +++ .../features/course-settings.feature | 3 ++ .../contentstore/features/section.feature | 1 + .../studio-overview-togglesection.feature | 31 ++++++++++--------- .../contentstore/features/subsection.feature | 17 +++++----- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/cms/djangoapps/contentstore/features/advanced-settings.feature b/cms/djangoapps/contentstore/features/advanced-settings.feature index 558294e890..ca5b62e596 100644 --- a/cms/djangoapps/contentstore/features/advanced-settings.feature +++ b/cms/djangoapps/contentstore/features/advanced-settings.feature @@ -11,6 +11,7 @@ Feature: Advanced (manual) course policy Given I am on the Advanced Course Settings page in Studio Then the settings are alphabetized + @skip-phantom Scenario: Test cancel editing key value Given I am on the Advanced Course Settings page in Studio When I edit the value of a policy key @@ -19,6 +20,7 @@ Feature: Advanced (manual) course policy And I reload the page Then the policy key value is unchanged + @skip-phantom Scenario: Test editing key value Given I am on the Advanced Course Settings page in Studio When I edit the value of a policy key and save @@ -26,6 +28,7 @@ Feature: Advanced (manual) course policy And I reload the page Then the policy key value is changed + @skip-phantom Scenario: Test how multi-line input appears Given I am on the Advanced Course Settings page in Studio When I create a JSON object as a value @@ -33,6 +36,7 @@ Feature: Advanced (manual) course policy And I reload the page Then it is displayed as formatted + @skip-phantom Scenario: Test automatic quoting of non-JSON values Given I am on the Advanced Course Settings page in Studio When I create a non-JSON value not in quotes diff --git a/cms/djangoapps/contentstore/features/course-settings.feature b/cms/djangoapps/contentstore/features/course-settings.feature index e869bfe47a..fc9641cb46 100644 --- a/cms/djangoapps/contentstore/features/course-settings.feature +++ b/cms/djangoapps/contentstore/features/course-settings.feature @@ -1,17 +1,20 @@ Feature: Course Settings As a course author, I want to be able to configure my course settings. + @skip-phantom Scenario: User can set course dates Given I have opened a new course in Studio When I select Schedule and Details And I set course dates Then I see the set dates on refresh + @skip-phantom Scenario: User can clear previously set course dates (except start date) Given I have set course dates And I clear all the dates except start Then I see cleared dates on refresh + @skip-phantom Scenario: User cannot clear the course start date Given I have set course dates And I clear the course start date diff --git a/cms/djangoapps/contentstore/features/section.feature b/cms/djangoapps/contentstore/features/section.feature index 08d38367bc..24cbeb3db9 100644 --- a/cms/djangoapps/contentstore/features/section.feature +++ b/cms/djangoapps/contentstore/features/section.feature @@ -3,6 +3,7 @@ Feature: Create Section As a course author I want to create and edit sections + @skip-phantom Scenario: Add a new section to a course Given I have opened a new course in Studio When I click the New Section link diff --git a/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature b/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature index 762dea6838..a0e0a48f9e 100644 --- a/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature +++ b/cms/djangoapps/contentstore/features/studio-overview-togglesection.feature @@ -1,32 +1,33 @@ Feature: Overview Toggle Section In order to quickly view the details of a course's section or to scan the inventory of sections - As a course author - I want to toggle the visibility of each section's subsection details in the overview listing + As a course author + I want to toggle the visibility of each section's subsection details in the overview listing Scenario: The default layout for the overview page is to show sections in expanded view Given I have a course with multiple sections - When I navigate to the course overview page - Then I see the "Collapse All Sections" link - And all sections are expanded + When I navigate to the course overview page + Then I see the "Collapse All Sections" link + And all sections are expanded Scenario: Expand /collapse for a course with no sections Given I have a course with no sections - When I navigate to the course overview page - Then I do not see the "Collapse All Sections" link + When I navigate to the course overview page + Then I do not see the "Collapse All Sections" link + @skip-phantom Scenario: Collapse link appears after creating first section of a course Given I have a course with no sections - When I navigate to the course overview page - And I add a section - Then I see the "Collapse All Sections" link - And all sections are expanded + When I navigate to the course overview page + And I add a section + Then I see the "Collapse All Sections" link + And all sections are expanded @skip-phantom Scenario: Collapse link is not removed after last section of a course is deleted Given I have a course with 1 section - And I navigate to the course overview page - When I press the "section" delete icon - And I confirm the alert + And I navigate to the course overview page + When I press the "section" delete icon + And I confirm the alert Then I see the "Collapse All Sections" link Scenario: Collapsing all sections when all sections are expanded @@ -57,4 +58,4 @@ Feature: Overview Toggle Section When I expand the first section And I click the "Expand All Sections" link Then I see the "Collapse All Sections" link - And all sections are expanded \ No newline at end of file + And all sections are expanded diff --git a/cms/djangoapps/contentstore/features/subsection.feature b/cms/djangoapps/contentstore/features/subsection.feature index cc3b2b1cbb..28285bf8a1 100644 --- a/cms/djangoapps/contentstore/features/subsection.feature +++ b/cms/djangoapps/contentstore/features/subsection.feature @@ -3,13 +3,15 @@ Feature: Create Subsection As a course author I want to create and edit subsections - Scenario: Add a new subsection to a section + @skip-phantom + Scenario: Add a new subsection to a section Given I have opened a new course section in Studio When I click the New Subsection link And I enter the subsection name and click save Then I see my subsection on the Courseware page - Scenario: Add a new subsection (with a name containing a quote) to a section (bug #216) + @skip-phantom + Scenario: Add a new subsection (with a name containing a quote) to a section (bug #216) Given I have opened a new course section in Studio When I click the New Subsection link And I enter a subsection name with a quote and click save @@ -17,7 +19,7 @@ Feature: Create Subsection And I click to edit the subsection name Then I see the complete subsection name with a quote in the editor - Scenario: Assign grading type to a subsection and verify it is still shown after refresh (bug #258) + Scenario: Assign grading type to a subsection and verify it is still shown after refresh (bug #258) Given I have opened a new course section in Studio And I have added a new subsection And I mark it as Homework @@ -25,20 +27,19 @@ Feature: Create Subsection And I reload the page Then I see it marked as Homework - Scenario: Set a due date in a different year (bug #256) + @skip-phantom + Scenario: Set a due date in a different year (bug #256) Given I have opened a new subsection in Studio And I have set a release date and due date in different years Then I see the correct dates And I reload the page Then I see the correct dates - @skip-phantom - Scenario: Delete a subsection + @skip-phantom + Scenario: Delete a subsection Given I have opened a new course section in Studio And I have added a new subsection And I see my subsection on the Courseware page When I press the "subsection" delete icon And I confirm the alert Then the subsection does not exist - - From 03daefb9246e8aa8a5e936c7e89e9267ef2dbcc0 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Thu, 18 Apr 2013 14:31:51 -0400 Subject: [PATCH 077/107] Change the name of the plot button --- common/lib/capa/capa/templates/matlabinput.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/templates/matlabinput.html b/common/lib/capa/capa/templates/matlabinput.html index 6c02e8e68e..04c91972d4 100644 --- a/common/lib/capa/capa/templates/matlabinput.html +++ b/common/lib/capa/capa/templates/matlabinput.html @@ -34,7 +34,7 @@

    - +
    - + - + @@ -97,8 +97,8 @@ from contentstore import utils
    1. - - Leeway on due dates + + Leeway on due dates (using HH:MM format)
    @@ -112,13 +112,13 @@ from contentstore import utils
      - -
    + +
    New Assignment Type - +
    From ba4c0b6d5318b1cd697e2a4c5b3176b4bff3eb55 Mon Sep 17 00:00:00 2001 From: Brian Talbot Date: Mon, 22 Apr 2013 09:43:57 -0400 Subject: [PATCH 095/107] studio - removing formatting suggestion from label --- cms/templates/settings_graders.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cms/templates/settings_graders.html b/cms/templates/settings_graders.html index a2f3f7022e..2e98409585 100644 --- a/cms/templates/settings_graders.html +++ b/cms/templates/settings_graders.html @@ -98,7 +98,7 @@ from contentstore import utils
  • - Leeway on due dates (using HH:MM format) + Leeway on due dates
    • From 506a9a20aaef0ce0d8d0997096e08071a4769d7d Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Mon, 22 Apr 2013 10:13:33 -0400 Subject: [PATCH 096/107] when updating the list of children of sequentials, we must use the 'direct' store otherwise we end up with draft revisions of sequentials, which is bad --- cms/djangoapps/contentstore/tests/test_contentstore.py | 9 +++++++-- common/lib/xmodule/xmodule/modulestore/xml_importer.py | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index e40d7c57b9..ed95d81d67 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -9,7 +9,6 @@ from tempdir import mkdtemp_clean from fs.osfs import OSFS import copy from json import loads -import traceback from datetime import timedelta from django.contrib.auth.models import User @@ -397,7 +396,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # We had a bug where orphaned draft nodes caused export to fail. This is here to cover that case. draft_store.clone_item(vertical.location, Location(['i4x', 'edX', 'full', - 'vertical', 'no_references', 'draft'])) + 'vertical', 'no_references', 'draft'])) for child in vertical.get_children(): draft_store.clone_item(child.location, child.location) @@ -478,6 +477,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): for child in vertical.get_children(): self.assertTrue(getattr(child, 'is_draft', False)) + # make sure that we don't have a sequential that is in draft mode + sequential = draft_store.get_item(Location(['i4x', 'edX', 'full', + 'sequential', 'Administrivia_and_Circuit_Elements', None])) + + self.assertFalse(getattr(sequential, 'is_draft', False)) + # verify that we have the private vertical test_private_vertical = draft_store.get_item(Location(['i4x', 'edX', 'full', 'vertical', 'vertical_66', None])) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 6355204d07..71c6983644 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -274,7 +274,7 @@ def import_from_xml(store, data_dir, course_dirs=None, # now import any 'draft' items if draft_store is not None: - import_course_draft(xml_module_store, draft_store, course_data_path, + import_course_draft(xml_module_store, store, draft_store, course_data_path, static_content_store, target_location_namespace if target_location_namespace is not None else course_location) @@ -339,7 +339,7 @@ def import_module(module, store, course_data_path, static_content_store, allow_n store.update_metadata(module.location, dict(own_metadata(module))) -def import_course_draft(xml_module_store, store, course_data_path, static_content_store, target_location_namespace): +def import_course_draft(xml_module_store, store, draft_store, course_data_path, static_content_store, target_location_namespace): ''' This will import all the content inside of the 'drafts' folder, if it exists NOTE: This is not a full course import, basically in our current application only verticals (and downwards) @@ -396,7 +396,7 @@ def import_course_draft(xml_module_store, store, course_data_path, static_conten del module.xml_attributes['parent_sequential_url'] del module.xml_attributes['index_in_children_list'] - import_module(module, store, course_data_path, static_content_store, allow_not_found=True) + import_module(module, draft_store, course_data_path, static_content_store, allow_not_found=True) for child in module.get_children(): _import_module(child) From 7994e1b344127b3a3c5d9463d6d24f1deaeadbf6 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 12:53:30 -0400 Subject: [PATCH 097/107] pep8 fixes --- .../django_comment_client/base/views.py | 35 ++++++------- .../django_comment_client/forum/views.py | 19 +++---- .../management/commands/reload_forum_users.py | 8 +-- .../tests/test_mustache_helpers.py | 1 - lms/djangoapps/django_comment_client/utils.py | 32 ++++++------ lms/lib/comment_client/comment.py | 22 ++++---- lms/lib/comment_client/thread.py | 50 ++++++++++--------- 7 files changed, 84 insertions(+), 83 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 84a543868e..1f20fcbc79 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -119,7 +119,7 @@ def create_thread(request, course_id, commentable_id): #patch for backward compatibility to comments service if not 'pinned' in thread.attributes: thread['pinned'] = False - + if post.get('auto_subscribe', 'false').lower() == 'true': user = cc.User.from_django_user(request.user) user.follow(thread) @@ -174,7 +174,7 @@ def _create_comment(request, course_id, thread_id=None, parent_id=None): user = cc.User.from_django_user(request.user) user.follow(comment.thread) if request.is_ajax(): - return ajax_content_response(request, course_id,comment.to_dict(), 'discussion/ajax_create_comment.html') + return ajax_content_response(request, course_id, comment.to_dict(), 'discussion/ajax_create_comment.html') else: return JsonResponse(utils.safe_content(comment.to_dict())) @@ -290,29 +290,32 @@ def vote_for_thread(request, course_id, thread_id, value): def flag_abuse_for_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - thread.flagAbuse(user,thread) + thread.flagAbuse(user, thread) return JsonResponse(utils.safe_content(thread.to_dict())) + @require_POST @login_required @permitted def un_flag_abuse_for_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) - + thread = cc.Thread.find(thread_id) removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) - thread.unFlagAbuse(user,thread,removeAll) + thread.unFlagAbuse(user, thread, removeAll) return JsonResponse(utils.safe_content(thread.to_dict())) + @require_POST @login_required @permitted def flag_abuse_for_comment(request, course_id, comment_id): user = cc.User.from_django_user(request.user) comment = cc.Comment.find(comment_id) - comment.flagAbuse(user,comment) + comment.flagAbuse(user, comment) return JsonResponse(utils.safe_content(comment.to_dict())) + @require_POST @login_required @permitted @@ -320,9 +323,10 @@ def un_flag_abuse_for_comment(request, course_id, comment_id): user = cc.User.from_django_user(request.user) removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) comment = cc.Comment.find(comment_id) - comment.unFlagAbuse(user,comment, removeAll) + comment.unFlagAbuse(user, comment, removeAll) return JsonResponse(utils.safe_content(comment.to_dict())) + @require_POST @login_required @permitted @@ -332,19 +336,21 @@ def undo_vote_for_thread(request, course_id, thread_id): user.unvote(thread) return JsonResponse(utils.safe_content(thread.to_dict())) + @require_POST @login_required @permitted def pin_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - thread.pin(user,thread_id) + thread.pin(user, thread_id) return JsonResponse(utils.safe_content(thread.to_dict())) + def un_pin_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) thread = cc.Thread.find(thread_id) - thread.un_pin(user,thread_id) + thread.un_pin(user, thread_id) return JsonResponse(utils.safe_content(thread.to_dict())) @@ -491,16 +497,11 @@ def upload(request, course_id): # ajax upload file to a question or answer if not file_extension in cc_settings.ALLOWED_UPLOAD_FILE_TYPES: file_types = "', '".join(cc_settings.ALLOWED_UPLOAD_FILE_TYPES) msg = _("allowed file types are '%(file_types)s'") % \ - {'file_types': file_types} + {'file_types': file_types} raise exceptions.PermissionDenied(msg) # generate new file name - new_file_name = str( - time.time() - ).replace( - '.', - str(random.randint(0, 100000)) - ) + file_extension + new_file_name = str(time.time()).replace('.', str(random.randint(0, 100000))) + file_extension file_storage = get_storage_class()() # use default storage to store file @@ -511,7 +512,7 @@ def upload(request, course_id): # ajax upload file to a question or answer if size > cc_settings.MAX_UPLOAD_FILE_SIZE: file_storage.delete(new_file_name) msg = _("maximum upload file size is %(file_size)sK") % \ - {'file_size': cc_settings.MAX_UPLOAD_FILE_SIZE} + {'file_size': cc_settings.MAX_UPLOAD_FILE_SIZE} raise exceptions.PermissionDenied(msg) except exceptions.PermissionDenied, e: diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index df65d5aae6..e53a0195e3 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -9,7 +9,7 @@ from django.contrib.auth.models import User from mitxmako.shortcuts import render_to_response, render_to_string from courseware.courses import get_course_with_access -from course_groups.cohorts import (is_course_cohorted, get_cohort_id, is_commentable_cohorted, +from course_groups.cohorts import (is_course_cohorted, get_cohort_id, is_commentable_cohorted, get_cohorted_commentables, get_course_cohorts, get_cohort_by_id) from courseware.access import has_access from django_comment_client.models import Role @@ -43,7 +43,7 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG 'course_id': course_id, 'user_id': request.user.id, } - + if not request.GET.get('sort_key'): # If the user did not select a sort key, use their last used sort key cc_user = cc.User.from_django_user(request.user) @@ -93,11 +93,11 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG else: thread['group_name'] = "" thread['group_string'] = "This post visible to everyone." - + #patch for backward compatibility to comments service if not 'pinned' in thread: thread['pinned'] = False - + query_params['page'] = page query_params['num_pages'] = num_pages @@ -242,10 +242,10 @@ def single_thread(request, course_id, discussion_id, thread_id): user_info = cc_user.to_dict() try: - thread = cc.Thread.find(thread_id).retrieve(recursive=True, user_id=request.user.id) + thread = cc.Thread.find(thread_id).retrieve(recursive=True, user_id=request.user.id) except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err: - log.error("Error loading single thread.") - raise Http404 + log.error("Error loading single thread.") + raise Http404 if request.is_ajax(): courseware_context = get_courseware_context(thread, course) @@ -302,9 +302,6 @@ def single_thread(request, course_id, discussion_id, thread_id): cohorts = get_course_cohorts(course_id) cohorted_commentables = get_cohorted_commentables(course_id) user_cohort = get_cohort_id(request.user, course_id) - - - context = { 'discussion_id': discussion_id, @@ -411,7 +408,7 @@ def followed_threads(request, course_id, user_id): 'user_info': saxutils.escape(json.dumps(user_info), escapedict), 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), # 'content': content, - } + } return render_to_response('discussion/user_profile.html', context) except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError): diff --git a/lms/djangoapps/django_comment_client/management/commands/reload_forum_users.py b/lms/djangoapps/django_comment_client/management/commands/reload_forum_users.py index 5e7e268270..e84771d615 100644 --- a/lms/djangoapps/django_comment_client/management/commands/reload_forum_users.py +++ b/lms/djangoapps/django_comment_client/management/commands/reload_forum_users.py @@ -6,10 +6,11 @@ from django.core.management.base import BaseCommand, CommandError from django.contrib.auth.models import User import comment_client as cc + class Command(BaseCommand): help = 'Reload forum (comment client) users from existing users' - def adduser(self,user): + def adduser(self, user): print user try: cc_user = cc.User.from_django_user(user) @@ -22,8 +23,7 @@ class Command(BaseCommand): uset = [User.objects.get(username=x) for x in args] else: uset = User.objects.all() - + for user in uset: self.adduser(user) - - \ No newline at end of file + diff --git a/lms/djangoapps/django_comment_client/tests/test_mustache_helpers.py b/lms/djangoapps/django_comment_client/tests/test_mustache_helpers.py index 7db3ba6e86..d5a403ecb8 100644 --- a/lms/djangoapps/django_comment_client/tests/test_mustache_helpers.py +++ b/lms/djangoapps/django_comment_client/tests/test_mustache_helpers.py @@ -39,4 +39,3 @@ class CloseThreadTextTest(TestCase): self.assertEqual(mustache_helpers.close_thread_text(self.contentOpen), 'Close thread') ######################################################################################### - diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 05615f3870..e9efb38aed 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -105,12 +105,12 @@ def filter_unstarted_categories(category_map): result_map = {} unfiltered_queue = [category_map] - filtered_queue = [result_map] + filtered_queue = [result_map] while len(unfiltered_queue) > 0: unfiltered_map = unfiltered_queue.pop() - filtered_map = filtered_queue.pop() + filtered_map = filtered_queue.pop() filtered_map["children"] = [] filtered_map["entries"] = {} @@ -187,8 +187,7 @@ def initialize_discussion_info(course): category = " / ".join([x.strip() for x in category.split("/")]) last_category = category.split("/")[-1] discussion_id_map[id] = {"location": module.location, "title": last_category + " / " + title} - unexpanded_category_map[category].append({"title": title, "id": id, - "sort_key": sort_key, "start_date": module.lms.start}) + unexpanded_category_map[category].append({"title": title, "id": id, "sort_key": sort_key, "start_date": module.lms.start}) category_map = {"entries": defaultdict(dict), "subcategories": defaultdict(dict)} for category_path, entries in unexpanded_category_map.items(): @@ -215,9 +214,9 @@ def initialize_discussion_info(course): level = path[-1] if level not in node: node[level] = {"subcategories": defaultdict(dict), - "entries": defaultdict(dict), - "sort_key": level, - "start_date": category_start_date} + "entries": defaultdict(dict), + "sort_key": level, + "start_date": category_start_date} else: if node[level]["start_date"] > category_start_date: node[level]["start_date"] = category_start_date @@ -297,12 +296,12 @@ class QueryCountDebugMiddleware(object): def get_ability(course_id, content, user): return { - 'editable': check_permissions_by_view(user, course_id, content, "update_thread" if content['type'] == 'thread' else "update_comment"), - 'can_reply': check_permissions_by_view(user, course_id, content, "create_comment" if content['type'] == 'thread' else "create_sub_comment"), - 'can_endorse': check_permissions_by_view(user, course_id, content, "endorse_comment") if content['type'] == 'comment' else False, - 'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if content['type'] == 'thread' else "delete_comment"), - 'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if content['type'] == 'thread' else False, - 'can_vote': check_permissions_by_view(user, course_id, content, "vote_for_thread" if content['type'] == 'thread' else "vote_for_comment"), + 'editable': check_permissions_by_view(user, course_id, content, "update_thread" if content['type'] == 'thread' else "update_comment"), + 'can_reply': check_permissions_by_view(user, course_id, content, "create_comment" if content['type'] == 'thread' else "create_sub_comment"), + 'can_endorse': check_permissions_by_view(user, course_id, content, "endorse_comment") if content['type'] == 'comment' else False, + 'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if content['type'] == 'thread' else "delete_comment"), + 'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if content['type'] == 'thread' else False, + 'can_vote': check_permissions_by_view(user, course_id, content, "vote_for_thread" if content['type'] == 'thread' else "vote_for_comment"), } #TODO: RENAME @@ -331,6 +330,7 @@ def get_annotated_content_infos(course_id, thread, user, user_info): Get metadata for a thread and its children """ infos = {} + def annotate(content): infos[str(content['id'])] = get_annotated_content_info(course_id, content, user, user_info) for child in content.get('children', []): @@ -395,8 +395,8 @@ def get_courseware_context(content, course): location = id_map[id]["location"].url() title = id_map[id]["title"] - url = reverse('jump_to', kwargs={"course_id":course.location.course_id, - "location": location}) + url = reverse('jump_to', kwargs={"course_id": course.location.course_id, + "location": location}) content_info = {"courseware_url": url, "courseware_title": title} return content_info @@ -410,7 +410,7 @@ def safe_content(content): 'at_position_list', 'children', 'highlighted_title', 'highlighted_body', 'courseware_title', 'courseware_url', 'tags', 'unread_comments_count', 'read', 'group_id', 'group_name', 'group_string', 'pinned', 'abuse_flaggers' - + ] if (content.get('anonymous') is False) and (content.get('anonymous_to_peers') is False): diff --git a/lms/lib/comment_client/comment.py b/lms/lib/comment_client/comment.py index 8010aaf60f..324de7923f 100644 --- a/lms/lib/comment_client/comment.py +++ b/lms/lib/comment_client/comment.py @@ -41,7 +41,7 @@ class Comment(models.Model): return cls.url_for_comments(params) else: return super(Comment, cls).url(action, params) - + def flagAbuse(self, user, voteable): if voteable.type == 'thread': url = _url_for_flag_abuse_thread(voteable.id) @@ -51,8 +51,8 @@ class Comment(models.Model): raise CommentClientError("Can only flag/unflag threads or comments") params = {'user_id': user.id} request = perform_request('put', url, params) - voteable.update_attributes(request) - + voteable.update_attributes(request) + def unFlagAbuse(self, user, voteable, removeAll): if voteable.type == 'thread': url = _url_for_unflag_abuse_thread(voteable.id) @@ -61,12 +61,12 @@ class Comment(models.Model): else: raise CommentClientError("Can flag/unflag for threads or comments") params = {'user_id': user.id} - + if removeAll: params['all'] = True - + request = perform_request('put', url, params) - voteable.update_attributes(request) + voteable.update_attributes(request) def _url_for_thread_comments(thread_id): @@ -75,9 +75,11 @@ def _url_for_thread_comments(thread_id): def _url_for_comment(comment_id): return "{prefix}/comments/{comment_id}".format(prefix=settings.PREFIX, comment_id=comment_id) - + + def _url_for_flag_abuse_comment(comment_id): - return "{prefix}/comments/{comment_id}/abuse_flags".format(prefix=settings.PREFIX, comment_id=comment_id) - + return "{prefix}/comments/{comment_id}/abuse_flags".format(prefix=settings.PREFIX, comment_id=comment_id) + + def _url_for_unflag_abuse_comment(comment_id): - return "{prefix}/comments/{comment_id}/abuse_unflags".format(prefix=settings.PREFIX, comment_id=comment_id) + return "{prefix}/comments/{comment_id}/abuse_unflags".format(prefix=settings.PREFIX, comment_id=comment_id) diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 8ecb0368be..60a68dc3ae 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -11,7 +11,6 @@ class Thread(models.Model): 'created_at', 'updated_at', 'comments_count', 'unread_comments_count', 'at_position_list', 'children', 'type', 'highlighted_title', 'highlighted_body', 'endorsed', 'read', 'group_id', 'group_name', 'pinned', 'abuse_flaggers' - ] updatable_fields = [ @@ -33,7 +32,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') or query_params.get('commentable_ids'): url = cls.url(action='search') else: @@ -56,7 +55,7 @@ class Thread(models.Model): @classmethod def url(cls, action, params={}): - + if action in ['get_all', 'post']: return cls.url_for_threads(params) elif action == 'search': @@ -70,11 +69,11 @@ class Thread(models.Model): def _retrieve(self, *args, **kwargs): url = self.url(action='get', params=self.attributes) request_params = { - 'recursive': kwargs.get('recursive'), - 'user_id': kwargs.get('user_id'), - 'mark_as_read': kwargs.get('mark_as_read', True), - } - + 'recursive': kwargs.get('recursive'), + 'user_id': kwargs.get('user_id'), + 'mark_as_read': kwargs.get('mark_as_read', True), + } + # user_id may be none, in which case it shouldn't be part of the # request. request_params = strip_none(request_params) @@ -91,8 +90,8 @@ class Thread(models.Model): raise CommentClientError("Can only flag/unflag threads or comments") params = {'user_id': user.id} request = perform_request('put', url, params) - voteable.update_attributes(request) - + voteable.update_attributes(request) + def unFlagAbuse(self, user, voteable, removeAll): if voteable.type == 'thread': url = _url_for_unflag_abuse_thread(voteable.id) @@ -104,31 +103,34 @@ class Thread(models.Model): #if you're an admin, when you unflag, remove ALL flags if removeAll: params['all'] = True - + request = perform_request('put', url, params) - voteable.update_attributes(request) - + voteable.update_attributes(request) + def pin(self, user, thread_id): url = _url_for_pin_thread(thread_id) params = {'user_id': user.id} request = perform_request('put', url, params) - self.update_attributes(request) + self.update_attributes(request) def un_pin(self, user, thread_id): url = _url_for_un_pin_thread(thread_id) params = {'user_id': user.id} request = perform_request('put', url, params) - self.update_attributes(request) - + self.update_attributes(request) + + def _url_for_flag_abuse_thread(thread_id): - return "{prefix}/threads/{thread_id}/abuse_flags".format(prefix=settings.PREFIX, thread_id=thread_id) - + return "{prefix}/threads/{thread_id}/abuse_flags".format(prefix=settings.PREFIX, thread_id=thread_id) + + def _url_for_unflag_abuse_thread(thread_id): - return "{prefix}/threads/{thread_id}/abuse_unflags".format(prefix=settings.PREFIX, thread_id=thread_id) - + return "{prefix}/threads/{thread_id}/abuse_unflags".format(prefix=settings.PREFIX, thread_id=thread_id) + + def _url_for_pin_thread(thread_id): - return "{prefix}/threads/{thread_id}/pin".format(prefix=settings.PREFIX, thread_id=thread_id) - + return "{prefix}/threads/{thread_id}/pin".format(prefix=settings.PREFIX, thread_id=thread_id) + + def _url_for_un_pin_thread(thread_id): - return "{prefix}/threads/{thread_id}/unpin".format(prefix=settings.PREFIX, thread_id=thread_id) - + return "{prefix}/threads/{thread_id}/unpin".format(prefix=settings.PREFIX, thread_id=thread_id) From 385026172e0b15d4b2564c98b9b5921f540afdb8 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 15:28:27 -0400 Subject: [PATCH 098/107] remove double Comment from merge conflict --- .../static/coffee/src/discussion/content.coffee | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/common/static/coffee/src/discussion/content.coffee b/common/static/coffee/src/discussion/content.coffee index e040c16c45..c44c321a3c 100644 --- a/common/static/coffee/src/discussion/content.coffee +++ b/common/static/coffee/src/discussion/content.coffee @@ -150,22 +150,6 @@ if Backbone? else @get("title") - class @Comments extends Backbone: -> - if @has("highlighted_title") - String(@get("highlighted_title")).replace(//g, '').replace(/<\/highlight>/g, '') - else - @get("title") - - toJSON: -> - json_attributes = _.clone(@attributes) - _.extend(json_attributes, { title: @display_title(), body: @display_body() }) - - created_at_date: -> - new Date(@get("created_at")) - - created_at_time: -> - new Date(@get("created_at")).getTime() - class @Comment extends @Content urlMappers: 'reply': -> DiscussionUtil.urlFor('create_sub_comment', @id) From 8a02001a4fc42832dfca4383540be4de16dc059c Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 15:40:19 -0400 Subject: [PATCH 099/107] minimize diff by restoring function locations --- .../coffee/src/discussion/content.coffee | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/common/static/coffee/src/discussion/content.coffee b/common/static/coffee/src/discussion/content.coffee index c44c321a3c..8197985a1c 100644 --- a/common/static/coffee/src/discussion/content.coffee +++ b/common/static/coffee/src/discussion/content.coffee @@ -78,6 +78,16 @@ if Backbone? if @getContent(id) @getContent(id).updateInfo(info) $.extend @contentInfos, infos + + pinThread: -> + pinned = @get("pinned") + @set("pinned",pinned) + @trigger "change", @ + + unPinThread: -> + pinned = @get("pinned") + @set("pinned",pinned) + @trigger "change", @ flagAbuse: -> temp_array = @get("abuse_flaggers") @@ -128,16 +138,6 @@ if Backbone? @get("votes")["up_count"] = parseInt(@get("votes")["up_count"]) - 1 @trigger "change", @ - pinThread: -> - pinned = @get("pinned") - @set("pinned",pinned) - @trigger "change", @ - - unPinThread: -> - pinned = @get("pinned") - @set("pinned",pinned) - @trigger "change", @ - display_body: -> if @has("highlighted_body") String(@get("highlighted_body")).replace(//g, '').replace(/<\/highlight>/g, '') From b8114b9eb2df17f57440885c9d6037dcbbbc641a Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 16:17:54 -0400 Subject: [PATCH 100/107] restore accidentally deleted chunk --- common/static/coffee/src/discussion/content.coffee | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/static/coffee/src/discussion/content.coffee b/common/static/coffee/src/discussion/content.coffee index 8197985a1c..6361a4b76e 100644 --- a/common/static/coffee/src/discussion/content.coffee +++ b/common/static/coffee/src/discussion/content.coffee @@ -150,6 +150,16 @@ if Backbone? else @get("title") + toJSON: -> + json_attributes = _.clone(@attributes) + _.extend(json_attributes, { title: @display_title(), body: @display_body() }) + + created_at_date: -> + new Date(@get("created_at")) + + created_at_time: -> + new Date(@get("created_at")).getTime() + class @Comment extends @Content urlMappers: 'reply': -> DiscussionUtil.urlFor('create_sub_comment', @id) From af1c411ddfc90b892f50c31ae23aa1c63ad79bd9 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 16:23:21 -0400 Subject: [PATCH 101/107] gentler url diff --- lms/djangoapps/django_comment_client/base/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index 203b591761..fb687f4e47 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -10,9 +10,9 @@ urlpatterns = patterns('django_comment_client.base.views', url(r'threads/(?P[\w\-]+)/reply$', 'create_comment', name='create_comment'), url(r'threads/(?P[\w\-]+)/delete', 'delete_thread', name='delete_thread'), url(r'threads/(?P[\w\-]+)/upvote$', 'vote_for_thread', {'value': 'up'}, name='upvote_thread'), + url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', name='downvote_thread'), url(r'threads/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_thread', name='flag_abuse_for_thread'), url(r'threads/(?P[\w\-]+)/unFlagAbuse$', 'un_flag_abuse_for_thread', name='un_flag_abuse_for_thread'), - url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', name='downvote_thread'), url(r'threads/(?P[\w\-]+)/unvote$', 'undo_vote_for_thread', name='undo_vote_for_thread'), url(r'threads/(?P[\w\-]+)/pin$', 'pin_thread', name='pin_thread'), url(r'threads/(?P[\w\-]+)/unpin$', 'un_pin_thread', name='un_pin_thread'), From 30104979e707de2e506ee0cc15e846c76b59c98f Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 16:25:16 -0400 Subject: [PATCH 102/107] fix downvoting url --- lms/djangoapps/django_comment_client/base/urls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/django_comment_client/base/urls.py b/lms/djangoapps/django_comment_client/base/urls.py index fb687f4e47..18efbec502 100644 --- a/lms/djangoapps/django_comment_client/base/urls.py +++ b/lms/djangoapps/django_comment_client/base/urls.py @@ -10,7 +10,7 @@ urlpatterns = patterns('django_comment_client.base.views', url(r'threads/(?P[\w\-]+)/reply$', 'create_comment', name='create_comment'), url(r'threads/(?P[\w\-]+)/delete', 'delete_thread', name='delete_thread'), url(r'threads/(?P[\w\-]+)/upvote$', 'vote_for_thread', {'value': 'up'}, name='upvote_thread'), - url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', name='downvote_thread'), + url(r'threads/(?P[\w\-]+)/downvote$', 'vote_for_thread', {'value': 'down'}, name='downvote_thread'), url(r'threads/(?P[\w\-]+)/flagAbuse$', 'flag_abuse_for_thread', name='flag_abuse_for_thread'), url(r'threads/(?P[\w\-]+)/unFlagAbuse$', 'un_flag_abuse_for_thread', name='un_flag_abuse_for_thread'), url(r'threads/(?P[\w\-]+)/unvote$', 'undo_vote_for_thread', name='undo_vote_for_thread'), From 32881ed265c771d04b89186f96415a220b786b65 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Mon, 22 Apr 2013 16:38:41 -0400 Subject: [PATCH 103/107] Change the name of the button so that it is clearer. --- common/lib/capa/capa/templates/matlabinput.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/capa/capa/templates/matlabinput.html b/common/lib/capa/capa/templates/matlabinput.html index 7ca9a1961f..69e412f43e 100644 --- a/common/lib/capa/capa/templates/matlabinput.html +++ b/common/lib/capa/capa/templates/matlabinput.html @@ -35,7 +35,7 @@ % if button_enabled:
    - +
    %endif From 068e02efd539a450fbf5e768930a8cee1ffcdbe4 Mon Sep 17 00:00:00 2001 From: Your Name Date: Mon, 22 Apr 2013 17:51:48 -0400 Subject: [PATCH 104/107] make unflag all permissions match javascript --- lms/djangoapps/django_comment_client/base/views.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 1f20fcbc79..d2fa25e979 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -20,7 +20,7 @@ from django.utils.translation import ugettext as _ from django.contrib.auth.models import User from mitxmako.shortcuts import render_to_response, render_to_string -from courseware.courses import get_course_with_access +from courseware.courses import get_course_with_access, get_course_by_id from course_groups.cohorts import get_cohort_id, is_commentable_cohorted from django_comment_client.utils import JsonResponse, JsonError, extract, get_courseware_context @@ -299,9 +299,9 @@ def flag_abuse_for_thread(request, course_id, thread_id): @permitted def un_flag_abuse_for_thread(request, course_id, thread_id): user = cc.User.from_django_user(request.user) - + course = get_course_by_id(course_id) thread = cc.Thread.find(thread_id) - removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) + removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) or has_access(request.user, course, 'staff') thread.unFlagAbuse(user, thread, removeAll) return JsonResponse(utils.safe_content(thread.to_dict())) @@ -321,7 +321,8 @@ def flag_abuse_for_comment(request, course_id, comment_id): @permitted def un_flag_abuse_for_comment(request, course_id, comment_id): user = cc.User.from_django_user(request.user) - removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) + course = get_course_by_id(course_id) + removeAll = cached_has_permission(request.user, 'openclose_thread', course_id) or has_access(request.user, course, 'staff') comment = cc.Comment.find(comment_id) comment.unFlagAbuse(user, comment, removeAll) return JsonResponse(utils.safe_content(comment.to_dict())) From 67f57a71454207b2cf6213bd582fcb97af0ed83c Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 23 Apr 2013 13:28:23 -0400 Subject: [PATCH 105/107] use new API verbs --- lms/lib/comment_client/comment.py | 4 ++-- lms/lib/comment_client/thread.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lms/lib/comment_client/comment.py b/lms/lib/comment_client/comment.py index 324de7923f..fb5a4ad0c3 100644 --- a/lms/lib/comment_client/comment.py +++ b/lms/lib/comment_client/comment.py @@ -78,8 +78,8 @@ def _url_for_comment(comment_id): def _url_for_flag_abuse_comment(comment_id): - return "{prefix}/comments/{comment_id}/abuse_flags".format(prefix=settings.PREFIX, comment_id=comment_id) + return "{prefix}/comments/{comment_id}/abuse_flag".format(prefix=settings.PREFIX, comment_id=comment_id) def _url_for_unflag_abuse_comment(comment_id): - return "{prefix}/comments/{comment_id}/abuse_unflags".format(prefix=settings.PREFIX, comment_id=comment_id) + return "{prefix}/comments/{comment_id}/abuse_unflag".format(prefix=settings.PREFIX, comment_id=comment_id) diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 60a68dc3ae..0b0be576b8 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -121,11 +121,11 @@ class Thread(models.Model): def _url_for_flag_abuse_thread(thread_id): - return "{prefix}/threads/{thread_id}/abuse_flags".format(prefix=settings.PREFIX, thread_id=thread_id) + return "{prefix}/threads/{thread_id}/abuse_flag".format(prefix=settings.PREFIX, thread_id=thread_id) def _url_for_unflag_abuse_thread(thread_id): - return "{prefix}/threads/{thread_id}/abuse_unflags".format(prefix=settings.PREFIX, thread_id=thread_id) + return "{prefix}/threads/{thread_id}/abuse_unflag".format(prefix=settings.PREFIX, thread_id=thread_id) def _url_for_pin_thread(thread_id): From bcce41078b4399ba00d5c18e97445072021a6faa Mon Sep 17 00:00:00 2001 From: ichuang Date: Tue, 23 Apr 2013 16:12:49 -0400 Subject: [PATCH 106/107] add more documentation about `showanswer` policy key --- doc/public/course_data_formats/course_xml.rst | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/public/course_data_formats/course_xml.rst b/doc/public/course_data_formats/course_xml.rst index 22d96d1432..f3e52bf32d 100644 --- a/doc/public/course_data_formats/course_xml.rst +++ b/doc/public/course_data_formats/course_xml.rst @@ -387,7 +387,14 @@ Inherited When this content should be shown to students. Note that anyone with staff access to the course will always see everything. `showanswer` - When to show answer. For 'attempted', will show answer after first attempt. Values: never, attempted, answered, closed. Default: closed. Optional. + When to show answer. Values: never, attempted, answered, closed. Default: closed. Optional. + - `never`: never show answer + - `attempted`: show answer after first attempt + - `answered` : this is slightly different from `attempted` -- resetting the problems makes "done" False, but leaves attempts unchanged. + - `closed` : show answer after problem is closed, ie due date is past, or maximum attempts exceeded. + - `finished` : show answer after problem closed, or is correctly answered. + - `past_due` : show answer after problem due date is past. + - `always` : always allow answer to be shown. `graded` Whether this section will count towards the students grade. "true" or "false". Defaults to "false". From a307696d679c2c1c88417441d622e09bc82d066e Mon Sep 17 00:00:00 2001 From: ichuang Date: Tue, 23 Apr 2013 16:15:46 -0400 Subject: [PATCH 107/107] more showanswer possibilities -> doc --- doc/public/course_data_formats/course_xml.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/public/course_data_formats/course_xml.rst b/doc/public/course_data_formats/course_xml.rst index f3e52bf32d..c17175bafa 100644 --- a/doc/public/course_data_formats/course_xml.rst +++ b/doc/public/course_data_formats/course_xml.rst @@ -387,7 +387,7 @@ Inherited When this content should be shown to students. Note that anyone with staff access to the course will always see everything. `showanswer` - When to show answer. Values: never, attempted, answered, closed. Default: closed. Optional. + When to show answer. Values: never, attempted, answered, closed, finished, past_due, always. Default: closed. Optional. - `never`: never show answer - `attempted`: show answer after first attempt - `answered` : this is slightly different from `attempted` -- resetting the problems makes "done" False, but leaves attempts unchanged.