api: writing custom query set

Fixed failing test

more test and improvements to code quality

lms/djangoapps/api_manager/groups/views.py

@@ -3,16 +3,14 @@ import uuid
 import json
 from collections import OrderedDict
 
-from django.contrib.auth.models import Group, User
+from django.contrib.auth.models import Group
 from django.core.exceptions import ObjectDoesNotExist
 from django.utils import timezone
 
 from rest_framework import status
 from rest_framework.response import Response
 
-
-
-from api_manager.models import GroupRelationship, CourseGroupRelationship, GroupProfile
+from api_manager.models import GroupRelationship, CourseGroupRelationship, GroupProfile, APIUser as User
 from xmodule.modulestore.django import modulestore
 from api_manager.permissions import SecureAPIView
 from xmodule.modulestore import Location, InvalidLocationError

lms/djangoapps/api_manager/models.py

@@ -6,6 +6,7 @@ from django.contrib.auth.models import Group, User
 from django.db import models
 from django.utils import timezone
 from model_utils.models import TimeStampedModel
+from .utils import is_int
 
 from projects.models import Workgroup
 
@@ -154,3 +155,30 @@ class CourseModuleCompletion(TimeStampedModel):
     user = models.ForeignKey(User, db_index=True, related_name="course_completions")
     course_id = models.CharField(max_length=255, db_index=True)
     content_id = models.CharField(max_length=255, db_index=True)
+
+
+class APIUserQuerySet(models.query.QuerySet):  # pylint: disable=R0924
+    """ Custom QuerySet to modify id based lookup """
+    def filter(self, *args, **kwargs):
+        if 'id' in kwargs and not is_int(kwargs['id']):
+            kwargs['anonymoususerid__anonymous_user_id'] = kwargs['id']
+            del kwargs['id']
+        return super(APIUserQuerySet, self).filter(*args, **kwargs)
+
+
+class APIUserManager(models.Manager):
+    """ Custom Manager """
+    def get_query_set(self):
+        return APIUserQuerySet(self.model)
+
+
+class APIUser(User):
+    """
+    A proxy model for django's auth.User to add AnonymousUserId fallback
+    support in User lookups
+    """
+    objects = APIUserManager()
+
+    class Meta:
+        """ Meta attribute to make this a proxy model"""
+        proxy = True

lms/djangoapps/api_manager/users/serializers.py

 """ Django REST Framework Serializers """
 
-from django.contrib.auth.models import User
-
+from api_manager.models import APIUser
 from rest_framework import serializers
 
 
@@ -9,6 +8,6 @@ class UserSerializer(serializers.ModelSerializer):
     """ Serializer for User model interactions """
     class Meta:
         """ Serializer/field specification """
-        model = User
+        model = APIUser
         fields = ("id", "email", "username", "first_name", "last_name")
         read_only_fields = ("id", "email", "username")

lms/djangoapps/api_manager/users/tests.py

@@ -13,6 +13,8 @@ from django.utils.translation import ugettext as _
 from django.core.cache import cache
 from django.test import TestCase, Client
 from django.test.utils import override_settings
+from student.tests.factories import UserFactory
+from student.models import anonymous_id_for_user
 from projects.models import Project
 
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
@@ -68,6 +70,7 @@ class UsersApiTests(TestCase):
             content_id=self.course_content.id + 'b2'
         )
 
+        self.user = UserFactory()
         self.client = SecureClient()
         cache.clear()
 
@@ -950,7 +953,8 @@ class UsersApiTests(TestCase):
             str(response.data['year_of_birth']), data["year_of_birth"])
 
     def test_user_organizations_list(self):
-        user_id = self._create_test_user()
+        user_id = self.user.id
+        anonymous_id = anonymous_id_for_user(self.user, self.course.id)
         for i in xrange(1, 7):
             data = {
                 'name': 'Org ' + str(i),
@@ -966,13 +970,20 @@ class UsersApiTests(TestCase):
         self.assertEqual(len(response.data['results']), 6)
         self.assertEqual(response.data['num_pages'], 1)
 
+        # test with anonymous user id
+        test_uri = '/api/users/{}/organizations/'.format(anonymous_id)
+        response = self.do_get(test_uri)
+        self.assertEqual(response.data['count'], 6)
+
         # test with invalid user
         response = self.do_get('/api/users/4356340/organizations/')
         self.assertEqual(response.status_code, 404)
 
     def test_user_workgroups_list(self):
         test_workgroups_uri = '/api/workgroups/'
-        user_id = self._create_test_user()
+        user_id = self.user.id
+        # create anonymous user
+        anonymous_id = anonymous_id_for_user(self.user, self.course.id)
         for i in xrange(1, 12):
             project_id = self.test_project.id
             if i > 7:  # set to other project
@@ -989,13 +1000,14 @@ class UsersApiTests(TestCase):
             response = self.do_post(users_uri, data)
             self.assertEqual(response.status_code, 201)
 
-        test_uri = '/api/users/{}/workgroups/?page_size=10'.format(user_id)
+        # test with anonymous user id
+        test_uri = '/api/users/{}/workgroups/?page_size=10'.format(anonymous_id)
         response = self.do_get(test_uri)
         self.assertEqual(response.data['count'], 11)
         self.assertEqual(len(response.data['results']), 10)
         self.assertEqual(response.data['num_pages'], 2)
 
-        # test with course_id filter
+        # test with course_id filter and integer user id
         response = self.do_get('/api/users/{}/workgroups/?course_id={}'.format(user_id, self.course.id))
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data['count'], 7)

