Merge pull request #8982 from edx/benmcmorran/team-discussion-context

TNL-1943 Support thread context for team discussions

lms/djangoapps/django_comment_client/base/tests.py

@@ -236,7 +236,7 @@ class ViewsTestCaseMixin(object):
             data["depth"] = 0
         self._set_mock_request_data(mock_request, data)
 
-    def create_thread_helper(self, mock_request):
+    def create_thread_helper(self, mock_request, extra_data=None):
         """
         Issues a request to create a thread and verifies the result.
         """
@@ -279,22 +279,27 @@ class ViewsTestCaseMixin(object):
             "anonymous": ["false"],
             "title": ["Hello"],
         }
+        if extra_data:
+            thread.update(extra_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)
         assert_true(mock_request.called)
+        expected_data = {
+            'thread_type': 'discussion',
+            'body': u'this is a post',
+            '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)
         mock_request.assert_called_with(
             'post',
             '{prefix}/i4x-MITx-999-course-Robot_Super_Course/threads'.format(prefix=CS_PREFIX),
-            data={
-                'thread_type': 'discussion',
-                'body': u'this is a post',
-                '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),
-            },
+            data=expected_data,
             params={'request_id': ANY},
             headers=ANY,
             timeout=5
@@ -378,6 +383,10 @@ 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):
+        # create_thread_helper verifies that extra data are passed through to the comments service
+        self.create_thread_helper(mock_request, extra_data={'context': 'standalone'})
+
     def test_delete_comment(self, mock_request):
         self._set_mock_request_data(mock_request, {
             "user_id": str(self.student.id),

lms/djangoapps/django_comment_client/base/views.py

@@ -174,16 +174,21 @@ def create_thread(request, course_id, commentable_id):
     if 'body' not in post or not post['body'].strip():
         return JsonError(_("Body can't be empty"))
 
-    thread = cc.Thread(
-        anonymous=anonymous,
-        anonymous_to_peers=anonymous_to_peers,
-        commentable_id=commentable_id,
-        course_id=course_key.to_deprecated_string(),
-        user_id=request.user.id,
-        thread_type=post["thread_type"],
-        body=post["body"],
-        title=post["title"]
-    )
+    params = {
+        'anonymous': anonymous,
+        'anonymous_to_peers': anonymous_to_peers,
+        'commentable_id': commentable_id,
+        'course_id': course_key.to_deprecated_string(),
+        'user_id': request.user.id,
+        'thread_type': post["thread_type"],
+        'body': post["body"],
+        'title': post["title"],
+    }
+
+    if 'context' in post:
+        params['context'] = post['context']
+
+    thread = cc.Thread(**params)
 
     # Cohort the thread if required
     try:

lms/djangoapps/django_comment_client/forum/tests.py

@@ -104,7 +104,16 @@ 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,
+        group_id=None,
+        group_name=None,
+        commentable_id=None,
+        context=None
+):
     thread_data = {
         "id": thread_id,
         "type": "thread",
@@ -116,7 +125,8 @@ def make_mock_thread_data(course, text, thread_id, num_children, group_id=None,
         "resp_total": 42,
         "resp_skip": 25,
         "resp_limit": 5,
-        "group_id": group_id
+        "group_id": group_id,
+        "context": context if context else "course"
     }
     if group_id is not None:
         thread_data['group_name'] = group_name
@@ -135,7 +145,8 @@ def make_mock_request_impl(
         thread_id="dummy_thread_id",
         group_id=None,
         commentable_id=None,
-        num_thread_responses=1
+        num_thread_responses=1,
+        context=None
 ):
     def mock_request_impl(*args, **kwargs):
         url = args[1]
@@ -149,13 +160,19 @@ def make_mock_request_impl(
                         thread_id=thread_id,
                         num_children=None,
                         group_id=group_id,
-                        commentable_id=commentable_id
+                        commentable_id=commentable_id,
+                        context=context
                     )
                 ]
             }
         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
+                course=course,
+                text=text,
+                thread_id=thread_id,
+                num_children=num_thread_responses,
+                group_id=group_id,
+                context=context
             )
         elif "/users/" in url:
             data = {
@@ -649,6 +666,33 @@ class SingleThreadContentGroupTestCase(ContentGroupTestCase):
         self.assert_can_access(self.non_cohorted_user, self.beta_module.discussion_id, thread_id, False)
 
 
+@patch('lms.lib.comment_client.utils.requests.request')
+class InlineDiscussionContextTestCase(ModuleStoreTestCase):
+    def setUp(self):
+        super(InlineDiscussionContextTestCase, self).setUp()
+        self.course = CourseFactory.create()
+        CourseEnrollmentFactory(user=self.user, course_id=self.course.id)
+
+    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"
+        )
+
+        request = RequestFactory().get("dummy_url")
+        request.user = self.user
+
+        response = views.inline_discussion(
+            request,
+            unicode(self.course.id),
+            "dummy_topic",
+        )
+
+        json_response = json.loads(response.content)
+        self.assertEqual(json_response['discussion_data'][0]['context'], 'standalone')
+
+
 @patch('lms.lib.comment_client.utils.requests.request')
 class InlineDiscussionGroupIdTestCase(
         CohortedTestCase,
@@ -953,6 +997,7 @@ class FollowedThreadsDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGr
         )
 
 
