diff --git a/lms/envs/common.py b/lms/envs/common.py index 7629aebd98..6c19fe88ec 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2962,52 +2962,45 @@ ACCOUNT_VISIBILITY_CONFIGURATION = { # The value is one of: 'all_users', 'private' "default_visibility": "all_users", - # The list of all fields that can be shared with other users - "shareable_fields": [ - 'username', - 'profile_image', - 'country', - 'time_zone', - 'date_joined', - 'language_proficiencies', - 'bio', - 'social_links', - 'account_privacy', - # Not an actual field, but used to signal whether badges should be public. - 'accomplishments_shared', - ], - # The list of account fields that are always public "public_fields": [ - 'username', - 'profile_image', 'account_privacy', + 'profile_image', + 'username', ], +} - # The list of account fields that are visible only to staff and users viewing their own profiles - "admin_fields": [ - "username", +# The list of all fields that can be shared with other users +ACCOUNT_VISIBILITY_CONFIGURATION["shareable_fields"] = ( + ACCOUNT_VISIBILITY_CONFIGURATION["public_fields"] + [ + 'bio', + 'country', + 'date_joined', + 'language_proficiencies', + "level_of_education", + 'social_links', + 'time_zone', + + # Not an actual field, but used to signal whether badges should be public. + 'accomplishments_shared', + ] +) + +# The list of account fields that are visible only to staff and users viewing their own profiles +ACCOUNT_VISIBILITY_CONFIGURATION["admin_fields"] = ( + ACCOUNT_VISIBILITY_CONFIGURATION["shareable_fields"] + [ "email", - "is_active", - "bio", - "country", - "date_joined", - "profile_image", - "language_proficiencies", - "social_links", - "name", + "extended_profile", "gender", "goals", - "year_of_birth", - "level_of_education", + "is_active", "mailing_address", + "name", "requires_parental_consent", - "account_privacy", - "accomplishments_shared", - "extended_profile", "secondary_email", + "year_of_birth", ] -} +) # The current list of social platforms to be shown to the user. # diff --git a/openedx/core/djangoapps/user_api/accounts/__init__.py b/openedx/core/djangoapps/user_api/accounts/__init__.py index d89a14600c..e8dceff472 100644 --- a/openedx/core/djangoapps/user_api/accounts/__init__.py +++ b/openedx/core/djangoapps/user_api/accounts/__init__.py @@ -26,6 +26,12 @@ ALL_USERS_VISIBILITY = 'all_users' # Indicates the user's preference that all their account information be private. PRIVATE_VISIBILITY = 'private' +# Indicates that the user has custom preferences for the visibility of their account information. +CUSTOM_VISIBILITY = 'custom' + +# Prefix prepended to user preferences related to custom account visibility preferences. +VISIBILITY_PREFIX = 'visibility.' + # Translators: This message is shown when the Unicode usernames are NOT allowed. # It is shown to users who attempt to create a new account using invalid characters # in the username. diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index b060b6a0c8..794c32b68d 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -24,8 +24,8 @@ from openedx.core.djangoapps.user_api.serializers import ReadOnlyFieldsSerialize from student.models import UserProfile, LanguageProficiency, SocialLink from . import ( - NAME_MIN_LENGTH, ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY, - ALL_USERS_VISIBILITY, + NAME_MIN_LENGTH, ACCOUNT_VISIBILITY_PREF_KEY, PRIVATE_VISIBILITY, CUSTOM_VISIBILITY, + ALL_USERS_VISIBILITY, VISIBILITY_PREFIX ) from .image_helpers import get_profile_image_urls_for_user from .utils import validate_social_link, format_social_link @@ -467,37 +467,53 @@ def get_extended_profile(user_profile): return extended_profile -def get_profile_visibility(user_profile, user, configuration=None): +def get_profile_visibility(user_profile, user, configuration): """ Returns the visibility level for the specified user profile. """ if user_profile.requires_parental_consent(): return PRIVATE_VISIBILITY - if not configuration: - configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION - # Calling UserPreference directly because the requesting user may be different from existing_user # (and does not have to be is_staff). profile_privacy = UserPreference.get_value(user, ACCOUNT_VISIBILITY_PREF_KEY) - return profile_privacy if profile_privacy else configuration.get('default_visibility') + if profile_privacy: + return profile_privacy + else: + return configuration.get('default_visibility') def _visible_fields(user_profile, user, configuration=None): """ - Return what fields should be visible based on user settings + Return what fields should be visible based on user's preferences :param user_profile: User profile object :param user: User object :param configuration: A visibility configuration dictionary. :return: whitelist List of fields to be shown """ - if not configuration: configuration = settings.ACCOUNT_VISIBILITY_CONFIGURATION profile_visibility = get_profile_visibility(user_profile, user, configuration) if profile_visibility == ALL_USERS_VISIBILITY: return configuration.get('shareable_fields') + + elif profile_visibility == CUSTOM_VISIBILITY: + return _visible_fields_from_custom_preferences(user, configuration) + else: return configuration.get('public_fields') + + +def _visible_fields_from_custom_preferences(user, configuration): + """ + Returns all fields that are marked to be shared with other users in the + given user's preferences. Includes fields that are always public. + """ + preferences = UserPreference.get_all_preferences(user) + fields_shared_with_all_users = [ + field_name for field_name in configuration.get('shareable_fields') + if preferences.get('{}{}'.format(VISIBILITY_PREFIX, field_name)) == 'all_users' + ] + return set(fields_shared_with_all_users + configuration.get('public_fields')) 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 59f0aa9bb4..a3df5f0b95 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -425,7 +425,8 @@ class AccountSettingsOnCreationTest(TestCase): 'account_privacy': PRIVATE_VISIBILITY, 'accomplishments_shared': False, 'extended_profile': [], - 'secondary_email': None + 'secondary_email': None, + 'time_zone': None, }) def test_normalize_password(self): 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 92b2b7ad6c..d1f1a69a69 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -24,17 +24,19 @@ from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_un from student.models import PendingEmailChange, UserProfile from student.tests.factories import TEST_PASSWORD, UserFactory -from .. import ALL_USERS_VISIBILITY, PRIVATE_VISIBILITY +from .. import ALL_USERS_VISIBILITY, PRIVATE_VISIBILITY, CUSTOM_VISIBILITY TEST_PROFILE_IMAGE_UPLOADED_AT = datetime.datetime(2002, 1, 9, 15, 43, 1, tzinfo=pytz.UTC) - # this is used in one test to check the behavior of profile image url # generation with a relative url in the config. TEST_PROFILE_IMAGE_BACKEND = deepcopy(settings.PROFILE_IMAGE_BACKEND) TEST_PROFILE_IMAGE_BACKEND['options']['base_url'] = '/profile-images/' +TEST_BIO_VALUE = u"Tired mother of twins" +TEST_LANGUAGE_PROFICIENCY_CODE = u"hi" + class UserAPITestCase(APITestCase): """ @@ -107,9 +109,9 @@ class UserAPITestCase(APITestCase): legacy_profile.goals = "world peace" legacy_profile.mailing_address = "Park Ave" legacy_profile.gender = "f" - legacy_profile.bio = "Tired mother of twins" + legacy_profile.bio = TEST_BIO_VALUE legacy_profile.profile_image_uploaded_at = TEST_PROFILE_IMAGE_UPLOADED_AT - legacy_profile.language_proficiencies.create(code='en') + legacy_profile.language_proficiencies.create(code=TEST_LANGUAGE_PROFICIENCY_CODE) legacy_profile.save() def _verify_profile_image_data(self, data, has_profile_image): @@ -213,53 +215,87 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.url = reverse("accounts_api", kwargs={'username': self.user.username}) + def _set_user_age_to_10_years(self, user): + """ + Sets the given user's age to 10. + Returns the calculated year of birth. + """ + legacy_profile = UserProfile.objects.get(id=user.id) + current_year = datetime.datetime.now().year + year_of_birth = current_year - 10 + legacy_profile.year_of_birth = year_of_birth + legacy_profile.save() + return year_of_birth + def _verify_full_shareable_account_response(self, response, account_privacy=None, badges_enabled=False): """ Verify that the shareable fields from the account are returned """ data = response.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) - self.assertIsNone(data["time_zone"]) - self.assertEqual([{"code": "en"}], data["language_proficiencies"]) - self.assertEqual("Tired mother of twins", data["bio"]) + self.assertEqual(11, len(data)) + + # public fields (3) self.assertEqual(account_privacy, data["account_privacy"]) + self._verify_profile_image_data(data, True) + self.assertEqual(self.user.username, data["username"]) + + # additional shareable fields (8) + self.assertEqual(TEST_BIO_VALUE, data["bio"]) + self.assertEqual("US", data["country"]) + self.assertIsNotNone(data["date_joined"]) + self.assertEqual([{"code": TEST_LANGUAGE_PROFICIENCY_CODE}], data["language_proficiencies"]) + self.assertEqual("m", data["level_of_education"]) + self.assertIsNotNone(data["social_links"]) + self.assertIsNone(data["time_zone"]) self.assertEqual(badges_enabled, data['accomplishments_shared']) - def _verify_private_account_response(self, response, requires_parental_consent=False, account_privacy=None): + def _verify_private_account_response(self, response, requires_parental_consent=False): """ Verify that only the public fields are returned if a user does not want to share account fields """ data = response.data self.assertEqual(3, len(data)) - self.assertEqual(self.user.username, data["username"]) + self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) self._verify_profile_image_data(data, not requires_parental_consent) - self.assertEqual(account_privacy, data["account_privacy"]) + self.assertEqual(self.user.username, data["username"]) - def _verify_full_account_response(self, response, requires_parental_consent=False): + def _verify_full_account_response(self, response, requires_parental_consent=False, year_of_birth=2000): """ Verify that all account fields are returned (even those that are not shareable). """ data = response.data - self.assertEqual(20, 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"]) - self.assertEqual("f", data["gender"]) - self.assertEqual(2000, data["year_of_birth"]) - self.assertEqual("m", data["level_of_education"]) - self.assertEqual("world peace", data["goals"]) - self.assertEqual("Park Ave", data['mailing_address']) - self.assertEqual(self.user.email, data["email"]) - self.assertTrue(data["is_active"]) - self.assertIsNotNone(data["date_joined"]) - self.assertEqual("Tired mother of twins", data["bio"]) + self.assertEqual(21, len(data)) + + # public fields (3) + expected_account_privacy = ( + PRIVATE_VISIBILITY if requires_parental_consent else + UserPreference.get_value(self.user, 'account_privacy') + ) + self.assertEqual(expected_account_privacy, data["account_privacy"]) self._verify_profile_image_data(data, not requires_parental_consent) + self.assertEqual(self.user.username, data["username"]) + + # additional shareable fields (8) + self.assertEqual(TEST_BIO_VALUE, data["bio"]) + self.assertEqual("US", data["country"]) + self.assertIsNotNone(data["date_joined"]) + self.assertEqual([{"code": TEST_LANGUAGE_PROFICIENCY_CODE}], data["language_proficiencies"]) + self.assertEqual("m", data["level_of_education"]) + self.assertIsNotNone(data["social_links"]) + self.assertEqual(UserPreference.get_value(self.user, 'time_zone'), data["time_zone"]) + self.assertIsNotNone(data["accomplishments_shared"]) + + # additional admin fields (10) + self.assertEqual(self.user.email, data["email"]) + self.assertIsNotNone(data["extended_profile"]) + self.assertEqual("f", data["gender"]) + self.assertEqual("world peace", data["goals"]) + self.assertTrue(data["is_active"]) + self.assertEqual("Park Ave", data['mailing_address']) + self.assertEqual(self.user.first_name + " " + self.user.last_name, data["name"]) self.assertEquals(requires_parental_consent, data["requires_parental_consent"]) - self.assertEqual([{"code": "en"}], data["language_proficiencies"]) - self.assertEqual(UserPreference.get_value(self.user, 'account_privacy'), data["account_privacy"]) + self.assertIsNone(data["secondary_email"]) + self.assertEqual(year_of_birth, data["year_of_birth"]) def test_anonymous_access(self): """ @@ -318,7 +354,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.create_mock_profile(self.user) with self.assertNumQueries(22): response = self.send_get(self.different_client) - self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY) + self._verify_private_account_response(response) @mock.patch.dict(settings.FEATURES, {'ENABLE_OPENBADGES': True}) @ddt.data( @@ -339,7 +375,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): Confirms that private fields are private, and public/shareable fields are public/shareable """ if preference_visibility == PRIVATE_VISIBILITY: - self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY) + self._verify_private_account_response(response) else: self._verify_full_shareable_account_response(response, ALL_USERS_VISIBILITY, badges_enabled=True) @@ -359,6 +395,70 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): response = self.send_get(client, query_parameters='view=shared') verify_fields_visible_to_all_users(response) + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ("different_client", "different_user"), + ) + @ddt.unpack + def test_custom_visibility_over_age(self, api_client, requesting_username): + self.create_mock_profile(self.user) + # set user's custom visibility preferences + set_user_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, CUSTOM_VISIBILITY) + shared_fields = ("bio", "language_proficiencies") + for field_name in shared_fields: + set_user_preference(self.user, "visibility.{}".format(field_name), ALL_USERS_VISIBILITY) + + # make API request + client = self.login_client(api_client, requesting_username) + response = self.send_get(client) + + # verify response + if requesting_username == "different_user": + data = response.data + self.assertEqual(5, len(data)) + + # public fields + self.assertEqual(self.user.username, data["username"]) + self.assertEqual(UserPreference.get_value(self.user, 'account_privacy'), data["account_privacy"]) + self._verify_profile_image_data(data, has_profile_image=True) + + # custom shared fields + self.assertEqual(TEST_BIO_VALUE, data["bio"]) + self.assertEqual([{"code": TEST_LANGUAGE_PROFICIENCY_CODE}], data["language_proficiencies"]) + else: + self._verify_full_account_response(response) + + @ddt.data( + ("client", "user"), + ("staff_client", "staff_user"), + ("different_client", "different_user"), + ) + @ddt.unpack + def test_custom_visibility_under_age(self, api_client, requesting_username): + self.create_mock_profile(self.user) + year_of_birth = self._set_user_age_to_10_years(self.user) + + # set user's custom visibility preferences + set_user_preference(self.user, ACCOUNT_VISIBILITY_PREF_KEY, CUSTOM_VISIBILITY) + shared_fields = ("bio", "language_proficiencies") + for field_name in shared_fields: + set_user_preference(self.user, "visibility.{}".format(field_name), ALL_USERS_VISIBILITY) + + # make API request + client = self.login_client(api_client, requesting_username) + response = self.send_get(client) + + # verify response + if requesting_username == "different_user": + self._verify_private_account_response(response, requires_parental_consent=True) + else: + self._verify_full_account_response( + response, + requires_parental_consent=True, + year_of_birth=year_of_birth, + ) + def test_get_account_default(self): """ Test that a client (logged in) can get her own account information (using default legacy profile information, @@ -372,7 +472,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): with self.assertNumQueries(queries): response = self.send_get(self.client) data = response.data - self.assertEqual(20, len(data)) + self.assertEqual(21, 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"): @@ -387,6 +487,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.assertTrue(data["requires_parental_consent"]) self.assertEqual([], data["language_proficiencies"]) self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) + self.assertIsNone(data["time_zone"]) # Badges aren't on by default, so should not be present. self.assertEqual(False, data["accomplishments_shared"]) @@ -771,22 +872,18 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): """ 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() + year_of_birth = self._set_user_age_to_10_years(self.user) 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(20, len(data)) + self.assertEqual(21, 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"]) + self.assertEqual(year_of_birth, 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"]) @@ -797,15 +894,11 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.assertTrue(data["requires_parental_consent"]) self.assertEqual(PRIVATE_VISIBILITY, data["account_privacy"]) else: - self._verify_private_account_response( - response, requires_parental_consent=True, account_privacy=PRIVATE_VISIBILITY - ) + 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, account_privacy=PRIVATE_VISIBILITY - ) + self._verify_private_account_response(response, requires_parental_consent=True) @skip_unless_lms diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 25565997ba..c4d98116bc 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -212,8 +212,12 @@ class AccountViewSet(ViewSet): * 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 - profile. Possible values are "all_users" or "private". + profile. Possible values are "all_users", "private", or "custom". + If "custom", the user has selectively chosen a subset of shareable + fields to make visible to others via the User Preferences API. + * accomplishments_shared: Signals whether badges are enabled on the platform and should be fetched. @@ -223,7 +227,7 @@ class AccountViewSet(ViewSet): If a user who does not have "is_staff" access requests account information for a different user, only a subset of these fields is - returned. The returns fields depend on the + returned. The returned fields depend on the ACCOUNT_VISIBILITY_CONFIGURATION configuration setting and the visibility preference of the user for whom data is requested.