From c370028bbd25815f236d4137b0f9eec257897624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrey=20Ca=C3=B1on?= <36200299+andrey-canon@users.noreply.github.com> Date: Wed, 17 Jul 2024 08:08:44 -0500 Subject: [PATCH] =?UTF-8?q?feat:=20modify=20PhoneNumberSerializer=20regex?= =?UTF-8?q?=20to=20allow=20plus=20symbol=20at=20the=20=E2=80=A6=20(#35117)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: modify PhoneNumberSerializer regex to allow plus symbol at the beginning * chore: update PhonenumberSerializer docstring --- .../0046_alter_userprofile_phone_number.py | 19 ++++++ common/djangoapps/student/models/user.py | 5 +- .../tests/test_user_profile_properties.py | 60 ++++++++++++------- .../user_api/accounts/serializers.py | 18 +++++- 4 files changed, 78 insertions(+), 24 deletions(-) create mode 100644 common/djangoapps/student/migrations/0046_alter_userprofile_phone_number.py diff --git a/common/djangoapps/student/migrations/0046_alter_userprofile_phone_number.py b/common/djangoapps/student/migrations/0046_alter_userprofile_phone_number.py new file mode 100644 index 000000000000..9cfe9588b6d9 --- /dev/null +++ b/common/djangoapps/student/migrations/0046_alter_userprofile_phone_number.py @@ -0,0 +1,19 @@ +# Generated by Django 4.2.14 on 2024-07-16 22:21 + +import django.core.validators +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('student', '0045_auto_20230808_0944'), + ] + + operations = [ + migrations.AlterField( + model_name='userprofile', + name='phone_number', + field=models.CharField(blank=True, max_length=50, null=True, validators=[django.core.validators.RegexValidator(message="Phone number must start with '+' (optional) followed by digits (0-9) only.", regex='^\\+?1?\\d*$')]), + ), + ] diff --git a/common/djangoapps/student/models/user.py b/common/djangoapps/student/models/user.py index 2a4a9deb3664..aa3de546ef2b 100644 --- a/common/djangoapps/student/models/user.py +++ b/common/djangoapps/student/models/user.py @@ -552,7 +552,10 @@ class Meta: goals = models.TextField(blank=True, null=True) bio = models.CharField(blank=True, null=True, max_length=3000, db_index=False) profile_image_uploaded_at = models.DateTimeField(null=True, blank=True) - phone_regex = RegexValidator(regex=r'^\+?1?\d*$', message="Phone number can only contain numbers.") + phone_regex = RegexValidator( + regex=r'^\+?1?\d*$', + message="Phone number must start with '+' (optional) followed by digits (0-9) only.", + ) phone_number = models.CharField(validators=[phone_regex], blank=True, null=True, max_length=50) @property diff --git a/common/djangoapps/student/tests/test_user_profile_properties.py b/common/djangoapps/student/tests/test_user_profile_properties.py index 8310759266bf..886c026421e6 100644 --- a/common/djangoapps/student/tests/test_user_profile_properties.py +++ b/common/djangoapps/student/tests/test_user_profile_properties.py @@ -107,23 +107,43 @@ def test_invalidate_cache_user_profile_country_updated(self): assert cache.get(cache_key) != country assert cache.get(cache_key) is None - def test_phone_number_can_only_contain_digits(self): - # validating the profile will fail, because there are letters - # in the phone number - self.profile.phone_number = 'abc' - pytest.raises(ValidationError, self.profile.full_clean) - # fail if mixed digits/letters - self.profile.phone_number = '1234gb' - pytest.raises(ValidationError, self.profile.full_clean) - # fail if whitespace - self.profile.phone_number = ' 123' - pytest.raises(ValidationError, self.profile.full_clean) - # fail with special characters - self.profile.phone_number = '123!@#$%^&*' - pytest.raises(ValidationError, self.profile.full_clean) - # valid phone number - self.profile.phone_number = '123456789' - try: - self.profile.full_clean() - except ValidationError: - self.fail("This phone number should be valid.") + def test_valid_phone_numbers(self): + """ + Test that valid phone numbers are accepted. + + Expected behavior: + - The phone number '+123456789' should be considered valid. + - The phone number '123456789' (without '+') should also be valid. + + This test verifies that valid phone numbers are accepted by the profile model validation. + """ + valid_numbers = ['+123456789', '123456789'] + + for number in valid_numbers: + self.profile.phone_number = number + + try: + self.profile.full_clean() + except ValidationError: + self.fail("This phone number should be valid.") + + def test_invalid_phone_numbers(self): + """ + Test that invalid phone numbers raise ValidationError. + + Expected behavior: + - Phone numbers with letters, mixed digits/letters, whitespace, + or special characters should raise a ValidationError. + + This test verifies that invalid phone numbers are rejected by the profile model validation. + """ + invalid_phone_numbers = [ + 'abc', # Letters in the phone number + '1234gb', # Mixed digits and letters + ' 123', # Whitespace + '123!@#$%^&*' # Special characters + ] + + for number in invalid_phone_numbers: + self.profile.phone_number = number + pytest.raises(ValidationError, self.profile.full_clean) diff --git a/openedx/core/djangoapps/user_api/accounts/serializers.py b/openedx/core/djangoapps/user_api/accounts/serializers.py index b17f479b452d..f7ffe15d2a4b 100644 --- a/openedx/core/djangoapps/user_api/accounts/serializers.py +++ b/openedx/core/djangoapps/user_api/accounts/serializers.py @@ -47,12 +47,24 @@ class PhoneNumberSerializer(serializers.BaseSerializer): # lint-amnesty, pylint: disable=abstract-method """ - Class to serialize phone number into a digit only representation + Class to serialize phone number into a digit only representation. + + This serializer removes all non-numeric characters from the phone number, + allowing '+' only at the beginning of the number. """ def to_internal_value(self, data): - """Remove all non numeric characters in phone number""" - return re.sub("[^0-9]", "", data) or None + """ + Remove all non-numeric characters from the phone number. + + Args: + data (str): The input phone number string. + + Returns: + str or None: The cleaned phone number string containing only digits, + with an optional '+' at the beginning. + """ + return re.sub(r'(?!^)\+|[^0-9+]', "", data) or None class LanguageProficiencySerializer(serializers.ModelSerializer):