+@patch('lms.lib.comment_client.utils.requests.request')
 class InlineDiscussionTestCase(ModuleStoreTestCase):
     def setUp(self):
         super(InlineDiscussionTestCase, self).setUp()
@@ -969,17 +1014,22 @@ class InlineDiscussionTestCase(ModuleStoreTestCase):
             discussion_target="Discussion1"
         )
 
-    @patch('lms.lib.comment_client.utils.requests.request')
-    def test_courseware_data(self, mock_request):
-        request = RequestFactory().get("dummy_url")
+    def send_request(self, mock_request, params=None):
+        """
+        Creates and returns a request with params set, and configures
+        mock_request to return appropriate values.
+        """
+        request = RequestFactory().get("dummy_url", params if params else {})
         request.user = self.student
         mock_request.side_effect = make_mock_request_impl(
             course=self.course, text="dummy content", commentable_id=self.discussion1.discussion_id
         )
-
-        response = views.inline_discussion(
+        return views.inline_discussion(
             request, self.course.id.to_deprecated_string(), self.discussion1.discussion_id
         )
+
+    def verify_response(self, response):
+        """Verifies that the response contains the appropriate courseware_url and courseware_title"""
         self.assertEqual(response.status_code, 200)
         response_data = json.loads(response.content)
         expected_courseware_url = '/courses/TestX/101/Test_Course/jump_to/i4x://TestX/101/discussion/Discussion1'
@@ -987,6 +1037,14 @@ class InlineDiscussionTestCase(ModuleStoreTestCase):
         self.assertEqual(response_data['discussion_data'][0]['courseware_url'], expected_courseware_url)
         self.assertEqual(response_data["discussion_data"][0]["courseware_title"], expected_courseware_title)
 
+    def test_courseware_data(self, mock_request):
+        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')
+        self.verify_response(response)
+
 
 @patch('requests.request')
 class UserProfileTestCase(ModuleStoreTestCase):

lms/djangoapps/django_comment_client/forum/views.py

@@ -149,6 +149,7 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
                     'flagged',
                     'unread',
                     'unanswered',
+                    'context',
                 ]
             )
         )

lms/djangoapps/django_comment_client/utils.py

@@ -561,7 +561,7 @@ def prepare_content(content, course_key, is_staff=False, course_is_cohorted=None
         'read', 'group_id', 'group_name', 'pinned', 'abuse_flaggers',
         'stats', 'resp_skip', 'resp_limit', 'resp_total', 'thread_type',
         'endorsed_responses', 'non_endorsed_responses', 'non_endorsed_resp_total',
-        'endorsement',
+        'endorsement', 'context'
     ]
 
     if (content.get('anonymous') is False) and ((content.get('anonymous_to_peers') is False) or is_staff):

lms/djangoapps/teams/migrations/0002_auto__add_field_courseteam_discussion_id.py

