From f72a6556a94465802194c1630b1d11eccc88694a Mon Sep 17 00:00:00 2001 From: Olivia Ruiz-Knott Date: Fri, 1 May 2020 18:34:13 -0400 Subject: [PATCH] MICROBA-311 Add US State field to UserProfile Add list of states and field to UserProfile model; add migration; add state field to relevant serializers and to ACCOUNT_VISIBILITY_CONFIGURATION. Removes state data if country is changed to something other than United States. --- .../migrations/0033_userprofile_state.py | 18 ++++++ common/djangoapps/student/models.py | 58 +++++++++++++++++++ lms/envs/common.py | 1 + .../core/djangoapps/user_api/accounts/api.py | 8 +++ .../user_api/accounts/serializers.py | 4 +- .../user_api/accounts/tests/test_api.py | 18 ++++++ .../user_api/accounts/tests/test_views.py | 17 ++++-- 7 files changed, 117 insertions(+), 7 deletions(-) create mode 100644 common/djangoapps/student/migrations/0033_userprofile_state.py diff --git a/common/djangoapps/student/migrations/0033_userprofile_state.py b/common/djangoapps/student/migrations/0033_userprofile_state.py new file mode 100644 index 0000000000..3aaec78862 --- /dev/null +++ b/common/djangoapps/student/migrations/0033_userprofile_state.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.12 on 2020-04-30 20:49 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0032_removed_logout_view_configuration'), + ] + + operations = [ + migrations.AddField( + model_name='userprofile', + name='state', + field=models.CharField(blank=True, choices=[('AL', 'Alabama'), ('AK', 'Alaska'), ('AZ', 'Arizona'), ('AR', 'Arkansas'), ('AA', 'Armed Forces Americas'), ('AE', 'Armed Forces Europe'), ('AP', 'Armed Forces Pacific'), ('CA', 'California'), ('CO', 'Colorado'), ('CT', 'Connecticut'), ('DE', 'Delaware'), ('DC', 'District Of Columbia'), ('FL', 'Florida'), ('GA', 'Georgia'), ('HI', 'Hawaii'), ('ID', 'Idaho'), ('IL', 'Illinois'), ('IN', 'Indiana'), ('IA', 'Iowa'), ('KS', 'Kansas'), ('KY', 'Kentucky'), ('LA', 'Louisiana'), ('ME', 'Maine'), ('MD', 'Maryland'), ('MA', 'Massachusetts'), ('MI', 'Michigan'), ('MN', 'Minnesota'), ('MS', 'Mississippi'), ('MO', 'Missouri'), ('MT', 'Montana'), ('NE', 'Nebraska'), ('NV', 'Nevada'), ('NH', 'New Hampshire'), ('NJ', 'New Jersey'), ('NM', 'New Mexico'), ('NY', 'New York'), ('NC', 'North Carolina'), ('ND', 'North Dakota'), ('OH', 'Ohio'), ('OK', 'Oklahoma'), ('OR', 'Oregon'), ('PA', 'Pennsylvania'), ('RI', 'Rhode Island'), ('SC', 'South Carolina'), ('SD', 'South Dakota'), ('TN', 'Tennessee'), ('TX', 'Texas'), ('UT', 'Utah'), ('VT', 'Vermont'), ('VA', 'Virginia'), ('WA', 'Washington'), ('WV', 'West Virginia'), ('WI', 'Wisconsin'), ('WY', 'Wyoming')], max_length=2, null=True), + ), + ] diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 172e3243a2..a11ad557f9 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -500,6 +500,64 @@ class UserProfile(models.Model): mailing_address = models.TextField(blank=True, null=True) city = models.TextField(blank=True, null=True) country = CountryField(blank=True, null=True) + COUNTRY_WITH_STATES = u'US' + STATE_CHOICES = ( + ('AL', 'Alabama'), + ('AK', 'Alaska'), + ('AZ', 'Arizona'), + ('AR', 'Arkansas'), + ('AA', 'Armed Forces Americas'), + ('AE', 'Armed Forces Europe'), + ('AP', 'Armed Forces Pacific'), + ('CA', 'California'), + ('CO', 'Colorado'), + ('CT', 'Connecticut'), + ('DE', 'Delaware'), + ('DC', 'District Of Columbia'), + ('FL', 'Florida'), + ('GA', 'Georgia'), + ('HI', 'Hawaii'), + ('ID', 'Idaho'), + ('IL', 'Illinois'), + ('IN', 'Indiana'), + ('IA', 'Iowa'), + ('KS', 'Kansas'), + ('KY', 'Kentucky'), + ('LA', 'Louisiana'), + ('ME', 'Maine'), + ('MD', 'Maryland'), + ('MA', 'Massachusetts'), + ('MI', 'Michigan'), + ('MN', 'Minnesota'), + ('MS', 'Mississippi'), + ('MO', 'Missouri'), + ('MT', 'Montana'), + ('NE', 'Nebraska'), + ('NV', 'Nevada'), + ('NH', 'New Hampshire'), + ('NJ', 'New Jersey'), + ('NM', 'New Mexico'), + ('NY', 'New York'), + ('NC', 'North Carolina'), + ('ND', 'North Dakota'), + ('OH', 'Ohio'), + ('OK', 'Oklahoma'), + ('OR', 'Oregon'), + ('PA', 'Pennsylvania'), + ('RI', 'Rhode Island'), + ('SC', 'South Carolina'), + ('SD', 'South Dakota'), + ('TN', 'Tennessee'), + ('TX', 'Texas'), + ('UT', 'Utah'), + ('VT', 'Vermont'), + ('VA', 'Virginia'), + ('WA', 'Washington'), + ('WV', 'West Virginia'), + ('WI', 'Wisconsin'), + ('WY', 'Wyoming'), + ) + state = models.CharField(blank=True, null=True, max_length=2, choices=STATE_CHOICES) goals = models.TextField(blank=True, null=True) allow_certificate = models.BooleanField(default=1) bio = models.CharField(blank=True, null=True, max_length=3000, db_index=False) diff --git a/lms/envs/common.py b/lms/envs/common.py index 5a6a81847e..fce2a1fa1e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -3302,6 +3302,7 @@ ACCOUNT_VISIBILITY_CONFIGURATION["admin_fields"] = ( "email", "extended_profile", "gender", + "state", "goals", "is_active", "mailing_address", diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index f693c61682..62d341c8a6 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -158,6 +158,7 @@ def update_account_settings(requesting_user, update, username=None): _notify_language_proficiencies_update_if_needed(update, user, user_profile, old_language_proficiencies) _store_old_name_if_needed(old_name, user_profile, requesting_user) _update_extended_profile_if_needed(update, user_profile) + _update_state_if_needed(update, user_profile) except PreferenceValidationError as err: raise AccountValidationError(err.preference_errors) @@ -290,6 +291,13 @@ def _update_extended_profile_if_needed(data, user_profile): user_profile.save() +def _update_state_if_needed(data, user_profile): + # If the country was changed to something other than US, remove the state. + if "country" in data and data['country'] != UserProfile.COUNTRY_WITH_STATES: + user_profile.state = None + user_profile.save() + + def _store_old_name_if_needed(old_name, user_profile, requesting_user): # If the name was changed, store information about the change operation. This is outside of the # serializer so that we can store who requested the change. diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index 0caa1314a6..7f197cb8b6 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -137,6 +137,7 @@ class UserReadOnlySerializer(serializers.Serializer): "is_active": user.is_active, "bio": None, "country": None, + "state": None, "profile_image": None, "language_proficiencies": None, "name": None, @@ -158,6 +159,7 @@ class UserReadOnlySerializer(serializers.Serializer): { "bio": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.bio), "country": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.country.code), + "state": AccountLegacyProfileSerializer.convert_empty_to_None(user_profile.state), "profile_image": AccountLegacyProfileSerializer.get_profile_image( user_profile, user, self.context.get('request') ), @@ -238,7 +240,7 @@ class AccountLegacyProfileSerializer(serializers.HyperlinkedModelSerializer, Rea class Meta(object): model = UserProfile fields = ( - "name", "gender", "goals", "year_of_birth", "level_of_education", "country", "social_links", + "name", "gender", "goals", "year_of_birth", "level_of_education", "country", "state", "social_links", "mailing_address", "bio", "profile_image", "requires_parental_consent", "language_proficiencies", "phone_number" ) 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 cc9b1de3f5..f0eac87b1c 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -481,6 +481,23 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAc self.assertIsNot(account_recovery, None) self.assertEqual(account_recovery.secondary_email, test_email) + def test_change_country_removes_state(self): + ''' + Test that changing the country (to something other than a country with + states) removes the state + ''' + # First set the country and state + update_account_settings(self.user, {"country": UserProfile.COUNTRY_WITH_STATES, "state": "MA"}) + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['country'], UserProfile.COUNTRY_WITH_STATES) + self.assertEqual(account_settings['state'], 'MA') + + # Change the country and check that state is removed + update_account_settings(self.user, {"country": ""}) + account_settings = get_account_settings(self.default_request)[0] + self.assertEqual(account_settings['country'], None) + self.assertEqual(account_settings['state'], None) + @patch('openedx.core.djangoapps.user_api.accounts.image_helpers._PROFILE_IMAGE_SIZES', [50, 10]) @patch.dict( @@ -521,6 +538,7 @@ class AccountSettingsOnCreationTest(CreateAccountMixin, TestCase): 'mailing_address': u'', 'year_of_birth': None, 'country': None, + 'state': None, 'social_links': [], 'bio': None, 'profile_image': { 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 e6ff25201d..96de95182e 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_views.py @@ -107,6 +107,7 @@ class UserAPITestCase(APITestCase): """ legacy_profile = UserProfile.objects.get(id=user.id) legacy_profile.country = "US" + legacy_profile.state = "MA" legacy_profile.level_of_education = "m" legacy_profile.year_of_birth = 2000 legacy_profile.goals = "world peace" @@ -266,7 +267,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): Verify that all account fields are returned (even those that are not shareable). """ data = response.data - self.assertEqual(24, len(data)) + self.assertEqual(25, len(data)) # public fields (3) expected_account_privacy = ( @@ -291,6 +292,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): # additional admin fields (10) self.assertEqual(self.user.email, data["email"]) self.assertIsNotNone(data["extended_profile"]) + self.assertEqual("MA", data["state"]) self.assertEqual("f", data["gender"]) self.assertEqual("world peace", data["goals"]) self.assertTrue(data["is_active"]) @@ -495,12 +497,13 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): with self.assertNumQueries(queries): response = self.send_get(self.client) data = response.data - self.assertEqual(24, len(data)) + self.assertEqual(25, 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"): self.assertIsNone(data[empty_field]) self.assertIsNone(data["country"]) + self.assertIsNone(data["state"]) self.assertEqual("m", data["gender"]) self.assertEqual("Learn a lot", data["goals"]) self.assertEqual(self.user.email, data["email"]) @@ -528,6 +531,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): """ legacy_profile = UserProfile.objects.get(id=self.user.id) legacy_profile.country = "" + legacy_profile.state = "" legacy_profile.level_of_education = "" legacy_profile.gender = "" legacy_profile.bio = "" @@ -536,7 +540,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): self.client.login(username=self.user.username, password=TEST_PASSWORD) with self.assertNumQueries(21): response = self.send_get(self.client) - for empty_field in ("level_of_education", "gender", "country", "bio"): + for empty_field in ("level_of_education", "gender", "country", "state", "bio",): self.assertIsNone(response.data[empty_field]) @ddt.data( @@ -572,6 +576,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): ("gender", "f", "not a gender", u'"not a gender" is not a valid choice.'), ("level_of_education", "none", u"ȻħȺɍłɇs", u'"ȻħȺɍłɇs" is not a valid choice.'), ("country", "GB", "XY", u'"XY" is not a valid choice.'), + ("state", "MA", "PY", u'"PY" is not a valid choice.'), ("year_of_birth", 2009, "not_an_int", u"A valid integer is required."), ("name", "bob", "z" * 256, u"Ensure this field has no more than 255 characters."), ("name", u"ȻħȺɍłɇs", " ", u"The name field must be at least 1 character long."), @@ -677,7 +682,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): Also verifies the behaviour when setting to None. """ self.client.login(username=self.user.username, password=TEST_PASSWORD) - for field_name in ["gender", "level_of_education", "country"]: + for field_name in ["gender", "level_of_education", "country", "state"]: response = self.send_patch(self.client, {field_name: ""}) # Although throwing a 400 might be reasonable, the default DRF behavior with ModelSerializer # is to convert to None, which also seems acceptable (and is difficult to override). @@ -906,12 +911,12 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase): response = self.send_get(client) if has_full_access: data = response.data - self.assertEqual(24, len(data)) + self.assertEqual(25, 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(year_of_birth, data["year_of_birth"]) - for empty_field in ("country", "level_of_education", "mailing_address", "bio"): + for empty_field in ("country", "level_of_education", "mailing_address", "bio", "state",): self.assertIsNone(data[empty_field]) self.assertEqual("m", data["gender"]) self.assertEqual("Learn a lot", data["goals"])