Skip to content

Test/years of experience #1121

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.

Sign up for GitHub

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

Closed
leslyclopez11 wants to merge 11 commits into from
Closed

Test/years of experience #1121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bce6c91
Added Drizzle to README.md
leslyclopez11 Sep 24, 2024
44b056c
Merge branch 'develop' into develop
NiallJoeMaher Sep 25, 2024
42966cb
Merge branch 'codu-code:develop' into develop
leslyclopez11 Sep 27, 2024
2be6c5e
Merge branch 'codu-code:develop' into develop
leslyclopez11 Oct 3, 2024
d9bfb08
added yearsOfExperiece
leslycatota1 Oct 8, 2024
f216eca
added yearsOfExperience ranges
leslycatota1 Oct 8, 2024
fba1ab2
Merge branch 'codu-code:develop' into develop
leslyclopez11 Oct 8, 2024
b506b6e
Merge branch 'develop' of https://github.com/codu-code/codu into develop
leslycatota1 Oct 10, 2024
13aea32
Merge branch 'develop' of https://github.com/leslyclopez11/codu into …
leslycatota1 Oct 10, 2024
80c56b6
Merge branch 'develop' of https://github.com/codu-code/codu into develop
leslycatota1 Oct 14, 2024
793e524
hopefully it works now
leslycatota1 Oct 8, 2024
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
32 changes: 30 additions & 2 deletions app/(app)/alpha/additional-details/_client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import { Button } from "@/components/ui-components/button";
import { Heading, Subheading } from "@/components/ui-components/heading";
import { Divider } from "@/components/ui-components/divider";

