From 12e5244720aa00cc8a54725da68162e9acc50400 Mon Sep 17 00:00:00 2001 From: Rene Sorel Date: Thu, 8 Sep 2016 13:16:49 +0200 Subject: [PATCH 1/2] user api endpoint for logged in users info --- .../user_api/accounts/tests/test_views.py | 142 ++++++++++-------- .../djangoapps/user_api/accounts/views.py | 13 ++ openedx/core/djangoapps/user_api/urls.py | 4 + 3 files changed, 99 insertions(+), 60 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index 72c03d0f8e..f3642b319a 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -112,28 +112,6 @@ class UserAPITestCase(APITestCase): legacy_profile.language_proficiencies.add(LanguageProficiency(code='en')) legacy_profile.save() - -@ddt.ddt -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') -@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) -@patch.dict( - 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', - {'full': 50, 'small': 10}, - clear=True -) -@attr(shard=2) -class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): - """ - Unit tests for the Account API. - """ - - ENABLED_CACHES = ['default'] - - def setUp(self): - super(TestAccountAPI, self).setUp() - - self.url = reverse("accounts_api", kwargs={'username': self.user.username}) - def _verify_profile_image_data(self, data, has_profile_image): """ Verify the profile image data in a GET response for self.user @@ -160,6 +138,88 @@ class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): } ) + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) +@patch.dict( + 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', + {'full': 50, 'small': 10}, + clear=True +) +@attr(shard=2) +class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): + """ + Unit tests for the Accounts API. + """ + + ENABLED_CACHES = ['default'] + + def setUp(self): + super(TestAccountAPI, self).setUp() + + self.url = reverse("account_api") + + def test_get_account_default(self): + """ + Test that a client (logged in) can get her own account information (using default legacy profile information, + as created by the test UserFactory). + """ + def verify_get_own_information(queries): + """ + Internal helper to perform the actual assertions + """ + with self.assertNumQueries(queries): + response = self.send_get(self.client) + data = response.data + self.assertEqual(17, len(data)) + self.assertEqual(self.user.username, data["username"]) + self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) + for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"): + self.assertIsNone(data[empty_field]) + self.assertIsNone(data["country"]) + self.assertEqual("m", data["gender"]) + self.assertEqual("Learn a lot", data["goals"]) + self.assertEqual(self.user.email, data["email"]) + self.assertIsNotNone(data["date_joined"]) + self.assertEqual(self.user.is_active, data["is_active"]) + self._verify_profile_image_data(data, False) + self.assertTrue(data["requires_parental_consent"]) + self.assertEqual([], data["language_proficiencies"]) + self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) + # Badges aren't on by default, so should not be present. + self.assertEqual(False, data["accomplishments_shared"]) + + self.client.login(username=self.user.username, password=self.test_password) + verify_get_own_information(17) + + # Now make sure that the user can get the same information, even if not active + self.user.is_active = False + self.user.save() + verify_get_own_information(11) + + +@ddt.ddt +@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') +@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) +@patch.dict( + 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', + {'full': 50, 'small': 10}, + clear=True +) +@attr(shard=2) +class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): + """ + Unit tests for the Accounts API. + """ + + ENABLED_CACHES = ['default'] + + def setUp(self): + super(TestAccountsAPI, self).setUp() + + self.url = reverse("accounts_api", kwargs={'username': self.user.username}) + def _verify_full_shareable_account_response(self, response, account_privacy=None, badges_enabled=False): """ Verify that the shareable fields from the account are returned @@ -306,44 +366,6 @@ class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): response = self.send_get(client, query_parameters='view=shared') verify_fields_visible_to_all_users(response) - def test_get_account_default(self): - """ - Test that a client (logged in) can get her own account information (using default legacy profile information, - as created by the test UserFactory). - """ - def verify_get_own_information(queries): - """ - Internal helper to perform the actual assertions - """ - with self.assertNumQueries(queries): - response = self.send_get(self.client) - data = response.data - self.assertEqual(17, len(data)) - self.assertEqual(self.user.username, data["username"]) - self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) - for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"): - self.assertIsNone(data[empty_field]) - self.assertIsNone(data["country"]) - self.assertEqual("m", data["gender"]) - self.assertEqual("Learn a lot", data["goals"]) - self.assertEqual(self.user.email, data["email"]) - self.assertIsNotNone(data["date_joined"]) - self.assertEqual(self.user.is_active, data["is_active"]) - self._verify_profile_image_data(data, False) - self.assertTrue(data["requires_parental_consent"]) - self.assertEqual([], data["language_proficiencies"]) - self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) - # Badges aren't on by default, so should not be present. - self.assertEqual(False, data["accomplishments_shared"]) - - self.client.login(username=self.user.username, password=self.test_password) - verify_get_own_information(17) - - # Now make sure that the user can get the same information, even if not active - self.user.is_active = False - self.user.save() - verify_get_own_information(11) - def test_get_account_empty_string(self): """ Test the conversion of empty strings to None for certain fields. diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index a95a1b6646..abf46ffef5 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -29,6 +29,7 @@ class AccountViewSet(ViewSet): **Example Requests** + GET /api/user/v1/me[?view=shared] GET /api/user/v1/accounts?usernames={username1,username2}[?view=shared] GET /api/user/v1/accounts/{username}/[?view=shared] @@ -147,6 +148,18 @@ class AccountViewSet(ViewSet): permission_classes = (permissions.IsAuthenticated,) parser_classes = (MergePatchParser,) + def get(self, request): + """ + GET /api/user/v1/me + """ + try: + account_settings = get_account_settings( + request, [request.user.username], view=request.query_params.get('view')) + except UserNotFound: + return Response(status=status.HTTP_403_FORBIDDEN if request.user.is_staff else status.HTTP_404_NOT_FOUND) + + return Response(account_settings[0]) + def list(self, request): """ GET /api/user/v1/accounts?username={username1,username2} diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 8aa44ad9ec..9655a6e523 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -10,6 +10,9 @@ from .accounts.views import AccountViewSet from .preferences.views import PreferencesView, PreferencesDetailView from .verification_api.views import PhotoVerificationStatusView +ME = AccountViewSet.as_view({ + 'get': 'get', +}) ACCOUNT_LIST = AccountViewSet.as_view({ 'get': 'list', @@ -22,6 +25,7 @@ ACCOUNT_DETAIL = AccountViewSet.as_view({ urlpatterns = patterns( '', + url(r'^v1/me$', ME, name='account_api'), url(r'^v1/accounts/{}$'.format(settings.USERNAME_PATTERN), ACCOUNT_DETAIL, name='accounts_api'), url(r'^v1/accounts$', ACCOUNT_LIST, name='accounts_detail_api'), url( From 68daaf435ab84c88e63f235a4079e0acbfdcc8b8 Mon Sep 17 00:00:00 2001 From: Sanford Student Date: Tue, 3 Jan 2017 11:49:32 -0500 Subject: [PATCH 2/2] return only username from /me endpoint, with associated test changes --- .../user_api/accounts/tests/test_views.py | 106 ++++++++++++------ .../djangoapps/user_api/accounts/views.py | 19 ++-- openedx/core/djangoapps/user_api/urls.py | 2 +- 3 files changed, 81 insertions(+), 46 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py index f3642b319a..91dc74468b 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -141,14 +141,8 @@ class UserAPITestCase(APITestCase): @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Account APIs are only supported in LMS') -@patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) -@patch.dict( - 'openedx.core.djangoapps.user_api.accounts.image_helpers.PROFILE_IMAGE_SIZES_MAP', - {'full': 50, 'small': 10}, - clear=True -) @attr(shard=2) -class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): +class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase): """ Unit tests for the Accounts API. """ @@ -156,47 +150,46 @@ class TestAccountAPI(CacheIsolationTestCase, UserAPITestCase): ENABLED_CACHES = ['default'] def setUp(self): - super(TestAccountAPI, self).setUp() + super(TestOwnUsernameAPI, self).setUp() - self.url = reverse("account_api") + self.url = reverse("own_username_api") - def test_get_account_default(self): + def _verify_get_own_username(self, queries, expected_status=200): """ - Test that a client (logged in) can get her own account information (using default legacy profile information, - as created by the test UserFactory). + Internal helper to perform the actual assertion """ - def verify_get_own_information(queries): - """ - Internal helper to perform the actual assertions - """ - with self.assertNumQueries(queries): - response = self.send_get(self.client) + with self.assertNumQueries(queries): + response = self.send_get(self.client, expected_status=expected_status) + if expected_status == 200: data = response.data - self.assertEqual(17, len(data)) + self.assertEqual(1, len(data)) self.assertEqual(self.user.username, data["username"]) - self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) - for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"): - self.assertIsNone(data[empty_field]) - self.assertIsNone(data["country"]) - self.assertEqual("m", data["gender"]) - self.assertEqual("Learn a lot", data["goals"]) - self.assertEqual(self.user.email, data["email"]) - self.assertIsNotNone(data["date_joined"]) - self.assertEqual(self.user.is_active, data["is_active"]) - self._verify_profile_image_data(data, False) - self.assertTrue(data["requires_parental_consent"]) - self.assertEqual([], data["language_proficiencies"]) - self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) - # Badges aren't on by default, so should not be present. - self.assertEqual(False, data["accomplishments_shared"]) + def test_get_username(self): + """ + Test that a client (logged in) can get her own username. + """ self.client.login(username=self.user.username, password=self.test_password) - verify_get_own_information(17) + self._verify_get_own_username(15) - # Now make sure that the user can get the same information, even if not active + def test_get_username_inactive(self): + """ + Test that a logged-in client can get their + username, even if inactive. + """ + self.client.login(username=self.user.username, password=self.test_password) self.user.is_active = False self.user.save() - verify_get_own_information(11) + self._verify_get_own_username(15) + + def test_get_username_not_logged_in(self): + """ + Test that a client (not logged in) gets a 401 + when trying to retrieve their username. + """ + + # verify that the endpoint is inaccessible when not logged in + self._verify_get_own_username(12, expected_status=401) @ddt.ddt @@ -366,6 +359,45 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): response = self.send_get(client, query_parameters='view=shared') verify_fields_visible_to_all_users(response) + def test_get_account_default(self): + """ + Test that a client (logged in) can get her own account information (using default legacy profile information, + as created by the test UserFactory). + """ + + def verify_get_own_information(queries): + """ + Internal helper to perform the actual assertions + """ + with self.assertNumQueries(queries): + response = self.send_get(self.client) + data = response.data + self.assertEqual(17, len(data)) + self.assertEqual(self.user.username, data["username"]) + self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) + for empty_field in ("year_of_birth", "level_of_education", "mailing_address", "bio"): + self.assertIsNone(data[empty_field]) + self.assertIsNone(data["country"]) + self.assertEqual("m", data["gender"]) + self.assertEqual("Learn a lot", data["goals"]) + self.assertEqual(self.user.email, data["email"]) + self.assertIsNotNone(data["date_joined"]) + self.assertEqual(self.user.is_active, data["is_active"]) + self._verify_profile_image_data(data, False) + self.assertTrue(data["requires_parental_consent"]) + self.assertEqual([], data["language_proficiencies"]) + self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) + # Badges aren't on by default, so should not be present. + self.assertEqual(False, data["accomplishments_shared"]) + + self.client.login(username=self.user.username, password=self.test_password) + verify_get_own_information(17) + + # Now make sure that the user can get the same information, even if not active + self.user.is_active = False + self.user.save() + verify_get_own_information(11) + def test_get_account_empty_string(self): """ Test the conversion of empty strings to None for certain fields. diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index abf46ffef5..a74b1834d3 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -35,7 +35,16 @@ class AccountViewSet(ViewSet): PATCH /api/user/v1/accounts/{username}/{"key":"value"} "application/merge-patch+json" - **Response Values for GET** + **Response Values for GET requests to the /me endpoint** + If the user is not logged in, an HTTP 401 "Not Authorized" response + is returned. + + Otherwise, an HTTP 200 "OK" response is returned. The response + contains the following value: + + * username: The username associated with the account. + + **Response Values for GET requests to /accounts endpoints** If no user exists with the specified username, an HTTP 404 "Not Found" response is returned. @@ -152,13 +161,7 @@ class AccountViewSet(ViewSet): """ GET /api/user/v1/me """ - try: - account_settings = get_account_settings( - request, [request.user.username], view=request.query_params.get('view')) - except UserNotFound: - return Response(status=status.HTTP_403_FORBIDDEN if request.user.is_staff else status.HTTP_404_NOT_FOUND) - - return Response(account_settings[0]) + return Response({'username': request.user.username}) def list(self, request): """ diff --git a/openedx/core/djangoapps/user_api/urls.py b/openedx/core/djangoapps/user_api/urls.py index 9655a6e523..d390ea11f2 100644 --- a/openedx/core/djangoapps/user_api/urls.py +++ b/openedx/core/djangoapps/user_api/urls.py @@ -25,7 +25,7 @@ ACCOUNT_DETAIL = AccountViewSet.as_view({ urlpatterns = patterns( '', - url(r'^v1/me$', ME, name='account_api'), + url(r'^v1/me$', ME, name='own_username_api'), url(r'^v1/accounts/{}$'.format(settings.USERNAME_PATTERN), ACCOUNT_DETAIL, name='accounts_api'), url(r'^v1/accounts$', ACCOUNT_LIST, name='accounts_detail_api'), url(