-
Notifications
You must be signed in to change notification settings - Fork 7
[Feature] Phone Plugin API #169
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
Conversation
🦋 Changeset detectedLatest commit: 86f4bbf The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Pull Request Overview
This PR implements a comprehensive Phone Plugin API for authentication using phone numbers. The feature provides seven endpoints supporting phone-based sign-up, forgot password, and phone number change workflows with OTP verification.
- Implements phone-based authentication with OTP verification for sign-up, password reset, and phone number changes
- Adds type definitions and interfaces to support phone-based authentication flows
- Updates Next.js resource handling with improved error handling and response validation
Reviewed Changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/core/prisma.types.ts | Updates Prisma type definitions from any to more specific Record<string, any> |
| packages/react/src/core/index.ts | Exports MockPrismaClient type for external use |
| packages/plugins/src/phone/types.ts | Defines TypeScript types and interfaces for phone plugin functionality |
| packages/plugins/src/phone/store.ts | Implements PhoneStore class with database operations for phone authentication |
| packages/plugins/src/phone/service.ts | Implements PhoneService class with business logic for phone authentication workflows |
| packages/plugins/src/phone/index.ts | Creates API endpoints for phone authentication using the service layer |
| packages/plugins/src/phone/helper.ts | Provides utility function for safe JSON parsing |
| packages/plugins/src/index.ts | Exports phone plugin functionality |
| packages/plugins/package.json | Adds new dependencies for the phone plugin |
| packages/next/src/resource.ts | Improves error handling and response validation |
| examples/erp/prisma/schema.prisma | Adds phone and phoneVerified fields to User model |
| examples/erp/prisma/migrations/20250828094031_add_phone_number/migration.sql | Database migration for phone fields |
| examples/erp/http.rest | HTTP test requests for phone API endpoints |
| examples/erp/genseki/plugins/phone.ts | Implementation example of phone plugin usage |
| examples/erp/genseki/plugins/me.ts | Plugin configuration for user profile functionality |
| examples/erp/genseki/config.tsx | Updates app configuration to include phone plugin |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| onOtpVerify: async (payload) => { | ||
| return Math.random() < 0.5 | ||
| }, |
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.
I'm quite not recommend using random for checking otp verify even in example application 😅
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.
Instead, we may have mock server to mock OTP verification which is a huge task. Maybe not in this PR
| const verifyStatus = await this.options.signUp | ||
| .onOtpVerify({ | ||
| phone: data.phone, | ||
| token: verification.id, | ||
| pin: data.pin, | ||
| }) | ||
| .then((result) => ok(result)) | ||
| .catch((error) => err(error)) |
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.
Can we use fromPromise instead ?
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.
Maybe like this
const result = await fromPromise(
this.store.createSignUpVerification({
id: otpResponse.value.token,
refCode: otpResponse.value.refCode,
value: {
phone: data.phone,
password: hashedPassword,
data: rest as z.output<TSignUpBodySchema>,
attempt: 0,
},
// TODO: Check the expiration time from config
expiredAt: new Date(Date.now() + 5 * 60 * 1000), // 5 minutes
}),
R.identity()
)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.
Sure, but the result is the same. I personally don't like fromPromise because of the indents
553af21 to
86f4bbf
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.
LGTM 🥳
Why did you create this PR
What did you do
Screenshots / Recordings
CleanShot.2568-08-28.at.17.33.47.mp4
Forgot password happy path
Forgot password unhappy path
Change password happy path
Change password unhappy path
Checklist