Merge pull request #52 from edx-solutions/mattdrayer/api-groupidfix

Mattdrayer/api groupidfix: Use group_id in response versus group_profile_id

lms/djangoapps/api_manager/courses/tests.py

@@ -8,6 +8,7 @@ import simplejson as json
 import uuid
 from random import randint
 
+from django.contrib.auth.models import Group
 from django.core.cache import cache
 from django.test import TestCase, Client
 from django.test.utils import override_settings
@@ -923,21 +924,32 @@ class CoursesApiTests(TestCase):
         test_uri = '{}/{}/groups'.format(self.base_course_content_uri, self.course_project.id)
         data = {'name': 'Alpha Group', 'type': 'test'}
         response = self.do_post(self.base_groups_uri, data)
-        group_id = response.data['id']
-        data = {'group_id': group_id}
+        alpha_group_id = response.data['id']
+        data = {'group_id': alpha_group_id}
         response = self.do_post(test_uri, data)
         self.assertEqual(response.status_code, 201)
 
+        # Add a profile-less group to the system to offset the identifiers
+        Group.objects.create(name='Offset Group')
+
         data = {'name': 'Beta Group', 'type': 'project'}
         response = self.do_post(self.base_groups_uri, data)
-        group_id = response.data['id']
-        data = {'group_id': group_id}
+
+        data = {'name': 'Delta Group', 'type': 'project'}
+        response = self.do_post(self.base_groups_uri, data)
+
+        data = {'name': 'Gamma Group', 'type': 'project'}
+        response = self.do_post(self.base_groups_uri, data)
+        gamma_group_id = response.data['id']
+        data = {'group_id': gamma_group_id}
         response = self.do_post(test_uri, data)
         self.assertEqual(response.status_code, 201)
 
         response = self.do_get(test_uri)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 2)
+        self.assertEqual(response.data[0]['group_id'], alpha_group_id)
+        self.assertEqual(response.data[1]['group_id'], gamma_group_id)
 
         test_uri = test_uri + '?type=project'
         response = self.do_get(test_uri)

lms/djangoapps/api_manager/courses/views.py

@@ -121,17 +121,6 @@ def _serialize_content_children(request, course_id, children):
     return data
 
 
