Commit 97e44ed2 by Daniel Friedman Committed by Andy Armstrong

Implement language proficiencies.

TNL-1488
parent 4125bf96
...@@ -1608,3 +1608,19 @@ class EntranceExamConfiguration(models.Model): ...@@ -1608,3 +1608,19 @@ class EntranceExamConfiguration(models.Model):
except EntranceExamConfiguration.DoesNotExist: except EntranceExamConfiguration.DoesNotExist:
can_skip = False can_skip = False
return can_skip return can_skip
class LanguageProficiency(models.Model):
"""
Represents a user's language proficiency.
"""
class Meta:
unique_together = (('code', 'user_profile'),)
user_profile = models.ForeignKey(UserProfile, db_index=True, related_name='language_proficiencies')
code = models.CharField(
max_length=16,
blank=False,
choices=settings.ALL_LANGUAGES,
help_text=ugettext_lazy("The ISO 639-1 language code for this language.")
)
...@@ -1924,6 +1924,8 @@ TIME_ZONE_DISPLAYED_FOR_DEADLINES = 'UTC' ...@@ -1924,6 +1924,8 @@ TIME_ZONE_DISPLAYED_FOR_DEADLINES = 'UTC'
# Source: # Source:
# http://loc.gov/standards/iso639-2/ISO-639-2_utf-8.txt according to http://en.wikipedia.org/wiki/ISO_639-1 # http://loc.gov/standards/iso639-2/ISO-639-2_utf-8.txt according to http://en.wikipedia.org/wiki/ISO_639-1
# Note that this is used as the set of choices to the `code` field of the
# `LanguageProficiency` model.
ALL_LANGUAGES = ( ALL_LANGUAGES = (
[u"aa", u"Afar"], [u"aa", u"Afar"],
[u"ab", u"Abkhazian"], [u"ab", u"Abkhazian"],
...@@ -2230,7 +2232,7 @@ ACCOUNT_VISIBILITY_CONFIGURATION = { ...@@ -2230,7 +2232,7 @@ ACCOUNT_VISIBILITY_CONFIGURATION = {
'profile_image', 'profile_image',
'country', 'country',
'time_zone', 'time_zone',
'languages', 'language_proficiencies',
'bio', 'bio',
], ],
......
...@@ -3,12 +3,34 @@ from django.contrib.auth.models import User ...@@ -3,12 +3,34 @@ from django.contrib.auth.models import User
from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH
from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerializerMixin
from student.models import UserProfile from student.models import UserProfile, LanguageProficiency
from .helpers import get_profile_image_url_for_user, PROFILE_IMAGE_SIZES_MAP from .helpers import get_profile_image_url_for_user, PROFILE_IMAGE_SIZES_MAP
PROFILE_IMAGE_KEY_PREFIX = 'image_url' PROFILE_IMAGE_KEY_PREFIX = 'image_url'
class LanguageProficiencySerializer(serializers.ModelSerializer):
"""
Class that serializes the LanguageProficiency model for account
information.
"""
class Meta:
model = LanguageProficiency
fields = ("code",)
def get_identity(self, data):
"""
This is used in bulk updates to determine the identity of an object.
The default is to use the id of an object, but we want to override that
and consider the language code to be the canonical identity of a
LanguageProficiency model.
"""
try:
return data.get('code', None)
except AttributeError:
return None
class AccountUserSerializer(serializers.HyperlinkedModelSerializer, ReadOnlyFieldsSerializerMixin): class AccountUserSerializer(serializers.HyperlinkedModelSerializer, ReadOnlyFieldsSerializerMixin):
""" """
Class that serializes the portion of User model needed for account information. Class that serializes the portion of User model needed for account information.
...@@ -26,12 +48,13 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea ...@@ -26,12 +48,13 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
""" """
profile_image = serializers.SerializerMethodField("get_profile_image") profile_image = serializers.SerializerMethodField("get_profile_image")
requires_parental_consent = serializers.SerializerMethodField("get_requires_parental_consent") requires_parental_consent = serializers.SerializerMethodField("get_requires_parental_consent")
language_proficiencies = LanguageProficiencySerializer(many=True, allow_add_remove=True, required=False)
class Meta: class Meta:
model = UserProfile model = UserProfile
fields = ( fields = (
"name", "gender", "goals", "year_of_birth", "level_of_education", "language", "country", "name", "gender", "goals", "year_of_birth", "level_of_education", "country",
"mailing_address", "bio", "profile_image", "requires_parental_consent", "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. # Currently no read-only field, but keep this so view code doesn't need to know.
read_only_fields = () read_only_fields = ()
...@@ -49,6 +72,14 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea ...@@ -49,6 +72,14 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea
return attrs return attrs
def validate_language_proficiencies(self, attrs, source):
""" Enforce all languages are unique. """
language_proficiencies = [language for language in attrs.get(source, [])]
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")
return attrs
def transform_gender(self, user_profile, value): def transform_gender(self, user_profile, value):
""" Converts empty string to None, to indicate not set. Replaced by to_representation in version 3. """ """ Converts empty string to None, to indicate not set. Replaced by to_representation in version 3. """
return AccountLegacyProfileSerializer.convert_empty_to_None(value) return AccountLegacyProfileSerializer.convert_empty_to_None(value)
......
...@@ -123,6 +123,20 @@ class TestAccountApi(TestCase): ...@@ -123,6 +123,20 @@ class TestAccountApi(TestCase):
{"profile_image": {"has_image": "not_allowed", "image_url": "not_allowed"}} {"profile_image": {"has_image": "not_allowed", "image_url": "not_allowed"}}
) )
# Check the various language_proficiencies validation failures.
# language_proficiencies must be a list of dicts, each containing a
# unique 'code' key representing the language code.
with self.assertRaises(AccountValidationError):
update_account_settings(
self.user,
{"language_proficiencies": "not_a_list"}
)
with self.assertRaises(AccountValidationError):
update_account_settings(
self.user,
{"language_proficiencies": [{}]}
)
def test_update_multiple_validation_errors(self): def test_update_multiple_validation_errors(self):
"""Test that all validation errors are built up and returned at once""" """Test that all validation errors are built up and returned at once"""
# Send a read-only error, serializer error, and email validation error. # Send a read-only error, serializer error, and email validation error.
...@@ -207,7 +221,6 @@ class AccountSettingsOnCreationTest(TestCase): ...@@ -207,7 +221,6 @@ class AccountSettingsOnCreationTest(TestCase):
'email': self.EMAIL, 'email': self.EMAIL,
'name': u'', 'name': u'',
'gender': None, 'gender': None,
'language': u'',
'goals': None, 'goals': None,
'is_active': False, 'is_active': False,
'level_of_education': None, 'level_of_education': None,
...@@ -221,6 +234,7 @@ class AccountSettingsOnCreationTest(TestCase): ...@@ -221,6 +234,7 @@ class AccountSettingsOnCreationTest(TestCase):
'image_url_small': 'http://example-storage.com/profile_images/default_10.jpg', 'image_url_small': 'http://example-storage.com/profile_images/default_10.jpg',
}, },
'requires_parental_consent': True, 'requires_parental_consent': True,
'language_proficiencies': [],
}) })
......
...@@ -13,7 +13,7 @@ from django.test.utils import override_settings ...@@ -13,7 +13,7 @@ from django.test.utils import override_settings
from rest_framework.test import APITestCase, APIClient from rest_framework.test import APITestCase, APIClient
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from student.models import UserProfile, PendingEmailChange from student.models import UserProfile, LanguageProficiency, PendingEmailChange
from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY from openedx.core.djangoapps.user_api.accounts import ACCOUNT_VISIBILITY_PREF_KEY
from openedx.core.djangoapps.user_api.preferences.api import set_user_preference from openedx.core.djangoapps.user_api.preferences.api import set_user_preference
from .. import PRIVATE_VISIBILITY, ALL_USERS_VISIBILITY from .. import PRIVATE_VISIBILITY, ALL_USERS_VISIBILITY
...@@ -91,6 +91,7 @@ class UserAPITestCase(APITestCase): ...@@ -91,6 +91,7 @@ class UserAPITestCase(APITestCase):
legacy_profile.gender = "f" legacy_profile.gender = "f"
legacy_profile.bio = "Tired mother of twins" legacy_profile.bio = "Tired mother of twins"
legacy_profile.has_profile_image = True legacy_profile.has_profile_image = True
legacy_profile.language_proficiencies.add(LanguageProficiency(code='en'))
legacy_profile.save() legacy_profile.save()
...@@ -138,7 +139,7 @@ class TestAccountAPI(UserAPITestCase): ...@@ -138,7 +139,7 @@ class TestAccountAPI(UserAPITestCase):
self.assertEqual("US", data["country"]) self.assertEqual("US", data["country"])
self._verify_profile_image_data(data, True) self._verify_profile_image_data(data, True)
self.assertIsNone(data["time_zone"]) self.assertIsNone(data["time_zone"])
self.assertIsNone(data["languages"]) self.assertEqual([{"code": "en"}], data["language_proficiencies"])
self.assertEqual("Tired mother of twins", data["bio"]) self.assertEqual("Tired mother of twins", data["bio"])
def _verify_private_account_response(self, response, requires_parental_consent=False): def _verify_private_account_response(self, response, requires_parental_consent=False):
...@@ -159,7 +160,6 @@ class TestAccountAPI(UserAPITestCase): ...@@ -159,7 +160,6 @@ class TestAccountAPI(UserAPITestCase):
self.assertEqual(self.user.username, data["username"]) self.assertEqual(self.user.username, data["username"])
self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"])
self.assertEqual("US", data["country"]) self.assertEqual("US", data["country"])
self.assertEqual("", data["language"])
self.assertEqual("f", data["gender"]) self.assertEqual("f", data["gender"])
self.assertEqual(2000, data["year_of_birth"]) self.assertEqual(2000, data["year_of_birth"])
self.assertEqual("m", data["level_of_education"]) self.assertEqual("m", data["level_of_education"])
...@@ -171,6 +171,7 @@ class TestAccountAPI(UserAPITestCase): ...@@ -171,6 +171,7 @@ class TestAccountAPI(UserAPITestCase):
self.assertEqual("Tired mother of twins", data["bio"]) self.assertEqual("Tired mother of twins", data["bio"])
self._verify_profile_image_data(data, not requires_parental_consent) self._verify_profile_image_data(data, not requires_parental_consent)
self.assertEquals(requires_parental_consent, data["requires_parental_consent"]) self.assertEquals(requires_parental_consent, data["requires_parental_consent"])
self.assertEqual([{"code": "en"}], data["language_proficiencies"])
def test_anonymous_access(self): def test_anonymous_access(self):
""" """
...@@ -279,7 +280,6 @@ class TestAccountAPI(UserAPITestCase): ...@@ -279,7 +280,6 @@ class TestAccountAPI(UserAPITestCase):
self.assertIsNone(data[empty_field]) self.assertIsNone(data[empty_field])
self.assertIsNone(data["country"]) self.assertIsNone(data["country"])
# TODO: what should the format of this be? # TODO: what should the format of this be?
self.assertEqual("", data["language"])
self.assertEqual("m", data["gender"]) self.assertEqual("m", data["gender"])
self.assertEqual("Learn a lot", data["goals"]) self.assertEqual("Learn a lot", data["goals"])
self.assertEqual(self.user.email, data["email"]) self.assertEqual(self.user.email, data["email"])
...@@ -287,6 +287,7 @@ class TestAccountAPI(UserAPITestCase): ...@@ -287,6 +287,7 @@ class TestAccountAPI(UserAPITestCase):
self.assertEqual(self.user.is_active, data["is_active"]) self.assertEqual(self.user.is_active, data["is_active"])
self._verify_profile_image_data(data, False) self._verify_profile_image_data(data, False)
self.assertTrue(data["requires_parental_consent"]) self.assertTrue(data["requires_parental_consent"])
self.assertEqual([], data["language_proficiencies"])
self.client.login(username=self.user.username, password=self.test_password) self.client.login(username=self.user.username, password=self.test_password)
verify_get_own_information() verify_get_own_information()
...@@ -347,7 +348,6 @@ class TestAccountAPI(UserAPITestCase): ...@@ -347,7 +348,6 @@ class TestAccountAPI(UserAPITestCase):
("year_of_birth", 2009, "not_an_int", u"Enter a whole number."), ("year_of_birth", 2009, "not_an_int", u"Enter a whole number."),
("name", "bob", "z" * 256, u"Ensure this value has at most 255 characters (it has 256)."), ("name", "bob", "z" * 256, u"Ensure this value has at most 255 characters (it has 256)."),
("name", u"ȻħȺɍłɇs", "z ", u"The name field must be at least 2 characters long."), ("name", u"ȻħȺɍłɇs", "z ", u"The name field must be at least 2 characters long."),
("language", "Creole"),
("goals", "Smell the roses"), ("goals", "Smell the roses"),
("mailing_address", "Sesame Street"), ("mailing_address", "Sesame Street"),
("bio", "Lacrosse-playing superhero"), ("bio", "Lacrosse-playing superhero"),
...@@ -355,6 +355,7 @@ class TestAccountAPI(UserAPITestCase): ...@@ -355,6 +355,7 @@ class TestAccountAPI(UserAPITestCase):
# Note that we store the raw data, so it is up to client to escape the HTML. # Note that we store the raw data, so it is up to client to escape the HTML.
("bio", "<html>fancy text</html>"), ("bio", "<html>fancy text</html>"),
# Note that email is tested below, as it is not immediately updated. # Note that email is tested below, as it is not immediately updated.
# Note that language_proficiencies is tested below as there are multiple error and success conditions.
) )
@ddt.unpack @ddt.unpack
def test_patch_account(self, field, value, fails_validation_value=None, developer_validation_message=None): def test_patch_account(self, field, value, fails_validation_value=None, developer_validation_message=None):
...@@ -535,6 +536,42 @@ class TestAccountAPI(UserAPITestCase): ...@@ -535,6 +536,42 @@ class TestAccountAPI(UserAPITestCase):
) )
self.assertEqual("Valid e-mail address required.", field_errors["email"]["user_message"]) self.assertEqual("Valid e-mail address required.", field_errors["email"]["user_message"])
def test_patch_language_proficiencies(self):
"""
Verify that patching the language_proficiencies field of the user
profile completely overwrites the previous value.
"""
client = self.login_client("client", "user")
# Patching language_proficiencies exercises the
# `LanguageProficiencySerializer.get_identity` method, which compares
# identifies language proficiencies based on their language code rather
# than django model id.
for proficiencies in ([{"code": "en"}, {"code": "fr"}, {"code": "es"}], [{"code": "fr"}], [{"code": "aa"}], []):
self.send_patch(client, {"language_proficiencies": proficiencies})
response = self.send_get(client)
self.assertItemsEqual(response.data["language_proficiencies"], proficiencies)
@ddt.data(
(u"not_a_list", [{u'non_field_errors': [u'Expected a list of items.']}]),
([u"not_a_JSON_object"], [{u'non_field_errors': [u'Invalid data']}]),
([{}], [{"code": [u"This field is required."]}]),
([{u"code": u"invalid_language_code"}], [{'code': [u'Select a valid choice. invalid_language_code is not one of the available choices.']}]),
([{u"code": u"kw"}, {u"code": u"el"}, {u"code": u"kw"}], [u'The language_proficiencies field must consist of unique languages']),
)
@ddt.unpack
def test_patch_invalid_language_proficiencies(self, patch_value, expected_error_message):
"""
Verify we handle error cases when patching the language_proficiencies
field.
"""
client = self.login_client("client", "user")
response = self.send_patch(client, {"language_proficiencies": patch_value}, expected_status=400)
self.assertEqual(
response.data["field_errors"]["language_proficiencies"]["developer_message"],
u"Value '{patch_value}' is not valid for field 'language_proficiencies': {error_message}".format(patch_value=patch_value, error_message=expected_error_message)
)
@patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save') @patch('openedx.core.djangoapps.user_api.accounts.serializers.AccountUserSerializer.save')
def test_patch_serializer_save_fails(self, serializer_save): def test_patch_serializer_save_fails(self, serializer_save):
""" """
...@@ -566,9 +603,52 @@ class TestAccountAPI(UserAPITestCase): ...@@ -566,9 +603,52 @@ class TestAccountAPI(UserAPITestCase):
"image_url_full": "http://testserver/profile_images/default_50.jpg", "image_url_full": "http://testserver/profile_images/default_50.jpg",
"image_url_small": "http://testserver/profile_images/default_10.jpg" "image_url_small": "http://testserver/profile_images/default_10.jpg"
} }
) )
@ddt.data(
("client", "user", True),
("different_client", "different_user", False),
("staff_client", "staff_user", True),
)
@ddt.unpack
def test_parental_consent(self, api_client, requesting_username, has_full_access):
"""
Verifies that under thirteens never return a public profile.
"""
client = self.login_client(api_client, requesting_username)
# Set the user to be ten years old with a public profile
legacy_profile = UserProfile.objects.get(id=self.user.id)
current_year = datetime.datetime.now().year
legacy_profile.year_of_birth = current_year - 10
legacy_profile.save()
set_user_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, ALL_USERS_VISIBILITY)
# Verify that the default view is still private (except for clients with full access)
response = self.send_get(client)
if has_full_access:
data = response.data
self.assertEqual(15, 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"])
self.assertEqual(current_year - 10, data["year_of_birth"])
for empty_field in ("country", "level_of_education", "mailing_address", "bio"):
self.assertIsNone(data[empty_field])
self.assertEqual("m", data["gender"])
self.assertEqual("Learn a lot", data["goals"])
self.assertTrue(data["is_active"])
self.assertIsNotNone(data["date_joined"])
self._verify_profile_image_data(data, False)
self.assertTrue(data["requires_parental_consent"])
else:
self._verify_private_account_response(response, requires_parental_consent=True)
# Verify that the shared view is still private
response = self.send_get(client, query_parameters='view=shared')
self._verify_private_account_response(response, requires_parental_consent=True)
@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
class TestAccountAPITransactions(TransactionTestCase): class TestAccountAPITransactions(TransactionTestCase):
""" """
......
...@@ -47,18 +47,16 @@ class AccountView(APIView): ...@@ -47,18 +47,16 @@ class AccountView(APIView):
format provided by datetime. format provided by datetime.
For example, "2014-08-26T17:52:11Z". For example, "2014-08-26T17:52:11Z".
* gender: One of the fullowing values: * gender: One of the following values:
* "m"
* "m" * "f"
* "f" * "o"
* "o" * null
* null
* year_of_birth: The year the user was born, as an integer, or * year_of_birth: The year the user was born, as an integer, or
null. null.
* level_of_education: One of the following values: * level_of_education: One of the following values:
* "p": PhD or Doctorate * "p": PhD or Doctorate
* "m": Master's or professional degree * "m": Master's or professional degree
* "b": Bachelor's degree * "b": Bachelor's degree
...@@ -72,10 +70,13 @@ class AccountView(APIView): ...@@ -72,10 +70,13 @@ class AccountView(APIView):
* language: The user's preferred language, or null. * language: The user's preferred language, or null.
* country: null (not set), or a Country corresponding to one of
the ISO 3166-1 countries.
* country: A ISO 3166 country code or null. * country: A ISO 3166 country code or null.
* mailing_address: The textual representation of the user's * mailing_address: The textual representation of the user's
mailing address, or null. mailing address, or null.
* goals: The textual representation of the user's goals, or null. * goals: The textual representation of the user's goals, or null.
...@@ -98,6 +99,10 @@ class AccountView(APIView): ...@@ -98,6 +99,10 @@ class AccountView(APIView):
* requires_parental_consent: true if the user is a minor * requires_parental_consent: true if the user is a minor
requiring parental consent. requiring parental consent.
* language_proficiencies: array of language preferences.
Each preference is a JSON object with the following keys:
* "code": string ISO 639-1 language code e.g. "en".
For all text fields, clients rendering the values should take care For all text fields, clients rendering the values should take care
to HTML escape them to avoid script injections, as the data is to HTML escape them to avoid script injections, as the data is
stored exactly as specified. The intention is that plain text is stored exactly as specified. The intention is that plain text is
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment