Add social links to learner profile.
LEARNER-1859 Added fields to add social links to the user account settings file. Added icons to the user profile when these links are set, only shown when users show their entire profile. Added jasmine tests for account settings and learner profile pages. Added python unit tests to test validation on the user account.
This commit is contained in:
@@ -11,7 +11,7 @@ from django.conf import settings
|
||||
from django.core.validators import validate_email, ValidationError
|
||||
from django.http import HttpResponseForbidden
|
||||
from openedx.core.djangoapps.user_api.preferences.api import update_user_preferences
|
||||
from openedx.core.djangoapps.user_api.errors import PreferenceValidationError
|
||||
from openedx.core.djangoapps.user_api.errors import PreferenceValidationError, AccountValidationError
|
||||
|
||||
from student.models import User, UserProfile, Registration
|
||||
from student import forms as student_forms
|
||||
@@ -216,7 +216,9 @@ def update_account_settings(requesting_user, update, username=None):
|
||||
existing_user_profile.save()
|
||||
|
||||
except PreferenceValidationError as err:
|
||||
raise errors.AccountValidationError(err.preference_errors)
|
||||
raise AccountValidationError(err.preference_errors)
|
||||
except AccountValidationError as err:
|
||||
raise err
|
||||
except Exception as err:
|
||||
raise errors.AccountUpdateError(
|
||||
u"Error thrown when saving account updates: '{}'".format(err.message)
|
||||
|
||||
@@ -10,15 +10,17 @@ from django.core.exceptions import ObjectDoesNotExist
|
||||
from django.core.urlresolvers import reverse
|
||||
|
||||
from lms.djangoapps.badges.utils import badges_enabled
|
||||
from openedx.core.djangoapps.user_api import errors
|
||||
from openedx.core.djangoapps.user_api.models import UserPreference
|
||||
from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin
|
||||
from student.models import UserProfile, LanguageProficiency, SocialLink
|
||||
|
||||
from . import (
|
||||
NAME_MIN_LENGTH, ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY,
|
||||
ALL_USERS_VISIBILITY,
|
||||
)
|
||||
from openedx.core.djangoapps.user_api.models import UserPreference
|
||||
from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin
|
||||
from student.models import UserProfile, LanguageProficiency
|
||||
from .image_helpers import get_profile_image_urls_for_user
|
||||
|
||||
from .utils import validate_social_link, format_social_link
|
||||
|
||||
PROFILE_IMAGE_KEY_PREFIX = 'image_url'
|
||||
LOGGER = logging.getLogger(__name__)
|
||||
@@ -46,6 +48,15 @@ class LanguageProficiencySerializer(serializers.ModelSerializer):
|
||||
return None
|
||||
|
||||
|
||||
class SocialLinkSerializer(serializers.ModelSerializer):
|
||||
"""
|
||||
Class that serializes the SocialLink model for the UserProfile object.
|
||||
"""
|
||||
class Meta(object):
|
||||
model = SocialLink
|
||||
fields = ("platform", "social_link")
|
||||
|
||||
|
||||
class UserReadOnlySerializer(serializers.Serializer):
|
||||
"""
|
||||
Class that serializes the User model and UserProfile model together.
|
||||
@@ -99,7 +110,8 @@ class UserReadOnlySerializer(serializers.Serializer):
|
||||
"mailing_address": None,
|
||||
"requires_parental_consent": None,
|
||||
"accomplishments_shared": accomplishments_shared,
|
||||
"account_privacy": self.configuration.get('default_visibility')
|
||||
"account_privacy": self.configuration.get('default_visibility'),
|
||||
"social_links": None,
|
||||
}
|
||||
|
||||
if user_profile:
|
||||
@@ -122,7 +134,10 @@ class UserReadOnlySerializer(serializers.Serializer):
|
||||
),
|
||||
"mailing_address": user_profile.mailing_address,
|
||||
"requires_parental_consent": user_profile.requires_parental_consent(),
|
||||
"account_privacy": get_profile_visibility(user_profile, user, self.configuration)
|
||||
"account_privacy": get_profile_visibility(user_profile, user, self.configuration),
|
||||
"social_links": SocialLinkSerializer(
|
||||
user_profile.social_links.all(), many=True
|
||||
).data,
|
||||
}
|
||||
)
|
||||
|
||||
@@ -168,11 +183,12 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
profile_image = serializers.SerializerMethodField("_get_profile_image")
|
||||
requires_parental_consent = serializers.SerializerMethodField()
|
||||
language_proficiencies = LanguageProficiencySerializer(many=True, required=False)
|
||||
social_links = SocialLinkSerializer(many=True, required=False)
|
||||
|
||||
class Meta(object):
|
||||
model = UserProfile
|
||||
fields = (
|
||||
"name", "gender", "goals", "year_of_birth", "level_of_education", "country",
|
||||
"name", "gender", "goals", "year_of_birth", "level_of_education", "country", "social_links",
|
||||
"mailing_address", "bio", "profile_image", "requires_parental_consent", "language_proficiencies"
|
||||
)
|
||||
# Currently no read-only field, but keep this so view code doesn't need to know.
|
||||
@@ -192,7 +208,15 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
language_proficiencies = [language for language in value]
|
||||
unique_language_proficiencies = set(language["code"] for language in language_proficiencies)
|
||||
if len(language_proficiencies) != len(unique_language_proficiencies):
|
||||
raise serializers.ValidationError("The language_proficiencies field must consist of unique languages")
|
||||
raise serializers.ValidationError("The language_proficiencies field must consist of unique languages.")
|
||||
return value
|
||||
|
||||
def validate_social_links(self, value):
|
||||
""" Enforce only one entry for a particular social platform. """
|
||||
social_links = [social_link for social_link in value]
|
||||
unique_social_links = set(social_link["platform"] for social_link in social_links)
|
||||
if len(social_links) != len(unique_social_links):
|
||||
raise serializers.ValidationError("The social_links field must consist of unique social platforms.")
|
||||
return value
|
||||
|
||||
def transform_gender(self, user_profile, value): # pylint: disable=unused-argument
|
||||
@@ -244,20 +268,22 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
def update(self, instance, validated_data):
|
||||
"""
|
||||
Update the profile, including nested fields.
|
||||
|
||||
Raises:
|
||||
errors.AccountValidationError: the update was not attempted because validation errors were found with
|
||||
the supplied update
|
||||
"""
|
||||
language_proficiencies = validated_data.pop("language_proficiencies", None)
|
||||
|
||||
# Update all fields on the user profile that are writeable,
|
||||
# except for "language_proficiencies", which we'll update separately
|
||||
update_fields = set(self.get_writeable_fields()) - set(["language_proficiencies"])
|
||||
# except for "language_proficiencies" and "social_links", which we'll update separately
|
||||
update_fields = set(self.get_writeable_fields()) - set(["language_proficiencies"]) - set(["social_links"])
|
||||
for field_name in update_fields:
|
||||
default = getattr(instance, field_name)
|
||||
field_value = validated_data.get(field_name, default)
|
||||
setattr(instance, field_name, field_value)
|
||||
|
||||
instance.save()
|
||||
|
||||
# Now update the related language proficiency
|
||||
# Update the related language proficiency
|
||||
if language_proficiencies is not None:
|
||||
instance.language_proficiencies.all().delete()
|
||||
instance.language_proficiencies.bulk_create([
|
||||
@@ -265,6 +291,39 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
|
||||
for language in language_proficiencies
|
||||
])
|
||||
|
||||
# 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(err.message),
|
||||
"user_message": err.message
|
||||
}
|
||||
})
|
||||
|
||||
# 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)
|
||||
])
|
||||
|
||||
instance.save()
|
||||
|
||||
return instance
|
||||
|
||||
|
||||
|
||||
@@ -297,6 +297,7 @@ class AccountSettingsOnCreationTest(TestCase):
|
||||
'mailing_address': None,
|
||||
'year_of_birth': None,
|
||||
'country': None,
|
||||
'social_links': [],
|
||||
'bio': None,
|
||||
'profile_image': {
|
||||
'has_image': False,
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
""" Unit tests for custom UserProfile properties. """
|
||||
|
||||
import ddt
|
||||
|
||||
from django.test import TestCase
|
||||
from openedx.core.djangolib.testing.utils import skip_unless_lms
|
||||
|
||||
from ..utils import validate_social_link, format_social_link
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class UserAccountSettingsTest(TestCase):
|
||||
"""Unit tests for setting Social Media Links."""
|
||||
|
||||
def setUp(self):
|
||||
super(UserAccountSettingsTest, self).setUp()
|
||||
|
||||
def validate_social_link(self, social_platform, link):
|
||||
"""
|
||||
Helper method that returns True if the social link is valid, False if
|
||||
the input link fails validation and will throw an error.
|
||||
"""
|
||||
try:
|
||||
validate_social_link(social_platform, link)
|
||||
except ValueError:
|
||||
return False
|
||||
return True
|
||||
|
||||
@ddt.data(
|
||||
('facebook', 'www.facebook.com/edX', 'https://www.facebook.com/edX', True),
|
||||
('facebook', 'facebook.com/edX/', 'https://www.facebook.com/edX', True),
|
||||
('facebook', 'HTTP://facebook.com/edX/', 'https://www.facebook.com/edX', True),
|
||||
('facebook', 'www.evilwebsite.com/123', None, False),
|
||||
('twitter', 'https://www.twiter.com/edX/', None, False),
|
||||
('twitter', 'https://www.twitter.com/edX/123s', None, False),
|
||||
('twitter', 'twitter.com/edX', 'https://www.twitter.com/edX', True),
|
||||
('twitter', 'twitter.com/edX?foo=bar', 'https://www.twitter.com/edX', True),
|
||||
('linkedin', 'www.linkedin.com/harryrein', None, False),
|
||||
('linkedin', 'www.linkedin.com/in/harryrein-1234', 'https://www.linkedin.com/in/harryrein-1234', True),
|
||||
('linkedin', 'www.evilwebsite.com/123?www.linkedin.com/edX', None, False),
|
||||
('linkedin', '', '', True),
|
||||
('linkedin', None, None, False),
|
||||
)
|
||||
@ddt.unpack
|
||||
@skip_unless_lms
|
||||
def test_social_link_input(self, platform_name, link_input, formatted_link_expected, is_valid_expected):
|
||||
"""
|
||||
Verify that social links are correctly validated and formatted.
|
||||
"""
|
||||
self.assertEqual(is_valid_expected, self.validate_social_link(platform_name, link_input))
|
||||
|
||||
self.assertEqual(formatted_link_expected, format_social_link(platform_name, link_input))
|
||||
@@ -222,7 +222,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
Verify that the shareable fields from the account are returned
|
||||
"""
|
||||
data = response.data
|
||||
self.assertEqual(9, len(data))
|
||||
self.assertEqual(10, len(data))
|
||||
self.assertEqual(self.user.username, data["username"])
|
||||
self.assertEqual("US", data["country"])
|
||||
self._verify_profile_image_data(data, True)
|
||||
@@ -247,7 +247,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
Verify that all account fields are returned (even those that are not shareable).
|
||||
"""
|
||||
data = response.data
|
||||
self.assertEqual(17, len(data))
|
||||
self.assertEqual(18, len(data))
|
||||
self.assertEqual(self.user.username, data["username"])
|
||||
self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
|
||||
self.assertEqual("US", data["country"])
|
||||
@@ -305,7 +305,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
"""
|
||||
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
|
||||
self.create_mock_profile(self.user)
|
||||
with self.assertNumQueries(19):
|
||||
with self.assertNumQueries(20):
|
||||
response = self.send_get(self.different_client)
|
||||
self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY)
|
||||
|
||||
@@ -320,7 +320,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
"""
|
||||
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
|
||||
self.create_mock_profile(self.user)
|
||||
with self.assertNumQueries(19):
|
||||
with self.assertNumQueries(20):
|
||||
response = self.send_get(self.different_client)
|
||||
self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY)
|
||||
|
||||
@@ -376,7 +376,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
with self.assertNumQueries(queries):
|
||||
response = self.send_get(self.client)
|
||||
data = response.data
|
||||
self.assertEqual(17, len(data))
|
||||
self.assertEqual(18, 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"):
|
||||
@@ -395,12 +395,12 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
self.assertEqual(False, data["accomplishments_shared"])
|
||||
|
||||
self.client.login(username=self.user.username, password=TEST_PASSWORD)
|
||||
verify_get_own_information(17)
|
||||
verify_get_own_information(18)
|
||||
|
||||
# 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)
|
||||
verify_get_own_information(12)
|
||||
|
||||
def test_get_account_empty_string(self):
|
||||
"""
|
||||
@@ -414,7 +414,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
legacy_profile.save()
|
||||
|
||||
self.client.login(username=self.user.username, password=TEST_PASSWORD)
|
||||
with self.assertNumQueries(17):
|
||||
with self.assertNumQueries(18):
|
||||
response = self.send_get(self.client)
|
||||
for empty_field in ("level_of_education", "gender", "country", "bio"):
|
||||
self.assertIsNone(response.data[empty_field])
|
||||
@@ -695,7 +695,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
),
|
||||
(
|
||||
[{u"code": u"kw"}, {u"code": u"el"}, {u"code": u"kw"}],
|
||||
[u'The language_proficiencies field must consist of unique languages']
|
||||
[u'The language_proficiencies field must consist of unique languages.']
|
||||
),
|
||||
)
|
||||
@ddt.unpack
|
||||
@@ -769,7 +769,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
|
||||
response = self.send_get(client)
|
||||
if has_full_access:
|
||||
data = response.data
|
||||
self.assertEqual(17, len(data))
|
||||
self.assertEqual(18, len(data))
|
||||
self.assertEqual(self.user.username, data["username"])
|
||||
self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
|
||||
self.assertEqual(self.user.email, data["email"])
|
||||
|
||||
90
openedx/core/djangoapps/user_api/accounts/utils.py
Normal file
90
openedx/core/djangoapps/user_api/accounts/utils.py
Normal file
@@ -0,0 +1,90 @@
|
||||
"""
|
||||
Utility methods for the account settings.
|
||||
"""
|
||||
import re
|
||||
from urlparse import urlparse
|
||||
|
||||
from django.conf import settings
|
||||
from django.utils.translation import ugettext as _
|
||||
|
||||
|
||||
def validate_social_link(platform_name, new_social_link):
|
||||
"""
|
||||
Given a new social link for a user, ensure that the link takes one of the
|
||||
following forms:
|
||||
|
||||
1) A valid url that comes from the correct social site.
|
||||
2) A valid username.
|
||||
3) A blank value.
|
||||
"""
|
||||
formatted_social_link = format_social_link(platform_name, new_social_link)
|
||||
|
||||
# Ensure that the new link is valid.
|
||||
if formatted_social_link is None:
|
||||
required_url_stub = settings.SOCIAL_PLATFORMS[platform_name]['url_stub']
|
||||
raise ValueError(_(
|
||||
' Make sure that you are providing a valid username or a URL that contains "' +
|
||||
required_url_stub + '". To remove the link from your edX profile, leave this field blank.'
|
||||
))
|
||||
|
||||
|
||||
def format_social_link(platform_name, new_social_link):
|
||||
"""
|
||||
Given a user's social link, returns a safe absolute url for the social link.
|
||||
|
||||
Returns the following based on the provided new_social_link:
|
||||
1) Given an empty string, returns ''
|
||||
1) Given a valid username, return 'https://www.[platform_name_base][username]'
|
||||
2) Given a valid URL, return 'https://www.[platform_name_base][username]'
|
||||
3) Given anything unparseable, returns None
|
||||
"""
|
||||
# Blank social links should return '' or None as was passed in.
|
||||
if not new_social_link:
|
||||
return new_social_link
|
||||
|
||||
url_stub = settings.SOCIAL_PLATFORMS[platform_name]['url_stub']
|
||||
username = _get_username_from_social_link(platform_name, new_social_link)
|
||||
if not username:
|
||||
return None
|
||||
|
||||
# For security purposes, always build up the url rather than using input from user.
|
||||
return 'https://www.{}{}'.format(url_stub, username)
|
||||
|
||||
|
||||
def _get_username_from_social_link(platform_name, new_social_link):
|
||||
"""
|
||||
Returns the username given a social link.
|
||||
|
||||
Uses the following logic to parse new_social_link into a username:
|
||||
1) If an empty string, returns it as the username.
|
||||
2) Given a URL, attempts to parse the username from the url and return it.
|
||||
3) Given a non-URL, returns the entire string as username if valid.
|
||||
4) If no valid username is found, returns None.
|
||||
"""
|
||||
# Blank social links should return '' or None as was passed in.
|
||||
if not new_social_link:
|
||||
return new_social_link
|
||||
|
||||
# Parse the social link as if it were a URL.
|
||||
parse_result = urlparse(new_social_link)
|
||||
url_domain_and_path = parse_result[1] + parse_result[2]
|
||||
url_stub = re.escape(settings.SOCIAL_PLATFORMS[platform_name]['url_stub'])
|
||||
username_match = re.search('(www\.)?' + url_stub + '(?P<username>.*?)[/]?$', url_domain_and_path, re.IGNORECASE)
|
||||
if username_match:
|
||||
username = username_match.group('username')
|
||||
else:
|
||||
username = new_social_link
|
||||
|
||||
# Ensure the username is a valid username.
|
||||
if not _is_valid_social_username(username):
|
||||
return None
|
||||
|
||||
return username
|
||||
|
||||
|
||||
def _is_valid_social_username(value):
|
||||
"""
|
||||
Given a particular string, returns whether the string can be considered a safe username.
|
||||
A safe username contains only hyphens, underscores or other alphanumerical characters.
|
||||
"""
|
||||
return bool(re.match('^[a-zA-Z0-9_-]*$', value))
|
||||
@@ -109,6 +109,12 @@ 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:
|
||||
|
||||
* "platform": A particular social platform, ex: 'facebook'
|
||||
* "social_link": The link to the user's profile on the particular platform
|
||||
|
||||
* username: The username associated with the account.
|
||||
* year_of_birth: The year the user was born, as an integer, or null.
|
||||
* account_privacy: The user's setting for sharing her personal
|
||||
|
||||
Reference in New Issue
Block a user