@@ -0,0 +1,83 @@
+# -*- coding: utf-8 -*-
+from south.utils import datetime_utils as datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'CourseTeam.discussion_topic_id'
+        db.add_column('teams_courseteam', 'discussion_topic_id',
+                      self.gf('django.db.models.fields.CharField')(default='', unique=True, max_length=255),
+                      keep_default=False)
+
+
+    def backwards(self, orm):
+        # Deleting field 'CourseTeam.discussion_topic_id'
+        db.delete_column('teams_courseteam', 'discussion_topic_id')
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'teams.courseteam': {
+            'Meta': {'object_name': 'CourseTeam'},
+            'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'blank': 'True'}),
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
+            'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'description': ('django.db.models.fields.CharField', [], {'max_length': '300'}),
+            'discussion_topic_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'language': ('student.models.LanguageField', [], {'max_length': '16', 'blank': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'team_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
+            'topic_id': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'users': ('django.db.models.fields.related.ManyToManyField', [], {'db_index': 'True', 'related_name': "'teams'", 'symmetrical': 'False', 'through': "orm['teams.CourseTeamMembership']", 'to': "orm['auth.User']"})
+        },
+        'teams.courseteammembership': {
+            'Meta': {'unique_together': "(('user', 'team'),)", 'object_name': 'CourseTeamMembership'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'team': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'membership'", 'to': "orm['teams.CourseTeam']"}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        }
+    }
+
+    complete_apps = ['teams']

lms/djangoapps/teams/models.py

@@ -1,5 +1,7 @@
 """Django models related to teams functionality."""
 
+from uuid import uuid4
+
 from django.contrib.auth.models import User
 from django.db import models
 from django.utils.translation import ugettext_lazy
@@ -15,6 +17,7 @@ class CourseTeam(models.Model):
     """This model represents team related info."""
 
     team_id = models.CharField(max_length=255, unique=True)
+    discussion_topic_id = models.CharField(max_length=255, unique=True)
     name = models.CharField(max_length=255)
     is_active = models.BooleanField(default=True)
     course_id = CourseKeyField(max_length=255, db_index=True)
@@ -48,9 +51,11 @@ class CourseTeam(models.Model):
         """
 
         team_id = generate_unique_readable_id(name, cls.objects.all(), 'team_id')
+        discussion_topic_id = uuid4().hex
 
         course_team = cls(
             team_id=team_id,
+            discussion_topic_id=discussion_topic_id,
             name=name,
             course_id=course_id,
             topic_id=topic_id if topic_id else '',

lms/djangoapps/teams/serializers.py

@@ -44,6 +44,7 @@ class CourseTeamSerializer(serializers.ModelSerializer):
         model = CourseTeam
         fields = (
             "id",
+            "discussion_topic_id",
             "name",
             "is_active",
             "course_id",
@@ -54,7 +55,7 @@ class CourseTeamSerializer(serializers.ModelSerializer):
             "language",
             "membership",
         )
-        read_only_fields = ("course_id", "date_created")
+        read_only_fields = ("course_id", "date_created", "discussion_topic_id")
 
 
 class CourseTeamCreationSerializer(serializers.ModelSerializer):

lms/djangoapps/teams/tests/factories.py

@@ -1,5 +1,7 @@
 """Factories for testing the Teams API."""
 
+from uuid import uuid4
+
 import factory
 from factory.django import DjangoModelFactory
 
@@ -15,5 +17,6 @@ class CourseTeamFactory(DjangoModelFactory):
     FACTORY_DJANGO_GET_OR_CREATE = ('team_id',)
 
     team_id = factory.Sequence('team-{0}'.format)
+    discussion_topic_id = factory.LazyAttribute(lambda a: uuid4().hex)
     name = "Awesome Team"
     description = "A simple description"

lms/djangoapps/teams/tests/test_views.py

@@ -381,6 +381,7 @@ class TestCreateTeamAPI(TeamAPITestCase):
         team = self.post_create_team(status, self.build_team_data(name="New Team"), user=user)
         if status == 200:
             self.assertEqual(team['id'], 'new-team')
+            self.assertIn('discussion_topic_id', team)
             teams = self.get_teams_list(user=user)
             self.assertIn("New Team", [team['name'] for team in teams['results']])
 
@@ -427,8 +428,9 @@ class TestCreateTeamAPI(TeamAPITestCase):
             language='fr'
         ))
 
-        # Remove date_created because it changes between test runs
+        # Remove date_created and discussion_topic_id because they change between test runs
         del team['date_created']
+        del team['discussion_topic_id']
         self.assertEquals(team, {
             'name': 'Fully specified team',
             'language': 'fr',
@@ -458,7 +460,8 @@ class TestDetailTeamAPI(TeamAPITestCase):
     def test_access(self, user, status):
         team = self.get_team_detail(self.test_team_1.team_id, status, user=user)
         if status == 200:
-            self.assertEquals(team['description'], self.test_team_1.description)
+            self.assertEqual(team['description'], self.test_team_1.description)
+            self.assertEqual(team['discussion_topic_id'], self.test_team_1.discussion_topic_id)
 
     def test_does_not_exist(self):
         self.get_team_detail('no_such_team', 404)

lms/lib/comment_client/thread.py

@@ -11,6 +11,7 @@ log = logging.getLogger(__name__)
 
 class Thread(models.Model):
 
+    # accessible_fields can be set and retrieved on the model
     accessible_fields = [
         'id', 'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id',
         'closed', 'tags', 'votes', 'commentable_id', 'username', 'user_id',
@@ -19,19 +20,23 @@ class Thread(models.Model):
         'highlighted_body', 'endorsed', 'read', 'group_id', 'group_name', 'pinned',
         'abuse_flaggers', 'resp_skip', 'resp_limit', 'resp_total', 'thread_type',
         'endorsed_responses', 'non_endorsed_responses', 'non_endorsed_resp_total',
+        'context',
     ]
 
+    # updateable_fields are sent in PUT requests
     updatable_fields = [
         'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id',
         'closed', 'user_id', 'commentable_id', 'group_id', 'group_name', 'pinned', 'thread_type'
     ]
 
+    # metric_tag_fields are used by Datadog to record metrics about the model
     metric_tag_fields = [
         'course_id', 'group_id', 'pinned', 'closed', 'anonymous', 'anonymous_to_peers',
         'endorsed', 'read'
     ]
 
-    initializable_fields = updatable_fields + ['thread_type']
+    # initializable_fields are sent in POST requests
+    initializable_fields = updatable_fields + ['thread_type', 'context']
 
     base_url = "{prefix}/threads".format(prefix=settings.PREFIX)
     default_retrieve_params = {'recursive': False}