From 01c22531d7f5ec9a0b6aa4c7c8cdc6857b60a028 Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Wed, 22 Jul 2015 17:24:07 -0400
Subject: [PATCH 1/3] Add team permission check on commentable_id.

TNL-2864
---
 .../django_comment_client/base/tests.py       | 246 +++++++++++++++++-
 .../django_comment_client/base/views.py       |  26 +-
 .../django_comment_client/forum/tests.py      |   8 +-
 .../django_comment_client/permissions.py      |  62 +++--
 lms/lib/comment_client/commentable.py         |  10 +
 5 files changed, 293 insertions(+), 59 deletions(-)

diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py
index 07458a17c5..dbcab2bd00 100644
--- a/lms/djangoapps/django_comment_client/base/tests.py
+++ b/lms/djangoapps/django_comment_client/base/tests.py
@@ -28,6 +28,8 @@ from xmodule.modulestore.tests.factories import check_mongo_calls
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore import ModuleStoreEnum
 
+from teams.tests.factories import CourseTeamFactory
+
 
 log = logging.getLogger(__name__)
 
@@ -99,7 +101,8 @@ class ThreadActionGroupIdTestCase(
                 "user_id": str(self.student.id),
                 "group_id": self.student_cohort.id,
                 "closed": False,
-                "type": "thread"
+                "type": "thread",
+                "commentable_id": "non_team_dummy_id"
             }
         )
         mock_request.return_value.status_code = 200
@@ -231,6 +234,7 @@ class ViewsTestCaseMixin(object):
         data = {
             "user_id": str(self.student.id),
             "closed": False,
+            "commentable_id": "non_team_dummy_id"
         }
         if include_depth:
             data["depth"] = 0
@@ -347,10 +351,10 @@ class ViewsQueryCountTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet
         return inner
 
     @ddt.data(
-        (ModuleStoreEnum.Type.mongo, 3, 4, 21),
-        (ModuleStoreEnum.Type.mongo, 20, 4, 21),
-        (ModuleStoreEnum.Type.split, 3, 13, 21),
-        (ModuleStoreEnum.Type.split, 20, 13, 21),
+        (ModuleStoreEnum.Type.mongo, 3, 4, 22),
+        (ModuleStoreEnum.Type.mongo, 20, 4, 22),
+        (ModuleStoreEnum.Type.split, 3, 13, 22),
+        (ModuleStoreEnum.Type.split, 20, 13, 22),
     )
     @ddt.unpack
     @count_queries
@@ -964,7 +968,9 @@ class CreateThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockReq
         request = RequestFactory().post("dummy_url", {"thread_type": "discussion", "body": text, "title": text})
         request.user = self.student
         request.view_name = "create_thread"
-        response = views.create_thread(request, course_id=self.course.id.to_deprecated_string(), commentable_id="test_commentable")
+        response = views.create_thread(
+            request, course_id=self.course.id.to_deprecated_string(), commentable_id="non_team_dummy_id"
+        )
 
         self.assertEqual(response.status_code, 200)
         self.assertTrue(mock_request.called)
@@ -1012,13 +1018,15 @@ class CreateCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockRe
 
     @patch('lms.lib.comment_client.utils.requests.request')
     def _test_unicode_data(self, text, mock_request):
+        commentable_id = "non_team_dummy_id"
         self._set_mock_request_data(mock_request, {
             "closed": False,
+            "commentable_id": commentable_id
         })
         # We have to get clever here due to Thread's setters and getters.
         # Patch won't work with it.
         try:
-            Thread.commentable_id = Mock()
+            Thread.commentable_id = commentable_id
             request = RequestFactory().post("dummy_url", {"body": text})
             request.user = self.student
             request.view_name = "create_comment"
@@ -1078,7 +1086,8 @@ class CreateSubCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, Moc
         self._set_mock_request_data(mock_request, {
             "closed": False,
             "depth": 1,
-            "thread_id": "test_thread"
+            "thread_id": "test_thread",
+            "commentable_id": "non_team_dummy_id"
         })
         request = RequestFactory().post("dummy_url", {"body": text})
         request.user = self.student
@@ -1086,7 +1095,7 @@ class CreateSubCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, Moc
         Thread.commentable_id = Mock()
         try:
             response = views.create_sub_comment(
-                request, course_id=self.course.id.to_deprecated_string(), comment_id="dummy_comment_id"
+                request, course_id=unicode(self.course.id), comment_id="dummy_comment_id"
             )
 
             self.assertEqual(response.status_code, 200)
@@ -1096,6 +1105,219 @@ class CreateSubCommentUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, Moc
             del Thread.commentable_id
 
 
