Merge pull request #173 from edx/edx/kdmccormick/post-course-summaries

EDUCATOR-464: Add POST method to course_summaries/

AUTHORS

@@ -9,3 +9,4 @@ Jason Bau <jbau@stanford.edu>
 John Jarvis <jarv@edx.org>
 Dmitry Viskov <dmitry.viskov@webenterprise.ru>
 Eric Fischer <efischer@edx.org>
+Kyle McCormick <kylemccor@gmail.com>

analytics_data_api/v0/tests/views/__init__.py

@@ -100,15 +100,29 @@ class APIListViewTestMixin(object):
     list_name = 'list'
     default_ids = []
     always_exclude = ['created']
+    test_post_method = False
 
-    def path(self, ids=None, fields=None, exclude=None, **kwargs):
-        query_params = {}
-        for query_arg, data in zip([self.ids_param, 'fields', 'exclude'], [ids, fields, exclude]) + kwargs.items():
-            if data:
-                query_params[query_arg] = ','.join(data)
-        query_string = '?{}'.format(urlencode(query_params))
+    def path(self, query_data=None):
+        query_data = query_data or {}
+        concat_query_data = {param: ','.join(arg) for param, arg in query_data.items() if arg}
+        query_string = '?{}'.format(urlencode(concat_query_data)) if concat_query_data else ''
         return '/api/v0/{}/{}'.format(self.list_name, query_string)
 
+    def validated_request(self, ids=None, fields=None, exclude=None, **extra_args):
+        params = [self.ids_param, 'fields', 'exclude']
+        args = [ids, fields, exclude]
+        data = {param: arg for param, arg in zip(params, args) if arg}
+        data.update(extra_args)
+
+        get_response = self.authenticated_get(self.path(data))
+        if self.test_post_method:
+            post_response = self.authenticated_post(self.path(), data=data)
+            self.assertEquals(get_response.status_code, post_response.status_code)
+            if 200 <= get_response.status_code < 300:
+                self.assertEquals(get_response.data, post_response.data)
+
+        return get_response
+
     def create_model(self, model_id, **kwargs):
         pass  # implement in subclass
 
@@ -134,19 +148,19 @@ class APIListViewTestMixin(object):
 
     def _test_all_items(self, ids):
         self.generate_data()
-        response = self.authenticated_get(self.path(ids=ids, exclude=self.always_exclude))
+        response = self.validated_request(ids=ids, exclude=self.always_exclude)
         self.assertEquals(response.status_code, 200)
         self.assertItemsEqual(response.data, self.all_expected_results(ids=ids))
 
     def _test_one_item(self, item_id):
         self.generate_data()
-        response = self.authenticated_get(self.path(ids=[item_id], exclude=self.always_exclude))
+        response = self.validated_request(ids=[item_id], exclude=self.always_exclude)
         self.assertEquals(response.status_code, 200)
         self.assertItemsEqual(response.data, [self.expected_result(item_id)])
 
     def _test_fields(self, fields):
         self.generate_data()
-        response = self.authenticated_get(self.path(fields=fields))
+        response = self.validated_request(fields=fields)
         self.assertEquals(response.status_code, 200)
 
         # remove fields not requested from expected results
@@ -158,10 +172,10 @@ class APIListViewTestMixin(object):
         self.assertItemsEqual(response.data, expected_results)
 
     def test_no_items(self):
-        response = self.authenticated_get(self.path())
+        response = self.validated_request()
         self.assertEquals(response.status_code, 404)
 
     def test_no_matching_items(self):
         self.generate_data()
-        response = self.authenticated_get(self.path(ids=['no/items/found']))
+        response = self.validated_request(ids=['no/items/found'])
         self.assertEquals(response.status_code, 404)

analytics_data_api/v0/tests/views/test_course_summaries.py

