diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index 041b6e0e24..07458a17c5 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/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), diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index f4c16280ba..5456e970db 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/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: diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 5095355d46..e542c4ba39 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/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): diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index aeb4a7ba75..0b2e1249c7 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/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', ] ) ) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index d1a0ccc42d..f403be3c9d 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/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): diff --git a/lms/djangoapps/teams/migrations/0002_auto__add_field_courseteam_discussion_id.py b/lms/djangoapps/teams/migrations/0002_auto__add_field_courseteam_discussion_id.py new file mode 100644 index 0000000000..f21feaba4f --- /dev/null +++ b/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'] \ No newline at end of file diff --git a/lms/djangoapps/teams/models.py b/lms/djangoapps/teams/models.py index 51f4fe9721..64c104b026 100644 --- a/lms/djangoapps/teams/models.py +++ b/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 '', diff --git a/lms/djangoapps/teams/serializers.py b/lms/djangoapps/teams/serializers.py index b0544fdb70..0ffa224b61 100644 --- a/lms/djangoapps/teams/serializers.py +++ b/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): diff --git a/lms/djangoapps/teams/tests/factories.py b/lms/djangoapps/teams/tests/factories.py index 23a04343a3..d3cc339ef2 100644 --- a/lms/djangoapps/teams/tests/factories.py +++ b/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" diff --git a/lms/djangoapps/teams/tests/test_views.py b/lms/djangoapps/teams/tests/test_views.py index 32d76b76e1..85db7abf48 100644 --- a/lms/djangoapps/teams/tests/test_views.py +++ b/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) diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 84839b87d0..2811f8975a 100644 --- a/lms/lib/comment_client/thread.py +++ b/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}