+@ddt.ddt
+@patch("lms.lib.comment_client.utils.requests.request")
+class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
+    # Most of the test points use the same ddt data.
+    # args: user, commentable_id, status_code
+    ddt_permissions_args = [
+        # Student in team can do operations on threads/comments within the team commentable.
+        ('student_in_team', 'team_commentable_id', 200),
+        # Non-team commentables can be edited by any student.
+        ('student_in_team', 'course_commentable_id', 200),
+        # Student not in team cannot do operations within the team commentable.
+        ('student_not_in_team', 'team_commentable_id', 401),
+        # Non-team commentables can be edited by any student.
+        ('student_not_in_team', 'course_commentable_id', 200),
+        # Moderators most be a member of the team for doing "student actions".
+        ('moderator', 'team_commentable_id', 401)
+    ]
+
+    @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
+    def setUp(self):
+        super(TeamsPermissionsTestCase, self).setUp()
+        self.password = "test password"
+        teams_configuration = {
+            'topics': [{'id': "topic_id", 'name': 'Solar Power', 'description': 'Solar power is hot'}]
+        }
+        self.course = CourseFactory.create(teams_configuration=teams_configuration)
+        seed_permissions_roles(self.course.id)
+
+        # Create 3 users-- student in team, student not in team, discussion moderator
+        self.student_in_team = UserFactory.create(password=self.password)
+        self.student_not_in_team = UserFactory.create(password=self.password)
+        self.moderator = UserFactory.create(password=self.password)
+        CourseEnrollmentFactory(user=self.student_in_team, course_id=self.course.id)
+        CourseEnrollmentFactory(user=self.student_not_in_team, course_id=self.course.id)
+        CourseEnrollmentFactory(user=self.moderator, course_id=self.course.id)
+        self.moderator.roles.add(Role.objects.get(name="Moderator", course_id=self.course.id))
+
+        # Create a team.
+        self.team_commentable_id = "team_discussion_id"
+        self.team = CourseTeamFactory.create(
+            name=u'The Only Team',
+            course_id=self.course.id,
+            topic_id='topic_id',
+            discussion_topic_id=self.team_commentable_id
+        )
+        self.team.add_user(self.student_in_team)
+
+        # Dummy commentable ID not linked to a team
+        self.course_commentable_id = "course_level_commentable"
+
+    def _setup_mock(self, user, mock_request, data):
+        user = getattr(self, user)
+        self._set_mock_request_data(mock_request, data)
+        self.client.login(username=user.username, password=self.password)
+
+    @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),
+        # students can only update their own posts
+        ('student_in_team', 'moderator', 'team_commentable_id', 401),
+        # 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),
+        # 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),
+    )
+    @ddt.unpack
+    def test_update_thread(self, user, thread_author, commentable_id, status_code, mock_request):
+        """
+        Verify that update_thread is limited to thread authors and privileged users (team membership does not matter).
+        """
+        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}
+        )
+        response = self.client.post(
+            reverse(
+                "update_thread",
+                kwargs={
+                    "course_id": unicode(self.course.id),
+                    "thread_id": "dummy"
+                }
+            ),
+            data={"body": "foo", "title": "foo"}
+        )
+        self.assertEqual(response.status_code, status_code)
+
+    @ddt.data(*ddt_permissions_args)
+    @ddt.unpack
+    def test_create_comment(self, user, commentable_id, status_code, mock_request):
+        """
+        Verify that create_comment is limited to members of the team.
+        """
+        commentable_id = getattr(self, commentable_id)
+        self._setup_mock(user, mock_request, {"closed": False, "commentable_id": commentable_id})
+
+        response = self.client.post(
+            reverse(
+                "create_comment",
+                kwargs={
+                    "course_id": unicode(self.course.id),
+                    "thread_id": "dummy"
+                }
+            ),
+            data={"body": "foo", "title": "foo"}
+        )
+        self.assertEqual(response.status_code, status_code)
+
+    @ddt.data(*ddt_permissions_args)
+    @ddt.unpack
+    def test_create_sub_comment(self, user, commentable_id, status_code, mock_request):
+        """
+        Verify that create_subcomment is limited to members of the team.
+        """
+        commentable_id = getattr(self, commentable_id)
+        self._setup_mock(
+            user, mock_request,
+            {"closed": False, "commentable_id": commentable_id, "thread_id": "dummy_thread"},
+        )
+        response = self.client.post(
+            reverse(
+                "create_sub_comment",
+                kwargs={
+                    "course_id": unicode(self.course.id),
+                    "comment_id": "dummy_comment"
+                }
+            ),
+            data={"body": "foo", "title": "foo"}
+        )
+        self.assertEqual(response.status_code, status_code)
+
+    @ddt.data(*ddt_permissions_args)
+    @ddt.unpack
+    def test_comment_actions(self, user, commentable_id, status_code, mock_request):
+        """
+        Verify that voting and flagging of comments is limited to members of the team.
+        """
+        commentable_id = getattr(self, commentable_id)
+        self._setup_mock(
+            user, mock_request,
+            {"closed": False, "commentable_id": commentable_id, "thread_id": "dummy_thread"},
+        )
+        for action in ["upvote_comment", "downvote_comment", "un_flag_abuse_for_comment", "flag_abuse_for_comment"]:
+            response = self.client.post(
+                reverse(
+                    action,
+                    kwargs={"course_id": unicode(self.course.id), "comment_id": "dummy_comment"}
+                )
+            )
+            self.assertEqual(response.status_code, status_code)
+
+    @ddt.data(*ddt_permissions_args)
+    @ddt.unpack
+    def test_threads_actions(self, user, commentable_id, status_code, mock_request):
+        """
+        Verify that voting, flagging, and following of threads is limited to members of the team.
+        """
+        commentable_id = getattr(self, commentable_id)
+        self._setup_mock(
+            user, mock_request,
+            {"closed": False, "commentable_id": commentable_id},
+        )
+        for action in ["upvote_thread", "downvote_thread", "un_flag_abuse_for_thread", "flag_abuse_for_thread",
+                       "follow_thread", "unfollow_thread"]:
+            response = self.client.post(
+                reverse(
+                    action,
+                    kwargs={"course_id": unicode(self.course.id), "thread_id": "dummy_thread"}
+                )
+            )
+            self.assertEqual(response.status_code, status_code)
+
+    @ddt.data(*ddt_permissions_args)
+    @ddt.unpack
+    def test_create_thread(self, user, commentable_id, status_code, __):
+        """
+        Verify that creation of threads is limited to members of the team.
+        """
+        commentable_id = getattr(self, commentable_id)
+        # mock_request is not used because Commentables don't exist in comment service.
+        self.client.login(username=getattr(self, user).username, password=self.password)
+        response = self.client.post(
+            reverse(
+                "create_thread",
+                kwargs={"course_id": unicode(self.course.id), "commentable_id": commentable_id}
+            ),
+            data={"body": "foo", "title": "foo", "thread_type": "discussion"}
+        )
+        self.assertEqual(response.status_code, status_code)
+
+    @ddt.data(*ddt_permissions_args)
+    @ddt.unpack
+    def test_commentable_actions(self, user, commentable_id, status_code, __):
+        """
+        Verify that following of commentables is limited to members of the team.
+        """
+        commentable_id = getattr(self, commentable_id)
+        # mock_request is not used because Commentables don't exist in comment service.
+        self.client.login(username=getattr(self, user).username, password=self.password)
+        for action in ["follow_commentable", "unfollow_commentable"]:
+            response = self.client.post(
+                reverse(
+                    action,
+                    kwargs={"course_id": unicode(self.course.id), "commentable_id": commentable_id}
+                )
+            )
+            self.assertEqual(response.status_code, status_code)
+
+
 class ForumEventTestCase(ModuleStoreTestCase, MockRequestSetupMixin):
     """
     Forum actions are expected to launch analytics events. Test these here.
@@ -1123,7 +1345,7 @@ class ForumEventTestCase(ModuleStoreTestCase, MockRequestSetupMixin):
         request.user = self.student
         request.view_name = "create_thread"
 
-        views.create_thread(request, course_id=self.course.id.to_deprecated_string(), commentable_id="test_commentable")
+        views.create_thread(request, course_id=unicode(self.course.id), commentable_id="test_commentable")
 
         event_name, event = mock_emit.call_args[0]
         self.assertEqual(event_name, 'edx.forum.thread.created')
@@ -1180,9 +1402,7 @@ class ForumEventTestCase(ModuleStoreTestCase, MockRequestSetupMixin):
         request = RequestFactory().post("dummy_url", {"body": "Another comment"})
         request.user = self.student
         request.view_name = "create_sub_comment"
-        views.create_sub_comment(
-            request, course_id=self.course.id.to_deprecated_string(), comment_id="dummy_comment_id"
-        )
+        views.create_sub_comment(request, course_id=unicode(self.course.id), comment_id="dummy_comment_id")
 
         event_name, event = mock_emit.call_args[0]
         self.assertEqual(event_name, "edx.forum.comment.created")
diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py
index 5456e970db..0111927ce6 100644
--- a/lms/djangoapps/django_comment_client/base/views.py
+++ b/lms/djangoapps/django_comment_client/base/views.py
@@ -51,6 +51,8 @@ def permitted(fn):
                 content = cc.Thread.find(kwargs["thread_id"]).to_dict()
             elif "comment_id" in kwargs:
                 content = cc.Comment.find(kwargs["comment_id"]).to_dict()
+            elif "commentable_id" in kwargs:
+                content = cc.Commentable.find(kwargs["commentable_id"]).to_dict()
             else:
                 content = None
             return content
@@ -605,16 +607,6 @@ def follow_commentable(request, course_id, commentable_id):
     return JsonResponse({})
 
 
-@require_POST
-@login_required
-@permitted
-def follow_user(request, course_id, followed_user_id):
-    user = cc.User.from_django_user(request.user)
-    followed_user = cc.User.find(followed_user_id)
-    user.follow(followed_user)
-    return JsonResponse({})
-
-
 @require_POST
 @login_required
 @permitted
@@ -643,20 +635,6 @@ def unfollow_commentable(request, course_id, commentable_id):
     return JsonResponse({})
 
 
-@require_POST
-@login_required
-@permitted
-def unfollow_user(request, course_id, followed_user_id):
-    """
-    given a course id and user id, stop following this user
-    ajax only
-    """
-    user = cc.User.from_django_user(request.user)
-    followed_user = cc.User.find(followed_user_id)
-    user.unfollow(followed_user)
-    return JsonResponse({})
-
-
 @require_POST
 @login_required
 @csrf.csrf_exempt
diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py
index e542c4ba39..4d076a5705 100644
--- a/lms/djangoapps/django_comment_client/forum/tests.py
+++ b/lms/djangoapps/django_comment_client/forum/tests.py
@@ -333,11 +333,11 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
 
     @ddt.data(
         # old mongo with cache
-        (ModuleStoreEnum.Type.mongo, 1, 7, 5, 13, 8),
-        (ModuleStoreEnum.Type.mongo, 50, 7, 5, 13, 8),
+        (ModuleStoreEnum.Type.mongo, 1, 7, 5, 14, 8),
+        (ModuleStoreEnum.Type.mongo, 50, 7, 5, 14, 8),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, 1, 3, 3, 13, 8),
-        (ModuleStoreEnum.Type.split, 50, 3, 3, 13, 8),
+        (ModuleStoreEnum.Type.split, 1, 3, 3, 14, 8),
+        (ModuleStoreEnum.Type.split, 50, 3, 3, 14, 8),
     )
     @ddt.unpack
     def test_number_of_mongo_queries(
diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py
index c7687fbe74..f0b72e20fe 100644
--- a/lms/djangoapps/django_comment_client/permissions.py
+++ b/lms/djangoapps/django_comment_client/permissions.py
@@ -10,6 +10,7 @@ from lms.lib.comment_client import Thread
 from opaque_keys.edx.keys import CourseKey
 
 from django_comment_common.models import all_permissions_for_user_in_course
+from teams.models import CourseTeam
 
 
 def has_permission(user, permission, course_id=None):
@@ -27,7 +28,7 @@ def has_permission(user, permission, course_id=None):
     return permission in all_permissions
 
 
-CONDITIONS = ['is_open', 'is_author', 'is_question_author']
+CONDITIONS = ['is_open', 'is_author', 'is_question_author', 'is_team_member_if_applicable']
 
 
 def _check_condition(user, condition, content):
@@ -55,10 +56,37 @@ def _check_condition(user, condition, content):
         except KeyError:
             return False
 
+    def check_team_member(user, content):
+        """
+        If the content has a commentable_id, verifies that either it is not associated with a team,
+        or if it is, that the user is a member of that team.
+        """
+        if not content:
+            return False
+        try:
+            commentable_id = content['commentable_id']
+            request_cache_dict = RequestCache.get_request_cache().data
+            cache_key = "django_comment_client.check_team_member.{}.{}".format(user.id, commentable_id)
+            if cache_key in request_cache_dict:
+                return request_cache_dict[cache_key]
+            team = CourseTeam.objects.get(discussion_topic_id=commentable_id)
+            passes_condition = team.users.filter(id=user.id).count() > 0
+            request_cache_dict[cache_key] = passes_condition
+        except KeyError:
+            # We do not expect KeyError in production-- it usually indicates an improper test mock.
+            logging.warning("Did not find key commentable_id in content.")
+            passes_condition = False
+        except CourseTeam.DoesNotExist:
+            passes_condition = True
+            request_cache_dict[cache_key] = passes_condition
+
+        return passes_condition
+
     handlers = {
         'is_open': check_open,
         'is_author': check_author,
         'is_question_author': check_question_author,
+        'is_team_member_if_applicable': check_team_member
     }
 
     return handlers[condition](user, content)
@@ -88,30 +116,28 @@ def _check_conditions_permissions(user, permissions, course_id, content):
 
 VIEW_PERMISSIONS = {
     'update_thread': ['edit_content', ['update_thread', 'is_open', 'is_author']],
-    'create_comment': [["create_comment", "is_open"]],
+    'create_comment': [["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']],
     'endorse_comment': ['endorse_comment', 'is_question_author'],
     'openclose_thread': ['openclose_thread'],
-    'create_sub_comment': [['create_sub_comment', 'is_open']],
+    'create_sub_comment': [['create_sub_comment', 'is_open', 'is_team_member_if_applicable']],
     'delete_comment': ['delete_comment', ['update_comment', 'is_open', 'is_author']],
-    'vote_for_comment': [['vote', 'is_open']],
-    'undo_vote_for_comment': [['unvote', 'is_open']],
-    'vote_for_thread': [['vote', 'is_open']],
-    'flag_abuse_for_thread': ['vote'],
-    'un_flag_abuse_for_thread': ['vote'],
-    'flag_abuse_for_comment': ['vote'],
-    'un_flag_abuse_for_comment': ['vote'],
-    'undo_vote_for_thread': [['unvote', 'is_open']],
+    'vote_for_comment': [['vote', 'is_open', 'is_team_member_if_applicable']],
+    'undo_vote_for_comment': [['unvote', 'is_open', 'is_team_member_if_applicable']],
+    'vote_for_thread': [['vote', 'is_open', 'is_team_member_if_applicable']],
+    'flag_abuse_for_thread': [['vote', 'is_team_member_if_applicable']],
+    'un_flag_abuse_for_thread': [['vote', 'is_team_member_if_applicable']],
+    'flag_abuse_for_comment': [['vote', 'is_team_member_if_applicable']],
+    'un_flag_abuse_for_comment': [['vote', 'is_team_member_if_applicable']],
+    'undo_vote_for_thread': [['unvote', 'is_open', 'is_team_member_if_applicable']],
     'pin_thread': ['openclose_thread'],
     'un_pin_thread': ['openclose_thread'],
-    'follow_thread': ['follow_thread'],
-    'follow_commentable': ['follow_commentable'],
-    'follow_user': ['follow_user'],
-    'unfollow_thread': ['unfollow_thread'],
-    'unfollow_commentable': ['unfollow_commentable'],
-    'unfollow_user': ['unfollow_user'],
-    'create_thread': ['create_thread'],
+    'follow_thread': [['follow_thread', 'is_team_member_if_applicable']],
+    'follow_commentable': [['follow_commentable', 'is_team_member_if_applicable']],
+    'unfollow_thread': [['unfollow_thread', 'is_team_member_if_applicable']],
+    'unfollow_commentable': [['unfollow_commentable', 'is_team_member_if_applicable']],
+    'create_thread': [['create_thread', 'is_team_member_if_applicable']],
 }
 
 
diff --git a/lms/lib/comment_client/commentable.py b/lms/lib/comment_client/commentable.py
index 9ce3b20053..93a2c28022 100644
--- a/lms/lib/comment_client/commentable.py
+++ b/lms/lib/comment_client/commentable.py
@@ -5,5 +5,15 @@ from lms.lib.comment_client import settings
 
 class Commentable(models.Model):
 
+    accessible_fields = ['id', 'commentable_id']
+
     base_url = "{prefix}/commentables".format(prefix=settings.PREFIX)
     type = 'commentable'
+
+    def retrieve(self, *args, **kwargs):
+        """
+        Override default behavior because commentables don't actually exist in the comment service.
+        """
+        self.attributes["commentable_id"] = self.attributes["id"]
+        self.retrieved = True
+        return self

From 9d3e2c1a08c2572991053c4a9b92bbef895fa837 Mon Sep 17 00:00:00 2001
From: cahrens <christina@edx.org>
Date: Thu, 30 Jul 2015 17:06:40 -0400
Subject: [PATCH 2/3] Ignore team membership for privileged users.

---
 .../django_comment_client/base/tests.py       | 19 ++++++-----
 .../django_comment_client/permissions.py      | 32 ++++++++++---------
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py
index dbcab2bd00..22541cc4a6 100644
--- a/lms/djangoapps/django_comment_client/base/tests.py
+++ b/lms/djangoapps/django_comment_client/base/tests.py
@@ -1119,8 +1119,8 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
         ('student_not_in_team', 'team_commentable_id', 401),
         # Non-team commentables can be edited by any student.
         ('student_not_in_team', 'course_commentable_id', 200),
-        # Moderators most be a member of the team for doing "student actions".
-        ('moderator', 'team_commentable_id', 401)
+        # Moderators can always operator on threads within a team, regardless of team membership.
+        ('moderator', 'team_commentable_id', 200)
     ]
 
     @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
@@ -1200,7 +1200,7 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
     @ddt.unpack
     def test_create_comment(self, user, commentable_id, status_code, mock_request):
         """
