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

Feature/complete onboarding form #1216

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,16 @@ export async function slideThreeSubmitAction(dataInput: TypeSlideThreeSchema) {
}

try {
const { professionalOrStudent, course, jobTitle, levelOfStudy, workplace } =
slideThreeSchema.parse(dataInput);
const {
professionalOrStudent,
course,
jobTitle,
levelOfStudy,
workplace,
yearsOfExperience,
} = slideThreeSchema.parse(dataInput);

const onboardingComplete = new Date().toISOString();
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider moving timestamp generation to database level

Instead of generating the timestamp in the application code, consider letting the database handle it for better consistency and reliability across all onboarding completions.

-    const onboardingComplete = new Date().toISOString();
+    // Let database handle the timestamp
     await db
       .update(user)
       .set({
         professionalOrStudent,
         course,
         jobTitle,
         levelOfStudy,
         workplace,
         yearsOfExperience,
-        onboardingComplete,
+        onboardingComplete: db.fn.now(), // Use database function
       })

Committable suggestion skipped: line range outside the PR's diff.


await db
.update(user)
Expand All @@ -91,6 +99,8 @@ export async function slideThreeSubmitAction(dataInput: TypeSlideThreeSchema) {
jobTitle,
levelOfStudy,
workplace,
yearsOfExperience,
onboardingComplete,
})
.where(eq(user.id, session.user.id));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
locationOptions,
levelOfStudyOptions,
monthsOptions,
} from "@/app/(app)/alpha/additional-details/selectOptions";
} from "@/app/(app)/onboarding/selectOptions";
import {
slideOneSubmitAction,
slideThreeSubmitAction,
Expand All @@ -39,6 +39,7 @@ import { Select } from "@/components/ui-components/select";
import { Button } from "@/components/ui-components/button";
import { Heading, Subheading } from "@/components/ui-components/heading";
import { Divider } from "@/components/ui-components/divider";
import { experienceRangeEnum } from "@/server/db/schema";

type UserDetails = {
username: string;
Expand All @@ -51,6 +52,7 @@ type UserDetails = {
levelOfStudy: string;
jobTitle: string;
workplace: string;
yearsOfExperience: "0-1" | "1-3" | "3-5" | "5-8" | "12+" | undefined;
};

export default function AdditionalSignUpDetails({
Expand Down Expand Up @@ -357,7 +359,7 @@ function SlideTwo({ details }: { details: UserDetails }) {
id="day"
aria-label="day"
value={day ? day : ""}
disabled={!month || undefined}
disabled={month === undefined || year === undefined}
required
onChange={(e) => setDay(Number(e.target.value))}
>
Expand Down Expand Up @@ -399,8 +401,14 @@ function SlideTwo({ details }: { details: UserDetails }) {
function SlideThree({ details }: { details: UserDetails }) {
const router = useRouter();

const { professionalOrStudent, workplace, jobTitle, course, levelOfStudy } =
details;
const {
professionalOrStudent,
workplace,
jobTitle,
course,
levelOfStudy,
yearsOfExperience,
} = details;

const {
register,
Expand All @@ -417,6 +425,13 @@ function SlideThree({ details }: { details: UserDetails }) {
jobTitle,
course,
levelOfStudy,
yearsOfExperience: yearsOfExperience as
| "0-1"
| "1-3"
| "3-5"
| "5-8"
| "12+"
| undefined,
},
});

Expand All @@ -427,7 +442,7 @@ function SlideThree({ details }: { details: UserDetails }) {
const professionalOrStudent = getValues("professionalOrStudent");

if (isError && professionalOrStudent === "Working professional") {
isError = await trigger(["workplace", "jobTitle"]);
isError = await trigger(["workplace", "jobTitle", "yearsOfExperience"]);
}

if (isError && professionalOrStudent === "Current student") {
Expand Down Expand Up @@ -512,6 +527,29 @@ function SlideThree({ details }: { details: UserDetails }) {
</ErrorMessage>
)}
</Field>

<Field className="mx-4 my-4">
<Label>Years of experience:</Label>
<Select
id="years-of-experience"
{...register("yearsOfExperience")}
defaultValue=""
>
<option value="" disabled>
Select years of experience
</option>
{experienceRangeEnum.enumValues.map((range) => (
<option key={range} value={range}>
{range} years
</option>
))}
</Select>
{errors.yearsOfExperience && (
<ErrorMessage className="text-red-500">
{errors.yearsOfExperience.message}
</ErrorMessage>
)}
</Field>
</>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export default async function Page() {
levelOfStudy: true,
jobTitle: true,
workplace: true,
yearsOfExperience: true,
},
where: (user, { eq }) => eq(user.id, userId),
});
Expand All @@ -37,6 +38,7 @@ export default async function Page() {
levelOfStudy: details?.levelOfStudy || "",
jobTitle: details?.jobTitle || "",
workplace: details?.workplace || "",
yearsOfExperience: details?.yearsOfExperience || undefined,
};

return <Content details={detailsWithNullsRemoved} />;
Expand Down
23 changes: 21 additions & 2 deletions context/AuthProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,29 @@
"use client";
import { SessionProvider } from "next-auth/react";
import { SessionProvider, useSession } from "next-auth/react";
import { useRouter } from "next/navigation";
import { useEffect } from "react";

export default function AuthProvider({
children,
}: {
children: React.ReactNode;
}) {
return <SessionProvider>{children}</SessionProvider>;
return (
<SessionProvider>
<OnboardingCheck>{children}</OnboardingCheck>
</SessionProvider>
);
}

function OnboardingCheck({ children }: { children: React.ReactNode }) {
const { data: session, status } = useSession();
const router = useRouter();

useEffect(() => {
if (status === "authenticated" && !session?.user?.isOnboardingComplete) {
router.push("/onboarding");
}
}, [status, session, router]);
John-Paul-Larkin marked this conversation as resolved.
Show resolved Hide resolved

return <>{children}</>;
}
9 changes: 9 additions & 0 deletions drizzle/0011_years-experience-user-table.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
DO $$ BEGIN
CREATE TYPE "public"."experience_range" AS ENUM('0-1', '1-3', '3-5', '5-8', '12+');
EXCEPTION
WHEN duplicate_object THEN null;
END $$;
--> statement-breakpoint
ALTER TABLE "session" ADD PRIMARY KEY ("sessionToken");--> statement-breakpoint
ALTER TABLE "user" ADD COLUMN "yearsOfExperience" "experience_range";--> statement-breakpoint
ALTER TABLE "user" ADD COLUMN "onboardingComplete" timestamp with time zone;
Comment on lines +8 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Strengthen column definitions for data integrity

The current column definitions could be improved to ensure data consistency:

  1. yearsOfExperience should be NOT NULL for new users going through onboarding
  2. onboardingComplete should have a default value for existing users

Consider this alternative implementation:

-ALTER TABLE "user" ADD COLUMN "yearsOfExperience" "experience_range";
-ALTER TABLE "user" ADD COLUMN "onboardingComplete" timestamp with time zone;
+ALTER TABLE "user" ADD COLUMN "yearsOfExperience" "experience_range" NOT NULL DEFAULT '0-1';
+ALTER TABLE "user" ADD COLUMN "onboardingComplete" timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP;

Note: The default value for yearsOfExperience should align with your business logic for existing users.

📝 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. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ALTER TABLE "user" ADD COLUMN "yearsOfExperience" "experience_range";--> statement-breakpoint
ALTER TABLE "user" ADD COLUMN "onboardingComplete" timestamp with time zone;
ALTER TABLE "user" ADD COLUMN "yearsOfExperience" "experience_range" NOT NULL DEFAULT '0-1';--> statement-breakpoint
ALTER TABLE "user" ADD COLUMN "onboardingComplete" timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP;

Loading
Loading