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..482a35e34d 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,11 @@ class DiscussionTabSingleThreadTest(UniqueCourseTest): self.thread_page = DiscussionTabSingleThreadPage(self.browser, self.course_id, thread_id) # pylint:disable=W0201 self.thread_page.visit() + # pylint:disable=W0613 + 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 +123,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..8539978779 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,19 +9,19 @@ 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 from edxmako import lookup_template import pystache_custom as pystache -from course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted +from course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted, is_course_cohorted from course_groups.models import CourseUserGroup -from xmodule.modulestore.django import modulestore -from django.utils.timezone import UTC 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__) @@ -425,7 +426,7 @@ def prepare_content(content, course_key, is_staff=False): # Only reveal endorser if requester can see author or if endorser is staff if ( endorser and - ("username" in fields or cached_has_permission(endorser, "endorse_comment", course_id)) + ("username" in fields or cached_has_permission(endorser, "endorse_comment", course_key)) ): endorsement["username"] = endorser.username else: @@ -438,9 +439,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 is_course_cohorted(course_key): + # 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