diff --git a/common/djangoapps/django_comment_common/models.py b/common/djangoapps/django_comment_common/models.py index 6e8ede6fd4..e47feb6e2a 100644 --- a/common/djangoapps/django_comment_common/models.py +++ b/common/djangoapps/django_comment_common/models.py @@ -16,6 +16,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError FORUM_ROLE_ADMINISTRATOR = ugettext_noop('Administrator') FORUM_ROLE_MODERATOR = ugettext_noop('Moderator') +FORUM_ROLE_GROUP_MODERATOR = ugettext_noop('Group Moderator') FORUM_ROLE_COMMUNITY_TA = ugettext_noop('Community TA') FORUM_ROLE_STUDENT = ugettext_noop('Student') diff --git a/common/djangoapps/django_comment_common/tests.py b/common/djangoapps/django_comment_common/tests.py index 482e23dce2..d988d6ab20 100644 --- a/common/djangoapps/django_comment_common/tests.py +++ b/common/djangoapps/django_comment_common/tests.py @@ -1,9 +1,9 @@ -from nose.plugins.attrib import attr - from django.test import TestCase +from nose.plugins.attrib import attr +from opaque_keys.edx.locations import SlashSeparatedCourseKey + from django_comment_common.models import Role from models import CourseDiscussionSettings -from opaque_keys.edx.locations import SlashSeparatedCourseKey from openedx.core.djangoapps.course_groups.cohorts import CourseCohortsSettings from student.models import CourseEnrollment, User from utils import get_course_discussion_settings, set_course_discussion_settings diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py index 7c6699da88..8a06ec0514 100644 --- a/common/djangoapps/django_comment_common/utils.py +++ b/common/djangoapps/django_comment_common/utils.py @@ -5,6 +5,7 @@ Common comment client utility functions. from django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_STUDENT, Role @@ -28,6 +29,9 @@ STUDENT_ROLE_PERMISSIONS = ["vote", "update_thread", "follow_thread", "unfollow_ MODERATOR_ROLE_PERMISSIONS = ["edit_content", "delete_thread", "openclose_thread", "endorse_comment", "delete_comment", "see_all_cohorts"] +GROUP_MODERATOR_ROLE_PERMISSIONS = ["group_edit_content", "group_delete_thread", "group_openclose_thread", + "group_endorse_comment", "group_delete_comment"] + ADMINISTRATOR_ROLE_PERMISSIONS = ["manage_moderator"] @@ -50,6 +54,7 @@ def seed_permissions_roles(course_key): """ administrator_role = _save_forum_role(course_key, FORUM_ROLE_ADMINISTRATOR) moderator_role = _save_forum_role(course_key, FORUM_ROLE_MODERATOR) + group_moderator_role = _save_forum_role(course_key, FORUM_ROLE_GROUP_MODERATOR) community_ta_role = _save_forum_role(course_key, FORUM_ROLE_COMMUNITY_TA) student_role = _save_forum_role(course_key, FORUM_ROLE_STUDENT) @@ -59,11 +64,14 @@ def seed_permissions_roles(course_key): for per in MODERATOR_ROLE_PERMISSIONS: moderator_role.add_permission(per) + for per in GROUP_MODERATOR_ROLE_PERMISSIONS: + group_moderator_role.add_permission(per) + for per in ADMINISTRATOR_ROLE_PERMISSIONS: administrator_role.add_permission(per) moderator_role.inherit_permissions(student_role) - + group_moderator_role.inherit_permissions(student_role) # For now, Community TA == Moderator, except for the styling. community_ta_role.inherit_permissions(moderator_role) @@ -78,6 +86,7 @@ def are_permissions_roles_seeded(course_id): try: administrator_role = Role.objects.get(name=FORUM_ROLE_ADMINISTRATOR, course_id=course_id) moderator_role = Role.objects.get(name=FORUM_ROLE_MODERATOR, course_id=course_id) + group_moderator_role = Role.objects.get(name=FORUM_ROLE_GROUP_MODERATOR, course_id=course_id) student_role = Role.objects.get(name=FORUM_ROLE_STUDENT, course_id=course_id) except: return False @@ -90,6 +99,10 @@ def are_permissions_roles_seeded(course_id): if not moderator_role.has_permission(per): return False + for per in GROUP_MODERATOR_ROLE_PERMISSIONS + STUDENT_ROLE_PERMISSIONS: + if not group_moderator_role.has_permission(per): + return False + for per in ADMINISTRATOR_ROLE_PERMISSIONS + MODERATOR_ROLE_PERMISSIONS + STUDENT_ROLE_PERMISSIONS: if not administrator_role.has_permission(per): return False diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index 723066cd9a..d75cb11dd8 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -139,7 +139,7 @@ class AutoAuthEnabledTestCase(AutoAuthTestCase): def test_set_roles(self, course_id, course_key): seed_permissions_roles(course_key) course_roles = dict((r.name, r) for r in Role.objects.filter(course_id=course_key)) - self.assertEqual(len(course_roles), 4) # sanity check + self.assertEqual(len(course_roles), 5) # sanity check # Student role is assigned by default on course enrollment. self._auto_auth({'username': 'a_student', 'course_id': course_id}) diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 1b690fbbf8..e039059ff7 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/lms/djangoapps/django_comment_client/base/tests.py @@ -15,6 +15,8 @@ from nose.tools import assert_equal, assert_true from opaque_keys.edx.keys import CourseKey from common.test.utils import MockSignalHandlerMixin, disable_signal +from course_modes.models import CourseMode +from course_modes.tests.factories import CourseModeFactory from django_comment_client.base import views from django_comment_client.tests.group_id import ( CohortedTopicGroupIdTestMixin, @@ -23,10 +25,12 @@ from django_comment_client.tests.group_id import ( ) from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_client.tests.utils import CohortedTestCase, ForumsEnableMixin -from django_comment_common.models import Role -from django_comment_common.utils import ThreadContext, seed_permissions_roles +from django_comment_common.models import CourseDiscussionSettings, Role, assign_role +from django_comment_common.utils import ThreadContext, seed_permissions_roles, set_course_discussion_settings from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory from lms.lib.comment_client import Thread +from openedx.core.djangoapps.course_groups.cohorts import set_course_cohorted +from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from student.tests.factories import CourseAccessRoleFactory, CourseEnrollmentFactory, UserFactory from util.testing import UrlResetMixin from xmodule.modulestore import ModuleStoreEnum @@ -380,7 +384,7 @@ class ViewsQueryCountTestCase( @ddt.data( (ModuleStoreEnum.Type.mongo, 3, 4, 31), - (ModuleStoreEnum.Type.split, 3, 13, 31), + (ModuleStoreEnum.Type.split, 3, 12, 31), ) @ddt.unpack @count_queries @@ -389,7 +393,7 @@ class ViewsQueryCountTestCase( @ddt.data( (ModuleStoreEnum.Type.mongo, 3, 3, 27), - (ModuleStoreEnum.Type.split, 3, 10, 27), + (ModuleStoreEnum.Type.split, 3, 9, 27), ) @ddt.unpack @count_queries @@ -1380,9 +1384,26 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto # Non-team commentables can be edited by any student. ('student_not_in_team', 'course_commentable_id', 200), # Moderators can always operator on threads within a team, regardless of team membership. - ('moderator', 'team_commentable_id', 200) + ('moderator', 'team_commentable_id', 200), + # Group moderators have regular student privileges for creating a thread and commenting + ('group_moderator', 'course_commentable_id', 200) ] + def change_divided_discussion_settings(self, scheme): + """ + Change divided discussion settings for the current course. + If dividing by cohorts, create and assign users to a cohort. + """ + enable_cohorts = True if scheme is CourseDiscussionSettings.COHORT else False + set_course_discussion_settings( + self.course.id, + enable_cohorts=enable_cohorts, + divided_discussions=[], + always_divide_inline_discussions=True, + division_scheme=scheme, + ) + set_course_cohorted(self.course.id, enable_cohorts) + @classmethod def setUpClass(cls): # pylint: disable=super-method-not-called @@ -1395,20 +1416,45 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto @classmethod def setUpTestData(cls): super(TeamsPermissionsTestCase, cls).setUpTestData() - + cls.course = CourseFactory.create() cls.password = "test password" seed_permissions_roles(cls.course.id) - # Create 3 users-- student in team, student not in team, discussion moderator - cls.student_in_team = UserFactory.create(password=cls.password) - cls.student_not_in_team = UserFactory.create(password=cls.password) - cls.moderator = UserFactory.create(password=cls.password) - CourseEnrollmentFactory(user=cls.student_in_team, course_id=cls.course.id) - CourseEnrollmentFactory(user=cls.student_not_in_team, course_id=cls.course.id) - CourseEnrollmentFactory(user=cls.moderator, course_id=cls.course.id) - cls.moderator.roles.add(Role.objects.get(name="Moderator", course_id=cls.course.id)) + # Create enrollment tracks + CourseModeFactory.create( + course_id=cls.course.id, + mode_slug=CourseMode.VERIFIED + ) + CourseModeFactory.create( + course_id=cls.course.id, + mode_slug=CourseMode.AUDIT + ) - # Create a team. + # Create 6 users-- + # student in team (in the team, audit) + # student not in team (not in the team, audit) + # cohorted (in the cohort, audit) + # verified (not in the cohort, verified) + # moderator (in the cohort, audit, moderator permissions) + # group moderator (in the cohort, verified, group moderator permissions) + def create_users_and_enroll(coursemode): + student = UserFactory.create(password=cls.password) + CourseEnrollmentFactory( + course_id=cls.course.id, + user=student, + mode=coursemode + ) + return student + + cls.student_in_team, cls.student_not_in_team, cls.moderator, cls.cohorted = ( + [create_users_and_enroll(CourseMode.AUDIT) for _ in range(4)]) + cls.verified, cls.group_moderator = [create_users_and_enroll(CourseMode.VERIFIED) for _ in range(2)] + + # Give moderator and group moderator permissions + cls.moderator.roles.add(Role.objects.get(name="Moderator", course_id=cls.course.id)) + assign_role(cls.course.id, cls.group_moderator, 'Group Moderator') + + # Create a team cls.team_commentable_id = "team_discussion_id" cls.team = CourseTeamFactory.create( name=u'The Only Team', @@ -1416,12 +1462,18 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto topic_id='topic_id', discussion_topic_id=cls.team_commentable_id ) - - cls.team.add_user(cls.student_in_team) + CourseTeamMembershipFactory.create(team=cls.team, user=cls.student_in_team) # Dummy commentable ID not linked to a team cls.course_commentable_id = "course_level_commentable" + # Create cohort and add students to it + CohortFactory( + course_id=cls.course.id, + name='Test Cohort', + users=[cls.group_moderator, cls.cohorted] + ) + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) def setUp(self): super(TeamsPermissionsTestCase, self).setUp() @@ -1433,30 +1485,43 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto @ddt.data( # student_in_team will be able to update his own post, regardless of team membership - ('student_in_team', 'student_in_team', 'team_commentable_id', 200), - ('student_in_team', 'student_in_team', 'course_commentable_id', 200), + ('student_in_team', 'student_in_team', 'team_commentable_id', 200, CourseDiscussionSettings.NONE), + ('student_in_team', 'student_in_team', 'course_commentable_id', 200, CourseDiscussionSettings.NONE), # students can only update their own posts - ('student_in_team', 'moderator', 'team_commentable_id', 401), + ('student_in_team', 'moderator', 'team_commentable_id', 401, CourseDiscussionSettings.NONE), # Even though student_not_in_team is not in the team, he can still modify posts he created while in the team. - ('student_not_in_team', 'student_not_in_team', 'team_commentable_id', 200), + ('student_not_in_team', 'student_not_in_team', 'team_commentable_id', 200, CourseDiscussionSettings.NONE), # Moderators can change their own posts and other people's posts. - ('moderator', 'moderator', 'team_commentable_id', 200), - ('moderator', 'student_in_team', 'team_commentable_id', 200), + ('moderator', 'moderator', 'team_commentable_id', 200, CourseDiscussionSettings.NONE), + ('moderator', 'student_in_team', 'team_commentable_id', 200, CourseDiscussionSettings.NONE), + # Group moderator can do operations on commentables within their group if the course is divided + ('group_moderator', 'verified', 'course_commentable_id', 200, CourseDiscussionSettings.ENROLLMENT_TRACK), + ('group_moderator', 'cohorted', 'course_commentable_id', 200, CourseDiscussionSettings.COHORT), + # Group moderators cannot do operations on commentables outside of their group + ('group_moderator', 'verified', 'course_commentable_id', 401, CourseDiscussionSettings.COHORT), + ('group_moderator', 'cohorted', 'course_commentable_id', 401, CourseDiscussionSettings.ENROLLMENT_TRACK), + # Group moderators cannot do operations when the course is not divided + ('group_moderator', 'verified', 'course_commentable_id', 401, CourseDiscussionSettings.NONE), + ('group_moderator', 'cohorted', 'course_commentable_id', 401, CourseDiscussionSettings.NONE) ) @ddt.unpack - def test_update_thread(self, user, thread_author, commentable_id, status_code, mock_request): + def test_update_thread(self, user, thread_author, commentable_id, status_code, division_scheme, mock_request): """ Verify that update_thread is limited to thread authors and privileged users (team membership does not matter). """ + self.change_divided_discussion_settings(division_scheme) commentable_id = getattr(self, commentable_id) # thread_author is who is marked as the author of the thread being updated. thread_author = getattr(self, thread_author) + self._setup_mock( user, mock_request, # user is the person making the request. { "user_id": str(thread_author.id), "closed": False, "commentable_id": commentable_id, - "context": "standalone" + "context": "standalone", + "username": thread_author.username, + "course_id": unicode(self.course.id) } ) response = self.client.post( @@ -1473,22 +1538,34 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto @ddt.data( # Students can delete their own posts - ('student_in_team', 'student_in_team', 'team_commentable_id', 200), + ('student_in_team', 'student_in_team', 'team_commentable_id', 200, CourseDiscussionSettings.NONE), # Moderators can delete any post - ('moderator', 'student_in_team', 'team_commentable_id', 200), + ('moderator', 'student_in_team', 'team_commentable_id', 200, CourseDiscussionSettings.NONE), # Others cannot delete posts - ('student_in_team', 'moderator', 'team_commentable_id', 401), - ('student_not_in_team', 'student_in_team', 'team_commentable_id', 401) + ('student_in_team', 'moderator', 'team_commentable_id', 401, CourseDiscussionSettings.NONE), + ('student_not_in_team', 'student_in_team', 'team_commentable_id', 401, CourseDiscussionSettings.NONE), + # Group moderator can do operations on commentables within their group if the course is divided + ('group_moderator', 'verified', 'team_commentable_id', 200, CourseDiscussionSettings.ENROLLMENT_TRACK), + ('group_moderator', 'cohorted', 'team_commentable_id', 200, CourseDiscussionSettings.COHORT), + # Group moderators cannot do operations on commentables outside of their group + ('group_moderator', 'verified', 'team_commentable_id', 401, CourseDiscussionSettings.COHORT), + ('group_moderator', 'cohorted', 'team_commentable_id', 401, CourseDiscussionSettings.ENROLLMENT_TRACK), + # Group moderators cannot do operations when the course is not divided + ('group_moderator', 'verified', 'team_commentable_id', 401, CourseDiscussionSettings.NONE), + ('group_moderator', 'cohorted', 'team_commentable_id', 401, CourseDiscussionSettings.NONE) ) @ddt.unpack - def test_delete_comment(self, user, comment_author, commentable_id, status_code, mock_request): + def test_delete_comment(self, user, comment_author, commentable_id, status_code, division_scheme, mock_request): commentable_id = getattr(self, commentable_id) comment_author = getattr(self, comment_author) + self.change_divided_discussion_settings(division_scheme) self._setup_mock(user, mock_request, { "closed": False, "commentable_id": commentable_id, - "user_id": str(comment_author.id) + "user_id": str(comment_author.id), + "username": comment_author.username, + "course_id": unicode(self.course.id) }) response = self.client.post( diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 001c358687..a7ff804d95 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -28,6 +28,8 @@ from django_comment_client.utils import ( get_annotated_content_info, get_cached_discussion_id_map, get_group_id_for_comments_service, + get_group_id_for_user, + get_user_group_ids, is_comment_too_deep, prepare_content ) @@ -169,6 +171,8 @@ def permitted(func): """ Extract the forum object from the keyword arguments to the view. """ + user_group_id = None + content_user_group_id = None if "thread_id" in kwargs: content = cc.Thread.find(kwargs["thread_id"]).to_dict() elif "comment_id" in kwargs: @@ -177,9 +181,16 @@ def permitted(func): content = cc.Commentable.find(kwargs["commentable_id"]).to_dict() else: content = None - return content + + if 'username' in content: + (user_group_id, content_user_group_id) = get_user_group_ids(course_key, content, request.user) + return content, user_group_id, content_user_group_id + course_key = CourseKey.from_string(kwargs['course_id']) - if check_permissions_by_view(request.user, course_key, fetch_content(), request.view_name): + content, user_group_id, content_user_group_id = fetch_content() + + if check_permissions_by_view(request.user, course_key, content, + request.view_name, user_group_id, content_user_group_id): return func(request, *args, **kwargs) else: return JsonError("unauthorized", status=401) @@ -203,7 +214,7 @@ def ajax_content_response(request, course_key, content): @permitted def create_thread(request, course_id, commentable_id): """ - Given a course and commentble ID, create the thread + Given a course and commentable ID, create the thread """ log.debug("Creating new thread in %r, id %r", course_id, commentable_id) diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py index a9250256bd..9731582cfb 100644 --- a/lms/djangoapps/django_comment_client/permissions.py +++ b/lms/djangoapps/django_comment_client/permissions.py @@ -7,7 +7,8 @@ from types import NoneType from opaque_keys.edx.keys import CourseKey -from django_comment_common.models import all_permissions_for_user_in_course +from django_comment_common.models import CourseDiscussionSettings, all_permissions_for_user_in_course +from django_comment_common.utils import get_course_discussion_settings from lms.djangoapps.teams.models import CourseTeam from lms.lib.comment_client import Thread from request_cache.middleware import RequestCache, request_cached @@ -44,6 +45,7 @@ def get_team(commentable_id): def _check_condition(user, condition, content): """ Check whether or not the given condition applies for the given user and content. """ + def check_open(_user, content): """ Check whether the content is open. """ try: @@ -106,7 +108,7 @@ def _check_condition(user, condition, content): return handlers[condition](user, content) -def _check_conditions_permissions(user, permissions, course_id, content): +def _check_conditions_permissions(user, permissions, course_id, content, user_group_id=None, content_user_group=None): """ Accepts a list of permissions and proceed if any of the permission is valid. Note that ["can_view", "can_edit"] will proceed if the user has either @@ -118,6 +120,17 @@ def _check_conditions_permissions(user, permissions, course_id, content): if isinstance(per, basestring): if per in CONDITIONS: return _check_condition(user, per, content) + if 'group_' in per: + # If a course does not have divided discussions + # or a course has divided discussions, but the current user's content group does not equal + # the content group of the commenter/poster, + # then the current user does not have group edit permissions. + division_scheme = get_course_discussion_settings(course_id).division_scheme + if (division_scheme is CourseDiscussionSettings.NONE + or user_group_id is None + or content_user_group is None + or user_group_id != content_user_group): + return False return has_permission(user, per, course_id=course_id) elif isinstance(per, list) and operator in ["and", "or"]: results = [test(user, x, operator="and") for x in per] @@ -125,42 +138,50 @@ def _check_conditions_permissions(user, permissions, course_id, content): return True in results elif operator == "and": return False not in results + return test(user, permissions, operator="or") # Note: 'edit_content' is being used as a generic way of telling if someone is a privileged user # (forum Moderator/Admin/TA), because there is a desire that team membership does not impact privileged users. VIEW_PERMISSIONS = { - 'update_thread': ['edit_content', ['update_thread', 'is_open', 'is_author']], - 'create_comment': ['edit_content', ["create_comment", "is_open", "is_team_member_if_applicable"]], - 'delete_thread': ['delete_thread', ['update_thread', 'is_author']], - 'update_comment': ['edit_content', ['update_comment', 'is_open', 'is_author']], + 'update_thread': ['group_edit_content', 'edit_content', ['update_thread', 'is_open', 'is_author']], + 'create_comment': ['group_edit_content', 'edit_content', ["create_comment", "is_open", + "is_team_member_if_applicable"]], + 'delete_thread': ['group_delete_thread', 'delete_thread', ['update_thread', 'is_author']], + 'update_comment': ['group_edit_content', 'edit_content', ['update_comment', 'is_open', 'is_author']], 'endorse_comment': ['endorse_comment', 'is_question_author'], - 'openclose_thread': ['openclose_thread'], - 'create_sub_comment': ['edit_content', ['create_sub_comment', 'is_open', 'is_team_member_if_applicable']], - 'delete_comment': ['delete_comment', ['update_comment', 'is_open', 'is_author']], - 'vote_for_comment': ['edit_content', ['vote', 'is_open', 'is_team_member_if_applicable']], - 'undo_vote_for_comment': ['edit_content', ['unvote', 'is_open', 'is_team_member_if_applicable']], - 'vote_for_thread': ['edit_content', ['vote', 'is_open', 'is_team_member_if_applicable']], - 'flag_abuse_for_thread': ['edit_content', ['vote', 'is_team_member_if_applicable']], - 'un_flag_abuse_for_thread': ['edit_content', ['vote', 'is_team_member_if_applicable']], - 'flag_abuse_for_comment': ['edit_content', ['vote', 'is_team_member_if_applicable']], - 'un_flag_abuse_for_comment': ['edit_content', ['vote', 'is_team_member_if_applicable']], - 'undo_vote_for_thread': ['edit_content', ['unvote', 'is_open', 'is_team_member_if_applicable']], - 'pin_thread': ['openclose_thread'], - 'un_pin_thread': ['openclose_thread'], - 'follow_thread': ['edit_content', ['follow_thread', 'is_team_member_if_applicable']], - 'follow_commentable': ['edit_content', ['follow_commentable', 'is_team_member_if_applicable']], - 'unfollow_thread': ['edit_content', ['unfollow_thread', 'is_team_member_if_applicable']], - 'unfollow_commentable': ['edit_content', ['unfollow_commentable', 'is_team_member_if_applicable']], - 'create_thread': ['edit_content', ['create_thread', 'is_team_member_if_applicable']], + 'openclose_thread': ['group_openclose_thread', 'openclose_thread'], + 'create_sub_comment': ['group_edit_content', 'edit_content', ['create_sub_comment', 'is_open', + 'is_team_member_if_applicable']], + 'delete_comment': ['group_delete_comment', 'delete_comment', ['update_comment', 'is_open', 'is_author']], + 'vote_for_comment': ['group_edit_content', 'edit_content', ['vote', 'is_open', 'is_team_member_if_applicable']], + 'undo_vote_for_comment': ['group_edit_content', 'edit_content', ['unvote', 'is_open', + 'is_team_member_if_applicable']], + 'vote_for_thread': ['group_edit_content', 'edit_content', ['vote', 'is_open', 'is_team_member_if_applicable']], + 'flag_abuse_for_thread': ['group_edit_content', 'edit_content', ['vote', 'is_team_member_if_applicable']], + 'un_flag_abuse_for_thread': ['group_edit_content', 'edit_content', ['vote', 'is_team_member_if_applicable']], + 'flag_abuse_for_comment': ['group_edit_content', 'edit_content', ['vote', 'is_team_member_if_applicable']], + 'un_flag_abuse_for_comment': ['group_edit_content', 'edit_content', ['vote', 'is_team_member_if_applicable']], + 'undo_vote_for_thread': ['group_edit_content', 'edit_content', ['unvote', 'is_open', + 'is_team_member_if_applicable']], + 'pin_thread': ['group_openclose_thread', 'openclose_thread'], + 'un_pin_thread': ['group_openclose_thread', 'openclose_thread'], + 'follow_thread': ['group_edit_content', 'edit_content', ['follow_thread', 'is_team_member_if_applicable']], + 'follow_commentable': ['group_edit_content', 'edit_content', ['follow_commentable', + 'is_team_member_if_applicable']], + 'unfollow_thread': ['group_edit_content', 'edit_content', ['unfollow_thread', 'is_team_member_if_applicable']], + 'unfollow_commentable': ['group_edit_content', 'edit_content', ['unfollow_commentable', + 'is_team_member_if_applicable']], + 'create_thread': ['group_edit_content', 'edit_content', ['create_thread', 'is_team_member_if_applicable']], } -def check_permissions_by_view(user, course_id, content, name): +def check_permissions_by_view(user, course_id, content, name, group_id=None, content_user_group=None): assert isinstance(course_id, CourseKey) + p = None try: p = VIEW_PERMISSIONS[name] except KeyError: logging.warning("Permission for view named %s does not exist in permissions.py", name) - return _check_conditions_permissions(user, p, course_id, content) + return _check_conditions_permissions(user, p, course_id, content, group_id, content_user_group) diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index ab48975294..76ddf1b7d2 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -4,6 +4,7 @@ import json import ddt import mock +from django.core.management import call_command from django.core.urlresolvers import reverse from django.test import RequestFactory, TestCase from django.utils.timezone import UTC as django_utc @@ -20,10 +21,20 @@ from django_comment_client.constants import TYPE_ENTRY, TYPE_SUBCATEGORY from django_comment_client.tests.factories import RoleFactory from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_client.tests.utils import config_course_discussions, topic_name_to_id -from django_comment_common.models import CourseDiscussionSettings, ForumsConfig -from django_comment_common.utils import get_course_discussion_settings, set_course_discussion_settings +from django_comment_common.models import ( + FORUM_ROLE_GROUP_MODERATOR, + CourseDiscussionSettings, + ForumsConfig, + Role, + assign_role +) +from django_comment_common.utils import ( + get_course_discussion_settings, + seed_permissions_roles, + set_course_discussion_settings +) from edxmako import add_lookup -from lms.djangoapps.teams.tests.factories import CourseTeamFactory +from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory from lms.lib.comment_client.utils import CommentClientMaintenanceError, perform_request from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.course_groups import cohorts @@ -1681,6 +1692,151 @@ class PermissionsTestCase(ModuleStoreTestCase): self.assertFalse(utils.is_content_authored_by(content, user)) +class GroupModeratorPermissionsTestCase(ModuleStoreTestCase): + """Test utils functionality related to forums "abilities" (permissions) for group moderators""" + + def _check_condition(user, condition, content): + """ + Mocks check_condition method because is_open and is_team_member_if_applicable must always be true + in order to interact with a thread or comment. + """ + return True if condition == 'is_open' or condition == 'is_team_member_if_applicable' else False + + def setUp(self): + super(GroupModeratorPermissionsTestCase, self).setUp() + + # Create course, seed permissions roles, and create team + self.course = CourseFactory.create() + seed_permissions_roles(self.course.id) + verified_coursemode = CourseModeFactory.create( + course_id=self.course.id, + mode_slug=CourseMode.VERIFIED + ) + audit_coursemode = CourseModeFactory.create( + course_id=self.course.id, + mode_slug=CourseMode.AUDIT + ) + + # Create four users: group_moderator (who is within the verified enrollment track and in the cohort), + # verified_user (who is in the verified enrollment track but not the cohort), + # cohorted_user (who is in the cohort but not the verified enrollment track), + # and plain_user (who is neither in the cohort nor the verified enrollment track) + self.group_moderator = UserFactory(username='group_moderator', email='group_moderator@edx.org') + self.group_moderator.id = 1 + CourseEnrollmentFactory( + course_id=self.course.id, + user=self.group_moderator, + mode=verified_coursemode + ) + self.verified_user = UserFactory(username='verified', email='verified@edx.org') + self.verified_user.id = 2 + CourseEnrollmentFactory( + course_id=self.course.id, + user=self.verified_user, + mode=verified_coursemode + ) + self.cohorted_user = UserFactory(username='cohort', email='cohort@edx.org') + self.cohorted_user.id = 3 + CourseEnrollmentFactory( + course_id=self.course.id, + user=self.cohorted_user, + mode=audit_coursemode + ) + self.plain_user = UserFactory(username='plain', email='plain@edx.org') + self.plain_user.id = 4 + CourseEnrollmentFactory( + course_id=self.course.id, + user=self.plain_user, + mode=audit_coursemode + ) + CohortFactory( + course_id=self.course.id, + name='Test Cohort', + users=[self.verified_user, self.cohorted_user] + ) + + # Give group moderator permissions to group_moderator + assign_role(self.course.id, self.group_moderator, 'Group Moderator') + + @mock.patch('django_comment_client.permissions._check_condition', side_effect=_check_condition) + def test_not_divided(self, check_condition_function): + """ + Group moderator should not have moderator permissions if the discussions are not divided. + """ + content = {'user_id': self.plain_user.id, 'type': 'thread', 'username': self.plain_user.username} + self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { + 'editable': False, + 'can_reply': True, + 'can_delete': False, + 'can_openclose': False, + 'can_vote': True, + 'can_report': True + }) + content = {'user_id': self.cohorted_user.id, 'type': 'thread'} + self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { + 'editable': False, + 'can_reply': True, + 'can_delete': False, + 'can_openclose': False, + 'can_vote': True, + 'can_report': True + }) + content = {'user_id': self.verified_user.id, 'type': 'thread'} + self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { + 'editable': False, + 'can_reply': True, + 'can_delete': False, + 'can_openclose': False, + 'can_vote': True, + 'can_report': True + }) + + @mock.patch('django_comment_client.permissions._check_condition', side_effect=_check_condition) + def test_divided_within_group(self, check_condition_function): + """ + Group moderator should have moderator permissions within their group if the discussions are divided. + """ + set_discussion_division_settings(self.course.id, enable_cohorts=True, + division_scheme=CourseDiscussionSettings.COHORT) + content = {'user_id': self.cohorted_user.id, 'type': 'thread', 'username': self.cohorted_user.username} + self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { + 'editable': True, + 'can_reply': True, + 'can_delete': True, + 'can_openclose': True, + 'can_vote': True, + 'can_report': True + }) + + set_discussion_division_settings(self.course.id, division_scheme=CourseDiscussionSettings.ENROLLMENT_TRACK) + content = {'user_id': self.verified_user.id, 'type': 'thread', 'username': self.verified_user.username} + self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { + 'editable': True, + 'can_reply': True, + 'can_delete': True, + 'can_openclose': True, + 'can_vote': True, + 'can_report': True + }) + + @mock.patch('django_comment_client.permissions._check_condition', side_effect=_check_condition) + def test_divided_outside_group(self, check_condition_function): + """ + Group moderator should not have moderator permissions outside of their group. + """ + content = {'user_id': self.plain_user.id, 'type': 'thread', 'username': self.plain_user.username} + set_discussion_division_settings(self.course.id, division_scheme=CourseDiscussionSettings.NONE) + + self.assertEqual(utils.get_ability(self.course.id, content, self.group_moderator), { + 'editable': False, + 'can_reply': True, + 'can_delete': False, + 'can_openclose': False, + 'can_vote': True, + 'can_report': True + }) + + class ClientConfigurationTestCase(TestCase): """Simple test cases to ensure enabling/disabling the use of the comment service works as intended.""" diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 8f68041931..a9eee26f58 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -25,6 +25,7 @@ from edxmako import lookup_template from openedx.core.djangoapps.content.course_structures.models import CourseStructure from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id, get_cohort_names, is_course_cohorted from request_cache.middleware import request_cached +from student.models import get_user_by_username_or_email from student.roles import GlobalStaff from xmodule.modulestore.django import modulestore from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID @@ -516,11 +517,33 @@ def get_ability(course_id, content, user): """ Return a dictionary of forums-oriented actions and the user's permission to perform them """ + (user_group_id, content_user_group_id) = get_user_group_ids(course_id, content, user) return { - 'editable': check_permissions_by_view(user, course_id, content, "update_thread" if content['type'] == 'thread' else "update_comment"), + 'editable': check_permissions_by_view( + user, + course_id, + content, + "update_thread" if content['type'] == 'thread' else "update_comment", + user_group_id, + content_user_group_id + ), 'can_reply': check_permissions_by_view(user, course_id, content, "create_comment" if content['type'] == 'thread' else "create_sub_comment"), - 'can_delete': check_permissions_by_view(user, course_id, content, "delete_thread" if content['type'] == 'thread' else "delete_comment"), - 'can_openclose': check_permissions_by_view(user, course_id, content, "openclose_thread") if content['type'] == 'thread' else False, + 'can_delete': check_permissions_by_view( + user, + course_id, + content, + "delete_thread" if content['type'] == 'thread' else "delete_comment", + user_group_id, + content_user_group_id + ), + 'can_openclose': check_permissions_by_view( + user, + course_id, + content, + "openclose_thread" if content['type'] == 'thread' else False, + user_group_id, + content_user_group_id + ), 'can_vote': not is_content_authored_by(content, user) and check_permissions_by_view( user, course_id, @@ -538,6 +561,26 @@ def get_ability(course_id, content, user): # TODO: RENAME +def get_user_group_ids(course_id, content, user=None): + """ + Given a user, course ID, and the content of the thread or comment, returns the group ID for the current user + and the user that posted the thread/comment. + """ + content_user_group_id = None + user_group_id = None + if course_id is not None: + course_discussion_settings = get_course_discussion_settings(course_id) + if content.get('username'): + try: + content_user = get_user_by_username_or_email(content.get('username')) + content_user_group_id = get_group_id_for_user(content_user, course_discussion_settings) + except User.DoesNotExist: + content_user_group_id = None + + user_group_id = get_group_id_for_user(user, course_discussion_settings) if user else None + return user_group_id, content_user_group_id + + def get_annotated_content_info(course_id, content, user, user_info): """ Get metadata for an individual content (thread or comment) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 4d6b24b878..7404678547 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -46,7 +46,13 @@ from courseware.access import has_access from courseware.courses import get_course_by_id, get_course_with_access from courseware.models import StudentModule from django_comment_client.utils import has_forum_access -from django_comment_common.models import FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR, Role +from django_comment_common.models import ( + Role, + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_MODERATOR, + FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_COMMUNITY_TA, +) from edxmako.shortcuts import render_to_string from lms.djangoapps.instructor.access import ROLES, allow_access, list_with_level, revoke_access, update_forum_role from lms.djangoapps.instructor.enrollment import ( @@ -2487,7 +2493,8 @@ def list_forum_members(request, course_id): return HttpResponseBadRequest("Operation requires instructor access.") # filter out unsupported for roles - if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: + if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_COMMUNITY_TA]: return HttpResponseBadRequest(strip_tags( "Unrecognized rolename '{}'.".format(rolename) )) @@ -2610,7 +2617,8 @@ def update_forum_role_membership(request, course_id): Query parameters: - `email` is the target users email - - `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] + - `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] - `action` is one of ['allow', 'revoke'] """ course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) @@ -2634,7 +2642,8 @@ def update_forum_role_membership(request, course_id): if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: return HttpResponseBadRequest("Operation requires instructor access.") - if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: + if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_COMMUNITY_TA]: return HttpResponseBadRequest(strip_tags( "Unrecognized rolename '{}'.".format(rolename) )) diff --git a/lms/templates/instructor/instructor_dashboard_2/membership.html b/lms/templates/instructor/instructor_dashboard_2/membership.html index 4b5c412023..0795335e85 100644 --- a/lms/templates/instructor/instructor_dashboard_2/membership.html +++ b/lms/templates/instructor/instructor_dashboard_2/membership.html @@ -248,6 +248,20 @@ from django.utils.translation import ugettext as _ data-add-button-label="${_("Add Moderator")}" > +
+