lms/djangoapps/api_manager/users/urls.py

@@ -8,16 +8,16 @@ from api_manager.users import views as users_views
 urlpatterns = patterns(
     '',
     url(r'/*$^', users_views.UsersList.as_view(), name='apimgr-users-list'),
-    url(r'^(?P<user_id>[0-9]+)$', users_views.UsersDetail.as_view(), name='apimgr-users-detail'),
-    url(r'^(?P<user_id>[0-9]+)/courses/*$', users_views.UsersCoursesList.as_view(), name='users-courses-list'),
-    url(r'^(?P<user_id>[0-9]+)/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)$', users_views.UsersCoursesDetail.as_view(), name='users-courses-detail'),
-    url(r'^(?P<user_id>[0-9]+)/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/grades$', users_views.UsersCoursesGradesDetail.as_view(), name='users-courses-grades-detail'),
-    url(r'^(?P<user_id>[0-9]+)/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/metrics/social/$', users_views.UsersSocialMetrics.as_view(), name='users-social-metrics'),
-    url(r'^(?P<user_id>[0-9]+)/groups/*$', users_views.UsersGroupsList.as_view(), name='users-groups-list'),
-    url(r'^(?P<user_id>[0-9]+)/groups/(?P<group_id>[0-9]+)$', users_views.UsersGroupsDetail.as_view(), name='users-groups-detail'),
-    url(r'^(?P<user_id>[0-9]+)/preferences$', users_views.UsersPreferences.as_view(), name='users-preferences-list'),
-    url(r'^(?P<user_id>[0-9]+)/organizations/$', users_views.UsersOrganizationsList.as_view(), name='users-organizations-list'),
-    url(r'^(?P<user_id>[0-9]+)/workgroups/$', users_views.UsersWorkgroupsList.as_view(), name='users-workgroups-list'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)$', users_views.UsersDetail.as_view(), name='apimgr-users-detail'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/courses/*$', users_views.UsersCoursesList.as_view(), name='users-courses-list'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)$', users_views.UsersCoursesDetail.as_view(), name='users-courses-detail'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/grades$', users_views.UsersCoursesGradesDetail.as_view(), name='users-courses-grades-detail'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/metrics/social/$', users_views.UsersSocialMetrics.as_view(), name='users-social-metrics'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/groups/*$', users_views.UsersGroupsList.as_view(), name='users-groups-list'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/groups/(?P<group_id>[0-9]+)$', users_views.UsersGroupsDetail.as_view(), name='users-groups-detail'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/preferences$', users_views.UsersPreferences.as_view(), name='users-preferences-list'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/organizations/$', users_views.UsersOrganizationsList.as_view(), name='users-organizations-list'),
+    url(r'^(?P<user_id>[a-zA-Z0-9]+)/workgroups/$', users_views.UsersWorkgroupsList.as_view(), name='users-workgroups-list'),
 )
 
 urlpatterns = format_suffix_patterns(urlpatterns)

lms/djangoapps/api_manager/users/views.py

@@ -2,7 +2,7 @@
 
 import logging
 
-from django.contrib.auth.models import User, Group
+from django.contrib.auth.models import Group
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import IntegrityError
 from django.core.validators import validate_email, validate_slug, ValidationError
@@ -15,7 +15,7 @@ from rest_framework.response import Response
 from django.db.models import Q
 
 from api_manager.permissions import SecureAPIView, SecureListAPIView
-from api_manager.models import GroupProfile
+from api_manager.models import GroupProfile, APIUser as User
 from api_manager.organizations.serializers import OrganizationSerializer
 from api_manager.utils import generate_base_uri
 from projects.serializers import BasicWorkgroupSerializer
@@ -525,7 +525,7 @@ class UsersGroupsList(SecureAPIView):
             return Response(response_data, status=status.HTTP_409_CONFLICT)
         except ObjectDoesNotExist:
             existing_user.groups.add(existing_group.id)
-            response_data['uri'] = '{}/{}'.format(base_uri, existing_user.id)
+            response_data['uri'] = '{}/{}'.format(base_uri, existing_group.id)
             response_data['group_id'] = str(existing_group.id)
             response_data['user_id'] = str(existing_user.id)
             return Response(response_data, status=status.HTTP_201_CREATED)
@@ -813,7 +813,7 @@ class UsersPreferences(SecureAPIView):
     * POSTing a duplicate preference will cause the existing preference to be overwritten (effectively a PUT operation)
     """
 
-    def get(self, request, user_id): # pylint: disable=W0613
+    def get(self, request, user_id):  # pylint: disable=W0613
         """
         GET returns the preferences for the specified user
         """

lms/djangoapps/api_manager/utils.py

@@ -44,3 +44,14 @@ def generate_base_uri(request, strip_qs=False):
         return request.build_absolute_uri(request.path)  # Don't need querystring that why giving location parameter
     else:
         return request.build_absolute_uri()
+
+
+def is_int(value):
+    """
+    checks if a string value can be interpreted as integer
+    """
+    try:
+        int(value)
+        return True
+    except ValueError:
+        return False