From 0e79129796f20ffab2d1fc6e9deab988650c758a Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Sun, 10 Feb 2019 08:38:21 -0500 Subject: [PATCH] Account API: Enhance Social Link API --- .../user_api/accounts/serializers.py | 80 +++++++++++-------- .../user_api/accounts/tests/test_api.py | 68 ++++++++++++++++ .../djangoapps/user_api/accounts/views.py | 12 ++- 3 files changed, 126 insertions(+), 34 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index e51b2e1149..b060b6a0c8 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -137,7 +137,7 @@ class UserReadOnlySerializer(serializers.Serializer): user_profile, user, self.context.get('request') ), "language_proficiencies": LanguageProficiencySerializer( - user_profile.language_proficiencies.all(), many=True + user_profile.language_proficiencies.all().order_by('code'), many=True ).data, "name": user_profile.name, "gender": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.gender), @@ -150,7 +150,7 @@ class UserReadOnlySerializer(serializers.Serializer): "requires_parental_consent": user_profile.requires_parental_consent(), "account_privacy": get_profile_visibility(user_profile, user, self.configuration), "social_links": SocialLinkSerializer( - user_profile.social_links.all(), many=True + user_profile.social_links.all().order_by('platform'), many=True ).data, "extended_profile": get_extended_profile(user_profile), } @@ -306,6 +306,49 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea """ return AccountLegacyProfileSerializer.get_profile_image(user_profile, user_profile.user) + def _update_social_links(self, instance, requested_social_links): + """ + Update the given profile instance's social links as requested. + """ + try: + new_social_links = [] + deleted_social_platforms = [] + for requested_link_data in requested_social_links: + requested_platform = requested_link_data['platform'] + requested_link_url = requested_link_data['social_link'] + validate_social_link(requested_platform, requested_link_url) + formatted_link = format_social_link(requested_platform, requested_link_url) + if not formatted_link: + deleted_social_platforms.append(requested_platform) + else: + new_social_links.append( + SocialLink(user_profile=instance, platform=requested_platform, social_link=formatted_link) + ) + + platforms_of_new_social_links = [s.platform for s in new_social_links] + current_social_links = list(instance.social_links.all()) + unreplaced_social_links = [ + social_link for social_link in current_social_links + if social_link.platform not in platforms_of_new_social_links + ] + pruned_unreplaced_social_links = [ + social_link for social_link in unreplaced_social_links + if social_link.platform not in deleted_social_platforms + ] + merged_social_links = new_social_links + pruned_unreplaced_social_links + + instance.social_links.all().delete() + instance.social_links.bulk_create(merged_social_links) + + except ValueError as err: + # If we have encountered any validation errors, return them to the user. + raise errors.AccountValidationError({ + 'social_links': { + "developer_message": u"Error when adding new social link: '{}'".format(text_type(err)), + "user_message": text_type(err) + } + }) + def update(self, instance, validated_data): """ Update the profile, including nested fields. @@ -333,38 +376,11 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea ]) # Update the user's social links - social_link_data = self._kwargs['data']['social_links'] if 'social_links' in self._kwargs['data'] else None - if social_link_data and len(social_link_data) > 0: - new_social_link = social_link_data[0] - current_social_links = list(instance.social_links.all()) - instance.social_links.all().delete() - - try: - # Add the new social link with correct formatting - validate_social_link(new_social_link['platform'], new_social_link['social_link']) - formatted_link = format_social_link(new_social_link['platform'], new_social_link['social_link']) - instance.social_links.bulk_create([ - SocialLink(user_profile=instance, platform=new_social_link['platform'], social_link=formatted_link) - ]) - except ValueError as err: - # If we have encountered any validation errors, return them to the user. - raise errors.AccountValidationError({ - 'social_links': { - "developer_message": u"Error thrown from adding new social link: '{}'".format(text_type(err)), - "user_message": text_type(err) - } - }) - - # Add back old links unless overridden by new link - for current_social_link in current_social_links: - if current_social_link.platform != new_social_link['platform']: - instance.social_links.bulk_create([ - SocialLink(user_profile=instance, platform=current_social_link.platform, - social_link=current_social_link.social_link) - ]) + requested_social_links = self._kwargs['data'].get('social_links') + if requested_social_links: + self._update_social_links(instance, requested_social_links) instance.save() - return instance diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index d31933b56a..59f0aa9bb4 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -159,6 +159,74 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, Retireme with self.assertRaises(UserNotFound): update_account_settings(self.user, {}) + def test_get_empty_social_links(self): + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['social_links'], []) + + def test_set_single_social_link(self): + social_links = [ + dict(platform="facebook", social_link="https://www.facebook.com/{}".format(self.user.username)) + ] + update_account_settings(self.user, {"social_links": social_links}) + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['social_links'], social_links) + + def test_set_multiple_social_links(self): + social_links = [ + dict(platform="facebook", social_link="https://www.facebook.com/{}".format(self.user.username)), + dict(platform="twitter", social_link="https://www.twitter.com/{}".format(self.user.username)), + ] + update_account_settings(self.user, {"social_links": social_links}) + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['social_links'], social_links) + + def test_add_social_links(self): + original_social_links = [ + dict(platform="facebook", social_link="https://www.facebook.com/{}".format(self.user.username)) + ] + update_account_settings(self.user, {"social_links": original_social_links}) + + extra_social_links = [ + dict(platform="twitter", social_link="https://www.twitter.com/{}".format(self.user.username)), + dict(platform="linkedin", social_link="https://www.linkedin.com/in/{}".format(self.user.username)), + ] + update_account_settings(self.user, {"social_links": extra_social_links}) + + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual( + account_settings['social_links'], + sorted(original_social_links + extra_social_links, key=lambda s: s['platform']), + ) + + def test_replace_social_links(self): + original_facebook_link = dict(platform="facebook", social_link="https://www.facebook.com/myself") + original_twitter_link = dict(platform="twitter", social_link="https://www.twitter.com/myself") + update_account_settings(self.user, {"social_links": [original_facebook_link, original_twitter_link]}) + + modified_facebook_link = dict(platform="facebook", social_link="https://www.facebook.com/new_me") + update_account_settings(self.user, {"social_links": [modified_facebook_link]}) + + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['social_links'], [modified_facebook_link, original_twitter_link]) + + def test_remove_social_link(self): + original_facebook_link = dict(platform="facebook", social_link="https://www.facebook.com/myself") + original_twitter_link = dict(platform="twitter", social_link="https://www.twitter.com/myself") + update_account_settings(self.user, {"social_links": [original_facebook_link, original_twitter_link]}) + + removed_facebook_link = dict(platform="facebook", social_link="") + update_account_settings(self.user, {"social_links": [removed_facebook_link]}) + + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['social_links'], [original_twitter_link]) + + def test_unsupported_social_link_platform(self): + social_links = [ + dict(platform="unsupported", social_link="https://www.unsupported.com/{}".format(self.user.username)) + ] + with self.assertRaises(AccountUpdateError): + update_account_settings(self.user, {"social_links": social_links}) + def test_update_error_validating(self): """Test that AccountValidationError is thrown if incorrect values are supplied.""" with self.assertRaises(AccountValidationError): diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 68eb9e2632..25565997ba 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -126,6 +126,14 @@ class AccountViewSet(ViewSet): PATCH /api/user/v1/accounts/{username}/{"key":"value"} "application/merge-patch+json" + **Notes for PATCH requests to /accounts endpoints** + * Requested updates to social_links are automatically merged with + previously set links. That is, any newly introduced platforms are + add to the previous list. Updated links to pre-existing platforms + replace their values in the previous list. Pre-existing platforms + can be removed by setting the value of the social_link to an + empty string (""). + **Response Values for GET requests to the /me endpoint** If the user is not logged in, an HTTP 401 "Not Authorized" response is returned. @@ -196,8 +204,8 @@ class AccountViewSet(ViewSet): * requires_parental_consent: True if the user is a minor requiring parental consent. - * social_links: Array of social links. Each - preference is a JSON object with the following keys: + * social_links: Array of social links, sorted alphabetically by + "platform". Each preference is a JSON object with the following keys: * "platform": A particular social platform, ex: 'facebook' * "social_link": The link to the user's profile on the particular platform