-def _serialize_content_groups(request, course_id, content_id, groups):
-    """
-    Loads the specified content group data into the response dict
-    This should probably evolve to use DRF serializers
-    """
-    return [
-        {'course_id': course_id, 'content_id': content_id, 'group_id': group.group_id}
-        for group in groups
-    ]
-
-
 def _serialize_content_with_children(request, course_descriptor, descriptor, depth):
     data = _serialize_content(
         request,
@@ -760,31 +749,34 @@ class CourseContentGroupsList(APIView):
         if group_id is None:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
         try:
-            existing_group = GroupProfile.objects.get(group_id=group_id)
+            existing_profile = GroupProfile.objects.get(group_id=group_id)
         except ObjectDoesNotExist:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
         try:
             existing_relationship = CourseContentGroupRelationship.objects.get(
                 course_id=course_id,
                 content_id=content_id,
-                group=existing_group
+                group_profile=existing_profile
             )
         except ObjectDoesNotExist:
             existing_relationship = None
         response_data = {}
         base_uri = _generate_base_uri(request)
-        response_data['uri'] = '{}/{}'.format(base_uri, existing_group.group_id)
+        response_data['uri'] = '{}/{}'.format(base_uri, existing_profile.group_id)
         if existing_relationship:
             response_data['message'] = "Relationship already exists."
             return Response(response_data, status=status.HTTP_409_CONFLICT)
         CourseContentGroupRelationship.objects.create(
             course_id=course_id,
             content_id=content_id,
-            group=existing_group
+            group_profile=existing_profile
         )
+        rels = CourseContentGroupRelationship.objects.all()
+        for rel in rels:
+            print rel.__dict__
         response_data['course_id'] = course_descriptor.id
         response_data['content_id'] = existing_content.id
-        response_data['group_id'] = str(existing_group.group_id)
+        response_data['group_id'] = str(existing_profile.group_id)
         return Response(response_data, status=status.HTTP_201_CREATED)
 
     def get(self, request, course_id, content_id):
@@ -794,7 +786,6 @@ class CourseContentGroupsList(APIView):
         """
         response_data = []
         group_type = request.QUERY_PARAMS.get('type')
-
         try:
             course_descriptor = get_course(course_id)
         except ValueError:
@@ -804,18 +795,16 @@ class CourseContentGroupsList(APIView):
             existing_content = store.get_instance(course_id, Location(content_id))
         except InvalidLocationError:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
-        groups = CourseContentGroupRelationship.objects.filter(
+        relationships = CourseContentGroupRelationship.objects.filter(
             course_id=course_id,
-            content_id=content_id
-        )
+            content_id=content_id,
+        ).select_related("groupprofile")
         if group_type:
-            groups = groups.filter(group__group_type=group_type)
-        response_data = _serialize_content_groups(
-            request,
-            course_id,
-            content_id,
-            groups
-        )
+            relationships = relationships.filter(group_profile__group_type=group_type)
+        response_data = [
+            {'course_id': course_id, 'content_id': content_id, 'group_id': relationship.group_profile.group_id}
+            for relationship in relationships
+        ]
         return Response(response_data, status=status.HTTP_200_OK)
 
 
@@ -837,10 +826,10 @@ class CourseContentGroupsDetail(APIView):
             return Response({}, status=status.HTTP_404_NOT_FOUND)
 
         try:
-            groups = CourseContentGroupRelationship.objects.get(
+            relationship = CourseContentGroupRelationship.objects.get(
                 course_id=course_id,
                 content_id=content_id,
-                group=group_id
+                group_profile__group_id=group_id
             )
         except ObjectDoesNotExist:
             return Response({}, status=status.HTTP_404_NOT_FOUND)
@@ -856,37 +845,38 @@ class CourseContentUsersList(generics.ListAPIView):
     """
     ### The CourseContentUsersList view allows clients to users enrolled and
     users not enrolled for course within all groups of course
-    - URI: ```/api/courses/{course_id}/content/{content_id}/users?enrolled={enrolment_status}&group_id={group_id}&type={group_type}```
+    - URI: ```/api/courses/{course_id}/content/{content_id}/users
+        * enrolled: boolean, filters user set by enrollment status
+        * group_id: numeric, filters user set by membership in a specific group
+        * type: string, filters user set by membership in groups matching the specified type
     - GET: Returns a JSON representation of users enrolled or not enrolled
     ### Use Cases/Notes:
-    * Use CourseContentUsersList to grab the users enrolled in Course content group
-    * Use CourseContentUsersList to grab the users not enrolled in Course content group
+    * Filtering related Users by enrollement status should be self-explanatory
+    * An example of specific group filtering is to get the set of users who are members of a particular workgroup related to the content
+    * An example of group type filtering is to get all users who are members of an organization group related to the content
     """
     permission_classes = (ApiKeyHeaderPermission,)
     serializer_class = UserSerializer
 
     def get_queryset(self):
         """
-        GET retrieves the list of users who registered for a given course content
-        and list of users who are not registered for that group course content.
-        'enrolled' query parameter for filtering user' enrolment status
-        'group_id' query parameter is available for filtering by group.
-        'type' query parameter is available for filtering by group_type.
+        GET /api/courses/{course_id}/content/{content_id}/users
         """
         course_id = self.kwargs['course_id']
         content_id = self.kwargs['content_id']
         enrolled = self.request.QUERY_PARAMS.get('enrolled', 'True')
         group_type = self.request.QUERY_PARAMS.get('type', None)
         group_id = self.request.QUERY_PARAMS.get('group_id', None)
-        groups = CourseContentGroupRelationship.objects.filter(course_id=course_id, content_id=content_id)
+        relationships = CourseContentGroupRelationship.objects.filter(
+            course_id=course_id, content_id=content_id).select_related("groupprofile")
 
         if group_id:
-            groups = groups.filter(group__group__id=group_id)
+            relationships = relationships.filter(group_profile__group__id=group_id)
 
         if group_type:
-            groups = groups.filter(group__group_type=group_type)
+            relationships = relationships.filter(group_profile__group_type=group_type)
 
-        lookup_group_ids = groups.values_list('group_id', flat=True)
+        lookup_group_ids = relationships.values_list('group_profile', flat=True)
         users = User.objects.filter(groups__id__in=lookup_group_ids)
         enrolled_users = CourseEnrollment.users_enrolled_in(course_id).filter(groups__id__in=lookup_group_ids)
         if enrolled in ['True', 'true']:

lms/djangoapps/api_manager/migrations/0007_groupidfix.py

+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Removing unique constraint on 'CourseContentGroupRelationship', fields ['course_id', 'content_id', 'group']
+        db.delete_unique('api_manager_coursecontentgrouprelationship', ['course_id', 'content_id', 'group_id'])
+
+        # Deleting field 'CourseContentGroupRelationship.group'
+        db.delete_column('api_manager_coursecontentgrouprelationship', 'group_id')
+
+        # Adding field 'CourseContentGroupRelationship.group_profile'
+        db.add_column('api_manager_coursecontentgrouprelationship', 'group_profile',
+                      self.gf('django.db.models.fields.related.ForeignKey')(default=0, to=orm['api_manager.GroupProfile']),
+                      keep_default=False)
+
+        # Adding unique constraint on 'CourseContentGroupRelationship', fields ['course_id', 'content_id', 'group_profile']
+        db.create_unique('api_manager_coursecontentgrouprelationship', ['course_id', 'content_id', 'group_profile_id'])
+
+
+    def backwards(self, orm):
+        # Removing unique constraint on 'CourseContentGroupRelationship', fields ['course_id', 'content_id', 'group_profile']
+        db.delete_unique('api_manager_coursecontentgrouprelationship', ['course_id', 'content_id', 'group_profile_id'])
+
+        # Adding field 'CourseContentGroupRelationship.group'
+        db.add_column('api_manager_coursecontentgrouprelationship', 'group',
+                      self.gf('django.db.models.fields.related.ForeignKey')(default=0, to=orm['api_manager.GroupProfile']),
+                      keep_default=False)
+
+        # Deleting field 'CourseContentGroupRelationship.group_profile'
+        db.delete_column('api_manager_coursecontentgrouprelationship', 'group_profile_id')
+
+        # Adding unique constraint on 'CourseContentGroupRelationship', fields ['course_id', 'content_id', 'group']
+        db.create_unique('api_manager_coursecontentgrouprelationship', ['course_id', 'content_id', 'group_id'])
+
+
+    models = {
+        'api_manager.coursecontentgrouprelationship': {
+            'Meta': {'unique_together': "(('course_id', 'content_id', 'group_profile'),)", 'object_name': 'CourseContentGroupRelationship'},
+            'content_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'group_profile': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['api_manager.GroupProfile']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'record_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'})
+        },
+        'api_manager.coursegrouprelationship': {
+            'Meta': {'object_name': 'CourseGroupRelationship'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'group': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.Group']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'record_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'})
+        },
+        'api_manager.groupprofile': {
+            'Meta': {'object_name': 'GroupProfile', 'db_table': "'auth_groupprofile'"},
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'data': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'group': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.Group']"}),
+            'group_type': ('django.db.models.fields.CharField', [], {'max_length': '32', 'null': 'True', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'record_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'})
+        },
+        'api_manager.grouprelationship': {
+            'Meta': {'object_name': 'GroupRelationship'},
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'group': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.Group']", 'unique': 'True', 'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'parent_group': ('django.db.models.fields.related.ForeignKey', [], {'default': '0', 'related_name': "'child_groups'", 'null': 'True', 'blank': 'True', 'to': "orm['api_manager.GroupRelationship']"}),
+            'record_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'})
+        },
+        'api_manager.linkedgrouprelationship': {
+            'Meta': {'object_name': 'LinkedGroupRelationship'},
+            'created': ('model_utils.fields.AutoCreatedField', [], {'default': 'datetime.datetime.now'}),
+            'from_group_relationship': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'from_group_relationships'", 'to': "orm['api_manager.GroupRelationship']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'modified': ('model_utils.fields.AutoLastModifiedField', [], {'default': 'datetime.datetime.now'}),
+            'record_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'to_group_relationship': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'to_group_relationships'", 'to': "orm['api_manager.GroupRelationship']"})
+        },
+        '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'})
+        },
+        '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'})
+        }
+    }
+
+    complete_apps = ['api_manager']
\ No newline at end of file

lms/djangoapps/api_manager/models.py

@@ -118,11 +118,11 @@ class CourseContentGroupRelationship(TimeStampedModel):
     """
     course_id = models.CharField(max_length=255, db_index=True)
     content_id = models.CharField(max_length=255, db_index=True)
-    group = models.ForeignKey(GroupProfile, db_index=True)
+    group_profile = models.ForeignKey(GroupProfile, db_index=True)
     record_active = models.BooleanField(default=True)
 
     class Meta:
         """
         Mapping model to enable grouping of course content such as chapters
         """
-        unique_together = ("course_id", "content_id", "group")
+        unique_together = ("course_id", "content_id", "group_profile")