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

EDUCATOR-464: Add POST method to course_summaries/ #173

Merged
merged 1 commit into from
Jun 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ Jason Bau <[email protected]>
John Jarvis <[email protected]>
Dmitry Viskov <[email protected]>
Eric Fischer <[email protected]>
Kyle McCormick <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

36 changes: 25 additions & 11 deletions analytics_data_api/v0/tests/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the function I'm most interested in feedback on. I'm making it so every request is done as both a GET and POST, and the results are compared before returning. Let me know what you think @efischer19.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it, very nice test addition

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

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

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
Expand All @@ -158,10 +172,10 @@ def _test_fields(self, fields):
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)
11 changes: 6 additions & 5 deletions analytics_data_api/v0/tests/views/test_course_summaries.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -135,7 +136,7 @@ def test_fields(self, fields):
)
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))

Expand All @@ -144,13 +145,13 @@ def test_empty_modes(self, modes):
[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')
Expand All @@ -161,13 +162,13 @@ def test_collapse_upcoming(self):

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)
42 changes: 39 additions & 3 deletions analytics_data_api/v0/views/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -142,13 +156,22 @@ 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.
fields -- The comma-separated fields to return in the response.
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
Expand All @@ -175,6 +198,19 @@ def get(self, request, *args, **kwargs):

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.
Expand Down
34 changes: 32 additions & 2 deletions analytics_data_api/v0/views/course_summaries.py
Original file line number Diff line number Diff line change
Expand Up @@ -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**

Expand All @@ -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.
Expand All @@ -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
Expand All @@ -68,6 +87,17 @@ def get(self, request, *args, **kwargs):
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.
Expand Down
10 changes: 9 additions & 1 deletion analyticsdataserver/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,15 @@ def setUp(self):

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
Expand Down