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

Commit 5061f38

Browse files
authored
Merge pull request #174 from edx/edx/kdmccormick/fix-ids-bug
EDUCATOR-604: Fix course_summaries/ and programs/ to expect correct query param
2 parents 9c92bfc + 70136af commit 5061f38

6 files changed

Lines changed: 22 additions & 5 deletions

File tree

analytics_data_api/v0/tests/views/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ def assertResponseFields(self, response, fields):
9494
class APIListViewTestMixin(object):
9595
model = None
9696
model_id = 'id'
97+
ids_param = 'ids'
9798
serializer = None
9899
expected_results = []
99100
list_name = 'list'
@@ -102,7 +103,7 @@ class APIListViewTestMixin(object):
102103

103104
def path(self, ids=None, fields=None, exclude=None, **kwargs):
104105
query_params = {}
105-
for query_arg, data in zip(['ids', 'fields', 'exclude'], [ids, fields, exclude]) + kwargs.items():
106+
for query_arg, data in zip([self.ids_param, 'fields', 'exclude'], [ids, fields, exclude]) + kwargs.items():
106107
if data:
107108
query_params[query_arg] = ','.join(data)
108109
query_string = '?{}'.format(urlencode(query_params))

analytics_data_api/v0/tests/views/test_course_summaries.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
class CourseSummariesViewTests(VerifyCourseIdMixin, TestCaseWithAuthentication, APIListViewTestMixin):
1717
model = models.CourseMetaSummaryEnrollment
1818
model_id = 'course_id'
19+
ids_param = 'course_ids'
1920
serializer = serializers.CourseMetaSummaryEnrollmentSerializer
2021
expected_summaries = []
2122
list_name = 'course_summaries'
@@ -143,7 +144,7 @@ def test_empty_modes(self, modes):
143144
[CourseSamples.course_ids[0], 'malformed-course-id'],
144145
)
145146
def test_bad_course_id(self, course_ids):
146-
response = self.authenticated_get(self.path(course_ids=course_ids))
147+
response = self.authenticated_get(self.path(ids=course_ids))
147148
self.verify_bad_course_id(response)
148149

149150
def test_collapse_upcoming(self):

analytics_data_api/v0/tests/views/test_programs.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
class ProgramsViewTests(TestCaseWithAuthentication, APIListViewTestMixin):
1212
model = models.CourseProgramMetadata
1313
model_id = 'program_id'
14+
ids_param = 'program_ids'
1415
serializer = serializers.CourseProgramMetadataSerializer
1516
expected_programs = []
1617
list_name = 'programs'

analytics_data_api/v0/views/__init__.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ class APIListView(generics.ListAPIView):
155155
exclude = None
156156
always_exclude = []
157157
model_id_field = 'id'
158+
ids_param = 'ids'
158159

159160
def get_serializer(self, *args, **kwargs):
160161
kwargs.update({
@@ -169,10 +170,19 @@ def get(self, request, *args, **kwargs):
169170
self.fields = split_query_argument(query_params.get('fields'))
170171
exclude = split_query_argument(query_params.get('exclude'))
171172
self.exclude = self.always_exclude + (exclude if exclude else [])
172-
self.ids = split_query_argument(query_params.get('ids'))
173+
self.ids = split_query_argument(query_params.get(self.ids_param))
174+
self.verify_ids()
173175

174176
return super(APIListView, self).get(request, *args, **kwargs)
175177

178+
def verify_ids(self):
179+
"""
180+
Optionally raise an exception if any of the IDs set as self.ids are invalid.
181+
By default, no verification is done.
182+
Subclasses can override this if they wish to perform verification.
183+
"""
184+
pass
185+
176186
def base_field_dict(self, item_id):
177187
"""Default result with fields pre-populated to default values."""
178188
field_dict = {

analytics_data_api/v0/views/course_summaries.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ class CourseSummariesView(APIListView):
5353
programs_serializer_class = serializers.CourseProgramMetadataSerializer
5454
model = models.CourseMetaSummaryEnrollment
5555
model_id_field = 'course_id'
56+
ids_param = 'course_ids'
5657
programs_model = models.CourseProgramMetadata
5758
count_fields = ('count', 'cumulative_count', 'count_change_7_days',
5859
'passing_users') # are initialized to 0 by default
@@ -64,12 +65,14 @@ def get(self, request, *args, **kwargs):
6465
programs = split_query_argument(query_params.get('programs'))
6566
if not programs:
6667
self.always_exclude = self.always_exclude + ['programs']
67-
self.ids = split_query_argument(query_params.get('course_ids'))
68-
self.verify_ids()
6968
response = super(CourseSummariesView, self).get(request, *args, **kwargs)
7069
return response
7170

7271
def verify_ids(self):
72+
"""
73+
Raise an exception if any of the course IDs set as self.ids are invalid.
74+
Overrides APIListView.verify_ids.
75+
"""
7376
if self.ids is not None:
7477
for item_id in self.ids:
7578
validate_course_id(item_id)

analytics_data_api/v0/views/programs.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class ProgramsView(APIListView):
3535
serializer_class = serializers.CourseProgramMetadataSerializer
3636
model = models.CourseProgramMetadata
3737
model_id_field = 'program_id'
38+
ids_param = 'program_ids'
3839
program_meta_fields = ['program_type', 'program_title']
3940

4041
def base_field_dict(self, program_id):

0 commit comments

Comments
 (0)