Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

Commit ff9fb9b

Browse files
committed
Add POST method to CourseSummariesView
POST method allows large number of course ID arguments to be passed as data, while GET method is restricted by URL length. EDUCATOR-464
1 parent 5061f38 commit ff9fb9b

6 files changed

Lines changed: 112 additions & 22 deletions

File tree

AUTHORS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ Jason Bau <jbau@stanford.edu>
99
John Jarvis <jarv@edx.org>
1010
Dmitry Viskov <dmitry.viskov@webenterprise.ru>
1111
Eric Fischer <efischer@edx.org>
12+
Kyle McCormick <kylemccor@gmail.com>

analytics_data_api/v0/tests/views/__init__.py

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,29 @@ class APIListViewTestMixin(object):
100100
list_name = 'list'
101101
default_ids = []
102102
always_exclude = ['created']
103+
test_post_method = False
103104

104-
def path(self, ids=None, fields=None, exclude=None, **kwargs):
105-
query_params = {}
106-
for query_arg, data in zip([self.ids_param, 'fields', 'exclude'], [ids, fields, exclude]) + kwargs.items():
107-
if data:
108-
query_params[query_arg] = ','.join(data)
109-
query_string = '?{}'.format(urlencode(query_params))
105+
def path(self, query_data=None):
106+
query_data = query_data or {}
107+
concat_query_data = {param: ','.join(arg) for param, arg in query_data.items() if arg}
108+
query_string = '?{}'.format(urlencode(concat_query_data)) if concat_query_data else ''
110109
return '/api/v0/{}/{}'.format(self.list_name, query_string)
111110

111+
def validated_request(self, ids=None, fields=None, exclude=None, **extra_args):
112+
params = [self.ids_param, 'fields', 'exclude']
113+
args = [ids, fields, exclude]
114+
data = {param: arg for param, arg in zip(params, args) if arg}
115+
data.update(extra_args)
116+
117+
get_response = self.authenticated_get(self.path(data))
118+
if self.test_post_method:
119+
post_response = self.authenticated_post(self.path(), data=data)
120+
self.assertEquals(get_response.status_code, post_response.status_code)
121+
if 200 <= get_response.status_code < 300:
122+
self.assertEquals(get_response.data, post_response.data)
123+
124+
return get_response
125+
112126
def create_model(self, model_id, **kwargs):
113127
pass # implement in subclass
114128

@@ -134,19 +148,19 @@ def all_expected_results(self, ids=None, **kwargs):
134148

135149
def _test_all_items(self, ids):
136150
self.generate_data()
137-
response = self.authenticated_get(self.path(ids=ids, exclude=self.always_exclude))
151+
response = self.validated_request(ids=ids, exclude=self.always_exclude)
138152
self.assertEquals(response.status_code, 200)
139153
self.assertItemsEqual(response.data, self.all_expected_results(ids=ids))
140154

141155
def _test_one_item(self, item_id):
142156
self.generate_data()
143-
response = self.authenticated_get(self.path(ids=[item_id], exclude=self.always_exclude))
157+
response = self.validated_request(ids=[item_id], exclude=self.always_exclude)
144158
self.assertEquals(response.status_code, 200)
145159
self.assertItemsEqual(response.data, [self.expected_result(item_id)])
146160

147161
def _test_fields(self, fields):
148162
self.generate_data()
149-
response = self.authenticated_get(self.path(fields=fields))
163+
response = self.validated_request(fields=fields)
150164
self.assertEquals(response.status_code, 200)
151165

152166
# remove fields not requested from expected results
@@ -158,10 +172,10 @@ def _test_fields(self, fields):
158172
self.assertItemsEqual(response.data, expected_results)
159173

160174
def test_no_items(self):
161-
response = self.authenticated_get(self.path())
175+
response = self.validated_request()
162176
self.assertEquals(response.status_code, 404)
163177

164178
def test_no_matching_items(self):
165179
self.generate_data()
166-
response = self.authenticated_get(self.path(ids=['no/items/found']))
180+
response = self.validated_request(ids=['no/items/found'])
167181
self.assertEquals(response.status_code, 404)

analytics_data_api/v0/tests/views/test_course_summaries.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class CourseSummariesViewTests(VerifyCourseIdMixin, TestCaseWithAuthentication,
2222
list_name = 'course_summaries'
2323
default_ids = CourseSamples.course_ids
2424
always_exclude = ['created', 'programs']
25+
test_post_method = True
2526

2627
def setUp(self):
2728
super(CourseSummariesViewTests, self).setUp()
@@ -135,7 +136,7 @@ def test_fields(self, fields):
135136
)
136137
def test_empty_modes(self, modes):
137138
self.generate_data(modes=modes)
138-
response = self.authenticated_get(self.path(exclude=self.always_exclude))
139+
response = self.validated_request(exclude=self.always_exclude)
139140
self.assertEquals(response.status_code, 200)
140141
self.assertItemsEqual(response.data, self.all_expected_results(modes=modes))
141142

