Skip to content

Commit

Permalink
feat: modify PhoneNumberSerializer regex to allow plus symbol at the … (
Browse files Browse the repository at this point in the history
#35117)

* feat: modify PhoneNumberSerializer regex to allow plus symbol at the beginning
* chore: update PhonenumberSerializer docstring
  • Loading branch information
andrey-canon committed Jul 17, 2024
1 parent 58de096 commit c370028
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -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*$')]),
),
]
5 changes: 4 additions & 1 deletion common/djangoapps/student/models/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 40 additions & 20 deletions common/djangoapps/student/tests/test_user_profile_properties.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
18 changes: 15 additions & 3 deletions openedx/core/djangoapps/user_api/accounts/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit c370028

Please sign in to comment.