From 87c3547d41c295eda413a1d8b435b8cc0b613740 Mon Sep 17 00:00:00 2001 From: Rene Sorel Date: Tue, 26 Jul 2016 09:09:09 +0200 Subject: [PATCH 1/4] mobile api user course enrollments allow filter by org --- lms/djangoapps/mobile_api/users/views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 311ff62e87..0ef5b53d2c 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -270,14 +270,18 @@ class UserCourseEnrollmentsList(generics.ListAPIView): # the default behavior by setting the pagination_class to None. pagination_class = None + def is_org(self, check_org, course_org): + return check_org == None or (check_org.lower() == course_org.lower()) + def get_queryset(self): enrollments = self.queryset.filter( user__username=self.kwargs['username'], is_active=True ).order_by('created').reverse() + org = self.request.query_params.get('org', None) return [ enrollment for enrollment in enrollments - if enrollment.course_overview and + if enrollment.course_overview and self.is_org(org, enrollment.course_overview.org) and is_mobile_available_for_user(self.request.user, enrollment.course_overview) ] From 14672ffe4fee2cde1e860ce5e79f7a5a0385c619 Mon Sep 17 00:00:00 2001 From: Rene Sorel Date: Thu, 28 Jul 2016 10:43:46 +0200 Subject: [PATCH 2/4] unit test for mobile api org filter, query string args for api calls --- lms/djangoapps/mobile_api/testutils.py | 8 +++---- lms/djangoapps/mobile_api/users/tests.py | 29 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/mobile_api/testutils.py b/lms/djangoapps/mobile_api/testutils.py index 1f1c93ad5d..194d0230cc 100644 --- a/lms/djangoapps/mobile_api/testutils.py +++ b/lms/djangoapps/mobile_api/testutils.py @@ -71,13 +71,13 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): self.login() self.enroll(course_id) - def api_response(self, reverse_args=None, expected_response_code=200, **kwargs): + def api_response(self, reverse_args=None, expected_response_code=200, qargs={}, **kwargs): """ Helper method for calling endpoint, verifying and returning response. If expected_response_code is None, doesn't verify the response' status_code. """ url = self.reverse_url(reverse_args, **kwargs) - response = self.url_method(url, **kwargs) + response = self.url_method(url, qargs=qargs, **kwargs) if expected_response_code is not None: self.assertEqual(response.status_code, expected_response_code) return response @@ -91,9 +91,9 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): reverse_args.update({'username': kwargs.get('username', self.user.username)}) return reverse(self.REVERSE_INFO['name'], kwargs=reverse_args) - def url_method(self, url, **kwargs): # pylint: disable=unused-argument + def url_method(self, url, qargs={}, **kwargs): # pylint: disable=unused-argument """Base implementation that returns response from the GET method of the URL.""" - return self.client.get(url) + return self.client.get(url, qargs) class MobileAuthTestMixin(object): diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 5038867bf0..d32530ebef 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -270,6 +270,35 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest self.assertIn('/api/discussion/v1/courses/{}'.format(self.course.id), response_discussion_url) + def test_org_query(self): + self.login() + + # Create list of courses with various organizations + courses = [ + CourseFactory.create(org='edX', mobile_available=True), + CourseFactory.create(org='edX', mobile_available=True), + CourseFactory.create(org='edX', mobile_available=True, visible_to_staff_only=True), + CourseFactory.create(org='Proversity.org', mobile_available=True), + CourseFactory.create(org='MITx', mobile_available=True), + CourseFactory.create(org='HarvardX', mobile_available=True), + ] + + # Enroll in all the courses + for course in courses: + self.enroll(course.id) + + response = self.api_response(qargs={'org':'edX'}) + + # Verify only edX courses are returned + self.assertEqual(len(response.data), 3) + for course_index in range(3): + result = response.data[course_index]['course'] + self.assertEqual(result['org'], 'edX') + + # Verify most recently enrolled course is staff only but still returned + self.assertFalse(response.data[0]['course']['courseware_access']['has_access']) + + @attr('shard_2') class CourseStatusAPITestCase(MobileAPITestCase): """ From 408bc223e5d74c0e9044da5cc2742150d0c428d2 Mon Sep 17 00:00:00 2001 From: Rene Sorel Date: Tue, 20 Sep 2016 11:55:10 +0200 Subject: [PATCH 3/4] requested changes for quality and performance --- lms/djangoapps/mobile_api/testutils.py | 8 ++++---- lms/djangoapps/mobile_api/users/tests.py | 11 +++-------- lms/djangoapps/mobile_api/users/views.py | 2 +- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lms/djangoapps/mobile_api/testutils.py b/lms/djangoapps/mobile_api/testutils.py index 853337f718..6cf07841ea 100644 --- a/lms/djangoapps/mobile_api/testutils.py +++ b/lms/djangoapps/mobile_api/testutils.py @@ -72,13 +72,13 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): self.login() self.enroll(course_id) - def api_response(self, reverse_args=None, expected_response_code=200, qargs={}, **kwargs): + def api_response(self, reverse_args=None, expected_response_code=200, data=None, **kwargs): """ Helper method for calling endpoint, verifying and returning response. If expected_response_code is None, doesn't verify the response' status_code. """ url = self.reverse_url(reverse_args, **kwargs) - response = self.url_method(url, qargs=qargs, **kwargs) + response = self.url_method(url, data=data, **kwargs) if expected_response_code is not None: self.assertEqual(response.status_code, expected_response_code) return response @@ -92,9 +92,9 @@ class MobileAPITestCase(ModuleStoreTestCase, APITestCase): reverse_args.update({'username': kwargs.get('username', self.user.username)}) return reverse(self.REVERSE_INFO['name'], kwargs=reverse_args) - def url_method(self, url, qargs={}, **kwargs): # pylint: disable=unused-argument + def url_method(self, url, data=None, **kwargs): # pylint: disable=unused-argument """Base implementation that returns response from the GET method of the URL.""" - return self.client.get(url, qargs) + return self.client.get(url, data=data) class MobileAuthTestMixin(object): diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index ccaa7fe8ee..75aab525ce 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -292,16 +292,11 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest for course in courses: self.enroll(course.id) - response = self.api_response(qargs={'org':'edX'}) + response = self.api_response(data={'org':'edX'}) # Verify only edX courses are returned - self.assertEqual(len(response.data), 3) - for course_index in range(3): - result = response.data[course_index]['course'] - self.assertEqual(result['org'], 'edX') - - # Verify most recently enrolled course is staff only but still returned - self.assertFalse(response.data[0]['course']['courseware_access']['has_access']) + for entry in response.data: + self.assertEqual(entry['course']['org'], 'edX') @attr(shard=2) diff --git a/lms/djangoapps/mobile_api/users/views.py b/lms/djangoapps/mobile_api/users/views.py index 0ef5b53d2c..be31dba4a0 100644 --- a/lms/djangoapps/mobile_api/users/views.py +++ b/lms/djangoapps/mobile_api/users/views.py @@ -271,7 +271,7 @@ class UserCourseEnrollmentsList(generics.ListAPIView): pagination_class = None def is_org(self, check_org, course_org): - return check_org == None or (check_org.lower() == course_org.lower()) + return check_org is None or (check_org.lower() == course_org.lower()) def get_queryset(self): enrollments = self.queryset.filter( From e503bb6fc2898335c24ca6ff6ad11a620bbe2071 Mon Sep 17 00:00:00 2001 From: Rene Sorel Date: Thu, 22 Sep 2016 09:44:12 +0200 Subject: [PATCH 4/4] correctly test count on org filtered courses --- lms/djangoapps/mobile_api/users/tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/lms/djangoapps/mobile_api/users/tests.py b/lms/djangoapps/mobile_api/users/tests.py index 75aab525ce..d40b3d835a 100644 --- a/lms/djangoapps/mobile_api/users/tests.py +++ b/lms/djangoapps/mobile_api/users/tests.py @@ -289,6 +289,7 @@ class TestUserEnrollmentApi(UrlResetMixin, MobileAPITestCase, MobileAuthUserTest ] # Enroll in all the courses + self.assertEqual(len(response.data), 3) for course in courses: self.enroll(course.id)