Skip to content

feat: add phone number validation to user APIs #5882

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

darcyYe
Copy link
Contributor

@darcyYe darcyYe commented May 16, 2024

Summary

Add phone number validation to user APIs.

  1. Put phone number validation utils to @logto/shared
  2. Refactor phone number validation in @logto/experience and @logto/console
  3. Add phone number validation to insertUser and updateUserById methods.

Testing

Covered by CI.

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

@github-actions github-actions bot added feature Cool stuff size/l labels May 16, 2024
@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch from 4a854dd to 13d885a Compare May 16, 2024 09:13
Copy link

github-actions bot commented May 16, 2024

COMPARE TO master

Total Size Diff 📈 +3.45 KB

Diff by File
Name Diff
.changeset/big-games-deny.md 📈 +478 Bytes
packages/console/package.json 📈 +1 Bytes
packages/console/src/pages/UserDetails/UserSettings/index.tsx 📈 +7 Bytes
packages/core/package.json 📈 +36 Bytes
packages/core/src/libraries/user.test.ts 📈 +62 Bytes
packages/core/src/libraries/user.ts 📈 +114 Bytes
packages/core/src/queries/user.ts 📈 +528 Bytes
packages/core/src/routes/account/email-and-phone.ts 📈 +5 Bytes
packages/core/src/routes/admin-user/basics.test.ts 0 Bytes
packages/core/src/routes/admin-user/mfa-verifications.test.ts 📈 +45 Bytes
packages/core/src/routes/admin-user/social.test.ts 0 Bytes
packages/core/src/routes/interaction/actions/submit-interaction.test.ts 0 Bytes
packages/core/src/routes/interaction/utils/single-sign-on.test.ts 0 Bytes
packages/core/src/routes/interaction/utils/single-sign-on.ts 📈 +34 Bytes
packages/core/src/utils/user.ts 📈 +955 Bytes
packages/experience/package.json 📈 +40 Bytes
packages/experience/src/utils/country-code.ts 📈 +158 Bytes
packages/experience/src/utils/form.ts 📈 +16 Bytes
packages/integration-tests/src/tests/api/admin-user.search.test.ts 📈 +89 Bytes
packages/integration-tests/src/tests/api/admin-user.test.ts 📈 +269 Bytes
packages/integration-tests/src/tests/api/role.user.test.ts 📈 +254 Bytes
packages/shared/package.json 0 Bytes
packages/shared/src/utils/phone.ts 📈 +887 Bytes
pnpm-lock.yaml 📈 +346 Bytes

@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch 4 times, most recently from 8a52112 to af48649 Compare May 17, 2024 03:10
@darcyYe darcyYe marked this pull request as ready for review May 17, 2024 03:36
@darcyYe darcyYe requested a review from a team May 17, 2024 03:36
@charIeszhao
Copy link
Member

Overall looks good to me. Need to resolve Simeng's comments, though.

@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch 2 times, most recently from 8ca8932 to b10179e Compare May 24, 2024 09:03
Copy link

github-actions bot commented Jun 8, 2024

This PR is stale because it has been open 10 for days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Jun 8, 2024
@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch 3 times, most recently from 182991a to c03b7a3 Compare June 12, 2024 10:19
@github-actions github-actions bot removed the stale label Jun 18, 2024
Copy link

github-actions bot commented Jul 2, 2024

This PR is stale because it has been open 10 for days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Jul 2, 2024
@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch from c03b7a3 to d5ab6b4 Compare April 9, 2025 12:12
@darcyYe darcyYe requested a review from xiaoyijun as a code owner April 9, 2025 12:12
@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch 3 times, most recently from a2081fb to 26c2ccc Compare April 10, 2025 04:22
@github-actions github-actions bot added size/m and removed size/l labels Apr 10, 2025
@simeng-li simeng-li requested a review from Copilot April 10, 2025 06:57
@simeng-li simeng-li removed the stale label Apr 10, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 18 out of 22 changed files in this pull request and generated no comments.

Files not reviewed (4)
  • packages/console/package.json: Language not supported
  • packages/core/package.json: Language not supported
  • packages/experience/package.json: Language not supported
  • packages/shared/package.json: Language not supported

@@ -220,7 +223,25 @@ export const createUserQueries = (pool: CommonQueryMethods) => {
id: string,
set: Partial<OmitAutoSetFields<CreateUser>>,
jsonbMode: 'replace' | 'merge' = 'merge'
) => updateUser({ set, where: { id }, jsonbMode });
) => {
if (typeof set.primaryPhone === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

if (set.primaryPhone) ?

});

const insertUser = async (data: OmitAutoSetFields<CreateUser>) => {
if (typeof data.primaryPhone === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

return value as '';
}

const result = value.startsWith('+') ? value : `+${value}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

The raw values start with + are not allowed in our DB?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See packages/shared/src/utils/phone.ts change, this is a legacy logic. But we refactored this.

@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch from 3bd5721 to e6630b4 Compare April 14, 2025 04:19
@github-actions github-actions bot added size/l and removed size/m labels Apr 14, 2025
@darcyYe darcyYe force-pushed the yemq-log-8880-add-phone-number-validation-to-user-APIs branch from e6630b4 to 685b003 Compare April 14, 2025 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants