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

Remove libphonenumber js #6222

Merged

Conversation

print-Sathvik
Copy link
Contributor

@print-Sathvik print-Sathvik commented Sep 4, 2023

WHAT

Replaced libphonenumber-js with custom functions

Proposed Changes

  • Fixes remove libphonenumber-js #6082
  • Deleted libphonenumber-js
  • Implemented methods to parse phone number, format phone number, check its validity, and fetch country code from phone number

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

HOW

  • For getting country name from phone number, the country whose country code is longer is returned in case there are multiple matches. For example when +1242 is typed, the country returned is Bahamas(+1242) instead of US(+1) or Canada(+1)
  • When there is a clash in country codes(like many North American countries have +1 as country code), their area code is checked.
  • Only Indian phone numbers are formatted as +91 nnnnn nnnnn. To format phone numbers of other countries we will need their area codes and there are many rules to be considered which is not practical without using library
  • Landline numbers will be formatted as cc nnnn nnnn or ccc nnn nnnn or cccc nnn nnn where 'c' is a digit of area code. Also the leading zeros for area codes are removed. This was how the function from libphonenumber-js was formatting.

🤖 Generated by Copilot at 1b89556

  • Remove libphonenumber-js dependency and replace it with custom phone number parsing and formatting functions (link)
  • Add AREACODES and IN_LANDLINE_AREA_CODES constants to store the area codes for different countries and Indian landline numbers (link)
  • Add parsePhoneNumber, formatPhoneNumber, and getCountryCode functions and CountryData interface to src/Utils/utils.ts to implement the custom phone number logic (link)
  • Change the code values for some countries in src/Common/static/countryPhoneAndFlags.json to remove the + and - symbols and handle countries with multiple area codes under the same country code (link, link, link, link, link, link, link, link, link, link, link, link)
  • Change the parse function for the support_phone field in the XLSXAssetImportSchema to use the custom PhoneNumberValidator function and handle the 1800 prefix differently (link)
  • Change the phone number fields validation, parsing, and formatting in various components to use the custom functions from src/Utils/utils.ts and handle the +91 and 1800 prefixes differently (link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link, link)
  • Remove unused libphonenumber-js imports from various files (link, link, link, link, link, link, link, link, link)
  • Add parsePhoneNumber and PhoneNumberValidator imports to various files to use the custom functions from src/Utils/utils.ts and src/Components/Form/FieldValidators.tsx (link, link, link, link, link, link, link, link, link, link, link, link)
  • Add useEffect hook to src/Components/Form/FormFields/PhoneNumberFormField.tsx to initialize the field.value with +91 if it is empty (link)

@print-Sathvik print-Sathvik requested a review from a team September 4, 2023 19:40
@print-Sathvik print-Sathvik requested a review from a team as a code owner September 4, 2023 19:40
@vercel
Copy link

vercel bot commented Sep 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
care-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2023 8:29am

@netlify
Copy link

netlify bot commented Sep 4, 2023

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit c9ce050
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/65095bb862c368000825403a
😎 Deploy Preview https://deploy-preview-6222--care-egov-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nihal467
Copy link
Member

nihal467 commented Sep 11, 2023

@print-Sathvik the cypress is failing back to back, fix it , PR looks good to me

@print-Sathvik
Copy link
Contributor Author

@rithviknishad New changes:

  • Phone number in excel file for importing asset does not have country code. So I adjusted that part by adding country code before validating.
  • After uploading the excel file location with value "Camera Locations" was being selected but there is no such location, so I changed it to "Camera Loc" as this is used in other tests.

@nihal467
Copy link
Member

LGTM

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

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

Nice Work! LGTM

@rithviknishad
Copy link
Member

Just a minor suggestion, you could've moved the phone number related utility methods to a seperate file under say: src/Utils/PhoneNumberUtils.ts

Currently, the utils.ts file feels too big and we should consider splitting it based on relevance.

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Sep 15, 2023
@github-actions
Copy link

👋 Hi, @print-Sathvik,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@print-Sathvik
Copy link
Contributor Author

Just a minor suggestion, you could've moved the phone number related utility methods to a seperate file under say: src/Utils/PhoneNumberUtils.ts

Currently, the utils.ts file feels too big and we should consider splitting it based on relevance.

I pulled, rebased and separated phone number utils in my local but that requires me to force push to current branch. Doing that will close this PR. So I will create a new PR for seperating phone number utils once this PR is merged.

@print-Sathvik print-Sathvik reopened this Sep 18, 2023
@print-Sathvik print-Sathvik removed the merge conflict pull requests with merge conflict label Sep 18, 2023
@nihal467
Copy link
Member

LGTM

@khavinshankar khavinshankar merged commit 4c8b840 into ohcnetwork:develop Sep 19, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove libphonenumber-js
4 participants