@@ -22,6 +22,7 @@ class CourseSummariesViewTests(VerifyCourseIdMixin, TestCaseWithAuthentication,
     list_name = 'course_summaries'
     default_ids = CourseSamples.course_ids
     always_exclude = ['created', 'programs']
+    test_post_method = True
 
     def setUp(self):
         super(CourseSummariesViewTests, self).setUp()
@@ -135,7 +136,7 @@ class CourseSummariesViewTests(VerifyCourseIdMixin, TestCaseWithAuthentication,
     )
     def test_empty_modes(self, modes):
         self.generate_data(modes=modes)
-        response = self.authenticated_get(self.path(exclude=self.always_exclude))
+        response = self.validated_request(exclude=self.always_exclude)
         self.assertEquals(response.status_code, 200)
         self.assertItemsEqual(response.data, self.all_expected_results(modes=modes))
 
@@ -144,13 +145,13 @@ class CourseSummariesViewTests(VerifyCourseIdMixin, TestCaseWithAuthentication,
         [CourseSamples.course_ids[0], 'malformed-course-id'],
     )
     def test_bad_course_id(self, course_ids):
-        response = self.authenticated_get(self.path(ids=course_ids))
+        response = self.validated_request(ids=course_ids)
         self.verify_bad_course_id(response)
 
     def test_collapse_upcoming(self):
         self.generate_data(availability='Starting Soon')
         self.generate_data(ids=['foo/bar/baz'], availability='Upcoming')
-        response = self.authenticated_get(self.path(exclude=self.always_exclude))
+        response = self.validated_request(exclude=self.always_exclude)
         self.assertEquals(response.status_code, 200)
 
         expected_summaries = self.all_expected_results(availability='Upcoming')
@@ -161,13 +162,13 @@ class CourseSummariesViewTests(VerifyCourseIdMixin, TestCaseWithAuthentication,
 
     def test_programs(self):
         self.generate_data(programs=True)
-        response = self.authenticated_get(self.path(exclude=self.always_exclude[:1], programs=['True']))
+        response = self.validated_request(exclude=self.always_exclude[:1], programs=['True'])
         self.assertEquals(response.status_code, 200)
         self.assertItemsEqual(response.data, self.all_expected_results(programs=True))
 
     @ddt.data('passing_users', )
     def test_exclude(self, field):
         self.generate_data()
-        response = self.authenticated_get(self.path(exclude=[field]))
+        response = self.validated_request(exclude=[field])
         self.assertEquals(response.status_code, 200)
         self.assertEquals(str(response.data).count(field), 0)

analytics_data_api/v0/views/__init__.py

@@ -119,19 +119,33 @@ class APIListView(generics.ListAPIView):
         GET /api/v0/some_endpoint/
             Returns full list of serialized models with all default fields.
 
-        GET /api/v0/some_endpoint/?ids={id},{id}
+        GET /api/v0/some_endpoint/?ids={id_1},{id_2}
             Returns list of serialized models with IDs that match an ID in the given
             `ids` query parameter with all default fields.
 
-        GET /api/v0/some_endpoint/?ids={id},{id}&fields={some_field},{some_field}
+        GET /api/v0/some_endpoint/?ids={id_1},{id_2}&fields={some_field_1},{some_field_2}
             Returns list of serialized models with IDs that match an ID in the given
             `ids` query parameter with only the fields in the given `fields` query parameter.
 
-        GET /api/v0/some_endpoint/?ids={id},{id}&exclude={some_field},{some_field}
+        GET /api/v0/some_endpoint/?ids={id_1},{id_2}&exclude={some_field_1},{some_field_2}
             Returns list of serialized models with IDs that match an ID in the given
             `ids` query parameter with all fields except those in the given `exclude` query
             parameter.
 
+        POST /api/v0/some_endpoint/
+        {
+            "ids": [
+                "{id_1}",
+                "{id_2}",
+                ...
+                "{id_200}"
+            ],
+            "fields": [
+                "{some_field_1}",
+                "{some_field_2}"
+            ]
+        }
+
     **Response Values**
 
         Since this is an abstract class, this view just returns an empty list.
@@ -142,6 +156,9 @@ class APIListView(generics.ListAPIView):
         explicitly specifying the fields to include in each result with `fields` as well of
         the fields to exclude with `exclude`.
 
+        For GET requests, these parameters are passed in the query string.
+        For POST requests, these parameters are passed as a JSON dict in the request body.
+
         ids -- The comma-separated list of identifiers for which results are filtered to.
             For example, 'edX/DemoX/Demo_Course,course-v1:edX+DemoX+Demo_2016'. Default is to
             return all courses.
@@ -149,6 +166,12 @@ class APIListView(generics.ListAPIView):
             For example, 'course_id,created'. Default is to return all fields.
         exclude -- The comma-separated fields to exclude in the response.
             For example, 'course_id,created'. Default is to not exclude any fields.
+
+    **Notes**
+
+        * GET is usable when the number of IDs is relatively low
+        * POST is required when the number of course IDs would cause the URL to be too long.
+        * POST functions the same as GET here. It does not modify any state.
     """
     ids = None
     fields = None
@@ -175,6 +198,19 @@ class APIListView(generics.ListAPIView):
 
         return super(APIListView, self).get(request, *args, **kwargs)
 
+    def post(self, request, *args, **kwargs):
+        # self.request.data is a QueryDict. For keys with singleton lists as values,
+        # QueryDicts return the singleton element of the list instead of the list itself,
+        # which is undesirable. So, we convert to a normal dict.
+        request_data_dict = dict(request.data)
+        self.fields = request_data_dict.get('fields')
+        exclude = request_data_dict.get('exclude')
+        self.exclude = self.always_exclude + (exclude if exclude else [])
+        self.ids = request_data_dict.get(self.ids_param)
+        self.verify_ids()
+
+        return super(APIListView, self).get(request, *args, **kwargs)
+
     def verify_ids(self):
         """
         Optionally raise an exception if any of the IDs set as self.ids are invalid.

analytics_data_api/v0/views/course_summaries.py

@@ -13,9 +13,19 @@ class CourseSummariesView(APIListView):
     """
     Returns summary information for courses.
 
-    **Example Request**
+    **Example Requests**
 
-        GET /api/v0/course_summaries/?course_ids={course_id},{course_id}
+        GET /api/v0/course_summaries/?course_ids={course_id_1},{course_id_2}
+
+        POST /api/v0/course_summaries/
+        {
+            "course_ids": [
+                "{course_id_1}",
+                "{course_id_2}",
+                ...
+                "{course_id_200}"
+            ]
+        }
 
     **Response Values**
 
@@ -39,6 +49,9 @@ class CourseSummariesView(APIListView):
 
         Results can be filed to the course IDs specified or limited to the fields.
 
+        For GET requests, these parameters are passed in the query string.
+        For POST requests, these parameters are passed as a JSON dict in the request body.
+
         course_ids -- The comma-separated course identifiers for which summaries are requested.
             For example, 'edX/DemoX/Demo_Course,course-v1:edX+DemoX+Demo_2016'.  Default is to
             return all courses.
@@ -48,6 +61,12 @@ class CourseSummariesView(APIListView):
             For example, 'course_id,created'.  Default is to exclude the programs array.
         programs -- If included in the query parameters, will find each courses' program IDs
             and include them in the response.
+
+    **Notes**
+
+        * GET is usable when the number of course IDs is relatively low
+        * POST is required when the number of course IDs would cause the URL to be too long.
+        * POST functions the same as GET for this endpoint. It does not modify any state.
     """
     serializer_class = serializers.CourseMetaSummaryEnrollmentSerializer
     programs_serializer_class = serializers.CourseProgramMetadataSerializer
@@ -68,6 +87,17 @@ class CourseSummariesView(APIListView):
         response = super(CourseSummariesView, self).get(request, *args, **kwargs)
         return response
 
+    def post(self, request, *args, **kwargs):
+        # self.request.data is a QueryDict. For keys with singleton lists as values,
+        # QueryDicts return the singleton element of the list instead of the list itself,
+        # which is undesirable. So, we convert to a normal dict.
+        request_data_dict = dict(self.request.data)
+        programs = request_data_dict.get('programs')
+        if not programs:
+            self.always_exclude = self.always_exclude + ['programs']
+        response = super(CourseSummariesView, self).post(request, *args, **kwargs)
+        return response
+
     def verify_ids(self):
         """
         Raise an exception if any of the course IDs set as self.ids are invalid.

analyticsdataserver/tests.py

@@ -27,7 +27,15 @@ class TestCaseWithAuthentication(TestCase):
 
     def authenticated_get(self, path, data=None, follow=True, **extra):
         data = data or {}
-        return self.client.get(path, data, follow, HTTP_AUTHORIZATION='Token ' + self.token.key, **extra)
+        return self.client.get(
+            path=path, data=data, follow=follow, HTTP_AUTHORIZATION='Token ' + self.token.key, **extra
+        )
+
+    def authenticated_post(self, path, data=None, follow=True, **extra):
+        data = data or {}
+        return self.client.post(
+            path=path, data=data, follow=follow, HTTP_AUTHORIZATION='Token ' + self.token.key, **extra
+        )
 
 
 @contextmanager