Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
112 changes: 110 additions & 2 deletions app/(app)/settings/_client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import { useEffect, useState } from "react";
import { Description, Field, Label, Switch } from "@headlessui/react";
import { zodResolver } from "@hookform/resolvers/zod";
import type { SubmitHandler } from "react-hook-form";
import { useForm } from "react-hook-form";
import { set, useForm } from "react-hook-form";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unnecessary import 'set' from 'react-hook-form'.

The set function is not used anywhere in the code. Removing unused imports helps keep the code clean and maintainable.

Apply this diff to fix the issue:

-import { set, useForm } from "react-hook-form";
+import { useForm } from "react-hook-form";
πŸ“ 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
import { set, useForm } from "react-hook-form";
import { useForm } from "react-hook-form";

import { api } from "@/server/trpc/react";
import { toast } from "sonner";
import type { saveSettingsInput } from "@/schema/profile";
import { saveSettingsSchema } from "@/schema/profile";

import { uploadFile } from "@/utils/s3helpers";
import type { user } from "@/server/db/schema";
import { Button } from "@/components/ui-components/button";
import { CheckCheck, Loader, Loader2 } from "lucide-react";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unused import 'Loader2' from 'lucide-react'.

The Loader2 component is not used in the code. Cleaning up unused imports reduces clutter and potential confusion.

Apply this diff to fix the issue:

-import { CheckCheck, Loader, Loader2 } from "lucide-react";
+import { CheckCheck, Loader } from "lucide-react";
πŸ“ 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
import { CheckCheck, Loader, Loader2 } from "lucide-react";
import { CheckCheck, Loader } from "lucide-react";


