Form select on status #791
Conversation
|
@John-Paul-Larkin is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe recent update involves a significant refactor in handling additional user sign-up details. The logic for determining the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (2)
- app/(app)/alpha/additional-details/_client.tsx (1 hunks)
- schema/additionalUserDetails.ts (2 hunks)
Additional comments: 10
schema/additionalUserDetails.ts (4)
- 1-1: The import statement for
zodis correct and follows standard practices for importing external libraries.- 1-5: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [3-12]
The
slideOneSchemacorrectly defines the validation rules for the first slide of the form. It ensures thatfirstName,surname, andusernameare strings, trimmed, and within specified length limits.locationis also validated as a non-empty string. This aligns well with best practices for form validation.
- 1-5: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [14-18]
The
slideTwoSchemais well-defined for validating gender as a non-empty string and dateOfBirth as a date. This schema is simple and follows best practices for data validation.
- 1-5: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [52-54]
The type exports
TypeSlideOneSchema,TypeSlideTwoSchema, andTypeSlideThreeSchemaare correctly defined using Zod'sTypeOfutility. This is a good practice for type safety and maintainability.app/(app)/alpha/additional-details/_client.tsx (6)
- 75-75: The redirection logic for users without a session to the
/get-startedroute is correctly implemented. This ensures that only authenticated users can access the onboarding forms.- 76-76: The rendering logic for different slides based on the
slidevariable is correctly implemented. This modular approach enhances readability and maintainability of the code.- 78-78: The
SlideOnecomponent correctly usesuseFormwithzodResolverfor form validation. This is a good practice for ensuring data integrity and user input validation.- 78-78: The form submission logic in
SlideOne,SlideTwo, andSlideThreecomponents correctly handles success and error states usingtoastnotifications. This enhances the user experience by providing immediate feedback.- 78-78: In
SlideThree, the logic for triggering validation based onprofessionalOrStudentstatus is innovative. However, ensure that this logic is thoroughly tested to catch any edge cases that might not be immediately apparent.- 78-78: The
SignupProgressBarcomponent effectively visualizes the user's progress through the onboarding process. This is a user-friendly feature that enhances the onboarding experience.
| import z from "zod"; | ||
|
|
||
| export const AdditionalDetailsSchema = z | ||
| .object({ | ||
| firstName: z | ||
| .string() | ||
| .trim() | ||
| .min(2, "Min name length is 2 characters.") | ||
| .max(50, "Max name length is 50 characters."), | ||
| surname: z | ||
| .string() | ||
| .trim() | ||
| .min(2, "Min name length is 2 characters.") | ||
| .max(50, "Max name length is 50 characters."), | ||
| username: z | ||
| .string() | ||
| .trim() | ||
| .min(3, "Min name length is 3 characters.") | ||
| .max(40, "Max name length is 40 characters."), | ||
| location: z.string().min(1, "Location is required"), | ||
| gender: z.string().min(1, "Gender is required"), | ||
| dateOfBirth: z.date(), | ||
| professionalOrStudent: z.string().min(1, "Select an option"), | ||
|
|
||
| workplace: z.string().max(30, "Max length is 30 characters."), | ||
| jobTitle: z.string().max(30, "Max length is 30 characters."), | ||
| levelOfStudy: z.string(), | ||
| course: z.string().max(30, "Max name length is 30 characters."), | ||
| }) | ||
| .superRefine((val, ctx) => { | ||
| if ( | ||
| val.professionalOrStudent === "Current student" && | ||
| val.levelOfStudy === "" | ||
| ) { | ||
| ctx.addIssue({ | ||
| path: ["levelOfStudy"], | ||
| code: "custom", | ||
| message: "required", | ||
| }); | ||
| } | ||
|
|
||
| if (val.professionalOrStudent === "Current student" && val.course === "") { | ||
| ctx.addIssue({ | ||
| path: ["course"], | ||
| code: "custom", | ||
| message: "required", | ||
| }); | ||
| } | ||
|
|
||
| if ( | ||
| val.professionalOrStudent === "Working professional" && | ||
| val.workplace === "" | ||
| ) { | ||
| ctx.addIssue({ | ||
| path: ["workplace"], | ||
| code: "custom", | ||
| message: "required", | ||
| }); | ||
| } | ||
|
|
||
| if ( | ||
| val.professionalOrStudent === "Working professional" && | ||
| val.jobTitle === "" | ||
| ) { | ||
| ctx.addIssue({ | ||
| path: ["jobTitle"], | ||
| code: "custom", | ||
| message: "required", | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| export const slideOneSchema = z.object({ | ||
| firstName: z | ||
| .string() |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [20-50]
The slideThreeSchema includes custom validation logic to ensure that certain fields are required based on the value of professionalOrStudent. This logic is correctly implemented using .superRefine(). However, consider adding comments to explain the custom validation logic for future maintainability.
+ // Custom validation to ensure required fields based on professionalOrStudent status
.superRefine((val, ctx) => {| const { | ||
| surname, | ||
| firstName, | ||
| username, | ||
| location, | ||
| dateOfBirth, | ||
| gender, | ||
| professionalOrStudent, | ||
| } = details; | ||
|
|
||
| let slide: number; | ||
| if (searchParams.get("slide")) { | ||
| slide = Number(searchParams.get("slide")); | ||
| } else if (!surname || !firstName || !username || !location) { | ||
| slide = 1; | ||
| } else if (!dateOfBirth || !gender) { | ||
| slide = 2; | ||
| } else if (!professionalOrStudent) { | ||
| slide = 3; | ||
| } else { | ||
| return redirect("/settings"); | ||
| } |
There was a problem hiding this comment.
The logic for determining the current slide based on user details and search parameters is clear and follows a logical progression. However, consider adding comments to explain the logic for future maintainability, especially for the conditions that determine the slide number.
+ // Determine the current slide based on user details and search parameters
let slide: number;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| const { | |
| surname, | |
| firstName, | |
| username, | |
| location, | |
| dateOfBirth, | |
| gender, | |
| professionalOrStudent, | |
| } = details; | |
| let slide: number; | |
| if (searchParams.get("slide")) { | |
| slide = Number(searchParams.get("slide")); | |
| } else if (!surname || !firstName || !username || !location) { | |
| slide = 1; | |
| } else if (!dateOfBirth || !gender) { | |
| slide = 2; | |
| } else if (!professionalOrStudent) { | |
| slide = 3; | |
| } else { | |
| return redirect("/settings"); | |
| } | |
| const { | |
| surname, | |
| firstName, | |
| username, | |
| location, | |
| dateOfBirth, | |
| gender, | |
| professionalOrStudent, | |
| } = details; | |
| // Determine the current slide based on user details and search parameters | |
| let slide: number; | |
| if (searchParams.get("slide")) { | |
| slide = Number(searchParams.get("slide")); | |
| } else if (!surname || !firstName || !username || !location) { | |
| slide = 1; | |
| } else if (!dateOfBirth || !gender) { | |
| slide = 2; | |
| } else if (!professionalOrStudent) { | |
| slide = 3; | |
| } else { | |
| return redirect("/settings"); | |
| } |
| } else { | ||
| return redirect("/settings"); | ||
| } | ||
|
|
||
| if (!session) { | ||
| return redirect("/get-started"); | ||
| } |
There was a problem hiding this comment.
Consider adding error handling for the useForm hook in SlideOne, SlideTwo, and SlideThree components to catch and handle potential errors during form initialization.
+ // Add error handling for useForm hook
const { register, handleSubmit, formState: { errors, isSubmitting } } = useForm<TypeSlideOneSchema>({Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| } | |
| // Add error handling for useForm hook | |
| const { register, handleSubmit, formState: { errors, isSubmitting } } = useForm<TypeSlideOneSchema>({ | |
| } |
The SlideButtons component is a reusable component for navigation buttons in the form slides. This is a good practice for code reuse and maintainability. However, consider adding prop types or TypeScript interfaces for better type safety and documentation.
+ // Add TypeScript interface for SlideButtonsProps
type SlideButtonsProps = {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| } | |
| // Add TypeScript interface for SlideButtonsProps | |
| type SlideButtonsProps = { | |
| } |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Pull Request details
Removed the unused additionalDetails Zod schema, as it has been separated into the appropriate form-specific schema.
Updated the main component to redirect users to the next incomplete onboarding form, based on their progress.
If all fields have been input we redirect to the /settings route.
Retained useSearchParams, to support back button functionality.
Open to discussion on whether the back button is necessary for the onboarding flow.
Next steps
Edit - Also need to remove the slide4 and slideButtons component as they are no longer required.
Summary by CodeRabbit