From 6114cf05f4a3e6e7111b2d44d41e894906ecc87d Mon Sep 17 00:00:00 2001 From: Andy Armstrong Date: Mon, 6 Oct 2014 16:59:05 -0400 Subject: [PATCH] Don't show cohort information when disabled TNL-552 --- .../acceptance/tests/discussion/helpers.py | 15 +++++++++++++- .../tests/discussion/test_cohorts.py | 18 ++++++++++++++++- lms/djangoapps/django_comment_client/utils.py | 20 +++++++++++++------ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/common/test/acceptance/tests/discussion/helpers.py b/common/test/acceptance/tests/discussion/helpers.py index e8aad4a58e..b38a9a36a0 100644 --- a/common/test/acceptance/tests/discussion/helpers.py +++ b/common/test/acceptance/tests/discussion/helpers.py @@ -30,6 +30,7 @@ class BaseDiscussionMixin(object): thread_fixture.addResponse(Response(id=str(i), body=str(i))) thread_fixture.push() self.setup_thread_page(thread_id) + return thread_id class CohortTestMixin(object): @@ -46,7 +47,19 @@ class CohortTestMixin(object): u"cohort_config": { "auto_cohort_groups": auto_cohort_groups or [], "cohorted_discussions": [], - "cohorted": True + "cohorted": True, + }, + }, + }) + + def disable_cohorting(self, course_fixture): + """ + Disables cohorting for the current course fixture. + """ + course_fixture._update_xblock(course_fixture._course_location, { + "metadata": { + u"cohort_config": { + "cohorted": False }, }, }) diff --git a/common/test/acceptance/tests/discussion/test_cohorts.py b/common/test/acceptance/tests/discussion/test_cohorts.py index dce9e50994..8ca505c52b 100644 --- a/common/test/acceptance/tests/discussion/test_cohorts.py +++ b/common/test/acceptance/tests/discussion/test_cohorts.py @@ -45,12 +45,17 @@ class CohortedDiscussionTestMixin(BaseDiscussionMixin, CohortTestMixin): def test_cohort_visibility_label(self): # Must be moderator to view content in a cohort other than your own AutoAuthPage(self.browser, course_id=self.course_id, roles="Moderator").visit() - self.setup_thread(1, group_id=self.cohort_1_id) + self.thread_id = self.setup_thread(1, group_id=self.cohort_1_id) self.assertEquals( self.thread_page.get_group_visibility_label(), "This post is visible only to {}.".format(self.cohort_1_name) ) + # Disable cohorts and verify that the post now shows as visible to everyone. + self.disable_cohorting(self.course_fixture) + self.refresh_thread_page(self.thread_id) + self.assertEquals(self.thread_page.get_group_visibility_label(), "This post is visible to everyone.") + class DiscussionTabSingleThreadTest(UniqueCourseTest): """ @@ -68,6 +73,10 @@ class DiscussionTabSingleThreadTest(UniqueCourseTest): self.thread_page = DiscussionTabSingleThreadPage(self.browser, self.course_id, thread_id) # pylint:disable=W0201 self.thread_page.visit() + def refresh_thread_page(self, thread_id): + self.browser.refresh() + self.thread_page.wait_for_page() + @attr('shard_1') class CohortedDiscussionTabSingleThreadTest(DiscussionTabSingleThreadTest, CohortedDiscussionTestMixin): @@ -113,12 +122,19 @@ class InlineDiscussionTest(UniqueCourseTest): def setup_thread_page(self, thread_id): CoursewarePage(self.browser, self.course_id).visit() + self.show_thread(thread_id) + + def show_thread(self, thread_id): discussion_page = InlineDiscussionPage(self.browser, self.discussion_id) discussion_page.expand_discussion() self.assertEqual(discussion_page.get_num_displayed_threads(), 1) self.thread_page = InlineDiscussionThreadPage(self.browser, thread_id) # pylint:disable=W0201 self.thread_page.expand() + def refresh_thread_page(self, thread_id): + self.browser.refresh() + self.show_thread(thread_id) + @attr('shard_1') class CohortedInlineDiscussionTest(InlineDiscussionTest, CohortedDiscussionTestMixin): diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index f88fc3d82c..f0f91739c8 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -1,3 +1,4 @@ +import json import pytz from collections import defaultdict import logging @@ -8,6 +9,8 @@ from django.core.urlresolvers import reverse from django.db import connection from django.http import HttpResponse from django.utils import simplejson +from django.utils.timezone import UTC + from django_comment_common.models import Role, FORUM_ROLE_STUDENT from django_comment_client.permissions import check_permissions_by_view, cached_has_permission @@ -16,11 +19,10 @@ import pystache_custom as pystache from course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted from course_groups.models import CourseUserGroup -from xmodule.modulestore.django import modulestore -from django.utils.timezone import UTC +from courseware.courses import get_course_by_id from opaque_keys.edx.locations import i4xEncoder from opaque_keys.edx.keys import CourseKey -import json +from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) @@ -393,6 +395,8 @@ def prepare_content(content, course_key, is_staff=False): @TODO: not all response pre-processing steps are currently integrated into this function. """ + course = get_course_by_id(course_key) + fields = [ 'id', 'title', 'body', 'course_id', 'anonymous', 'anonymous_to_peers', 'endorsed', 'parent_id', 'thread_id', 'votes', 'closed', 'created_at', @@ -438,9 +442,13 @@ def prepare_content(content, course_key, is_staff=False): ] content[child_content_key] = children - # Augment the specified thread info to include the group name if a group id is present. - if content.get('group_id') is not None: - content['group_name'] = get_cohort_by_id(course_key, content.get('group_id')).name + if course.is_cohorted: + # Augment the specified thread info to include the group name if a group id is present. + if content.get('group_id') is not None: + content['group_name'] = get_cohort_by_id(course_key, content.get('group_id')).name + else: + # Remove any cohort information that might remain if the course had previously been cohorted. + content.pop('group_id', None) return content