Merge pull request #19777 from edx/arch/account-api-social-links

Account API: Enhance Social Link API

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
 
 

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):

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