@@ -144,13 +145,13 @@ def test_empty_modes(self, modes):
144145
[CourseSamples.course_ids[0], 'malformed-course-id'],
145146
)
146147
def test_bad_course_id(self, course_ids):
147-
response = self.authenticated_get(self.path(ids=course_ids))
148+
response = self.validated_request(ids=course_ids)
148149
self.verify_bad_course_id(response)
149150

150151
def test_collapse_upcoming(self):
151152
self.generate_data(availability='Starting Soon')
152153
self.generate_data(ids=['foo/bar/baz'], availability='Upcoming')
153-
response = self.authenticated_get(self.path(exclude=self.always_exclude))
154+
response = self.validated_request(exclude=self.always_exclude)
154155
self.assertEquals(response.status_code, 200)
155156

156157
expected_summaries = self.all_expected_results(availability='Upcoming')
@@ -161,13 +162,13 @@ def test_collapse_upcoming(self):
161162

162163
def test_programs(self):
163164
self.generate_data(programs=True)
164-
response = self.authenticated_get(self.path(exclude=self.always_exclude[:1], programs=['True']))
165+
response = self.validated_request(exclude=self.always_exclude[:1], programs=['True'])
165166
self.assertEquals(response.status_code, 200)
166167
self.assertItemsEqual(response.data, self.all_expected_results(programs=True))
167168

168169
@ddt.data('passing_users', )
169170
def test_exclude(self, field):
170171
self.generate_data()
171-
response = self.authenticated_get(self.path(exclude=[field]))
172+
response = self.validated_request(exclude=[field])
172173
self.assertEquals(response.status_code, 200)
173174
self.assertEquals(str(response.data).count(field), 0)

analytics_data_api/v0/views/__init__.py

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,19 +119,33 @@ class APIListView(generics.ListAPIView):
119119
GET /api/v0/some_endpoint/
120120
Returns full list of serialized models with all default fields.
121121
122-
GET /api/v0/some_endpoint/?ids={id},{id}
122+
GET /api/v0/some_endpoint/?ids={id_1},{id_2}
123123
Returns list of serialized models with IDs that match an ID in the given
124124
`ids` query parameter with all default fields.
125125
126-
GET /api/v0/some_endpoint/?ids={id},{id}&fields={some_field},{some_field}
126+
GET /api/v0/some_endpoint/?ids={id_1},{id_2}&fields={some_field_1},{some_field_2}
127127
Returns list of serialized models with IDs that match an ID in the given
128128
`ids` query parameter with only the fields in the given `fields` query parameter.
129129
130-
GET /api/v0/some_endpoint/?ids={id},{id}&exclude={some_field},{some_field}
130+
GET /api/v0/some_endpoint/?ids={id_1},{id_2}&exclude={some_field_1},{some_field_2}
131131
Returns list of serialized models with IDs that match an ID in the given
132132
`ids` query parameter with all fields except those in the given `exclude` query
133133
parameter.
134134
135+
POST /api/v0/some_endpoint/
136+
{
137+
"ids": [
138+
"{id_1}",
139+
"{id_2}",
140+
...
141+
"{id_200}"
142+
],
143+
"fields": [
144+
"{some_field_1}",
145+
"{some_field_2}"
146+
]
147+
}
148+
135149
**Response Values**
136150
137151
Since this is an abstract class, this view just returns an empty list.
@@ -142,13 +156,22 @@ class APIListView(generics.ListAPIView):
142156
explicitly specifying the fields to include in each result with `fields` as well of
143157
the fields to exclude with `exclude`.
144158
159+
For GET requests, these parameters are passed in the query string.
160+
For POST requests, these parameters are passed as a JSON dict in the request body.
161+
145162
ids -- The comma-separated list of identifiers for which results are filtered to.
146163
For example, 'edX/DemoX/Demo_Course,course-v1:edX+DemoX+Demo_2016'. Default is to
147164
return all courses.
148165
fields -- The comma-separated fields to return in the response.
149166
For example, 'course_id,created'. Default is to return all fields.
150167
exclude -- The comma-separated fields to exclude in the response.
151168
For example, 'course_id,created'. Default is to not exclude any fields.
169+
170+
**Notes**
171+
172+
* GET is usable when the number of IDs is relatively low
173+
* POST is required when the number of course IDs would cause the URL to be too long.
174+
* POST functions the same as GET here. It does not modify any state.
152175
"""
153176
ids = None
154177
fields = None
@@ -175,6 +198,19 @@ def get(self, request, *args, **kwargs):
175198

176199
return super(APIListView, self).get(request, *args, **kwargs)
177200

201+
def post(self, request, *args, **kwargs):
202+
# self.request.data is a QueryDict. For keys with singleton lists as values,
203+
# QueryDicts return the singleton element of the list instead of the list itself,
204+
# which is undesirable. So, we convert to a normal dict.
205+
request_data_dict = dict(request.data)
206+
self.fields = request_data_dict.get('fields')
207+
exclude = request_data_dict.get('exclude')
208+
self.exclude = self.always_exclude + (exclude if exclude else [])
209+
self.ids = request_data_dict.get(self.ids_param)
210+
self.verify_ids()
211+
212+
return super(APIListView, self).get(request, *args, **kwargs)
213+
178214
def verify_ids(self):
179215
"""
180216
Optionally raise an exception if any of the IDs set as self.ids are invalid.

