-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
base: master
Are you sure you want to change the base?
feat: add phone number validation to user APIs #5882
Conversation
4a854dd
to
13d885a
Compare
COMPARE TO
|
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 |
8a52112
to
af48649
Compare
Overall looks good to me. Need to resolve Simeng's comments, though. |
8ca8932
to
b10179e
Compare
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. |
182991a
to
c03b7a3
Compare
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. |
c03b7a3
to
d5ab6b4
Compare
a2081fb
to
26c2ccc
Compare
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.
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
packages/core/src/queries/user.ts
Outdated
@@ -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') { |
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.
if (set.primaryPhone)
?
packages/core/src/queries/user.ts
Outdated
}); | ||
|
||
const insertUser = async (data: OmitAutoSetFields<CreateUser>) => { | ||
if (typeof data.primaryPhone === 'string') { |
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.
ditto
return value as ''; | ||
} | ||
|
||
const result = value.startsWith('+') ? value : `+${value}`; |
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.
The raw values start with + are not allowed in our DB?
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.
See packages/shared/src/utils/phone.ts change, this is a legacy logic. But we refactored this.
3bd5721
to
e6630b4
Compare
e6630b4
to
685b003
Compare
Summary
Add phone number validation to user APIs.
insertUser
andupdateUserById
methods.Testing
Covered by CI.
Checklist
.changeset