From 2af06ef1d4075d01150a43f241ae5d7067dbdd11 Mon Sep 17 00:00:00 2001 From: Victor Shnayder Date: Wed, 6 Feb 2013 20:29:17 -0500 Subject: [PATCH] Revert "Merge pull request #1402 from MITx/feature/kevin/groups_ui_changes" This reverts commit 66889b872443078c9fa71c82abf7c7382f40897c, reversing changes made to 0dd9c24e7410e900c1ee29167fa7f170d5517944. This was merged too early--it's still missing the filtering and display-your-cohort changes. Will need to revert this revert later. --- common/djangoapps/course_groups/cohorts.py | 16 ---- .../discussion/discussion_module_view.coffee | 6 -- .../discussion_thread_view_inline.coffee | 5 +- .../views/new_post_inline_vew.coffee | 2 - .../src/discussion/views/new_post_view.coffee | 10 +-- .../django_comment_client/base/views.py | 24 ++---- .../django_comment_client/forum/views.py | 83 ++----------------- lms/djangoapps/django_comment_client/utils.py | 2 +- lms/lib/comment_client/thread.py | 4 +- lms/static/sass/_discussion.scss | 32 +------ .../discussion/_filter_dropdown.html | 2 +- lms/templates/discussion/_new_post.html | 23 +---- lms/templates/discussion/_single_thread.html | 5 -- .../discussion/_thread_list_template.html | 2 +- .../discussion/_underscore_templates.html | 4 - .../discussion/mustache/_content.mustache | 1 - .../mustache/_inline_discussion.mustache | 2 + .../mustache/_inline_thread.mustache | 1 + .../mustache/_inline_thread_cohorted.mustache | 22 ----- 19 files changed, 30 insertions(+), 216 deletions(-) delete mode 100644 lms/templates/discussion/mustache/_inline_thread_cohorted.mustache diff --git a/common/djangoapps/course_groups/cohorts.py b/common/djangoapps/course_groups/cohorts.py index 1752376175..155f82e0c7 100644 --- a/common/djangoapps/course_groups/cohorts.py +++ b/common/djangoapps/course_groups/cohorts.py @@ -64,23 +64,7 @@ def is_commentable_cohorted(course_id, commentable_id): ans)) return ans - -def get_cohorted_commentables(course_id): - """ - Given a course_id return a list of strings representing cohorted commentables - """ - course = courses.get_course_by_id(course_id) - - if not course.is_cohorted: - # this is the easy case :) - ans = [] - else: - ans = course.cohorted_discussions - - return ans - - def get_cohort(user, course_id): """ Given a django User and a course_id, return the user's cohort in that diff --git a/common/static/coffee/src/discussion/discussion_module_view.coffee b/common/static/coffee/src/discussion/discussion_module_view.coffee index 077210bc4f..63bd6bc733 100644 --- a/common/static/coffee/src/discussion/discussion_module_view.coffee +++ b/common/static/coffee/src/discussion/discussion_module_view.coffee @@ -73,13 +73,7 @@ if Backbone? # $elem.html("Hide Discussion") @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 - $discussion = $(Mustache.render $("script#_inline_discussion").html(), {'threads':response.discussion_data, 'discussionId': discussionId, 'allow_anonymous_to_peers': allow_anonymous_to_peers, 'allow_anonymous': allow_anonymous}) - if @$('section.discussion').length @$('section.discussion').replaceWith($discussion) else diff --git a/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee b/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee index e648955d08..7dab9ae342 100644 --- a/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee +++ b/common/static/coffee/src/discussion/views/discussion_thread_view_inline.coffee @@ -16,10 +16,7 @@ if Backbone? @$delegateElement = @$local render: -> - if @model.has('group_id') - @template = DiscussionUtil.getTemplate("_inline_thread_cohorted") - else - @template = DiscussionUtil.getTemplate("_inline_thread") + @template = DiscussionUtil.getTemplate("_inline_thread") if not @model.has('abbreviatedBody') @abbreviateBody() 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 index ffd43ff7bf..ed5ee13919 100644 --- a/common/static/coffee/src/discussion/views/new_post_inline_vew.coffee +++ b/common/static/coffee/src/discussion/views/new_post_inline_vew.coffee @@ -25,7 +25,6 @@ if Backbone? event.preventDefault() title = @$(".new-post-title").val() body = @$(".new-post-body").find(".wmd-input").val() - group = @$(".new-post-group option:selected").attr("value") # TODO tags: commenting out til we know what to do with them #tags = @$(".new-post-tags").val() @@ -46,7 +45,6 @@ if Backbone? data: title: title body: body - group_id: group # TODO tags: commenting out til we know what to do with them #tags: tags 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 be146587df..1c49fdbc8e 100644 --- a/common/static/coffee/src/discussion/views/new_post_view.coffee +++ b/common/static/coffee/src/discussion/views/new_post_view.coffee @@ -14,9 +14,8 @@ if Backbone? @setSelectedTopic() DiscussionUtil.makeWmdEditor @$el, $.proxy(@$, @), "new-post-body" - @$(".new-post-tags").tagsInput DiscussionUtil.tagsInputOptions() - + events: "submit .new-post-form": "createPost" "click .topic_dropdown_button": "toggleTopicDropdown" @@ -66,11 +65,6 @@ if Backbone? @topicText = @getFullTopicName($target) @topicId = $target.data('discussion_id') @setSelectedTopic() - if $target.attr('cohorted') == "True" - $('.choose-cohort').show(); - else - $('.choose-cohort').hide(); - setSelectedTopic: -> @dropdownButton.html(@fitName(@topicText) + ' ▾') @@ -122,7 +116,6 @@ if Backbone? title = @$(".new-post-title").val() body = @$(".new-post-body").find(".wmd-input").val() tags = @$(".new-post-tags").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") @@ -144,7 +137,6 @@ if Backbone? 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 diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index c9bc9330dc..7ca00cb37c 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -91,29 +91,23 @@ def create_thread(request, course_id, commentable_id): 'user_id': request.user.id, }) - - user = cc.User.from_django_user(request.user) - #kevinchugh because the new requirement is that all groups will be determined - #by the group id in the request this all goes away - # Cohort the thread if the commentable is cohorted. - #if is_commentable_cohorted(course_id, commentable_id): - # user_group_id = get_cohort_id(user, course_id) + if is_commentable_cohorted(course_id, commentable_id): + user_group_id = get_cohort_id(request.user, course_id) # TODO (vshnayder): once we have more than just cohorts, we'll want to # change this to a single get_group_for_user_and_commentable function # that can do different things depending on the commentable_id - # if cached_has_permission(request.user, "see_all_cohorts", course_id): + if cached_has_permission(request.user, "see_all_cohorts", course_id): # admins can optionally choose what group to post as - # group_id = post.get('group_id', user_group_id) - # else: + group_id = post.get('group_id', user_group_id) + else: # regular users always post with their own id. - # group_id = user_group_id - if 'group_id' in post.keys(): - thread.update_attributes(group_id=post['group_id']) - + group_id = user_group_id + + thread.update_attributes(group_id=group_id) + thread.save() - print thread if post.get('auto_subscribe', 'false').lower() == 'true': user = cc.User.from_django_user(request.user) user.follow(thread) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index f7d34f6643..70d9f40fcf 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -11,12 +11,12 @@ 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 * +from course_groups.cohorts import get_cohort_id from courseware.access import has_access from urllib import urlencode from operator import methodcaller -from django_comment_client.permissions import check_permissions_by_view, cached_has_permission +from django_comment_client.permissions import check_permissions_by_view from django_comment_client.utils import (merge_dict, extract, strip_none, strip_blank, get_courseware_context) @@ -62,8 +62,7 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG #if the course-user is cohorted, then add the group id - group_id = get_cohort_id(request.user, course_id) - + group_id = get_cohort_id(user, course_id) if group_id: default_query_params["group_id"] = group_id @@ -74,16 +73,6 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG 'tags', 'commentable_ids']))) threads, page, num_pages = cc.Thread.search(query_params) - - - #now add the group name if the thread has a group id - for thread in threads: - if thread.get('group_id'): - thread['group_name'] = get_cohort_by_id(course_id, thread.get('group_id')).name - thread['group_string'] = "This post visible only to Group %s." % (thread['group_name']) - else: - thread['group_name'] = "" - thread['group_string'] = "This post visible to everyone." query_params['page'] = page query_params['num_pages'] = num_pages @@ -92,7 +81,6 @@ def get_threads(request, course_id, discussion_id=None, per_page=THREADS_PER_PAG def inline_discussion(request, course_id, discussion_id): - """ Renders JSON for DiscussionModules """ @@ -101,8 +89,7 @@ def inline_discussion(request, course_id, discussion_id): try: threads, query_params = get_threads(request, course_id, discussion_id, per_page=INLINE_THREADS_PER_PAGE) - user = cc.User.from_django_user(request.user) - user_info = user.to_dict() + user_info = cc.User.from_django_user(request.user).to_dict() except (cc.utils.CommentClientError, cc.utils.CommentClientUnknownError) as err: # TODO (vshnayder): since none of this code seems to be aware of the fact that # sometimes things go wrong, I suspect that the js client is also not @@ -114,39 +101,7 @@ def inline_discussion(request, course_id, discussion_id): allow_anonymous = course.metadata.get("allow_anonymous", True) allow_anonymous_to_peers = course.metadata.get("allow_anonymous_to_peers", False) - - #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) - cohorts_list = list() - - if is_cohorted: - - #if you're a mod, send all cohorts and let you pick - if cached_has_permission(request.user, "see_all_cohorts", course_id): - cohorts = get_course_cohorts(course_id) - for c in cohorts: - cohorts_list.append({'name':c.name, 'id':c.id}) - - else: - #otherwise, just make a dictionary of two - user_cohort = get_cohort(user, course_id) - if user_cohort: - user_cohort_name = user_cohort.name - user_cohort_id = user_cohort.id - else: - user_cohort_name = user_cohort_id = None - - - cohorts_list.append({'name':'All Groups','id':None}) - if user_cohort: - cohorts_list.append({'name':user_cohort_name, 'id':user_cohort_id}) - else: - cohorts_list = None - - return utils.JsonResponse({ 'discussion_data': map(utils.safe_content, threads), 'user_info': user_info, @@ -156,14 +111,11 @@ def inline_discussion(request, course_id, discussion_id): 'roles': utils.get_role_ids(course_id), 'allow_anonymous_to_peers': allow_anonymous_to_peers, 'allow_anonymous': allow_anonymous, - 'cohorts': cohorts_list, - 'is_cohorted': is_cohorted }) @login_required def forum_form_discussion(request, course_id): - """ Renders the main Discussion page, potentially filtered by a search query """ @@ -177,8 +129,7 @@ def forum_form_discussion(request, course_id): log.error("Error loading forum discussion threads: %s" % str(err)) raise Http404 - user = cc.User.from_django_user(request.user) - user_info = user.to_dict() + user_info = cc.User.from_django_user(request.user).to_dict() annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_info) @@ -203,10 +154,6 @@ def forum_form_discussion(request, course_id): #trending_tags = cc.search_trending_tags( # course_id, #) - cohorts = get_course_cohorts(course_id) - cohorted_commentables = get_cohorted_commentables(course_id) - - user_cohort_id = get_cohort_id(request.user, course_id) context = { 'csrf': csrf(request)['csrf_token'], @@ -221,11 +168,6 @@ def forum_form_discussion(request, course_id): 'course_id': course.id, 'category_map': category_map, 'roles': saxutils.escape(json.dumps(utils.get_role_ids(course_id)), escapedict), - '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) } # print "start rendering.." return render_to_response('discussion/index.html', context) @@ -233,6 +175,7 @@ def forum_form_discussion(request, course_id): @login_required def single_thread(request, course_id, discussion_id, thread_id): + course = get_course_with_access(request.user, course_id, 'load') cc_user = cc.User.from_django_user(request.user) user_info = cc_user.to_dict() @@ -246,7 +189,7 @@ def single_thread(request, course_id, discussion_id, thread_id): if request.is_ajax(): courseware_context = get_courseware_context(thread, course) - + annotated_content_info = utils.get_annotated_content_infos(course_id, thread, request.user, user_info=user_info) context = {'thread': thread.to_dict(), 'course_id': course_id} # TODO: Remove completely or switch back to server side rendering @@ -276,8 +219,6 @@ def single_thread(request, course_id, discussion_id, thread_id): courseware_context = get_courseware_context(thread, course) if courseware_context: thread.update(courseware_context) - if thread.get('group_id') and not thread.get('group_name'): - thread['group_name'] = get_cohort_by_id(course_id, thread.get('group_id')).name threads = [utils.safe_content(thread) for thread in threads] @@ -291,11 +232,8 @@ def single_thread(request, course_id, discussion_id, thread_id): # course_id, #) + annotated_content_info = utils.get_metadata_for_threads(course_id, threads, request.user, user_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, @@ -312,11 +250,6 @@ def single_thread(request, course_id, discussion_id, thread_id): 'category_map': category_map, 'roles': saxutils.escape(json.dumps(utils.get_role_ids(course_id)), escapedict), '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), - 'cohorts': cohorts, - 'user_cohort': get_cohort_id(request.user, course_id), - 'cohorted_commentables': cohorted_commentables } return render_to_response('discussion/single_thread.html', context) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 06f3492285..1f1a80e2b4 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -406,7 +406,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', 'group_id', 'group_name', 'group_string' + 'read', ] if (content.get('anonymous') is False) and (content.get('anonymous_to_peers') is False): diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index ca607d3ff3..912ae1af18 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -11,12 +11,12 @@ 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', 'group_id', 'group_name' + 'highlighted_body', 'endorsed', 'read', 'group_id' ] updatable_fields = [ 'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id', - 'closed', 'tags', 'user_id', 'commentable_id', 'group_id', 'group_name' + 'closed', 'tags', 'user_id', 'commentable_id', 'group_id' ] initializable_fields = updatable_fields diff --git a/lms/static/sass/_discussion.scss b/lms/static/sass/_discussion.scss index a914751280..809c968fe6 100644 --- a/lms/static/sass/_discussion.scss +++ b/lms/static/sass/_discussion.scss @@ -169,12 +169,6 @@ body.discussion { } } - .form-group-label { - display: block; - padding-top: 5px; - color:#fff; - } - .topic_dropdown_button { position: relative; z-index: 1000; @@ -187,7 +181,7 @@ body.discussion { .drop-arrow { float: right; color: #999; - line-height: 37px; + line-height: 36px; } } @@ -1026,18 +1020,6 @@ body.discussion { } } - .group-filter-label { - width: 40px; - margin-left:10px; - } - - .group-filter-select { - margin: 5px 0px 5px 5px; - width: 80px; - font-size:10px; - background: transparent; - border-color: #ccc; - } } .post-list-wrapper { @@ -1345,8 +1327,6 @@ body.discussion { margin-left: 40px; } - - .post-tools { @include clearfix; margin-top: 15px; @@ -1377,8 +1357,6 @@ body.discussion { margin-bottom: 20px; } - - .responses { list-style: none; margin-top: 40px; @@ -2434,11 +2412,3 @@ body.discussion { .discussion-user-threads { @extend .discussion-module } - - -.group-visibility-label { - font-size: 12px; - color:#000; - font-style: italic; - background-color:#fff; - } \ No newline at end of file diff --git a/lms/templates/discussion/_filter_dropdown.html b/lms/templates/discussion/_filter_dropdown.html index 8272fdd062..484ee05101 100644 --- a/lms/templates/discussion/_filter_dropdown.html +++ b/lms/templates/discussion/_filter_dropdown.html @@ -30,7 +30,7 @@