function classNames(...classes: string[]) {
return classes.filter(Boolean).join(" ");
Expand All @@ -27,6 +29,8 @@ type User = Pick<
| "emailNotifications"
| "newsletter"
| "image"
| "email"
| "id"
>;

type ProfilePhoto = {
Expand All @@ -42,7 +46,11 @@ const Settings = ({ profile }: { profile: User }) => {
formState: { errors },
} = useForm<saveSettingsInput>({
resolver: zodResolver(saveSettingsSchema),
defaultValues: { ...profile, username: profile.username || "" },
defaultValues: {
...profile,
username: profile.username || "",
email: profile.email || "",
},
});

const bio = watch("bio");
Expand All @@ -52,6 +60,9 @@ const Settings = ({ profile }: { profile: User }) => {

const [emailNotifications, setEmailNotifications] = useState(eNotifications);
const [weeklyNewsletter, setWeeklyNewsletter] = useState(newsletter);
const [newEmail, setNewEmail] = useState("");
const [sendForVerification, setSendForVerification] = useState(false);
const [loading, setLoading] = useState(false);
Comment thread
NiallJoeMaher marked this conversation as resolved.

const [profilePhoto, setProfilePhoto] = useState<ProfilePhoto>({
status: "idle",
Expand Down Expand Up @@ -127,6 +138,44 @@ const Settings = ({ profile }: { profile: User }) => {
}
};

const handleNewEmailUpdate = async () => {
setLoading(true);
try {
await fetch("/api/update_email", {
method: "POST",
body: JSON.stringify({ userId: profile.id, newEmail }),
headers: {
"Content-Type": "application/json",
},
})
.then((res) => res.json())
.then((res) => {
console.log(res);
if (res.status === 400) {
setLoading(false);
toast.error(
"Something went wrong while sending the verification link",
);
} else {
toast.success(
"Verification link has been sent to your new email address.",
);
setLoading(false);
setSendForVerification(true);
}
})
.catch((error) => {
setLoading(false);
toast.error(
"Something went wrong while sending the verification link.",
);
});
} catch (error) {
setLoading(false);
toast.error("Something went wrong while sending the verification link.");
}
};
Comment on lines +141 to +159
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect response handling in 'handleNewEmailUpdate' function.

The response handling in your fetch call is incorrect. After calling res.json(), you lose access to the res.status property, as res now contains the parsed JSON body. Additionally, mixing async/await with .then() and .catch() can lead to confusion and is not considered best practice.

Here's how you can refactor the function:

  • Use async/await consistently for better readability.
  • Access the response status code before parsing the JSON.
  • Improve error handling to accurately reflect the response.

Apply this diff to fix the issue:

 const handleNewEmailUpdate = async () => {
   setLoading(true);
   try {
-    await fetch("/api/update_email", {
+    const res = await fetch("/api/update_email", {
       method: "POST",
       body: JSON.stringify({ userId: profile.id, newEmail }),
       headers: {
         "Content-Type": "application/json",
       },
-    })
-      .then((res) => res.json())
-      .then((res) => {
-        console.log(res);
-        if (res.status === 400) {
-          setLoading(false);
-          toast.error(
-            "Something went wrong while sending the verification link",
-          );
-        } else {
-          toast.success(
-            "Verification link has been sent to your new email address.",
-          );
-          setLoading(false);
-          setSendForVerification(true);
-        }
-      })
-      .catch((error) => {
-        setLoading(false);
-        toast.error(
-          "Something went wrong while sending the verification link.",
-        );
-      });
+    });
+    if (res.status === 400) {
+      toast.error(
+        "Something went wrong while sending the verification link"
+      );
+    } else if (res.ok) {
+      toast.success(
+        "Verification link has been sent to your new email address."
+      );
+      setSendForVerification(true);
+    } else {
+      toast.error(
+        "Unexpected error occurred while sending the verification link."
+      );
+    }
+  } catch (error) {
+    toast.error(
+      "Something went wrong while sending the verification link."
+    );
   } finally {
     setLoading(false);
   }
 };
πŸ“ 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
const handleNewEmailUpdate = async () => {
setLoading(true);
try {
await fetch("/api/update_email", {
method: "POST",
body: JSON.stringify({ userId: profile.id, newEmail }),
headers: {
"Content-Type": "application/json",
},
})
.then((res) => res.json())
.then((res) => {
console.log(res);
if (res.status === 400) {
setLoading(false);
toast.error(
"Something went wrong while sending the verification link",
);
} else {
toast.success(
"Verification link has been sent to your new email address.",
);
setLoading(false);
setSendForVerification(true);
}
})
.catch((error) => {
setLoading(false);
toast.error(
"Something went wrong while sending the verification link.",
);
});
} catch (error) {
setLoading(false);
toast.error("Something went wrong while sending the verification link.");
}
};
const handleNewEmailUpdate = async () => {
setLoading(true);
try {
const res = await fetch("/api/update_email", {
method: "POST",
body: JSON.stringify({ userId: profile.id, newEmail }),
headers: {
"Content-Type": "application/json",
},
});
if (res.status === 400) {
toast.error(
"Something went wrong while sending the verification link"
);
} else if (res.ok) {
toast.success(
"Verification link has been sent to your new email address."
);
setSendForVerification(true);
} else {
toast.error(
"Unexpected error occurred while sending the verification link."
);
}
} catch (error) {
toast.error(
"Something went wrong while sending the verification link."
);
} finally {
setLoading(false);
}
};

Comment on lines +141 to +159
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Ensure robust error handling in handleNewEmailUpdate function.

In the catch block, accessing error.data.code and error.data.message without confirming that error.data exists may lead to runtime errors if error.data is undefined. To prevent potential issues, use optional chaining when accessing these properties.

Apply this diff to enhance error handling:

 const handleNewEmailUpdate = async () => {
   setLoading(true);
   try {
     await updateEmail({ newEmail });

     toast.success("Verification link sent to your email.");
     setSendForVerification(true);
   } catch (error: any) {
-    if (
-      error.data.code === "BAD_REQUEST" ||
-      error.data?.code === "INTERNAL_SERVER_ERROR"
-    ) {
-      toast.error(error.data.message);
+    const errorCode = error.data?.code;
+    if (
+      errorCode === "BAD_REQUEST" ||
+      errorCode === "INTERNAL_SERVER_ERROR"
+    ) {
+      toast.error(error.data?.message || "An error occurred.");
     } else {
       toast.error("Something went wrong");
     }
   } finally {
     setLoading(false);
   }
 };
πŸ“ 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
const handleNewEmailUpdate = async () => {
setLoading(true);
try {
await updateEmail({ newEmail });
toast.success("Verification link sent to your email.");
setSendForVerification(true);
} catch (error: any) {
if (
error.data.code === "BAD_REQUEST" ||
error.data?.code === "INTERNAL_SERVER_ERROR"
) {
toast.error(error.data.message);
} else {
toast.error("Something went wrong");
}
}
setLoading(false);
};
const handleNewEmailUpdate = async () => {
setLoading(true);
try {
await updateEmail({ newEmail });
toast.success("Verification link sent to your email.");
setSendForVerification(true);
} catch (error: any) {
const errorCode = error.data?.code;
if (
errorCode === "BAD_REQUEST" ||
errorCode === "INTERNAL_SERVER_ERROR"
) {
toast.error(error.data?.message || "An error occurred.");
} else {
toast.error("Something went wrong");
}
} finally {
setLoading(false);
}
};

Comment on lines +140 to +159
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Enhance error handling in handleNewEmailUpdate.

The current error handling in handleNewEmailUpdate is generic. Consider providing more specific error messages based on the error type or code returned by the API.

Here's a suggested improvement:

const handleNewEmailUpdate = async () => {
  setLoading(true);
  await updateEmail(
    { newEmail },
    {
      onError(error: any) {
        setLoading(false);
        if (error.data?.code === "INVALID_EMAIL") {
          return toast.error("The provided email is invalid. Please check and try again.");
        } else if (error.data?.code === "EMAIL_IN_USE") {
          return toast.error("This email is already in use. Please use a different email.");
        }
        return toast.error(
          "Something went wrong sending the verification link.",
        );
      },
      onSuccess() {
        setLoading(false);
        toast.success("Verification link sent to your email.");
        setSendForVerification(true);
      },
    },
  );
};

This change provides more specific error messages based on potential API error codes, improving the user experience.


return (
<div className="old-input py-8">
<div className="mx-auto flex w-full max-w-2xl flex-grow flex-col justify-center px-4 sm:px-6 lg:col-span-9">
Expand Down Expand Up @@ -338,6 +387,65 @@ const Settings = ({ profile }: { profile: User }) => {
</div>
</div>
</div>
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
{...register("email")}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={newEmail || loading ? false : true}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Simplify the 'disabled' prop expression in the 'Send verification link' button.

The use of a ternary operator with boolean literals is unnecessary. You can simplify the expression by using the logical NOT operator for clarity.

Apply this diff to fix the issue:

-                            disabled={newEmail || loading ? false : true}
+                            disabled={!(newEmail || loading)}
πŸ“ 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
disabled={newEmail || loading ? false : true}
disabled={!(newEmail || loading)}
🧰 Tools
πŸͺ› Biome

[error] 421-421: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

onClick={handleNewEmailUpdate}
>
{loading && (
<Loader className="text-primary h-4 w-4 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[200px]"
disabled={newEmail || loading ? false : true}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Simplify the 'disabled' prop expression in the 'Resend verification link' button.

Similarly, simplify the ternary expression to make the code more readable.

Apply this diff to fix the issue:

-                              disabled={newEmail || loading ? false : true}
+                              disabled={!(newEmail || loading)}
πŸ“ 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
disabled={newEmail || loading ? false : true}
disabled={!(newEmail || loading)}
🧰 Tools
πŸͺ› Biome

[error] 437-437: Unnecessary use of boolean literals in conditional expression.

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with

(lint/complexity/noUselessTernary)

onClick={handleNewEmailUpdate}
>
{loading && (
<Loader className="text-primary h-8 w-8 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>
Comment on lines +372 to +433
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Improve button disabled state logic.

The current implementation of the disabled state for the "Send verification link" and "Resend verification link" buttons can be improved for better user experience and logic clarity.

Consider the following changes:

  1. The button should be disabled when there's no new email entered or when it's the same as the current email.
  2. The loading state should also be considered in the disabled logic.

Apply this diff to improve the logic:

-                        disabled={!(newEmail || loading)}
+                        disabled={!newEmail || newEmail === profile.email || loading}

This change ensures that the button is disabled when:

  • No new email is entered
  • The new email is the same as the current email
  • The form is in a loading state

This provides a more intuitive user experience and prevents unnecessary API calls.

πŸ“ 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
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
value={profile.email!}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={!(newEmail || loading)}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[250px]"
disabled={
!newEmail || newEmail === profile.email || loading
}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
value={profile.email!}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={!newEmail || newEmail === profile.email || loading}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[250px]"
disabled={
!newEmail || newEmail === profile.email || loading
}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>

Comment on lines +372 to +433
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Improve button disabled state logic.

The current implementation of the disabled state for the "Send verification link" and "Resend verification link" buttons can be improved for better user experience and logic clarity.

Consider the following changes:

  1. The button should be disabled when there's no new email entered or when it's the same as the current email.
  2. The loading state should also be considered in the disabled logic.

Apply this diff to improve the logic:

-                        disabled={!(newEmail || loading)}
+                        disabled={!newEmail || newEmail === profile.email || loading}

This change ensures that the button is disabled when:

  • No new email is entered
  • The new email is the same as the current email
  • The form is in a loading state

This provides a more intuitive user experience and prevents unnecessary API calls.

πŸ“ 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
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
value={profile.email!}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={
!newEmail || newEmail === profile.email || loading
}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[250px]"
disabled={
!newEmail || newEmail === profile.email || loading
}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
value={profile.email!}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={!newEmail || newEmail === profile.email || loading}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[250px]"
disabled={!newEmail || newEmail === profile.email || loading}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>

</div>
Comment on lines +372 to +434
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Improve button disabled state logic.

The current implementation of the disabled state for the "Send verification link" and "Resend verification link" buttons can be improved for better user experience and logic clarity.

Consider the following changes:

  1. The button should be disabled when there's no new email entered or when it's the same as the current email.
  2. The loading state should also be considered in the disabled logic.

Apply this diff to improve the logic:

-                        disabled={!(newEmail || loading)}
+                        disabled={!newEmail || newEmail === profile.email || loading}

This change ensures that the button is disabled when:

  • No new email is entered
  • The new email is the same as the current email
  • The form is in a loading state

This provides a more intuitive user experience and prevents unnecessary API calls.

πŸ“ 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
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
value={profile.email!}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={!(newEmail || loading)}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[250px]"
disabled={!(newEmail || loading)}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>
</div>
<div className="mt-6 text-neutral-600 dark:text-neutral-400">
<h2 className="text-xl font-bold tracking-tight text-neutral-800 dark:text-white">
Update email
</h2>
<p className="mt-1 text-sm">Change your email here.</p>
<div className="mt-2 flex flex-col gap-2">
<div className="flex flex-col">
<label htmlFor="currEmail">Current email</label>
<div>
<input
type="email"
id="currEmail"
value={profile.email!}
disabled
/>
</div>
</div>
<div className="flex flex-col">
<label htmlFor="newEmail">Update email</label>
<div>
<input
type="email"
id="newEmail"
onChange={(e) => setNewEmail(e.target.value)}
value={newEmail}
/>
</div>
</div>
{!sendForVerification ? (
<Button
className="w-[200px]"
disabled={!newEmail || newEmail === profile.email || loading}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Send verification link
</Button>
) : (
<div className="mt-2 flex flex-row gap-2">
<h2 className="flex items-center gap-2 text-sm italic text-green-400">
<CheckCheck />
Verification link sent
</h2>
<Button
className="w-[250px]"
disabled={!newEmail || newEmail === profile.email || loading}
onClick={handleNewEmailUpdate}
>
{loading && (
<Loader2 className="text-primary h-6 w-6 animate-spin" />
)}
Resend verification link
</Button>
</div>
)}
</div>
</div>

<div className="divide-y divide-neutral-200 pt-6">
<div>
<div className="text-neutral-600 dark:text-neutral-400">
Expand Down
4 changes: 4 additions & 0 deletions app/(app)/settings/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export default async function Page() {

const existingUser = await db.query.user.findFirst({
columns: {
id: true,
name: true,
username: true,
bio: true,
Expand All @@ -31,6 +32,7 @@ export default async function Page() {
emailNotifications: true,
newsletter: true,
image: true,
email: true,
},
where: (users, { eq }) => eq(users.id, session.user!.id),
});
Expand All @@ -50,6 +52,7 @@ export default async function Page() {
.set({ username: initialUsername })
.where(eq(user.id, session.user.id))
.returning({
id: user.id,
name: user.name,
username: user.username,
bio: user.bio,
Expand All @@ -58,6 +61,7 @@ export default async function Page() {
emailNotifications: user.emailNotifications,
newsletter: user.newsletter,
image: user.image,
email: user.email,
});
return <Content profile={newUser} />;
}
Expand Down
25 changes: 25 additions & 0 deletions app/api/update_email/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {
generateEmailToken,
sendVerificationEmail,
storeTokenInDb,
} from "@/utils/emailToken";
import { NextRequest, NextResponse } from "next/server";

export async function POST(req: NextRequest, res: NextResponse) {
const { userId, newEmail } = await req.json();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could let someone change someone elses email. You would need to grab the user/userId from the current users session. Also for mutating requests like this we use trpc. You could add this here: https://github.com/codu-code/codu/blob/develop/server/api/router/profile.ts

And you'll see the pattern we have for cleaning data with schemas and getting the user from ctx.session.user

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes followed this way now.


if (!newEmail || !userId)
return NextResponse.json({ message: "Invalid request" }, { status: 400 });

const token = generateEmailToken();
const expiresAt = new Date(Date.now() + 1000 * 60 * 60); //1 hour
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Consider making token expiration configurable

The token expiration is currently hardcoded to 1 hour. Consider making this configurable to allow for flexibility in different environments or use cases.

Here's a suggested improvement:

import { TOKEN_EXPIRATION_TIME } from "@/config/constants"; // Assume this exists

// ...

const expiresAt = new Date(Date.now() + TOKEN_EXPIRATION_TIME);

This change allows you to easily adjust the token expiration time across your application by modifying a single constant.


await storeTokenInDb(userId, token, expiresAt, newEmail);

await sendVerificationEmail(newEmail, token);

return NextResponse.json(
{ message: "Verification email sent" },
{ status: 200 },
);
}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 6, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Enhance error handling and input validation

While the overall structure of the POST function is good, there are several areas where it can be improved:

  1. Error handling: Add try-catch blocks around database operations and email sending to handle potential errors gracefully.
  2. Input validation:
    • Check if the new email is different from the current one to avoid unnecessary processing.
    • Validate the email format before proceeding.
    • Verify that the user exists in the database before updating their email.
  3. Response consistency: Ensure all responses follow the same structure for consistency.

Here's a suggested improvement:

import { isValidEmail, getUserById } from "@/utils/validators"; // Assume these exist

export async function POST(req: NextRequest, res: NextResponse) {
  try {
    const { userId, newEmail } = await req.json();

    if (!newEmail || !userId) {
      return NextResponse.json({ error: "Invalid request" }, { status: 400 });
    }

    if (!isValidEmail(newEmail)) {
      return NextResponse.json({ error: "Invalid email format" }, { status: 400 });
    }

    const user = await getUserById(userId);
    if (!user) {
      return NextResponse.json({ error: "User not found" }, { status: 404 });
    }

    if (user.email === newEmail) {
      return NextResponse.json({ message: "New email is the same as the current email" }, { status: 200 });
    }

    const token = generateEmailToken();
    const expiresAt = new Date(Date.now() + 1000 * 60 * 60); // 1 hour

    await storeTokenInDb(userId, token, expiresAt, newEmail);
    await sendVerificationEmail(newEmail, token);

    return NextResponse.json({ message: "Verification email sent" }, { status: 200 });
  } catch (error) {
    console.error("Error updating email:", error);
    return NextResponse.json({ error: "Internal server error" }, { status: 500 });
  }
}

This implementation adds error handling, more comprehensive input validation, and consistent response structures.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed! A try catch is useful here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher, glad to hear that you're implementing the try-catch block for improved error handling!

(^_^/)


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

33 changes: 33 additions & 0 deletions app/api/verify_email/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On the verify email route we could probably create a GET call which takes the token ID and redirects the user to a success or an error page depending on what they do (that's the normal pattern)

So have a route that might be /verify-email?token=eyJhbGciOiJIUzI1NiIsInR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done βœ…

deleteTokenFromDb,
getTokenFromDb,
updateEmail,
} from "@/utils/emailToken";
import { NextRequest, NextResponse } from "next/server";

export async function POST(req: NextRequest, res: NextResponse) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix the function signature to comply with Next.js route handlers

The POST function should only accept a single NextRequest parameter and return a NextResponse. The res: NextResponse parameter is unnecessary and may cause unexpected behavior.

Apply this diff to correct the function signature:

-export async function POST(req: NextRequest, res: NextResponse) {
+export async function POST(req: NextRequest) {
πŸ“ 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
export async function POST(req: NextRequest, res: NextResponse) {
export async function POST(req: NextRequest) {

const token = req.nextUrl.searchParams.get("token");
const { userId } = await req.json();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid accepting userId from client input for security reasons

Accepting userId from the request body can lead to security vulnerabilities, such as account hijacking if a malicious user provides a different userId. Instead, derive the userId from the token or use authentication mechanisms to identify the user securely.

[security]

Consider modifying the code to obtain userId from a secure source, such as decoding it from the token or retrieving it from the session.


if (!token)
return NextResponse.json({ message: "Invalid request" }, { status: 400 });

const tokenFromDb = await getTokenFromDb(token, userId);

console.log("tokenFromDb", tokenFromDb);
if (!tokenFromDb.length)
return NextResponse.json({ message: "Invalid token" }, { status: 400 });

const { expiresAt, email } = tokenFromDb[0];
if (expiresAt < new Date())
return NextResponse.json({ message: "Token expired" }, { status: 400 });

await updateEmail(userId, email);

await deleteTokenFromDb(token);

return NextResponse.json(
{ message: "Email successfully verified" },
{ status: 200 },
);
}
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 6, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add error handling to catch unexpected exceptions

The function lacks a try...catch block to handle unexpected errors during asynchronous operations like database access. Unhandled exceptions could cause the server to crash or leak sensitive information. Wrapping the logic in a try...catch block ensures that errors are gracefully handled.

Apply this diff to add error handling:

 export async function POST(req: NextRequest) {
+  try {
     const token = req.nextUrl.searchParams.get("token");
     const { userId } = await req.json();

     if (!token)
       return NextResponse.json({ message: "Invalid request" }, { status: 400 });

     const tokenFromDb = await getTokenFromDb(token, userId);

-    console.log("tokenFromDb", tokenFromDb);
     if (!tokenFromDb.length)
       return NextResponse.json({ message: "Invalid token" }, { status: 400 });

     const { expiresAt, email } = tokenFromDb[0];
     if (expiresAt < new Date())
       return NextResponse.json({ message: "Token expired" }, { status: 400 });

     await updateEmail(userId, email);

     await deleteTokenFromDb(token);

     return NextResponse.json(
       { message: "Email successfully verified" },
       { status: 200 },
     );
+  } catch (error) {
+    console.error('Error verifying email:', error);
+    return NextResponse.json({ message: 'Internal server error' }, { status: 500 });
+  }
 }
πŸ“ 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
export async function POST(req: NextRequest, res: NextResponse) {
const token = req.nextUrl.searchParams.get("token");
const { userId } = await req.json();
if (!token)
return NextResponse.json({ message: "Invalid request" }, { status: 400 });
const tokenFromDb = await getTokenFromDb(token, userId);
console.log("tokenFromDb", tokenFromDb);
if (!tokenFromDb.length)
return NextResponse.json({ message: "Invalid token" }, { status: 400 });
const { expiresAt, email } = tokenFromDb[0];
if (expiresAt < new Date())
return NextResponse.json({ message: "Token expired" }, { status: 400 });
await updateEmail(userId, email);
await deleteTokenFromDb(token);
return NextResponse.json(
{ message: "Email successfully verified" },
{ status: 200 },
);
}
export async function POST(req: NextRequest) {
try {
const token = req.nextUrl.searchParams.get("token");
const { userId } = await req.json();
if (!token)
return NextResponse.json({ message: "Invalid request" }, { status: 400 });
const tokenFromDb = await getTokenFromDb(token, userId);
if (!tokenFromDb.length)
return NextResponse.json({ message: "Invalid token" }, { status: 400 });
const { expiresAt, email } = tokenFromDb[0];
if (expiresAt < new Date())
return NextResponse.json({ message: "Token expired" }, { status: 400 });
await updateEmail(userId, email);
await deleteTokenFromDb(token);
return NextResponse.json(
{ message: "Email successfully verified" },
{ status: 200 },
);
} catch (error) {
console.error('Error verifying email:', error);
return NextResponse.json({ message: 'Internal server error' }, { status: 500 });
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good suggestions here!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher, glad you found the suggestions helpful!

((^.^)/)

106 changes: 106 additions & 0 deletions app/verify_email/_client.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
"use client";

import { Button } from "@headlessui/react";
import { AlertCircle, CheckCircle, Loader } from "lucide-react";
import { useRouter, useSearchParams } from "next/navigation";
import React, { useEffect, useState } from "react";

function Content({ userId }: { userId: string }) {
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 6, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider retrieving userId on the server side to enhance security

Passing userId as a prop to a client-side component may expose sensitive information and could potentially be manipulated. To enhance security, consider retrieving userId on the server side and not exposing it to the client directly. The server can then use the session or authentication token to identify the user.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good comment again. Nitpick, but the url would look better if it was verify-email.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher, I've found the occurrences of verify_email in your codebase:

  • In utils/emailToken.ts:

    const verificationLink = `${process.env.NEXT_PUBLIC_BACKEND_URL}/verify_email?token=${token}`;
  • In app/verify_email/_client.tsx:

    const res = await fetch(`/api/verify_email?token=${token}`, {
      // ...
    });

To update the URL to verify-email, you can make the following changes:

  • In utils/emailToken.ts:

    const verificationLink = `${process.env.NEXT_PUBLIC_BACKEND_URL}/verify-email?token=${token}`;
  • Rename the directory:

    Rename app/verify_email to app/verify-email.

  • In app/verify-email/_client.tsx (after renaming the directory):

    const res = await fetch(`/api/verify-email?token=${token}`, {
      // ...
    });

These updates will ensure that your URLs use hyphens instead of underscores, improving readability.

(^β€Ώ^)/

πŸ”— Analysis chain

@NiallJoeMaher, great suggestion! Changing the URL to verify-email would indeed improve readability.

To implement this, you can rename the directory app/verify_email to app/verify-email and update any references in your codebase.

This will help you identify all instances where verify_email is used, so you can update them to verify-email.

(^β€Ώ^)/

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Find all occurrences of 'verify_email' in the codebase
rg 'verify_email'

Length of output: 233


✏️ Learnings added
Learnt from: NiallJoeMaher
PR: codu-code/codu#1069
File: app/verify_email/_client.tsx:8-8
Timestamp: 2024-10-06T07:42:19.183Z
Learning: Prefer using hyphens instead of underscores in URLs for better readability.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const params = useSearchParams();
const router = useRouter();
const [status, setStatus] = useState<
"idle" | "loading" | "success" | "error"
>("idle");
const [message, setMessage] = useState("");
const [token, setToken] = useState<string | null>(null);

useEffect(() => {
const tokenParam = params.get("token");
if (tokenParam && !token) {
setToken(tokenParam);
}
}, [params, token]);

useEffect(() => {
const verifyEmail = async () => {
if (!token) {
setStatus("error");
setMessage(
"No verification token found. Please check your email for the correct link.",
);
return;
}
setStatus("loading");

try {
const res = await fetch(`/api/verify_email?token=${token}`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ userId }),
});
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 6, 2024

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Consider passing the token in the request body instead of the query parameter

Currently, the token is being passed via the query parameter in a POST request, which is unconventional and may lead to potential security issues, such as the token being logged in server access logs. It's generally more secure and conventional to include data in the body of POST requests.

Apply this diff to pass the token in the request body:

-        const res = await fetch(`/api/verify_email?token=${token}`, {
+        const res = await fetch(`/api/verify_email`, {
           method: "POST",
           headers: {
             "Content-Type": "application/json",
           },
-          body: JSON.stringify({ userId }),
+          body: JSON.stringify({ userId, token }),
         });
πŸ“ 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
const res = await fetch(`/api/verify_email?token=${token}`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ userId }),
});
const res = await fetch(`/api/verify_email`, {
method: "POST",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({ userId, token }),
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The user should be able to click a link to a GET route which will auto-magically validate the token (instead of having to post a request). This ensures that the user clicked their email always.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher, thank you for the clarification. I understand that you prefer to have the user click a link to a GET route that automatically validates the token, ensuring that the user clicked their email.

I've noted this preference for future reviews.


✏️ Learnings added
Learnt from: NiallJoeMaher
PR: codu-code/codu#1069
File: app/verify_email/_client.tsx:36-42
Timestamp: 2024-10-06T07:41:03.924Z
Learning: In the email verification process in `app/verify_email/_client.tsx`, we use a GET route that automatically validates the token when the user clicks the email link, instead of making a POST request. This ensures that the user clicked their email.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

const data = await res.json();
console.log("data", data);
if (res.ok) {
setStatus("success");
} else {
setStatus("error");
}
setMessage(data.message);
} catch (error) {
setStatus("error");
setMessage(
"An error occurred during verification. Please try again later.",
);
}
};

verifyEmail();
}, [token]);

return (
<div className="flex min-h-screen items-center justify-center bg-gray-100">
<div className="w-[350px] rounded-lg border bg-white shadow-sm">
<div className="flex flex-col space-y-1.5 p-6 text-center">
<div className="text-2xl font-bold">Email Verification</div>
<div className="text-gray-400">Verifying your email address</div>
</div>
<div className="min-h-12 p-6 pt-0">
{status === "loading" && (
<div className="flex flex-col items-center justify-center py-4">
<Loader className="text-primary h-4 w-4 animate-spin" />
<p className="text-muted-foreground mt-2 text-sm">
Verifying your email...
</p>
</div>
)}
{status === "success" && (
<div className="flex flex-col items-center justify-center py-4">
<CheckCircle className="h-8 w-8 text-green-500" />
<p className="mt-2 text-center text-sm">{message}</p>
</div>
)}
{status === "error" && (
<div className="flex flex-col items-center justify-center py-4">
<AlertCircle className="h-8 w-8 text-red-500" />
<p className="mt-2 text-center text-sm">{message}</p>
</div>
)}
</div>
{status === "success" && (
<div className="flex items-center justify-center p-6 pt-0">
<Button
onClick={() => router.push("/settings")}
className="mt-4 h-10 rounded-md bg-gray-200 px-4 py-2 transition-colors hover:bg-gray-300"
>
Return to Settings
</Button>
</div>
)}
</div>
</div>
);
}

export default Content;
30 changes: 30 additions & 0 deletions app/verify_email/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { getServerAuthSession } from "@/server/auth";
import { redirect } from "next/navigation";
import React from "react";
import Content from "./_client";
import { db } from "@/server/db";

export const metadata = {
title: "Verify Email",
};

export default async function Page() {
const session = await getServerAuthSession();
console.log("session", session);
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 6, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove console.log to prevent logging sensitive information

The console.log("session", session); statement logs the entire session object, which may contain sensitive user information. Logging such data can lead to security vulnerabilities if logs are exposed.

Apply this diff to remove the debug statement:

   const session = await getServerAuthSession();
-  console.log("session", session);
   if (!session || !session.user) {
πŸ“ 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
console.log("session", session);
const session = await getServerAuthSession();
if (!session || !session.user) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher, thanks for addressing this!

((^_^)/


If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!

if (!session || !session.user) {
redirect("/not-found");
}

const existingUser = await db.query.user.findFirst({
columns: {
id: true,
},
where: (users, { eq }) => eq(users.id, session.user!.id),
});

Comment on lines +18 to +
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Oct 6, 2024

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

Simplify user retrieval by using findUnique

Since you're querying by a unique identifier (id), you can use findUnique instead of findFirst for clarity and potential performance benefits.

Apply this diff to use findUnique:

-  const existingUser = await db.query.user.findFirst({
-    columns: {
-      id: true,
-    },
-    where: (users, { eq }) => eq(users.id, session.user!.id),
-  });
+  const existingUser = await db.query.user.findUnique({
+    where: { id: session.user.id },
+    select: { id: true },
+  });
πŸ“ 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
const existingUser = await db.query.user.findFirst({
columns: {
id: true,
},
where: (users, { eq }) => eq(users.id, session.user!.id),
});
const existingUser = await db.query.user.findUnique({
where: { id: session.user.id },
select: { id: true },
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@NiallJoeMaher, glad you agree!

(_/)
(^.^)
/πŸŽ‰\

if (!existingUser) {
redirect("/not-found");
}

return <Content userId={existingUser.id} />;
}
Loading