Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: this changes the current phone number regex to allow the plus s… #31

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

andrey-canon
Copy link
Collaborator

Description

This modifies the current serializer regex that removes all characters that are not numbers, to allow the plus symbol at the beginning.

Testing instructions

import re

data = "+57+26100*4-8"
re.sub(r'(?!^)\+|[^0-9+]', "", data) # Expected result  "+572610048"

@andrey-canon andrey-canon requested a review from johanseto July 12, 2024 22:55
Copy link
Collaborator

@johanseto johanseto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this PR going to be proposed in upstream?

@@ -53,7 +53,7 @@ class PhoneNumberSerializer(serializers.BaseSerializer): # lint-amnesty, pylint

def to_internal_value(self, data):
"""Remove all non numeric characters in phone number"""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the docstring. + is not numeric

@andrey-canon andrey-canon requested a review from johanseto July 16, 2024 21:24
@andrey-canon andrey-canon force-pushed the and/modify_phone_number_regex branch from c50c432 to 469a776 Compare July 16, 2024 22:27
Copy link
Collaborator

@johanseto johanseto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andrey-canon andrey-canon merged commit b657b30 into open-release/palm.nelp Jul 17, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants