diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 64d1dd9a93..11c353677b 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -83,40 +83,39 @@ if Backbone? renderDiscussion: ($elem, response, textStatus, discussionId) => $elem.focus() - window.user = new DiscussionUser(response.user_info) + user = new DiscussionUser(response.user_info) + window.user = user + DiscussionUtil.setUser(user) Content.loadContentInfos(response.annotated_content_info) DiscussionUtil.loadRoles(response.roles) - allow_anonymous = response.allow_anonymous - allow_anonymous_to_peers = response.allow_anonymous_to_peers - cohorts = response.cohorts - # $elem.html("Hide Discussion") + + @course_settings = new DiscussionCourseSettings(response.course_settings) @discussion = new Discussion() @discussion.reset(response.discussion_data, {silent: false}) - #use same discussion template but different thread templated - #determined in the coffeescript based on whether or not there's a - #group id - - if response.is_cohorted and response.is_moderator - source = "script#_inline_discussion_cohorted" - else - source = "script#_inline_discussion" - - $discussion = $(Mustache.render $(source).html(), {'threads':response.discussion_data, 'discussionId': discussionId, 'allow_anonymous_to_peers': allow_anonymous_to_peers, 'allow_anonymous': allow_anonymous, 'cohorts':cohorts}) + $discussion = $(Mustache.render $("script#_inline_discussion").html(), {'threads':response.discussion_data, 'discussionId': discussionId}) if @$('section.discussion').length @$('section.discussion').replaceWith($discussion) else @$el.append($discussion) + @newPostForm = $('.new-post-article') @threadviews = @discussion.map (thread) -> new DiscussionThreadInlineView el: @$("article#thread_#{thread.id}"), model: thread _.each @threadviews, (dtv) -> dtv.render() DiscussionUtil.bulkUpdateContentInfo(window.$$annotated_content_info) - @newPostView = new NewPostInlineView el: @$('.new-post-article'), collection: @discussion + @newPostView = new NewPostView( + el: @newPostForm, + collection: @discussion, + course_settings: @course_settings + ) + @newPostView.render() @discussion.on "add", @addThread + @retrieved = true @showed = true @renderPagination(response.num_pages) + if @isWaitingOnNewPost @newPostForm.show() diff --git a/common/static/coffee/src/discussion/discussion_router.coffee b/common/static/coffee/src/discussion/discussion_router.coffee index c535c3100f..1477164e8a 100644 --- a/common/static/coffee/src/discussion/discussion_router.coffee +++ b/common/static/coffee/src/discussion/discussion_router.coffee @@ -6,15 +6,23 @@ if Backbone? initialize: (options) -> @discussion = options['discussion'] + @course_settings = options['course_settings'] + @nav = new DiscussionThreadListView(collection: @discussion, el: $(".sidebar")) @nav.on "thread:selected", @navigateToThread @nav.on "thread:removed", @navigateToAllThreads @nav.on "threads:rendered", @setActiveThread + @nav.on "thread:created", @navigateToThread @nav.render() - @newPostView = new NewPostView(el: $(".new-post-article"), collection: @discussion) - @nav.on "thread:created", @navigateToThread @newPost = $('.new-post-article') + @newPostView = new NewPostView( + el: @newPost, + collection: @discussion, + course_settings: @course_settings, + mode: "tab" + ) + @newPostView.render() $('.new-post-btn').bind "click", @showNewPost $('.new-post-btn').bind "keydown", (event) => DiscussionUtil.activateOnSpace(event, @showNewPost) $('.new-post-cancel').bind "click", @hideNewPost diff --git a/common/static/coffee/src/discussion/main.coffee b/common/static/coffee/src/discussion/main.coffee index 42fd2260ff..29e97161d3 100644 --- a/common/static/coffee/src/discussion/main.coffee +++ b/common/static/coffee/src/discussion/main.coffee @@ -10,10 +10,13 @@ if Backbone? threads = element.data("threads") thread_pages = element.data("thread-pages") content_info = element.data("content-info") - window.user = new DiscussionUser(user_info) + user = new DiscussionUser(user_info) + DiscussionUtil.setUser(user) + window.user = user Content.loadContentInfos(content_info) discussion = new Discussion(threads, {pages: thread_pages, sort: sort_preference}) - new DiscussionRouter({discussion: discussion}) + course_settings = new DiscussionCourseSettings(element.data("course-settings")) + new DiscussionRouter({discussion: discussion, course_settings: course_settings}) Backbone.history.start({pushState: true, root: "/courses/#{$$course_id}/discussion/forum/"}) DiscussionProfileApp = start: (elem) -> diff --git a/common/static/coffee/src/discussion/models/discussion_course_settings.coffee b/common/static/coffee/src/discussion/models/discussion_course_settings.coffee new file mode 100644 index 0000000000..97275a639b --- /dev/null +++ b/common/static/coffee/src/discussion/models/discussion_course_settings.coffee @@ -0,0 +1,2 @@ +if Backbone? + class @DiscussionCourseSettings extends Backbone.Model diff --git a/common/static/coffee/src/discussion/utils.coffee b/common/static/coffee/src/discussion/utils.coffee index e634c79975..8242779c8a 100644 --- a/common/static/coffee/src/discussion/utils.coffee +++ b/common/static/coffee/src/discussion/utils.coffee @@ -18,6 +18,9 @@ class @DiscussionUtil @getTemplate: (id) -> $("script##{id}").html() + @setUser: (user) -> + @user = user + @loadRoles: (roles)-> @roleIds = roles @@ -29,10 +32,12 @@ class @DiscussionUtil @loadFlagModerator($("#discussion-container").data("flag-moderator")) @isStaff: (user_id) -> + user_id ?= @user.id staff = _.union(@roleIds['Moderator'], @roleIds['Administrator']) _.include(staff, parseInt(user_id)) @isTA: (user_id) -> + user_id ?= @user.id ta = _.union(@roleIds['Community TA']) _.include(ta, parseInt(user_id)) diff --git a/common/static/coffee/src/discussion/views/new_post_inline_vew.coffee b/common/static/coffee/src/discussion/views/new_post_inline_vew.coffee deleted file mode 100644 index 97d1730402..0000000000 --- a/common/static/coffee/src/discussion/views/new_post_inline_vew.coffee +++ /dev/null @@ -1,57 +0,0 @@ -if Backbone? - class @NewPostInlineView extends Backbone.View - - initialize: () -> - - @topicId = @$(".topic").first().data("discussion-id") - - @maxNameWidth = 100 - - DiscussionUtil.makeWmdEditor @$el, $.proxy(@$, @), "new-post-body" - - events: - "submit .new-post-form": "createPost" - - # Because we want the behavior that when the body is clicked the menu is - # closed, we need to ignore clicks in the search field and stop propagation. - # Without this, clicking the search field would also close the menu. - ignoreClick: (event) -> - event.stopPropagation() - - createPost: (event) -> - event.preventDefault() - title = @$(".new-post-title").val() - body = @$(".new-post-body").find(".wmd-input").val() - group = @$(".new-post-group option:selected").attr("value") - - anonymous = false || @$("input.discussion-anonymous").is(":checked") - anonymous_to_peers = false || @$("input.discussion-anonymous-to-peers").is(":checked") - follow = false || @$("input.discussion-follow").is(":checked") - - url = DiscussionUtil.urlFor('create_thread', @topicId) - - DiscussionUtil.safeAjax - $elem: $(event.target) - $loading: $(event.target) if event - url: url - type: "POST" - dataType: 'json' - async: false # TODO when the rest of the stuff below is made to work properly.. - data: - title: title - body: body - group_id: group - - anonymous: anonymous - anonymous_to_peers: anonymous_to_peers - auto_subscribe: follow - error: DiscussionUtil.formErrorHandler(@$(".new-post-form-errors")) - success: (response, textStatus) => - # TODO: Move this out of the callback, this makes it feel sluggish - thread = new Thread response['content'] - DiscussionUtil.clearFormErrors(@$(".new-post-form-errors")) - @$el.hide() - @$(".new-post-title").val("").attr("prev-text", "") - @$(".new-post-body textarea").val("").attr("prev-text", "") - - @collection.add thread diff --git a/common/static/coffee/src/discussion/views/new_post_view.coffee b/common/static/coffee/src/discussion/views/new_post_view.coffee index ef7d1e86e9..6812cd09c9 100644 --- a/common/static/coffee/src/discussion/views/new_post_view.coffee +++ b/common/static/coffee/src/discussion/views/new_post_view.coffee @@ -1,25 +1,74 @@ if Backbone? class @NewPostView extends Backbone.View - initialize: () -> - @dropdownButton = @$(".topic_dropdown_button") - @topicMenu = @$(".topic_menu_wrapper") - - @menuOpen = @dropdownButton.hasClass('dropped') - - @topicId = @$(".topic").first().data("discussion_id") - @topicText = @getFullTopicName(@$(".topic").first()) - + initialize: (options) -> + @mode = options.mode or "inline" # allowed values are "tab" or "inline" + if @mode not in ["tab", "inline"] + throw new Error("invalid mode: " + @mode) + @course_settings = options.course_settings @maxNameWidth = 100 - @setSelectedTopic() + render: () -> + if @mode is "tab" + @$el.html( + _.template( + $("#new-post-tab-template").html(), { + topic_dropdown_html: @getTopicDropdownHTML(), + options_html: @getOptionsHTML(), + editor_html: @getEditorHTML() + } + ) + ) + # set up the topic dropdown in tab mode + @dropdownButton = @$(".topic_dropdown_button") + @topicMenu = @$(".topic_menu_wrapper") + @menuOpen = @dropdownButton.hasClass('dropped') + @topicId = @$(".topic").first().data("discussion_id") + @topicText = @getFullTopicName(@$(".topic").first()) + $('.choose-cohort').hide() unless @$(".topic_menu li a").first().is("[cohorted=true]") + @setSelectedTopic() + else # inline + @$el.html( + _.template( + $("#new-post-inline-template").html(), { + options_html: @getOptionsHTML(), + editor_html: @getEditorHTML() + } + ) + ) DiscussionUtil.makeWmdEditor @$el, $.proxy(@$, @), "new-post-body" - - if @$($(".topic_menu li a")[0]).attr('cohorted') != "True" - $('.choose-cohort').hide(); - - - + + getTopicDropdownHTML: () -> + # populate the category menu (topic dropdown) + _renderCategoryMap = (map) -> + category_template = _.template($("#new-post-menu-category-template").html()) + entry_template = _.template($("#new-post-menu-entry-template").html()) + html = "" + for name in map.children + if name of map.entries + entry = map.entries[name] + html += entry_template({text: name, id: entry.id, is_cohorted: entry.is_cohorted}) + else # subcategory + html += category_template({text: name, entries: _renderCategoryMap(map.subcategories[name])}) + html + topics_html = _renderCategoryMap(@course_settings.get("category_map")) + _.template($("#new-post-topic-dropdown-template").html(), {topics_html: topics_html}) + + getEditorHTML: () -> + _.template($("#new-post-editor-template").html(), {}) + + getOptionsHTML: () -> + # cohort options? + if @course_settings.get("is_cohorted") and DiscussionUtil.isStaff() + user_cohort_id = $("#discussion-container").data("user-cohort-id") + cohort_options = _.map @course_settings.get("cohorts"), (cohort) -> + {value: cohort.id, text: cohort.name, selected: cohort.id==user_cohort_id} + else + cohort_options = null + context = _.clone(@course_settings.attributes) + context.cohort_options = cohort_options + _.template($("#new-post-options-template").html(), context) + events: "submit .new-post-form": "createPost" "click .topic_dropdown_button": "toggleTopicDropdown" @@ -33,6 +82,43 @@ if Backbone? ignoreClick: (event) -> event.stopPropagation() + createPost: (event) -> + event.preventDefault() + title = @$(".new-post-title").val() + body = @$(".new-post-body").find(".wmd-input").val() + group = @$(".new-post-group option:selected").attr("value") + + anonymous = false || @$("input.discussion-anonymous").is(":checked") + anonymous_to_peers = false || @$("input.discussion-anonymous-to-peers").is(":checked") + follow = false || @$("input.discussion-follow").is(":checked") + + url = DiscussionUtil.urlFor('create_thread', @topicId) + + DiscussionUtil.safeAjax + $elem: $(event.target) + $loading: $(event.target) if event + url: url + type: "POST" + dataType: 'json' + async: false # TODO when the rest of the stuff below is made to work properly.. + data: + title: title + body: body + anonymous: anonymous + anonymous_to_peers: anonymous_to_peers + auto_subscribe: follow + group_id: group + error: DiscussionUtil.formErrorHandler(@$(".new-post-form-errors")) + success: (response, textStatus) => + # TODO: Move this out of the callback, this makes it feel sluggish + thread = new Thread response['content'] + DiscussionUtil.clearFormErrors(@$(".new-post-form-errors")) + @$el.hide() + @$(".new-post-title").val("").attr("prev-text", "") + @$(".new-post-body textarea").val("").attr("prev-text", "") + @$(".wmd-preview p").html("") # only line not duplicated in new post inline view + @collection.add thread + toggleTopicDropdown: (event) -> event.stopPropagation() if @menuOpen @@ -69,11 +155,10 @@ if Backbone? @topicText = @getFullTopicName($target) @topicId = $target.data('discussion_id') @setSelectedTopic() - if $target.attr('cohorted') == "True" + if $target.is('[cohorted=true]') $('.choose-cohort').show(); else $('.choose-cohort').hide(); - setSelectedTopic: -> @dropdownButton.html(@fitName(@topicText) + ' ▾') @@ -119,44 +204,6 @@ if Backbone? return name - - createPost: (event) -> - event.preventDefault() - title = @$(".new-post-title").val() - body = @$(".new-post-body").find(".wmd-input").val() - group = @$(".new-post-group option:selected").attr("value") - - anonymous = false || @$("input.discussion-anonymous").is(":checked") - anonymous_to_peers = false || @$("input.discussion-anonymous-to-peers").is(":checked") - follow = false || @$("input.discussion-follow").is(":checked") - - url = DiscussionUtil.urlFor('create_thread', @topicId) - - DiscussionUtil.safeAjax - $elem: $(event.target) - $loading: $(event.target) if event - url: url - type: "POST" - dataType: 'json' - async: false # TODO when the rest of the stuff below is made to work properly.. - data: - title: title - body: body - anonymous: anonymous - anonymous_to_peers: anonymous_to_peers - auto_subscribe: follow - group_id: group - error: DiscussionUtil.formErrorHandler(@$(".new-post-form-errors")) - success: (response, textStatus) => - # TODO: Move this out of the callback, this makes it feel sluggish - thread = new Thread response['content'] - DiscussionUtil.clearFormErrors(@$(".new-post-form-errors")) - @$el.hide() - @$(".new-post-title").val("").attr("prev-text", "") - @$(".new-post-body textarea").val("").attr("prev-text", "") - @$(".wmd-preview p").html("") - @collection.add thread - setActiveItem: (event) -> if event.which == 13 $(".topic_menu_wrapper .focused").click() diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index ce3bb42777..4ca8fc2baa 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -26,10 +26,34 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey THREADS_PER_PAGE = 20 INLINE_THREADS_PER_PAGE = 20 PAGES_NEARBY_DELTA = 2 -escapedict = {'"': '"'} log = logging.getLogger("edx.discussions") +def _attr_safe_json(obj): + """ + return a JSON string for obj which is safe to embed as the value of an attribute in a DOM node + """ + return saxutils.escape(json.dumps(obj), {'"': '"'}) + +@newrelic.agent.function_trace() +def make_course_settings(course, include_category_map=False): + """ + Generate a JSON-serializable model for course settings, which will be used to initialize a + DiscussionCourseSettings object on the client. + """ + + obj = { + 'is_cohorted': is_course_cohorted(course.id), + 'allow_anonymous': course.allow_anonymous, + 'allow_anonymous_to_peers': course.allow_anonymous_to_peers, + 'cohorts': [{"id": str(g.id), "name": g.name} for g in get_course_cohorts(course.id)], + } + + if include_category_map: + obj['category_map'] = utils.get_discussion_category_map(course) + + return obj + @newrelic.agent.function_trace() def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAGE): """ @@ -125,31 +149,6 @@ def inline_discussion(request, course_id, discussion_id): with newrelic.agent.FunctionTrace(nr_transaction, "get_metadata_for_threads"): annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info) - allow_anonymous = course.allow_anonymous - allow_anonymous_to_peers = course.allow_anonymous_to_peers - - #since inline is all one commentable, only show or allow the choice of cohorts - #if the commentable is cohorted, otherwise everything is not cohorted - #and no one has the option of choosing a cohort - is_cohorted = is_course_cohorted(course_id) and is_commentable_cohorted(course_id, discussion_id) - is_moderator = cached_has_permission(request.user, "see_all_cohorts", course_id) - - cohorts_list = list() - - if is_cohorted: - cohorts_list.append({'name': _('All Groups'), 'id': None}) - - #if you're a mod, send all cohorts and let you pick - - if is_moderator: - cohorts = get_course_cohorts(course_id) - for cohort in cohorts: - cohorts_list.append({'name': cohort.name, 'id': cohort.id}) - - else: - #students don't get to choose - cohorts_list = None - return utils.JsonResponse({ 'discussion_data': map(utils.safe_content, threads), 'user_info': user_info, @@ -157,14 +156,9 @@ def inline_discussion(request, course_id, discussion_id): 'page': query_params['page'], 'num_pages': query_params['num_pages'], 'roles': utils.get_role_ids(course_id), - 'allow_anonymous_to_peers': allow_anonymous_to_peers, - 'allow_anonymous': allow_anonymous, - 'cohorts': cohorts_list, - 'is_moderator': is_moderator, - 'is_cohorted': is_cohorted + 'course_settings': make_course_settings(course) }) - @login_required def forum_form_discussion(request, course_id): """ @@ -174,8 +168,7 @@ def forum_form_discussion(request, course_id): nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load_forum', course_id) - with newrelic.agent.FunctionTrace(nr_transaction, "get_discussion_category_map"): - category_map = utils.get_discussion_category_map(course) + course_settings = make_course_settings(course, include_category_map=True) try: unsafethreads, query_params = get_threads(request, course_id) # This might process a search query @@ -203,9 +196,6 @@ def forum_form_discussion(request, course_id): }) else: with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): - cohorts = get_course_cohorts(course_id) - cohorted_commentables = get_cohorted_commentables(course_id) - user_cohort_id = get_cohort_id(request.user, course_id) context = { @@ -213,20 +203,20 @@ def forum_form_discussion(request, course_id): 'course': course, #'recent_active_threads': recent_active_threads, 'staff_access': has_access(request.user, 'staff', course), - 'threads': saxutils.escape(json.dumps(threads), escapedict), + 'threads': _attr_safe_json(threads), 'thread_pages': query_params['num_pages'], - 'user_info': saxutils.escape(json.dumps(user_info), escapedict), + 'user_info': _attr_safe_json(user_info), 'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course), - 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), + 'annotated_content_info': _attr_safe_json(annotated_content_info), 'course_id': course.id.to_deprecated_string(), - 'category_map': category_map, - 'roles': saxutils.escape(json.dumps(utils.get_role_ids(course_id)), escapedict), + 'roles': _attr_safe_json(utils.get_role_ids(course_id)), 'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_id), - 'cohorts': cohorts, - 'user_cohort': user_cohort_id, - 'cohorted_commentables': cohorted_commentables, - 'is_course_cohorted': is_course_cohorted(course_id), + 'cohorts': course_settings["cohorts"], # still needed to render _thread_list_template + 'user_cohort': user_cohort_id, # read from container in NewPostView + 'is_course_cohorted': is_course_cohorted(course_id), # still needed to render _thread_list_template 'sort_preference': user.default_sort_key, + 'category_map': course_settings["category_map"], + 'course_settings': _attr_safe_json(course_settings) } # print "start rendering.." return render_to_response('discussion/index.html', context) @@ -239,6 +229,7 @@ def single_thread(request, course_id, discussion_id, thread_id): nr_transaction = newrelic.agent.current_transaction() course = get_course_with_access(request.user, 'load_forum', course_id) + course_settings = make_course_settings(course, include_category_map=True) cc_user = cc.User.from_django_user(request.user) user_info = cc_user.to_dict() @@ -269,14 +260,9 @@ def single_thread(request, course_id, discussion_id, thread_id): }) else: - with newrelic.agent.FunctionTrace(nr_transaction, "get_discussion_category_map"): - category_map = utils.get_discussion_category_map(course) - threads, query_params = get_threads(request, course_id) threads.append(thread.to_dict()) - course = get_course_with_access(request.user, 'load_forum', course_id) - with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): add_courseware_context(threads, course) @@ -294,30 +280,29 @@ def single_thread(request, course_id, discussion_id, thread_id): annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info) with newrelic.agent.FunctionTrace(nr_transaction, "get_cohort_info"): - 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, 'csrf': csrf(request)['csrf_token'], 'init': '', # TODO: What is this? - 'user_info': saxutils.escape(json.dumps(user_info), escapedict), - 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), + 'user_info': _attr_safe_json(user_info), + 'annotated_content_info': _attr_safe_json(annotated_content_info), 'course': course, #'recent_active_threads': recent_active_threads, 'course_id': course.id.to_deprecated_string(), # TODO: Why pass both course and course.id to template? 'thread_id': thread_id, - 'threads': saxutils.escape(json.dumps(threads), escapedict), - 'category_map': category_map, - 'roles': saxutils.escape(json.dumps(utils.get_role_ids(course_id)), escapedict), + 'threads': _attr_safe_json(threads), + 'roles': _attr_safe_json(utils.get_role_ids(course_id)), + 'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_id), 'thread_pages': query_params['num_pages'], 'is_course_cohorted': is_course_cohorted(course_id), - 'is_moderator': cached_has_permission(request.user, "see_all_cohorts", course_id), 'flag_moderator': cached_has_permission(request.user, 'openclose_thread', course.id) or has_access(request.user, 'staff', course), - 'cohorts': cohorts, - 'user_cohort': get_cohort_id(request.user, course_id), - 'cohorted_commentables': cohorted_commentables, + 'cohorts': course_settings["cohorts"], + 'user_cohort': user_cohort, 'sort_preference': cc_user.default_sort_key, + 'category_map': course_settings["category_map"], + 'course_settings': _attr_safe_json(course_settings) } return render_to_response('discussion/index.html', context) @@ -350,7 +335,7 @@ def user_profile(request, course_id, user_id): 'discussion_data': map(utils.safe_content, threads), 'page': query_params['page'], 'num_pages': query_params['num_pages'], - 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), + 'annotated_content_info': _attr_safe_json(annotated_content_info), }) else: context = { @@ -358,9 +343,9 @@ def user_profile(request, course_id, user_id): 'user': request.user, 'django_user': User.objects.get(id=user_id), 'profiled_user': profiled_user.to_dict(), - 'threads': saxutils.escape(json.dumps(threads), escapedict), - 'user_info': saxutils.escape(json.dumps(user_info), escapedict), - 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), + 'threads': _attr_safe_json(threads), + 'user_info': _attr_safe_json(user_info), + 'annotated_content_info': _attr_safe_json(annotated_content_info), 'page': query_params['page'], 'num_pages': query_params['num_pages'], # 'content': content, @@ -408,9 +393,9 @@ def followed_threads(request, course_id, user_id): 'user': request.user, 'django_user': User.objects.get(id=user_id), 'profiled_user': profiled_user.to_dict(), - 'threads': saxutils.escape(json.dumps(threads), escapedict), - 'user_info': saxutils.escape(json.dumps(user_info), escapedict), - 'annotated_content_info': saxutils.escape(json.dumps(annotated_content_info), escapedict), + 'threads': _attr_safe_json(threads), + 'user_info': _attr_safe_json(user_info), + 'annotated_content_info': _attr_safe_json(annotated_content_info), # 'content': content, } diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index f9bd599bb2..64ee59b00e 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -152,7 +152,7 @@ class CategoryMapTestCase(ModuleStoreTestCase): def create_discussion(self, discussion_category, discussion_target, **kwargs): self.discussion_num += 1 - ItemFactory.create( + return ItemFactory.create( parent_location=self.course.location, category="discussion", discussion_id="discussion{}".format(self.discussion_num), @@ -179,17 +179,38 @@ class CategoryMapTestCase(ModuleStoreTestCase): "Topic B": {"id": "Topic_B"}, "Topic C": {"id": "Topic_C"} } - self.assertCategoryMapEquals( - { - "entries": { - "Topic A": {"id": "Topic_A", "sort_key": "Topic A"}, - "Topic B": {"id": "Topic_B", "sort_key": "Topic B"}, - "Topic C": {"id": "Topic_C", "sort_key": "Topic C"}, - }, - "subcategories": {}, - "children": ["Topic A", "Topic B", "Topic C"] - } - ) + + def check_cohorted_topics(expected_ids): + self.assertCategoryMapEquals( + { + "entries": { + "Topic A": {"id": "Topic_A", "sort_key": "Topic A", "is_cohorted": "Topic_A" in expected_ids}, + "Topic B": {"id": "Topic_B", "sort_key": "Topic B", "is_cohorted": "Topic_B" in expected_ids}, + "Topic C": {"id": "Topic_C", "sort_key": "Topic C", "is_cohorted": "Topic_C" in expected_ids}, + }, + "subcategories": {}, + "children": ["Topic A", "Topic B", "Topic C"] + } + ) + + check_cohorted_topics([]) # default (empty) cohort config + + self.course.cohort_config = {"cohorted": False, "cohorted_discussions": []} + check_cohorted_topics([]) + + self.course.cohort_config = {"cohorted": True, "cohorted_discussions": []} + check_cohorted_topics([]) + + self.course.cohort_config = {"cohorted": True, "cohorted_discussions": ["Topic_B", "Topic_C"]} + check_cohorted_topics(["Topic_B", "Topic_C"]) + + self.course.cohort_config = {"cohorted": True, "cohorted_discussions": ["Topic_A", "Some_Other_Topic"]} + check_cohorted_topics(["Topic_A"]) + + # unlikely case, but make sure it works. + self.course.cohort_config = {"cohorted": False, "cohorted_discussions": ["Topic_A"]} + check_cohorted_topics([]) + def test_single_inline(self): self.create_discussion("Chapter", "Discussion") @@ -201,7 +222,8 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion": { "id": "discussion1", - "sort_key": None + "sort_key": None, + "is_cohorted": False, } }, "subcategories": {}, @@ -220,81 +242,101 @@ class CategoryMapTestCase(ModuleStoreTestCase): self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion") self.create_discussion("Chapter 3 / Section 1", "Discussion") - self.assertCategoryMapEquals( - { - "entries": {}, - "subcategories": { - "Chapter 1": { - "entries": { - "Discussion 1": { - "id": "discussion1", - "sort_key": None + def check_cohorted(is_cohorted): + + self.assertCategoryMapEquals( + { + "entries": {}, + "subcategories": { + "Chapter 1": { + "entries": { + "Discussion 1": { + "id": "discussion1", + "sort_key": None, + "is_cohorted": is_cohorted, + }, + "Discussion 2": { + "id": "discussion2", + "sort_key": None, + "is_cohorted": is_cohorted, + } }, - "Discussion 2": { - "id": "discussion2", - "sort_key": None - } + "subcategories": {}, + "children": ["Discussion 1", "Discussion 2"] }, - "subcategories": {}, - "children": ["Discussion 1", "Discussion 2"] - }, - "Chapter 2": { - "entries": { - "Discussion": { - "id": "discussion3", - "sort_key": None - } - }, - "subcategories": { - "Section 1": { - "entries": {}, - "subcategories": { - "Subsection 1": { - "entries": { - "Discussion": { - "id": "discussion4", - "sort_key": None - } + "Chapter 2": { + "entries": { + "Discussion": { + "id": "discussion3", + "sort_key": None, + "is_cohorted": is_cohorted, + } + }, + "subcategories": { + "Section 1": { + "entries": {}, + "subcategories": { + "Subsection 1": { + "entries": { + "Discussion": { + "id": "discussion4", + "sort_key": None, + "is_cohorted": is_cohorted, + } + }, + "subcategories": {}, + "children": ["Discussion"] }, - "subcategories": {}, - "children": ["Discussion"] + "Subsection 2": { + "entries": { + "Discussion": { + "id": "discussion5", + "sort_key": None, + "is_cohorted": is_cohorted, + } + }, + "subcategories": {}, + "children": ["Discussion"] + } }, - "Subsection 2": { - "entries": { - "Discussion": { - "id": "discussion5", - "sort_key": None - } - }, - "subcategories": {}, - "children": ["Discussion"] - } - }, - "children": ["Subsection 1", "Subsection 2"] - } + "children": ["Subsection 1", "Subsection 2"] + } + }, + "children": ["Discussion", "Section 1"] }, - "children": ["Discussion", "Section 1"] + "Chapter 3": { + "entries": {}, + "subcategories": { + "Section 1": { + "entries": { + "Discussion": { + "id": "discussion6", + "sort_key": None, + "is_cohorted": is_cohorted, + } + }, + "subcategories": {}, + "children": ["Discussion"] + } + }, + "children": ["Section 1"] + } }, - "Chapter 3": { - "entries": {}, - "subcategories": { - "Section 1": { - "entries": { - "Discussion": { - "id": "discussion6", - "sort_key": None - } - }, - "subcategories": {}, - "children": ["Discussion"] - } - }, - "children": ["Section 1"] - } - }, - "children": ["Chapter 1", "Chapter 2", "Chapter 3"] - } - ) + "children": ["Chapter 1", "Chapter 2", "Chapter 3"] + } + ) + + # empty / default config + check_cohorted(False) + + # explicitly disabled cohorting + self.course.cohort_config = {"cohorted": False} + check_cohorted(False) + + # explicitly enabled cohorting + self.course.cohort_config = {"cohorted": True} + check_cohorted(True) + def test_start_date_filter(self): now = datetime.now() @@ -314,7 +356,8 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion 1": { "id": "discussion1", - "sort_key": None + "sort_key": None, + "is_cohorted": False, } }, "subcategories": {}, @@ -324,7 +367,8 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion": { "id": "discussion3", - "sort_key": None + "sort_key": None, + "is_cohorted": False, } }, "subcategories": {}, @@ -350,23 +394,28 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion 1": { "id": "discussion1", - "sort_key": "D" + "sort_key": "D", + "is_cohorted": False, }, "Discussion 2": { "id": "discussion2", - "sort_key": "A" + "sort_key": "A", + "is_cohorted": False, }, "Discussion 3": { "id": "discussion3", - "sort_key": "E" + "sort_key": "E", + "is_cohorted": False, }, "Discussion 4": { "id": "discussion4", - "sort_key": "C" + "sort_key": "C", + "is_cohorted": False, }, "Discussion 5": { "id": "discussion5", - "sort_key": "B" + "sort_key": "B", + "is_cohorted": False, } }, "subcategories": {}, @@ -392,9 +441,9 @@ class CategoryMapTestCase(ModuleStoreTestCase): self.assertCategoryMapEquals( { "entries": { - "Topic A": {"id": "Topic_A", "sort_key": "B"}, - "Topic B": {"id": "Topic_B", "sort_key": "C"}, - "Topic C": {"id": "Topic_C", "sort_key": "A"}, + "Topic A": {"id": "Topic_A", "sort_key": "B", "is_cohorted": False}, + "Topic B": {"id": "Topic_B", "sort_key": "C", "is_cohorted": False}, + "Topic C": {"id": "Topic_C", "sort_key": "A", "is_cohorted": False}, }, "subcategories": {}, "children": ["Topic C", "Topic A", "Topic B"] @@ -418,23 +467,28 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion D": { "id": "discussion1", - "sort_key": "Discussion D" + "sort_key": "Discussion D", + "is_cohorted": False, }, "Discussion A": { "id": "discussion2", - "sort_key": "Discussion A" + "sort_key": "Discussion A", + "is_cohorted": False, }, "Discussion E": { "id": "discussion3", - "sort_key": "Discussion E" + "sort_key": "Discussion E", + "is_cohorted": False, }, "Discussion C": { "id": "discussion4", - "sort_key": "Discussion C" + "sort_key": "Discussion C", + "is_cohorted": False, }, "Discussion B": { "id": "discussion5", - "sort_key": "Discussion B" + "sort_key": "Discussion B", + "is_cohorted": False, } }, "subcategories": {}, @@ -466,11 +520,13 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion 1": { "id": "discussion3", - "sort_key": None + "sort_key": None, + "is_cohorted": False, }, "Discussion 2": { "id": "discussion5", - "sort_key": None + "sort_key": None, + "is_cohorted": False, } }, "subcategories": {}, @@ -480,11 +536,13 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion 1": { "id": "discussion4", - "sort_key": None + "sort_key": None, + "is_cohorted": False, }, "Discussion 2": { "id": "discussion1", - "sort_key": None + "sort_key": None, + "is_cohorted": False, } }, "subcategories": {}, @@ -494,7 +552,8 @@ class CategoryMapTestCase(ModuleStoreTestCase): "entries": { "Discussion": { "id": "discussion2", - "sort_key": None + "sort_key": None, + "is_cohorted": False, } }, "subcategories": {}, diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 9d19eb8e1a..6d4ec4a02c 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -135,6 +135,9 @@ def get_discussion_category_map(course): modules = _get_discussion_modules(course) + is_course_cohorted = course.is_cohorted + cohorted_discussion_ids = course.cohorted_discussions + for module in modules: id = module.discussion_id title = module.discussion_target @@ -179,7 +182,8 @@ def get_discussion_category_map(course): for entry in entries: node[level]["entries"][entry["title"]] = {"id": entry["id"], "sort_key": entry["sort_key"], - "start_date": entry["start_date"]} + "start_date": entry["start_date"], + "is_cohorted": is_course_cohorted} # TODO. BUG! : course location is not unique across multiple course runs! # (I think Kevin already noticed this) Need to send course_id with requests, store it @@ -187,7 +191,8 @@ def get_discussion_category_map(course): for topic, entry in course.discussion_topics.items(): category_map['entries'][topic] = {"id": entry["id"], "sort_key": entry.get("sort_key", topic), - "start_date": datetime.now(UTC())} + "start_date": datetime.now(UTC()), + "is_cohorted": is_course_cohorted and entry["id"] in cohorted_discussion_ids} _sort_map_entries(category_map, course.discussion_sort_alpha) diff --git a/lms/templates/discussion/_filter_dropdown.html b/lms/templates/discussion/_filter_dropdown.html index b108be6d2e..4a44f2cd5a 100644 --- a/lms/templates/discussion/_filter_dropdown.html +++ b/lms/templates/discussion/_filter_dropdown.html @@ -12,7 +12,7 @@ <%def name="render_entry(entries, entry)"> -
  • ${entry}
    • +
  • ${entry}
    • <%def name="render_category(categories, category)"> diff --git a/lms/templates/discussion/_inline_new_post.html b/lms/templates/discussion/_inline_new_post.html deleted file mode 100644 index ae108a4d78..0000000000 --- a/lms/templates/discussion/_inline_new_post.html +++ /dev/null @@ -1,51 +0,0 @@ -<%! from django.utils.translation import ugettext as _ %> -
    -
    -
    -
    -
    -
    -
    - -
    - % if course.allow_anonymous: - - %elif course.allow_anonymous_to_peers: - - %endif - %if is_course_cohorted: -
    - ## Translators: This labels the selector for which group of students can view a post - ${_("Make visible to:")} - -
    - %endif -
    -
    -
    -
    - - -
    -
    -
    - -
    - - ${_("Cancel")} -
    -
    -
    -
    diff --git a/lms/templates/discussion/_new_post.html b/lms/templates/discussion/_new_post.html deleted file mode 100644 index 8066e34eca..0000000000 --- a/lms/templates/discussion/_new_post.html +++ /dev/null @@ -1,82 +0,0 @@ -<%! from django.utils.translation import ugettext as _ %> -<%def name="render_form_filter_dropdown(map)"> - % for child in map["children"]: - % if child in map["entries"]: - ${render_entry(map["entries"], child)} - %else: - ${render_category(map["subcategories"], child)} - %endif - %endfor - - -<%def name="render_entry(entries, entry)"> -
  • ${entry}
    • - - -<%def name="render_category(categories, category)"> -
  • - ${category} - -
    • - -
    -
    -
    -
    - ${_("Create new post about:")} -
    - ${_("Show All Discussions")} -
    - - -
    -
    -
    - -
    - % if course.allow_anonymous: - - %elif course.allow_anonymous_to_peers: - - %endif - %if is_course_cohorted and is_moderator: -
    - ## Translators: This labels the selector for which group of students can view a post - ${_("Make visible to:")} - -
    - %endif -
    -
    -
    -
      -
      - - -
      -
      -
      - -
      - - ${_("Cancel")} -
      -
      -
      -
      diff --git a/lms/templates/discussion/_thread_list_template.html b/lms/templates/discussion/_thread_list_template.html index 67f2cd9237..90255a1db8 100644 --- a/lms/templates/discussion/_thread_list_template.html +++ b/lms/templates/discussion/_thread_list_template.html @@ -34,13 +34,12 @@
    • ${_("comments")}
      • - %if is_course_cohorted and is_moderator: ${_("Show:")} %endif diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html index 32335bf832..a1c3585bee 100644 --- a/lms/templates/discussion/_underscore_templates.html +++ b/lms/templates/discussion/_underscore_templates.html @@ -319,3 +319,108 @@ + + + + + + + + + + + + + + diff --git a/lms/templates/discussion/index.html b/lms/templates/discussion/index.html index 88bb49df6b..12be8542e3 100644 --- a/lms/templates/discussion/index.html +++ b/lms/templates/discussion/index.html @@ -23,9 +23,19 @@ <%include file="_discussion_course_navigation.html" args="active_page='discussion'" /> -<%include file="_new_post.html" /> +
      -
      +
      diff --git a/lms/templates/discussion/mustache/_inline_discussion.mustache b/lms/templates/discussion/mustache/_inline_discussion.mustache index 62948bec7a..56f22b0538 100644 --- a/lms/templates/discussion/mustache/_inline_discussion.mustache +++ b/lms/templates/discussion/mustache/_inline_discussion.mustache @@ -1,33 +1,7 @@ <%! from django.utils.translation import ugettext as _ %>
      -
      - -
      -
      -
      -
      -
      - -
      -
      -
      -
      - - ${_("Cancel")} -
      - -
      - {{#allow_anonymous}} - - {{/allow_anonymous}} - {{#allow_anonymous_to_peers}} - - {{/allow_anonymous_to_peers}} -
      -
      -
      -
      +
      {{#threads}} diff --git a/lms/templates/discussion/mustache/_inline_discussion_cohorted.mustache b/lms/templates/discussion/mustache/_inline_discussion_cohorted.mustache deleted file mode 100644 index cb75c6f20b..0000000000 --- a/lms/templates/discussion/mustache/_inline_discussion_cohorted.mustache +++ /dev/null @@ -1,54 +0,0 @@ -<%! from django.utils.translation import ugettext as _ %> - -
      -
      - -
      -
      -
      -
      -
      - -
      -
      -
      -
      - - ${_("Cancel")} -
      - -
      - {{#allow_anonymous}} - - {{/allow_anonymous}} - {{#allow_anonymous_to_peers}} - - {{/allow_anonymous_to_peers}} - -
      - ## Translators: This labels the selector for which group of students can view a post - ${_("Make visible to:")} - -
      - -
      -
      -
      -
      - -
      - {{#threads}} -
      -
      - {{/threads}} -
      - -
      -
      -