Fix Python unit tests for Discussion

common/lib/xmodule/xmodule/x_module.py

@@ -1613,7 +1613,7 @@ class DiscussionService(object):
         request.user = user
         user_info = cc.User.from_django_user(self.runtime.user).to_dict()
         course_id = self.runtime.course_id
-        course = get_course_with_access(self.runtime.user, 'load_forum', course_id)
+        course = get_course_with_access(self.runtime.user, 'load', course_id)
         user_cohort_id = get_cohort_id(user, course_id)
 
         unsafethreads, query_params = get_threads(request, course_id)
@@ -1674,7 +1674,7 @@ class DiscussionService(object):
         course_id = self.runtime.course_id
         user = self.runtime.user
 
-        course = get_course_with_access(user, 'load_forum', course_id)
+        course = get_course_with_access(user, 'load', course_id)
         category_map = get_discussion_category_map(course, user)
 
         is_moderator = has_permission(user, "see_all_cohorts", course_id)

lms/djangoapps/api_manager/users/views.py

@@ -18,7 +18,7 @@ from rest_framework.response import Response
 
 from courseware import grades, module_render
 from courseware.model_data import FieldDataCache
-from openedx.core.djangoapps.course_groups.models import CourseUserGroup
+from openedx.core.djangoapps.course_groups.models import CourseUserGroup, CourseCohort
 from openedx.core.djangoapps.course_groups.cohorts import (
     get_cohort_by_name,
     add_cohort,
@@ -746,7 +746,7 @@ class UsersCoursesList(SecureAPIView):
         try:
             default_cohort = get_cohort_by_name(course_key, CourseUserGroup.default_cohort_name)
         except CourseUserGroup.DoesNotExist:
-            default_cohort = add_cohort(course_key, CourseUserGroup.default_cohort_name)
+            default_cohort = add_cohort(course_key, CourseUserGroup.default_cohort_name, CourseCohort.RANDOM)
         add_user_to_cohort(default_cohort, user.username)
         log.debug('User "{}" has been automatically added in cohort "{}" for course "{}"'.format(
             user.username, default_cohort.name, course_descriptor.display_name)

lms/djangoapps/django_comment_client/base/tests.py

@@ -14,10 +14,11 @@ from django_comment_client.base import views
 from django_comment_client.tests.group_id import (
     CohortedTopicGroupIdTestMixin, NonCohortedTopicGroupIdTestMixin, GroupIdAssertionMixin
 )
-from django_comment_client.tests.utils import CohortedContentTestCase
+from django_comment_client.tests.utils import CohortedTestCase
 from django_comment_client.tests.unicode import UnicodeTestMixin
 from django_comment_common.models import Role
 from django_comment_common.utils import seed_permissions_roles
+from openedx.core.djangoapps.course_groups.models import CourseCohort
 from student.tests.factories import CourseEnrollmentFactory, UserFactory, CourseAccessRoleFactory
 from util.testing import UrlResetMixin
 from xmodule.modulestore.tests.factories import CourseFactory
@@ -49,7 +50,7 @@ class MockRequestSetupMixin(object):
 @patch('lms.lib.comment_client.utils.requests.request')
 class CreateThreadGroupIdTestCase(
         MockRequestSetupMixin,
-        CohortedContentTestCase,
+        CohortedTestCase,
         CohortedTopicGroupIdTestMixin,
         NonCohortedTopicGroupIdTestMixin
 ):
@@ -84,7 +85,7 @@ class CreateThreadGroupIdTestCase(
 @patch('lms.lib.comment_client.utils.requests.request')
 class ThreadActionGroupIdTestCase(
         MockRequestSetupMixin,
-        CohortedContentTestCase,
+        CohortedTestCase,
         GroupIdAssertionMixin
 ):
     def call_view(
@@ -171,7 +172,7 @@ class ThreadActionGroupIdTestCase(
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class CreateCohortedThreadTestCase(CohortedContentTestCase):
+class CreateCohortedThreadTestCase(CohortedTestCase):
     """
     Tests how `views.create_thread` passes `group_id` to the comments service
     for cohorted topics.
@@ -204,7 +205,7 @@ class CreateCohortedThreadTestCase(CohortedContentTestCase):
 
     def test_moderator_without_group_id(self, mock_request):
         self._create_thread_in_cohorted_topic(self.moderator, mock_request, None, pass_group_id=False)
-        self._assert_mock_request_called_with_group_id(mock_request, self.moderator_cohort.id)
+        self._assert_mock_request_called_without_group_id(mock_request)
 
     def test_moderator_none_group_id(self, mock_request):
         self._create_thread_in_cohorted_topic(self.moderator, mock_request, None, expected_status_code=400)
@@ -225,7 +226,7 @@ class CreateCohortedThreadTestCase(CohortedContentTestCase):
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class CreateNonCohortedThreadTestCase(CohortedContentTestCase):
+class CreateNonCohortedThreadTestCase(CohortedTestCase):
     """
     Tests how `views.create_thread` passes `group_id` to the comments service
     for non-cohorted topics.
@@ -502,8 +503,8 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
             assert_equal(get_notifications_count_for_user(self.student.id), 1)
 
         # create cohorts
-        groupA = add_cohort(self.course.id, "CohortA")
-        groupB = add_cohort(self.course.id, "CohortB")
+        groupA = add_cohort(self.course.id, "CohortA", CourseCohort.RANDOM)
+        groupB = add_cohort(self.course.id, "CohortB", CourseCohort.RANDOM)
 
         add_user_to_cohort(groupA, self.student.username)
 
@@ -514,7 +515,6 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
         CourseEnrollmentFactory(user=a_user, course_id=self.course_id)
         add_user_to_cohort(groupA, a_user.username)
 
-
         b_user = User.objects.create_user('cohortB', 'cohortBemail', 'test')
         b_user.is_active = True
         b_user.save()

lms/djangoapps/django_comment_client/base/views.py

@@ -219,33 +219,6 @@ def create_thread(request, course_id, commentable_id):
     if group_id is not None:
         thread.group_id = group_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
-    #not anymore, only for admins
-
-    # Cohort the thread if the commentable is cohorted.
-    if is_commentable_cohorted(course_key, commentable_id):
-        user_group_id = get_cohort_id(user, course_key)
-
-        # 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 has_permission(request.user, "see_all_cohorts", course_key):
-            # admins can optionally choose what group to post as
-            try:
-                group_id = int(post.get('group_id', user_group_id))
-                get_cohort_by_id(course_key, group_id)
-            except (ValueError, CourseUserGroup.DoesNotExist):
-                return HttpResponseBadRequest("Invalid cohort id")
-        else:
-            # regular users always post with their own id.
-            group_id = user_group_id
-
-        if group_id:
-            thread.group_id = group_id
-
     thread.save()
 
     # patch for backward compatibility to comments service
@@ -288,7 +261,6 @@ def create_thread(request, course_id, commentable_id):
     # rescore this user
     _update_user_engagement_score(course_key, request.user.id)
 
-    add_courseware_context([data], course, user)
     add_thread_group_name(data, course_key)
     if thread.get('group_id') and not thread.get('group_name'):
         thread['group_name'] = get_cohort_by_id(course_key, thread.get('group_id')).name
@@ -412,7 +384,7 @@ def update_thread(request, course_id, thread_id):
         thread.thread_type = request.POST["thread_type"]
     if "commentable_id" in request.POST:
         course = get_course_with_access(request.user, 'load', course_key)
-        commentable_ids = get_discussion_categories_ids(course)
+        commentable_ids = get_discussion_categories_ids(course, request.user)
         if request.POST.get("commentable_id") in commentable_ids:
             thread.commentable_id = request.POST["commentable_id"]
         else:

lms/djangoapps/django_comment_client/forum/views.py

@@ -25,7 +25,6 @@ from openedx.core.djangoapps.course_groups.cohorts import (
     get_cohort_id,
     get_course_cohorts,
     is_commentable_cohorted,
-    get_cohorted_threads_privacy,
     get_cohorted_commentables,
     get_cohort_by_id
 )
@@ -148,7 +147,7 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
             group_id = None
     else:
         group_id = get_cohort_id(request.user, course.id)
-        if not group_id and get_cohorted_threads_privacy(course.id) == 'cohort-only':
+        if not group_id:
             default_query_params['exclude_groups'] = True
 
     if group_id:
@@ -377,11 +376,11 @@ def single_thread(request, course_key, discussion_id, thread_id):
     is_staff = has_permission(request.user, 'openclose_thread', course.id)
     if request.is_ajax():
         with newrelic.agent.FunctionTrace(nr_transaction, "get_annotated_content_infos"):
-            annotated_content_info = utils.get_annotated_content_infos(course_key, thread, request.user, user_info=user_info)
+            annotated_content_info = utils.get_annotated_content_infos(
+                course_key, thread, request.user, user_info=user_info
+            )
         content = utils.prepare_content(thread.to_dict(), course_key, is_staff)
         add_thread_group_name(content, course_key)
-        with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
-            add_courseware_context([content], course, request.user)
         return utils.JsonResponse({
             'content': content,
             'annotated_content_info': annotated_content_info,

lms/djangoapps/django_comment_client/tests/utils.py

@@ -15,37 +15,31 @@ from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 
 
-class CohortedContentTestCase(ModuleStoreTestCase):
+class CohortedTestCase(ModuleStoreTestCase):
     """
     Sets up a course with a student, a moderator and their cohorts.
     """
     @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
     def setUp(self):
-        super(CohortedContentTestCase, self).setUp()
+        super(CohortedTestCase, self).setUp()
 
         self.course = CourseFactory.create(
-            discussion_topics={
-                "cohorted topic": {"id": "cohorted_topic"},
-                "non-cohorted topic": {"id": "non_cohorted_topic"},
-            },
             cohort_config={
                 "cohorted": True,
                 "cohorted_discussions": ["cohorted_topic"]
             }
         )
-        self.course.discussion_topics["cohorted topic"] = {"id": "cohorted_topic"}
-        self.course.discussion_topics["non-cohorted topic"] = {"id": "non_cohorted_topic"}
-        self.store.update_item(self.course, self.user.id)
-        self.student_cohort = CourseUserGroup.objects.create(
+        self.student_cohort = CohortFactory.create(
             name="student_cohort",
-            course_id=self.course.id,
-            group_type=CourseUserGroup.COHORT
+            course_id=self.course.id
         )
-        self.moderator_cohort = CourseUserGroup.objects.create(
+        self.moderator_cohort = CohortFactory.create(
             name="moderator_cohort",
-            course_id=self.course.id,
-            group_type=CourseUserGroup.COHORT
+            course_id=self.course.id
         )
+        self.course.discussion_topics["cohorted topic"] = {"id": "cohorted_topic"}
+        self.course.discussion_topics["non-cohorted topic"] = {"id": "non_cohorted_topic"}
+        self.store.update_item(self.course, self.user.id)
 
         seed_permissions_roles(self.course.id)
         self.student = UserFactory.create()

lms/djangoapps/django_comment_client/utils.py

@@ -78,7 +78,6 @@ def get_accessible_discussion_modules(course, user, include_all=False):  # pylin
                 log.warning("Required key '%s' not in discussion %s, leaving out of category map" % (key, module.location))
                 return False
         return True
-
     return [
         module for module in all_modules
         if has_required_keys(module) and (include_all or has_access(user, 'load', module, course.id))

openedx/core/djangoapps/course_groups/cohorts.py

@@ -326,16 +326,6 @@ def get_cohort_by_id(course_key, cohort_id):
     )
 
 
-def get_cohorted_threads_privacy(course_key):
-    """
-    Given a course key, return the cohorted threads privacy setting.
-
-    Raises:
-       Http404 if the course doesn't exist.
-    """
-    return courses.get_course_by_id(course_key).cohorted_threads_privacy
-
-
 def add_cohort(course_key, name, assignment_type):
     """
     Add a cohort to a course.  Raises ValueError if a cohort of the same name already

openedx/core/djangoapps/course_groups/management/commands/revert_workgroup.py

@@ -9,7 +9,7 @@ from django.contrib.auth.models import User
 
 from xmodule.modulestore.django import modulestore
 from xmodule.course_module import CourseDescriptor
-from openedx.core.djangoapps.course_groups.models import CourseUserGroup
+from openedx.core.djangoapps.course_groups.models import CourseUserGroup, CourseCohort
 from openedx.core.djangoapps.course_groups.cohorts import (
     get_cohort_by_name,
     add_cohort,
@@ -76,7 +76,9 @@ class Command(BaseCommand):
                         try:
                             default_cohort = get_cohort_by_name(course.id, CourseUserGroup.default_cohort_name)
                         except CourseUserGroup.DoesNotExist:
-                            default_cohort = add_cohort(course.id, CourseUserGroup.default_cohort_name)
+                            default_cohort = add_cohort(
+                                course.id, CourseUserGroup.default_cohort_name, CourseCohort.RANDOM
+                            )
                             self.stdout.write(
                                 'Default cohort "{}" created for course "{}"'.format(
                                     default_cohort.name, course.display_name
@@ -108,8 +110,11 @@ class Command(BaseCommand):
                                     user.username, cohort.name, course.display_name
                                 )
                             )
-                        self.stdout.write("User '{}' is now only in cohort '{}' in course '{}'.\n".format(
-                            user.username, cohort.name, course.display_name)
+                        self.stdout.write(
+                            "User '{}' is now only in cohort '{}' in course '{}'.\n".format(
+                                # pylint: disable=undefined-loop-variable
+                                user.username, cohort.name, course.display_name
+                            )
                         )
                     else:
                         error = True

openedx/core/djangoapps/course_groups/tests/test_scope_resolver.py

@@ -16,6 +16,7 @@ class ScopeResolverTests(TestCase):
 
     def setUp(self):
         """Creates cohorts for testing"""
+        super(ScopeResolverTests, self).setUp()
         self.course_id = 'foo/bar/baz'
         self.cohort1_users = [UserFactory.create() for _ in range(3)]
         self.cohort2_users = [UserFactory.create() for _ in range(2)]