export const yearsOfExperienceOptions = ["0-1", "1-3", "3-5", "5-8", "8-12", "12+"] as const;
export type YearsOfExperience = typeof yearsOfExperienceOptions[number];
type UserDetails = {
username: string;
name: string;
Expand All @@ -51,6 +53,7 @@ type UserDetails = {
levelOfStudy: string;
jobTitle: string;
workplace: string;
yearsOfExperience: YearsOfExperience;
};

export default function AdditionalSignUpDetails({
Expand All @@ -68,6 +71,7 @@ export default function AdditionalSignUpDetails({
dateOfBirth,
gender,
professionalOrStudent,
yearsOfExperience,
} = details;

let slide: number;
Expand Down Expand Up @@ -399,7 +403,7 @@ function SlideTwo({ details }: { details: UserDetails }) {
function SlideThree({ details }: { details: UserDetails }) {
const router = useRouter();

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

const {
Expand All @@ -417,6 +421,7 @@ function SlideThree({ details }: { details: UserDetails }) {
jobTitle,
course,
levelOfStudy,
yearsOfExperience,
},
});

Expand All @@ -427,7 +432,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 +517,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')}
>
<option value="" disabled>
Select range
</option>
<option value="0-1">0-1 years</option>
<option value="1-3">1-3 years</option>
<option value="3-5">3-5 years</option>
<option value="5-8">5-8 years</option>
<option value="8-12">8-12 years</option>
<option value="12+">12+ years</option>
</Select>
{errors.yearsOfExperience && (
<ErrorMessage className="text-red-500">
{errors.yearsOfExperience.message}
</ErrorMessage>
)}
</Field>
Comment on lines +520 to +542
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 14, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor to use yearsOfExperienceOptions in Select component

Currently, the options in the Select component for "Years of experience" are hardcoded, which duplicates data and can lead to inconsistencies. Refactor to map over the yearsOfExperienceOptions array for better maintainability.

Apply this change:

<Field className="mx-4 my-4 ">
  <Label>Years of experience:</Label>
  <Select
    id="years-of-experience"
    {...register('yearsOfExperience')}
  >
    <option value="" disabled>
      Select range
    </option>
-   <option value="0-1">0-1 years</option>
-   <option value="1-3">1-3 years</option>
-   <option value="3-5">3-5 years</option>
-   <option value="5-8">5-8 years</option>
-   <option value="8-12">8-12 years</option>
-   <option value="12+">12+ years</option>
+   {yearsOfExperienceOptions.map((range) => (
+     <option key={range} value={range}>
+       {range} years
+     </option>
+   ))}
  </Select>
  {errors.yearsOfExperience && (
    <ErrorMessage className="text-red-500">
      {errors.yearsOfExperience.message}
    </ErrorMessage>
  )}
</Field>

Committable suggestion was skipped due to low confidence.

</>
)}

Expand Down
7 changes: 7 additions & 0 deletions app/(app)/alpha/additional-details/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { redirect } from "next/navigation";
import Content from "./_client";
import { getServerAuthSession } from "@/server/auth";
import { db } from "@/server/db";
import {YearsOfExperience} from "./_client";


export default async function Page() {
const session = await getServerAuthSession();
Expand All @@ -22,6 +24,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 +40,10 @@ export default async function Page() {
levelOfStudy: details?.levelOfStudy || "",
jobTitle: details?.jobTitle || "",
workplace: details?.workplace || "",
yearsOfExperience: details?.yearsOfExperience &&
["0-1", "1-3", "3-5", "5-8", "8-12", "12+"].includes(details.yearsOfExperience)
? details.yearsOfExperience as YearsOfExperience
: "0-1",
};

return <Content details={detailsWithNullsRemoved} />;
Expand Down
1 change: 1 addition & 0 deletions drizzle/0010_gorgeous_living_tribunal.sql
View file
Open in desktop
Copy link
Contributor

@NiallJoeMaher NiallJoeMaher Oct 28, 2024

Choose a reason for hiding this comment

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

There should be only one migration file for the updates. Let me know if you still want to take this on or else we can take it over.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE "user" ADD COLUMN "yearsOfExperience" varchar DEFAULT '0-1' NOT NULL;
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 14, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Add constraints to the yearsOfExperience column for data integrity

The frontend implementation restricts yearsOfExperience to predefined options, ensuring valid user input. However, the current database schema does not enforce these constraints, which could allow invalid data entries if bypassed.

To align the database schema with the application's validation logic and enhance data integrity, consider updating the column definition as follows:

ALTER TABLE "user" 
  ADD COLUMN "yearsOfExperience" varchar(20) 
  CHECK ("yearsOfExperience" IN ('0-1', '1-3', '3-5', '5-8', '8-12', '12+', 'Student', 'None')) 
  DEFAULT 'None' 
  NOT NULL;

This modification introduces a length limit and enforces that only the specified values can be stored in the yearsOfExperience column, ensuring consistency between the frontend and backend.

🔗 Analysis chain

Consider refining the yearsOfExperience column definition

The addition of the yearsOfExperience column aligns with the PR objectives. However, there are a few points to consider:

  1. The varchar type allows for any string input, which might not be ideal if we're using a dropdown with predefined options. Consider using an enum type if your database supports it, or add a CHECK constraint to limit the possible values.

  2. The default value '0-1' might not cover all scenarios mentioned in the PR objectives, such as "Current student" or "None of the above". Consider if a different default value or making the column nullable would be more appropriate.

  3. There's no explicit length limit on the varchar field. It's generally a good practice to set a reasonable maximum length to prevent potential issues with very long strings.

Here's a suggested refinement:

ALTER TABLE "user" ADD COLUMN "yearsOfExperience" varchar(20) CHECK ("yearsOfExperience" IN ('0-1', '1-3', '3-5', '5-10', '10+', 'Student', 'None')) NOT NULL DEFAULT 'None';

This suggestion adds a length limit, includes a CHECK constraint for predefined options, and uses 'None' as the default value. Adjust the options and default value as needed to match your specific requirements.

To ensure this change aligns with the frontend implementation, let's check the related React components:

Please review the script output to ensure the backend and frontend implementations are consistent.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the implementation of yearsOfExperience in React components

# Test: Search for yearsOfExperience in TypeScript/JavaScript files
rg --type-add 'web:*.{ts,tsx,js,jsx}' -t web 'yearsOfExperience'

# Test: Look for dropdown or select components related to years of experience
rg --type-add 'web:*.{ts,tsx,js,jsx}' -t web -i 'dropdown|select.*years.*experience'

Length of output: 4236

12 changes: 12 additions & 0 deletions drizzle/0011_yearsOfExperience
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
DO $$ BEGIN
ALTER TABLE "User" ADD COLUMN "yearsOfExperience" text;
EXCEPTION
WHEN duplicate_object THEN null;
END $$;

DO $$ BEGIN
ALTER TABLE "User" ADD CONSTRAINT "User_yearsOfExperience_check"
CHECK ("yearsOfExperience" IN ('0-1', '1-3', '3-5', '5-8', '8-12', '12+'));
EXCEPTION
WHEN duplicate_object THEN null;
END $$;
7 changes: 7 additions & 0 deletions schema/additionalUserDetails.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import z from "zod";
const yearsOfExperienceOptions = ["0-1", "1-3", "3-5", "5-8", "8-12", "12+"] as const;

export const slideOneSchema = z.object({
name: z
Expand Down Expand Up @@ -30,6 +31,12 @@ export const slideThreeSchema = z
jobTitle: z.string().max(30, "Max length is 30 characters."),
levelOfStudy: z.string(),
course: z.string().max(30, "Max name length is 30 characters."),
yearsOfExperience: z
.string()
.optional()
.refine((val) => val === undefined || yearsOfExperienceOptions.includes(val as typeof yearsOfExperienceOptions[number]), {
message: "Please select a valid years of experience.",
}),
Comment on lines +34 to +39
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 14, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider making yearsOfExperience a required field.

The implementation of yearsOfExperience is well done, using Zod for schema validation as required. The refinement ensures only valid options are accepted, which is great. However, the PR objectives state that this field should be required, but it's currently defined as optional.

Consider updating the schema to make yearsOfExperience a required field:

-    yearsOfExperience: z
-      .string()
-      .optional()
+    yearsOfExperience: z
+      .string()
      .refine((val) => val === undefined || yearsOfExperienceOptions.includes(val as typeof yearsOfExperienceOptions[number]), {
        message: "Please select a valid years of experience.",
      }),

This change will ensure the field aligns fully with the PR objectives.

📝 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
yearsOfExperience: z
.string()
.optional()
.refine((val) => val === undefined || yearsOfExperienceOptions.includes(val as typeof yearsOfExperienceOptions[number]), {
message: "Please select a valid years of experience.",
}),
yearsOfExperience: z
.string()
.refine((val) => val === undefined || yearsOfExperienceOptions.includes(val as typeof yearsOfExperienceOptions[number]), {
message: "Please select a valid years of experience.",
}),

})
.superRefine((val, ctx) => {
if (
Expand Down
1 change: 1 addition & 0 deletions server/db/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ export const user = pgTable(
levelOfStudy: text("levelOfStudy"),
course: text("course"),
role: role("role").default("USER").notNull(),
yearsOfExperience: varchar("yearsOfExperience").default("0-1").notNull(),
},
(table) => {
return {
Expand Down