analytics_data_api/v0/views/course_summaries.py

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,19 @@ class CourseSummariesView(APIListView):
1313
"""
1414
Returns summary information for courses.
1515
16-
**Example Request**
16+
**Example Requests**
1717
18-
GET /api/v0/course_summaries/?course_ids={course_id},{course_id}
18+
GET /api/v0/course_summaries/?course_ids={course_id_1},{course_id_2}
19+
20+
POST /api/v0/course_summaries/
21+
{
22+
"course_ids": [
23+
"{course_id_1}",
24+
"{course_id_2}",
25+
...
26+
"{course_id_200}"
27+
]
28+
}
1929
2030
**Response Values**
2131
@@ -39,6 +49,9 @@ class CourseSummariesView(APIListView):
3949
4050
Results can be filed to the course IDs specified or limited to the fields.
4151
52+
For GET requests, these parameters are passed in the query string.
53+
For POST requests, these parameters are passed as a JSON dict in the request body.
54+
4255
course_ids -- The comma-separated course identifiers for which summaries are requested.
4356
For example, 'edX/DemoX/Demo_Course,course-v1:edX+DemoX+Demo_2016'. Default is to
4457
return all courses.
@@ -48,6 +61,12 @@ class CourseSummariesView(APIListView):
4861
For example, 'course_id,created'. Default is to exclude the programs array.
4962
programs -- If included in the query parameters, will find each courses' program IDs
5063
and include them in the response.
64+
65+
**Notes**
66+
67+
* GET is usable when the number of course IDs is relatively low
68+
* POST is required when the number of course IDs would cause the URL to be too long.
69+
* POST functions the same as GET for this endpoint. It does not modify any state.
5170
"""
5271
serializer_class = serializers.CourseMetaSummaryEnrollmentSerializer
5372
programs_serializer_class = serializers.CourseProgramMetadataSerializer
@@ -68,6 +87,17 @@ def get(self, request, *args, **kwargs):
6887
response = super(CourseSummariesView, self).get(request, *args, **kwargs)
6988
return response
7089

90+
def post(self, request, *args, **kwargs):
91+
# self.request.data is a QueryDict. For keys with singleton lists as values,
92+
# QueryDicts return the singleton element of the list instead of the list itself,
93+
# which is undesirable. So, we convert to a normal dict.
94+
request_data_dict = dict(self.request.data)
95+
programs = request_data_dict.get('programs')
96+
if not programs:
97+
self.always_exclude = self.always_exclude + ['programs']
98+
response = super(CourseSummariesView, self).post(request, *args, **kwargs)
99+
return response
100+
71101
def verify_ids(self):
72102
"""
73103
Raise an exception if any of the course IDs set as self.ids are invalid.

analyticsdataserver/tests.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,15 @@ def setUp(self):
2727

2828
def authenticated_get(self, path, data=None, follow=True, **extra):
2929
data = data or {}
30-
return self.client.get(path, data, follow, HTTP_AUTHORIZATION='Token ' + self.token.key, **extra)
30+
return self.client.get(
31+
path=path, data=data, follow=follow, HTTP_AUTHORIZATION='Token ' + self.token.key, **extra
32+
)
33+
34+
def authenticated_post(self, path, data=None, follow=True, **extra):
35+
data = data or {}
36+
return self.client.post(
37+
path=path, data=data, follow=follow, HTTP_AUTHORIZATION='Token ' + self.token.key, **extra
38+
)
3139

3240

3341
@contextmanager

0 commit comments

Comments
 (0)