Implement forum cohort visibility labels correctly

TNL-25

lms/djangoapps/django_comment_client/base/views.py

@@ -37,6 +37,7 @@ from django_comment_client.utils import (
     get_group_id_for_comments_service,
     get_discussion_categories_ids,
     get_discussion_id_map,
+    add_thread_group_name
 )
 from django_comment_client.permissions import check_permissions_by_view, has_permission
 from eventtracking import tracker
@@ -260,7 +261,8 @@ def create_thread(request, course_id, commentable_id):
     _update_user_engagement_score(course_key, request.user.id)
 
     add_courseware_context([data], course, user)
-
+    add_thread_group_name(data, course_key)
+    add_courseware_context([data], course)
     if request.is_ajax():
         return ajax_content_response(request, course_key, data)
     else:

lms/djangoapps/django_comment_client/forum/tests.py

@@ -105,7 +105,9 @@ class ViewsExceptionTestCase(UrlResetMixin, ModuleStoreTestCase):
         self.assertEqual(self.response.status_code, 404)
 
 
-def make_mock_thread_data(course, text, thread_id, num_children, group_id=None, group_name=None, commentable_id=None):
+def make_mock_thread_data(
+        course, text, thread_id, num_children, include_children, group_id=None, group_name=None, commentable_id=None
+):
     thread_data = {
         "id": thread_id,
         "type": "thread",
@@ -122,6 +124,9 @@ def make_mock_thread_data(course, text, thread_id, num_children, group_id=None,
     if group_id is not None:
         thread_data['group_name'] = group_name
     if num_children is not None:
+        thread_data['group_id'] = group_id
+        thread_data['group_name'] = group_name
+    if include_children:
         thread_data["children"] = [{
             "id": "dummy_comment_id_{}".format(i),
             "type": "comment",
@@ -150,14 +155,17 @@ def make_mock_request_impl(
                         thread_id=thread_id,
                         num_children=None,
                         group_id=group_id,
-                        commentable_id=commentable_id
+                        commentable_id=commentable_id,
+                        include_children=False
                     )
                 ]
             }
         elif thread_id and url.endswith(thread_id):
-            data = make_mock_thread_data(
-                course=course, text=text, thread_id=thread_id, num_children=num_thread_responses, group_id=group_id
-            )
+            data = {
+                "collection": [make_mock_thread_data(course=course, text=text, thread_id=thread_id,
+                                                     num_children=num_thread_responses, include_children=False,
+                                                     group_id=group_id)]
+            }
         elif "/users/" in url:
             data = {
                 "default_sort_key": "date",
@@ -228,7 +236,9 @@ class SingleThreadTestCase(ModuleStoreTestCase):
         # django view performs prior to writing thread data to the response
         self.assertEquals(
             response_data["content"],
-            strip_none(make_mock_thread_data(course=self.course, text=text, thread_id=thread_id, num_children=1))
+            strip_none(make_mock_thread_data(
+                course=self.course, text=text, thread_id=thread_id, num_children=1, include_children=False
+            ))
         )
         mock_request.assert_called_with(
             "get",
@@ -408,7 +418,8 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
                 thread_id=self.mock_thread_id,
                 num_children=1,
                 group_id=self.student_cohort.id,
-                group_name=self.student_cohort.name
+                group_name=self.student_cohort.name,
+                include_children=False,
             )
         )
 
@@ -990,6 +1001,74 @@ class InlineDiscussionTestCase(ModuleStoreTestCase):
 
 
 @patch('requests.request')
+class SingleCohortedThreadTestCase(ModuleStoreTestCase):
+    def setUp(self):
+        self.course = CourseFactory.create()
+        self.student = UserFactory.create()
+        CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
+        self.student_cohort = CourseUserGroup.objects.create(
+            name="student_cohort",
+            course_id=self.course.id,
+            group_type=CourseUserGroup.COHORT
+        )
+
+    def _create_mock_cohorted_thread(self, mock_request):
+        self.mock_text = "dummy content"
+        self.mock_thread_id = "test_thread_id"
+        mock_request.side_effect = make_mock_request_impl(
+            self.mock_text, self.mock_thread_id,
+            group_id=self.student_cohort.id
+        )
+
+    def test_ajax(self, mock_request):
+        self._create_mock_cohorted_thread(mock_request)
+
+        request = RequestFactory().get(
+            "dummy_url",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest"
+        )
+        request.user = self.student
+        response = views.single_thread(
+            request,
+            self.course.id.to_deprecated_string(),
+            "dummy_discussion_id",
+            self.mock_thread_id
+        )
+
+        self.assertEquals(response.status_code, 200)
+        response_data = json.loads(response.content)
+        self.assertEquals(
+            response_data["content"],
+            make_mock_thread_data(
+                self.course,
+                self.mock_text, self.mock_thread_id, 1, True,
+                group_id=self.student_cohort.id,
+                group_name=self.student_cohort.name,
+            )
+        )
+
+    def test_html(self, mock_request):
+        self._create_mock_cohorted_thread(mock_request)
+
+        request = RequestFactory().get("dummy_url")
+        request.user = self.student
+        mako_middleware_process_request(request)
+        response = views.single_thread(
+            request,
+            self.course.id.to_deprecated_string(),
+            "dummy_discussion_id",
+            self.mock_thread_id
+        )
+
+        self.assertEquals(response.status_code, 200)
+        self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8')
+        html = response.content
+
+        # Verify that the group name is correctly included in the HTML
+        self.assertRegexpMatches(html, r'"group_name": "student_cohort"')
+
+
+@patch('requests.request')
 class UserProfileTestCase(ModuleStoreTestCase):
 
     TEST_THREAD_TEXT = 'userprofile-test-text'

lms/djangoapps/django_comment_client/forum/views.py

@@ -24,6 +24,7 @@ from openedx.core.djangoapps.course_groups.cohorts import (
     get_cohort_id,
     get_course_cohorts,
     is_commentable_cohorted,
+    get_cohorted_threads_privacy,
     get_cohorted_commentables)
 from courseware.tabs import EnrolledTab
 from courseware.access import has_access
@@ -38,6 +39,8 @@ from django_comment_client.utils import (
     add_courseware_context,
     get_group_id_for_comments_service
 )
+
+from django_comment_client.utils import (merge_dict, extract, strip_none, add_courseware_context, add_thread_group_name)
 import django_comment_client.utils as utils
 import lms.lib.comment_client as cc
 
@@ -110,6 +113,7 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
         'sort_order': 'desc',
         'text': '',
         'course_id': unicode(course.id),
+        'commentable_id': discussion_id,
         'user_id': request.user.id,
         'group_id': get_group_id_for_comments_service(request, course.id, discussion_id),  # may raise ValueError
     }
@@ -135,6 +139,24 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
     #is user a moderator
     #did the user request a group
 
+    #if the user requested a group explicitly, give them that group, otherwise, if mod, show all, else if student, use cohort
+
+    group_id = request.GET.get('group_id')
+
+    if group_id == "all":
+        group_id = None
+
+    if not group_id:
+        if not has_permission(request.user, "see_all_cohorts", course.id):
+            group_id = get_cohort_id(request.user, course.id)
+            if not group_id and get_cohorted_threads_privacy(course.id) == 'cohort-only':
+                default_query_params['exclude_groups'] = True
+
+    if group_id:
+        default_query_params["group_id"] = group_id
+
+    #so by default, a moderator sees all items, and a student sees his cohort
+
     query_params = merge_dict(
         default_query_params,
         strip_none(
@@ -307,6 +329,7 @@ def single_thread(request, course_key, discussion_id, thread_id):
 
     course = get_course_with_access(request.user, 'load', course_key, check_if_enrolled=True)
     course_settings = make_course_settings(course, request.user)
+
     cc_user = cc.User.from_django_user(request.user)
     user_info = cc_user.to_dict()
     is_moderator = has_permission(request.user, "see_all_cohorts", course_key)
@@ -341,6 +364,8 @@ def single_thread(request, course_key, discussion_id, thread_id):
         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)
         content = utils.prepare_content(thread.to_dict(), course_key, is_staff)
+        content = utils.safe_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({
@@ -353,12 +378,14 @@ def single_thread(request, course_key, discussion_id, thread_id):
             threads, query_params = get_threads(request, course)
         except ValueError:
             return HttpResponseBadRequest("Invalid group_id")
+        threads, query_params = get_threads(request, course_key)
         threads.append(thread.to_dict())
 
         with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
             add_courseware_context(threads, course, request.user)
 
         for thread in threads:
+            add_thread_group_name(thread, course_key)
             # patch for backward compatibility with comments service
             if "pinned" not in thread:
                 thread["pinned"] = False
@@ -395,11 +422,11 @@ def single_thread(request, course_key, discussion_id, thread_id):
             'sort_preference': cc_user.default_sort_key,
             'category_map': course_settings["category_map"],
             'course_settings': _attr_safe_json(course_settings),
-            'cohorted_commentables': (get_cohorted_commentables(course_id)),
-            'has_permission_to_create_thread': cached_has_permission(request.user, "create_thread", course_id),
-            'has_permission_to_create_comment': cached_has_permission(request.user, "create_comment", course_id),
-            'has_permission_to_create_subcomment': cached_has_permission(request.user, "create_subcomment", course_id),
-            'has_permission_to_openclose_thread': cached_has_permission(request.user, "openclose_thread", course_id)
+            'cohorted_commentables': (get_cohorted_commentables(course.id)),
+            'has_permission_to_create_thread': has_permission(request.user, "create_thread", course.id),
+            'has_permission_to_create_comment': has_permission(request.user, "create_comment", course.id),
+            'has_permission_to_create_subcomment': has_permission(request.user, "create_subcomment", course.id),
+            'has_permission_to_openclose_thread': has_permission(request.user, "openclose_thread", course.id)
         }
         return render_to_response('discussion/index.html', context)
 

lms/djangoapps/django_comment_client/utils.py

@@ -2,6 +2,7 @@ from collections import defaultdict
 from datetime import datetime
 import json
 import logging
+import string
 
 import pytz
 from django.contrib.auth.models import User
@@ -25,6 +26,11 @@ from openedx.core.djangoapps.course_groups.cohorts import (
 )
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup
 
+from xmodule.modulestore.django import modulestore
+from django.utils.timezone import UTC
+from opaque_keys.edx.locations import i4xEncoder
+from opaque_keys.edx.keys import CourseKey
+import json
 
 log = logging.getLogger(__name__)
 
@@ -584,6 +590,31 @@ def get_group_id_for_comments_service(request, course_key, commentable_id=None):
         return None
 
 
+def add_thread_group_name(thread_info, course_key):
+    """
+    Augment the specified thread info to include the group name if a group id is present.
+    """
+    if thread_info.get('group_id') is not None:
+        thread_info['group_name'] = get_cohort_by_id(course_key, thread_info.get('group_id')).name
+
+
+def format_filename(filename):
+    """Take a string and return a valid filename constructed from the string.
+    Uses a whitelist approach: any characters not present in valid_chars are
+    removed. Also spaces are replaced with underscores.
+
+    Note: this method may produce invalid filenames such as ``, `.` or `..`
+    When I use this method I prepend a date string like '2009_01_15_19_46_32_'
+    and append a file extension like '.txt', so I avoid the potential of using
+    an invalid filename.
+
+    https://gist.github.com/seanh/93666
+    """
+    valid_chars = "-_.() %s%s" % (string.ascii_letters, string.digits)
+    filename = ''.join(c for c in filename if c in valid_chars)
+    filename = filename.replace(' ', '_')
+    return filename
+
 def is_comment_too_deep(parent):
     """
     Determine whether a comment with the given parent violates MAX_COMMENT_DEPTH

openedx/core/djangoapps/course_groups/cohorts.py

@@ -326,6 +326,16 @@ 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