-
Notifications
You must be signed in to change notification settings - Fork 2
feat(leads): normalize lead phone numbers to E.164 (create & update) #202
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
Merged
+97
−0
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| """Lead phone numbers are normalized to E.164 on create, so the CRM is | ||
| consistent and click-to-call / SMS / dedup-grouping work. Normalization is | ||
| best-effort and never drops data (unrecognized formats pass through unchanged). | ||
| """ | ||
|
|
||
| import asyncio | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| sys.path.insert(0, str(Path(__file__).parent)) | ||
|
|
||
| from lead_management import Lead, LeadManager, normalize_phone | ||
|
|
||
|
|
||
| def test_normalize_phone_us_formats(): | ||
| assert normalize_phone("(281) 324-3020") == "+12813243020" | ||
| assert normalize_phone("281-324-3020") == "+12813243020" | ||
| assert normalize_phone("2813243020") == "+12813243020" | ||
| assert normalize_phone("1 (281) 324-3020") == "+12813243020" | ||
| assert normalize_phone("+12813243020") == "+12813243020" # idempotent | ||
|
|
||
|
|
||
| def test_normalize_phone_never_loses_data(): | ||
| assert normalize_phone(None) is None | ||
| assert normalize_phone("") == "" | ||
| assert normalize_phone("123") == "123" # too short -> unchanged | ||
| assert normalize_phone("+44 20 7946 0958") == "+44 20 7946 0958" # intl -> unchanged | ||
|
|
||
|
|
||
| def test_create_lead_normalizes_phone(): | ||
| saved = {} | ||
|
|
||
| class _Doc: | ||
| def set(self, data, **k): | ||
| saved.update(data) | ||
|
|
||
| class _Coll: | ||
| def document(self, _id): | ||
| return _Doc() | ||
|
|
||
| class _DB: | ||
| def collection(self, _n): | ||
| return _Coll() | ||
|
|
||
| lm = LeadManager.__new__(LeadManager) # bypass firestore.Client() | ||
| lm.db = _DB() | ||
| lm.collection_name = "leads" | ||
| lead = Lead(lead_id="x", user_id="u", session_id="s", phone="(281) 324-3020") | ||
| asyncio.run(lm.create_lead(lead)) | ||
| assert lead.phone == "+12813243020" | ||
| # Firestore stores via Lead.to_dict() -> asdict(), i.e. snake_case field | ||
| # names. (The capitalized "Phone" key only exists in to_csv_row().) | ||
| assert saved.get("phone") == "+12813243020" | ||
|
|
||
|
|
||
| def test_update_lead_normalizes_phone(): | ||
| """The 'stored phones are E.164' invariant must survive edits, not just | ||
| creation -- otherwise an admin CRM edit re-introduces a raw number.""" | ||
| saved = {} | ||
|
|
||
| class _Doc: | ||
| def set(self, data, **k): | ||
| saved.update(data) | ||
|
|
||
| class _Coll: | ||
| def document(self, _id): | ||
| return _Doc() | ||
|
|
||
| class _DB: | ||
| def collection(self, _n): | ||
| return _Coll() | ||
|
|
||
| lm = LeadManager.__new__(LeadManager) # bypass firestore.Client() | ||
| lm.db = _DB() | ||
| lm.collection_name = "leads" | ||
| lead = Lead(lead_id="x", user_id="u", session_id="s", phone="281-324-3020") | ||
| asyncio.run(lm.update_lead(lead)) | ||
| assert lead.phone == "+12813243020" | ||
| assert saved.get("phone") == "+12813243020" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For international inputs that already include a non-US country code but happen to contain exactly 10 digits, this strips the
+and rewrites the number as a US number. For example,+64 9 123 4567becomes+16491234567instead of being left unchanged as promised, so a valid lead phone can be corrupted and become uncallable; check for an explicit+/non-1country code before the 10-digit US fallback.Useful? React with 👍 / 👎.