-        Verify that create_comment is limited to members of the team.
+        Verify that create_comment is limited to members of the team or users with 'edit_content' permission.
         """
         commentable_id = getattr(self, commentable_id)
         self._setup_mock(user, mock_request, {"closed": False, "commentable_id": commentable_id})
@@ -1221,7 +1221,7 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
     @ddt.unpack
     def test_create_sub_comment(self, user, commentable_id, status_code, mock_request):
         """
-        Verify that create_subcomment is limited to members of the team.
+        Verify that create_subcomment is limited to members of the team or users with 'edit_content' permission.
         """
         commentable_id = getattr(self, commentable_id)
         self._setup_mock(
@@ -1244,7 +1244,8 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
     @ddt.unpack
     def test_comment_actions(self, user, commentable_id, status_code, mock_request):
         """
-        Verify that voting and flagging of comments is limited to members of the team.
+        Verify that voting and flagging of comments is limited to members of the team or users with
+        'edit_content' permission.
         """
         commentable_id = getattr(self, commentable_id)
         self._setup_mock(
@@ -1264,7 +1265,8 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
     @ddt.unpack
     def test_threads_actions(self, user, commentable_id, status_code, mock_request):
         """
-        Verify that voting, flagging, and following of threads is limited to members of the team.
+        Verify that voting, flagging, and following of threads is limited to members of the team or users with
+        'edit_content' permission.
         """
         commentable_id = getattr(self, commentable_id)
         self._setup_mock(
@@ -1285,7 +1287,7 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
     @ddt.unpack
     def test_create_thread(self, user, commentable_id, status_code, __):
         """
-        Verify that creation of threads is limited to members of the team.
+        Verify that creation of threads is limited to members of the team or users with 'edit_content' permission.
         """
         commentable_id = getattr(self, commentable_id)
         # mock_request is not used because Commentables don't exist in comment service.
@@ -1303,7 +1305,8 @@ class TeamsPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSe
     @ddt.unpack
     def test_commentable_actions(self, user, commentable_id, status_code, __):
         """
-        Verify that following of commentables is limited to members of the team.
+        Verify that following of commentables is limited to members of the team or users with
+        'edit_content' permission.
         """
         commentable_id = getattr(self, commentable_id)
         # mock_request is not used because Commentables don't exist in comment service.
diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py
index f0b72e20fe..dd490c45dc 100644
--- a/lms/djangoapps/django_comment_client/permissions.py
+++ b/lms/djangoapps/django_comment_client/permissions.py
@@ -114,30 +114,32 @@ def _check_conditions_permissions(user, permissions, course_id, content):
     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': [["create_comment", "is_open", "is_team_member_if_applicable"]],
+    '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']],
     'endorse_comment': ['endorse_comment', 'is_question_author'],
     'openclose_thread': ['openclose_thread'],
-    'create_sub_comment': [['create_sub_comment', 'is_open', 'is_team_member_if_applicable']],
+    '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': [['vote', 'is_open', 'is_team_member_if_applicable']],
-    'undo_vote_for_comment': [['unvote', 'is_open', 'is_team_member_if_applicable']],
-    'vote_for_thread': [['vote', 'is_open', 'is_team_member_if_applicable']],
-    'flag_abuse_for_thread': [['vote', 'is_team_member_if_applicable']],
-    'un_flag_abuse_for_thread': [['vote', 'is_team_member_if_applicable']],
-    'flag_abuse_for_comment': [['vote', 'is_team_member_if_applicable']],
-    'un_flag_abuse_for_comment': [['vote', 'is_team_member_if_applicable']],
-    'undo_vote_for_thread': [['unvote', 'is_open', 'is_team_member_if_applicable']],
+    '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': [['follow_thread', 'is_team_member_if_applicable']],
-    'follow_commentable': [['follow_commentable', 'is_team_member_if_applicable']],
-    'unfollow_thread': [['unfollow_thread', 'is_team_member_if_applicable']],
-    'unfollow_commentable': [['unfollow_commentable', 'is_team_member_if_applicable']],
-    'create_thread': [['create_thread', 'is_team_member_if_applicable']],
+    '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']],
 }
 
 

From c12c2933a734d8b213ca3371fb2a5b15b23d2279 Mon Sep 17 00:00:00 2001
From: Diana Huang <dkh@edx.org>
Date: Thu, 30 Jul 2015 16:19:12 -0400
Subject: [PATCH 3/3] Make context for threads more implicit.

---
 .../djangoapps/django_comment_common/utils.py |  7 +++
 .../django_comment_client/base/tests.py       | 27 ++++++++----
 .../django_comment_client/base/views.py       | 10 +++--
 .../django_comment_client/forum/tests.py      | 44 +++++++++++++------
 .../django_comment_client/forum/views.py      |  8 +++-
 .../django_comment_client/permissions.py      | 33 +++++++++++---
 6 files changed, 96 insertions(+), 33 deletions(-)

diff --git a/common/djangoapps/django_comment_common/utils.py b/common/djangoapps/django_comment_common/utils.py
index 2bc8aec97a..bccc127733 100644
--- a/common/djangoapps/django_comment_common/utils.py
+++ b/common/djangoapps/django_comment_common/utils.py
@@ -1,5 +1,12 @@
 from django_comment_common.models import Role
 
+
+class ThreadContext(object):
+    """ An enumeration that represents the context of a thread. Used primarily by the comments service. """
+    STANDALONE = 'standalone'
+    COURSE = 'course'
+
+
 _STUDENT_ROLE_PERMISSIONS = ["vote", "update_thread", "follow_thread", "unfollow_thread",
                              "update_comment", "create_sub_comment", "unvote", "create_thread",
                              "follow_commentable", "unfollow_commentable", "create_comment", ]
diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py
index dbcab2bd00..87fd8d5a1c 100644
--- a/lms/djangoapps/django_comment_client/base/tests.py
+++ b/lms/djangoapps/django_comment_client/base/tests.py
@@ -19,7 +19,7 @@ from django_comment_client.tests.group_id import CohortedTopicGroupIdTestMixin,
 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 django_comment_common.utils import seed_permissions_roles, ThreadContext
 from student.tests.factories import CourseEnrollmentFactory, UserFactory, CourseAccessRoleFactory
 from util.testing import UrlResetMixin
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -240,7 +240,7 @@ class ViewsTestCaseMixin(object):
             data["depth"] = 0
         self._set_mock_request_data(mock_request, data)
 
-    def create_thread_helper(self, mock_request, extra_data=None):
+    def create_thread_helper(self, mock_request, extra_request_data=None, extra_response_data=None):
         """
         Issues a request to create a thread and verifies the result.
         """
@@ -283,8 +283,8 @@ class ViewsTestCaseMixin(object):
             "anonymous": ["false"],
             "title": ["Hello"],
         }
-        if extra_data:
-            thread.update(extra_data)
+        if extra_request_data:
+            thread.update(extra_request_data)
         url = reverse('create_thread', kwargs={'commentable_id': 'i4x-MITx-999-course-Robot_Super_Course',
                                                'course_id': self.course_id.to_deprecated_string()})
         response = self.client.post(url, data=thread)
@@ -292,14 +292,15 @@ class ViewsTestCaseMixin(object):
         expected_data = {
             'thread_type': 'discussion',
             'body': u'this is a post',
+            'context': ThreadContext.COURSE,
             'anonymous_to_peers': False, 'user_id': 1,
             'title': u'Hello',
             'commentable_id': u'i4x-MITx-999-course-Robot_Super_Course',
             'anonymous': False,
             'course_id': unicode(self.course_id),
         }
-        if extra_data:
-            expected_data.update(extra_data)
+        if extra_response_data:
+            expected_data.update(extra_response_data)
         mock_request.assert_called_with(
             'post',
             '{prefix}/i4x-MITx-999-course-Robot_Super_Course/threads'.format(prefix=CS_PREFIX),
@@ -387,9 +388,19 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin, V
     def test_create_thread(self, mock_request):
         self.create_thread_helper(mock_request)
 
-    def test_create_thread_with_context(self, mock_request):
+    def test_create_thread_standalone(self, mock_request):
+        team = CourseTeamFactory.create(
+            name="A Team",
+            course_id=self.course_id,
+            topic_id='topic_id',
+            discussion_topic_id="i4x-MITx-999-course-Robot_Super_Course"
+        )
+
+        # Add the student to the team so they can post to the commentable.
+        team.add_user(self.student)
+
         # create_thread_helper verifies that extra data are passed through to the comments service
-        self.create_thread_helper(mock_request, extra_data={'context': 'standalone'})
+        self.create_thread_helper(mock_request, extra_response_data={'context': ThreadContext.STANDALONE})
 
     def test_delete_comment(self, mock_request):
         self._set_mock_request_data(mock_request, {
diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py
index 0111927ce6..119bd59697 100644
--- a/lms/djangoapps/django_comment_client/base/views.py
+++ b/lms/djangoapps/django_comment_client/base/views.py
@@ -18,6 +18,7 @@ from courseware.access import has_access
 from util.file import store_uploaded_file
 from courseware.courses import get_course_with_access, get_course_by_id
 import django_comment_client.settings as cc_settings
+from django_comment_common.utils import ThreadContext
 from django_comment_client.utils import (
     add_courseware_context,
     get_annotated_content_info,
@@ -30,7 +31,7 @@ from django_comment_client.utils import (
     discussion_category_id_access,
     get_cached_discussion_id_map,
 )
-from django_comment_client.permissions import check_permissions_by_view, has_permission
+from django_comment_client.permissions import check_permissions_by_view, has_permission, get_team
 from eventtracking import tracker
 import lms.lib.comment_client as cc
 
@@ -187,8 +188,11 @@ def create_thread(request, course_id, commentable_id):
         'title': post["title"],
     }
 
-    if 'context' in post:
-        params['context'] = post['context']
+    # Check for whether this commentable belongs to a team, and add the right context
+    if get_team(commentable_id) is not None:
+        params['context'] = ThreadContext.STANDALONE
+    else:
+        params['context'] = ThreadContext.COURSE
 
     thread = cc.Thread(**params)
 
diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py
index 4d076a5705..2dc54201d1 100644
--- a/lms/djangoapps/django_comment_client/forum/tests.py
+++ b/lms/djangoapps/django_comment_client/forum/tests.py
@@ -8,7 +8,9 @@ from django.test.client import Client, RequestFactory
 from django.test.utils import override_settings
 from edxmako.tests import mako_middleware_process_request
 
+from django_comment_common.utils import ThreadContext
 from django_comment_client.forum import views
+from django_comment_client.permissions import get_team
 from django_comment_client.tests.group_id import (
     CohortedTopicGroupIdTestMixin,
     NonCohortedTopicGroupIdTestMixin
@@ -33,6 +35,8 @@ from mock import patch, Mock, ANY, call
 
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup
 
+from teams.tests.factories import CourseTeamFactory
+
 log = logging.getLogger(__name__)
 
 # pylint: disable=missing-docstring
@@ -112,21 +116,23 @@ def make_mock_thread_data(
         group_id=None,
         group_name=None,
         commentable_id=None,
-        context=None
 ):
+    data_commentable_id = (
+        commentable_id or course.discussion_topics.get('General', {}).get('id') or "dummy_commentable_id"
+    )
     thread_data = {
         "id": thread_id,
         "type": "thread",
         "title": text,
         "body": text,
-        "commentable_id": (
-            commentable_id or course.discussion_topics.get('General', {}).get('id') or "dummy_commentable_id"
-        ),
+        "commentable_id": data_commentable_id,
         "resp_total": 42,
         "resp_skip": 25,
         "resp_limit": 5,
         "group_id": group_id,
-        "context": context if context else "course"
+        "context": (
+            ThreadContext.COURSE if get_team(data_commentable_id) is None else ThreadContext.STANDALONE
+        )
     }
     if group_id is not None:
         thread_data['group_name'] = group_name
@@ -146,7 +152,6 @@ def make_mock_request_impl(
         group_id=None,
         commentable_id=None,
         num_thread_responses=1,
-        context=None
 ):
     def mock_request_impl(*args, **kwargs):
         url = args[1]
@@ -161,7 +166,6 @@ def make_mock_request_impl(
                         num_children=None,
                         group_id=group_id,
                         commentable_id=commentable_id,
-                        context=context
                     )
                 ]
             }
@@ -172,7 +176,6 @@ def make_mock_request_impl(
                 thread_id=thread_id,
                 num_children=num_thread_responses,
                 group_id=group_id,
-                context=context
             )
         elif "/users/" in url:
             data = {
@@ -672,12 +675,20 @@ class InlineDiscussionContextTestCase(ModuleStoreTestCase):
         super(InlineDiscussionContextTestCase, self).setUp()
         self.course = CourseFactory.create()
         CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
+        self.discussion_topic_id = "dummy_topic"
+        self.team = CourseTeamFactory(
+            name="A team",
+            course_id=self.course.id,
+            topic_id='topic_id',
+            discussion_topic_id=self.discussion_topic_id
+        )
+        self.team.add_user(self.user)  # pylint: disable=no-member
 
     def test_context_can_be_standalone(self, mock_request):
         mock_request.side_effect = make_mock_request_impl(
             course=self.course,
             text="dummy text",
-            context="standalone"
+            commentable_id=self.discussion_topic_id
         )
 
         request = RequestFactory().get("dummy_url")
@@ -686,11 +697,11 @@ class InlineDiscussionContextTestCase(ModuleStoreTestCase):
         response = views.inline_discussion(
             request,
             unicode(self.course.id),
-            "dummy_topic",
+            self.discussion_topic_id,
         )
 
         json_response = json.loads(response.content)
-        self.assertEqual(json_response['discussion_data'][0]['context'], 'standalone')
+        self.assertEqual(json_response['discussion_data'][0]['context'], ThreadContext.STANDALONE)
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
@@ -1041,8 +1052,15 @@ class InlineDiscussionTestCase(ModuleStoreTestCase):
         self.verify_response(self.send_request(mock_request))
 
     def test_context(self, mock_request):
-        response = self.send_request(mock_request, {'context': 'standalone'})
-        self.assertEqual(mock_request.call_args[1]['params']['context'], 'standalone')
+        team = CourseTeamFactory(
+            name='Team Name',
+            topic_id='A topic',
+            course_id=self.course.id,
+            discussion_topic_id=self.discussion1.discussion_id
+        )
+        team.add_user(self.student)  # pylint: disable=no-member
+        response = self.send_request(mock_request)
+        self.assertEqual(mock_request.call_args[1]['params']['context'], ThreadContext.STANDALONE)
         self.verify_response(response)
 
 
diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py
index 1521b0d303..853e30e35f 100644
--- a/lms/djangoapps/django_comment_client/forum/views.py
+++ b/lms/djangoapps/django_comment_client/forum/views.py
@@ -30,7 +30,8 @@ from courseware.access import has_access
 from xmodule.modulestore.django import modulestore
 from ccx.overrides import get_current_ccx
 
-from django_comment_client.permissions import has_permission
+from django_comment_common.utils import ThreadContext
+from django_comment_client.permissions import has_permission, get_team
 from django_comment_client.utils import (
     merge_dict,
     extract,
@@ -111,6 +112,7 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
         'text': '',
         'course_id': unicode(course.id),
         'user_id': request.user.id,
+        'context': ThreadContext.COURSE,
         'group_id': get_group_id_for_comments_service(request, course.id, discussion_id),  # may raise ValueError
     }
 
@@ -118,6 +120,9 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
     # comments_service.
     if discussion_id is not None:
         default_query_params['commentable_id'] = discussion_id
+        # Use the discussion id/commentable id to determine the context we are going to pass through to the backend.
+        if get_team(discussion_id) is not None:
+            default_query_params['context'] = ThreadContext.STANDALONE
 
     if not request.GET.get('sort_key'):
         # If the user did not select a sort key, use their last used sort key
@@ -149,7 +154,6 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
                     'flagged',
                     'unread',
                     'unanswered',
-                    'context',
                 ]
             )
         )
diff --git a/lms/djangoapps/django_comment_client/permissions.py b/lms/djangoapps/django_comment_client/permissions.py
index f0b72e20fe..e2534a2d50 100644
--- a/lms/djangoapps/django_comment_client/permissions.py
+++ b/lms/djangoapps/django_comment_client/permissions.py
@@ -31,20 +31,40 @@ def has_permission(user, permission, course_id=None):
 CONDITIONS = ['is_open', 'is_author', 'is_question_author', 'is_team_member_if_applicable']
 
 
+def get_team(commentable_id):
+    """ Returns the team that the commentable_id belongs to if it exists. Returns None otherwise. """
+    request_cache_dict = RequestCache.get_request_cache().data
+    cache_key = "django_comment_client.team_commentable.{}".format(commentable_id)
+    if cache_key in request_cache_dict:
+        return request_cache_dict[cache_key]
+
+    try:
+        team = CourseTeam.objects.get(discussion_topic_id=commentable_id)
+    except CourseTeam.DoesNotExist:
+        team = None
+
+    request_cache_dict[cache_key] = team
+    return team
+
+
 def _check_condition(user, condition, content):
-    def check_open(user, 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:
             return content and not content['closed']
         except KeyError:
             return False
 
     def check_author(user, content):
+        """ Check if the given user is the author of the content. """
         try:
             return content and content['user_id'] == str(user.id)
         except KeyError:
             return False
 
     def check_question_author(user, content):
+        """ Check if the given user is the author of the original question for both threads and comments. """
         if not content:
             return False
         try:
@@ -69,17 +89,16 @@ def _check_condition(user, condition, content):
             cache_key = "django_comment_client.check_team_member.{}.{}".format(user.id, commentable_id)
             if cache_key in request_cache_dict:
                 return request_cache_dict[cache_key]
-            team = CourseTeam.objects.get(discussion_topic_id=commentable_id)
-            passes_condition = team.users.filter(id=user.id).count() > 0
+            team = get_team(commentable_id)
+            if team is None:
+                passes_condition = True
+            else:
+                passes_condition = team.users.filter(id=user.id).exists()
             request_cache_dict[cache_key] = passes_condition
         except KeyError:
             # We do not expect KeyError in production-- it usually indicates an improper test mock.
             logging.warning("Did not find key commentable_id in content.")
             passes_condition = False
-        except CourseTeam.DoesNotExist:
-            passes_condition = True
-            request_cache_dict[cache_key] = passes_condition
-
         return passes_condition
 
     handlers = {