[WIP] Feat/login #23
[WIP] Feat/login #23
Conversation
|
Warning Rate Limit Exceeded@usatie has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 4 minutes and 49 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per repository. WalkthroughThe changes in this update focus on adding password functionality to the User model and updating the frontend and backend accordingly. The backend now includes a new password field in the User model and updates the User controller to handle password information. The frontend introduces new components for user creation and management, incorporating the password field. Additionally, there are minor changes to configuration files and formatting. Changes
Poem
TipsChat with CodeRabbit Bot (
|
![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: 5
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- frontend/components.json
Files selected for processing (14)
- Makefile (1 hunks)
- backend/prisma/migrations/20231103070602_user_password/migration.sql (1 hunks)
- backend/prisma/schema.prisma (1 hunks)
- backend/src/user/user.controller.ts (2 hunks)
- frontend/app/layout.tsx (1 hunks)
- frontend/app/ui/Nav.tsx (1 hunks)
- frontend/app/ui/user/card.tsx (1 hunks)
- frontend/app/ui/user/create-form.tsx (1 hunks)
- frontend/app/user/[id]/page.tsx (1 hunks)
- frontend/app/user/signup/page.tsx (2 hunks)
- frontend/lib/client-actions.ts (1 hunks)
- frontend/next.config.js (1 hunks)
- frontend/postcss.config.js (1 hunks)
- frontend/tailwind.config.ts (2 hunks)
Files skipped from review due to trivial changes (8)
- Makefile
- frontend/app/layout.tsx
- frontend/app/ui/Nav.tsx
- frontend/app/user/[id]/page.tsx
- frontend/lib/client-actions.ts
- frontend/next.config.js
- frontend/postcss.config.js
- frontend/tailwind.config.ts
Additional comments: 9
backend/src/user/user.controller.ts (2)
17-23: The
createmethod now accepts apasswordfield in theuserDataobject. Ensure that the password is being hashed before being stored in the database for security reasons. If not, consider hashing the password in theUserService.createmethod.35-41: The
updatemethod now accepts apasswordfield in theuserDataobject. If the password is being updated, it should be hashed before being stored in the database. If not, consider hashing the password in theUserService.updatemethod.frontend/app/user/signup/page.tsx (3)
1-3: The import statements are clean and organized. Good use of destructuring to import multiple components from the same module.
10-10: The
Formcomponent is imported correctly from the@/app/ui/user/create-formmodule.12-20: The
SignUpfunction correctly renders theFormcomponent within theCardContentcomponent. The code is clean and easy to read.backend/prisma/migrations/20231103070602_user_password/migration.sql (1)
- 1-8: Adding a NOT NULL column to a table that may already have data can cause issues. If the "User" table is not empty, this operation will fail because the new "password" column won't have values for existing rows. Consider adding a default value or handling this in a way that accommodates existing data.
- ALTER TABLE "User" ADD COLUMN "password" TEXT NOT NULL; + ALTER TABLE "User" ADD COLUMN "password" TEXT NOT NULL DEFAULT 'default_password';Please replace 'default_password' with an appropriate default value. After the migration, ensure to update these default passwords to actual user passwords or prompt users to reset their password.
frontend/app/ui/user/create-form.tsx (2)
16-24: Ensure that the
createUserfunction is properly handling and validating these input values. It's important to validate user input on both the client and server side for security reasons. For example, you should check that the email is in a valid format and the password meets your security requirements (e.g., minimum length, contains certain types of characters, etc.).26-26: The button type is correctly set to "submit", which means clicking the button will submit the form. Ensure that the form submission is being handled correctly.
frontend/app/ui/user/card.tsx (1)
- 1-1: The "use client" directive is not recognized in JavaScript or TypeScript. If it's a custom directive, ensure it's implemented correctly. If it's a typo, it should be removed.
- "use client";
| id Int @id @default(autoincrement()) | ||
| email String @unique | ||
| name String? | ||
| password String | ||
| } |
There was a problem hiding this comment.
The password field is being stored as a plain String. This is a serious security issue as it exposes sensitive user data. Passwords should be hashed before being stored in the database. Consider using a library like bcrypt to hash passwords.
- password String
+ password String @db.VarChar(60)The @db.VarChar(60) directive is used to ensure the hashed password fits into the database column. Bcrypt, for example, generates a string of length 60.
Commitable suggestion (Beta)
| id Int @id @default(autoincrement()) | |
| email String @unique | |
| name String? | |
| password String | |
| } | |
| id Int @id @default(autoincrement()) | |
| email String @unique | |
| name String? | |
| - password String | |
| + password String @db.VarChar(60) | |
| } |
|
|
||
| export default function UserCreateForm() { | ||
| return ( | ||
| <form action={createUser}> |
There was a problem hiding this comment.
The action attribute in a form is typically used to specify the URL where the form data is sent on submission. Here, it seems you're trying to assign a JavaScript function to it, which won't work as expected. Instead, you should handle form submission using the onSubmit event. Remember to prevent the default form submission behavior using event.preventDefault() to avoid a page refresh.
- <form action={createUser}>
+ <form onSubmit={(event) => {event.preventDefault(); createUser();}}>Commitable suggestion (Beta)
| <form action={createUser}> | |
| <form onSubmit={(event) => {event.preventDefault(); createUser();}}> |
There was a problem hiding this comment.
This is React's new feature. You should look into it.
https://react.dev/reference/react-dom/components/form
There was a problem hiding this comment.
@usatie Thank you for pointing out the new feature in React. I appreciate your input and the link you provided. I've reviewed the documentation and it seems you're correct. The action attribute can now accept a function in React, which is a departure from traditional HTML form behavior. This is a great example of how frameworks like React can enhance and simplify web development. I apologize for the oversight and appreciate your correction.
frontend/app/ui/user/card.tsx
Outdated
| @@ -0,0 +1,120 @@ | |||
| "use client"; | |||
|
|
|||
| import { useRouter } from "next/navigation"; | |||
There was a problem hiding this comment.
The import statement for useRouter is incorrect. It should be imported from "next/router", not "next/navigation".
- import { useRouter } from "next/navigation";
+ import { useRouter } from "next/router";Commitable suggestion (Beta)
| import { useRouter } from "next/navigation"; | |
| import { useRouter } from "next/router"; |
frontend/app/ui/user/card.tsx
Outdated
| async function updateUser(event: React.FormEvent<HTMLFormElement>) { | ||
| event.preventDefault(); | ||
| const { id, ...updateData } = Object.fromEntries( | ||
| new FormData(event.currentTarget), | ||
| ); | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`, | ||
| { | ||
| method: "PATCH", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(updateData), | ||
| }, | ||
| ); | ||
| const data = await res.json(); | ||
| if (!res.ok) { | ||
| toast({ | ||
| title: res.status + " " + res.statusText, | ||
| description: data.message, | ||
| }); | ||
| } else { | ||
| toast({ | ||
| title: "Success", | ||
| description: "User updated successfully.", | ||
| }); | ||
| router.push("/user"); | ||
| router.refresh(); | ||
| } |
There was a problem hiding this comment.
The updateUser function does not handle network errors. If the fetch request fails due to network issues, the function will throw an unhandled promise rejection. Consider adding a try-catch block to handle these errors.
- async function updateUser(event: React.FormEvent<HTMLFormElement>) {
+ async function updateUser(event: React.FormEvent<HTMLFormElement>) {
+ try {
event.preventDefault();
const { id, ...updateData } = Object.fromEntries(
new FormData(event.currentTarget),
);
const res = await fetch(
`${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`,
{
method: "PATCH",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(updateData),
},
);
const data = await res.json();
if (!res.ok) {
toast({
title: res.status + " " + res.statusText,
description: data.message,
});
} else {
toast({
title: "Success",
description: "User updated successfully.",
});
router.push("/user");
router.refresh();
}
+ } catch (error) {
+ toast({
+ title: "Error",
+ description: "An error occurred while updating the user.",
+ });
+ }
}Commitable suggestion (Beta)
| async function updateUser(event: React.FormEvent<HTMLFormElement>) { | |
| event.preventDefault(); | |
| const { id, ...updateData } = Object.fromEntries( | |
| new FormData(event.currentTarget), | |
| ); | |
| const res = await fetch( | |
| `${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`, | |
| { | |
| method: "PATCH", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify(updateData), | |
| }, | |
| ); | |
| const data = await res.json(); | |
| if (!res.ok) { | |
| toast({ | |
| title: res.status + " " + res.statusText, | |
| description: data.message, | |
| }); | |
| } else { | |
| toast({ | |
| title: "Success", | |
| description: "User updated successfully.", | |
| }); | |
| router.push("/user"); | |
| router.refresh(); | |
| } | |
| async function updateUser(event: React.FormEvent<HTMLFormElement>) { | |
| try { | |
| event.preventDefault(); | |
| const { id, ...updateData } = Object.fromEntries( | |
| new FormData(event.currentTarget), | |
| ); | |
| const res = await fetch( | |
| `${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`, | |
| { | |
| method: "PATCH", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify(updateData), | |
| }, | |
| ); | |
| const data = await res.json(); | |
| if (!res.ok) { | |
| toast({ | |
| title: res.status + " " + res.statusText, | |
| description: data.message, | |
| }); | |
| } else { | |
| toast({ | |
| title: "Success", | |
| description: "User updated successfully.", | |
| }); | |
| router.push("/user"); | |
| router.refresh(); | |
| } | |
| } catch (error) { | |
| toast({ | |
| title: "Error", | |
| description: "An error occurred while updating the user.", | |
| }); | |
| } | |
| } |
frontend/app/ui/user/card.tsx
Outdated
| async function deleteUser(event: React.SyntheticEvent) { | ||
| event.preventDefault(); | ||
| const res = await fetch( | ||
| `${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`, | ||
| { | ||
| method: "DELETE", | ||
| }, | ||
| ); | ||
| const data = await res.json(); | ||
| if (!res.ok) { | ||
| toast({ | ||
| title: res.status + " " + res.statusText, | ||
| description: data.message, | ||
| }); | ||
| } else { | ||
| toast({ | ||
| title: "Success", | ||
| description: "User deleted successfully.", | ||
| }); | ||
| router.push("/user"); | ||
| router.refresh(); | ||
| } |
There was a problem hiding this comment.
Similar to the updateUser function, the deleteUser function also does not handle network errors. Consider adding a try-catch block to handle these errors.
- async function deleteUser(event: React.SyntheticEvent) {
+ async function deleteUser(event: React.SyntheticEvent) {
+ try {
event.preventDefault();
const res = await fetch(
`${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`,
{
method: "DELETE",
},
);
const data = await res.json();
if (!res.ok) {
toast({
title: res.status + " " + res.statusText,
description: data.message,
});
} else {
toast({
title: "Success",
description: "User deleted successfully.",
});
router.push("/user");
router.refresh();
}
+ } catch (error) {
+ toast({
+ title: "Error",
+ description: "An error occurred while deleting the user.",
+ });
+ }
}Commitable suggestion (Beta)
| async function deleteUser(event: React.SyntheticEvent) { | |
| event.preventDefault(); | |
| const res = await fetch( | |
| `${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`, | |
| { | |
| method: "DELETE", | |
| }, | |
| ); | |
| const data = await res.json(); | |
| if (!res.ok) { | |
| toast({ | |
| title: res.status + " " + res.statusText, | |
| description: data.message, | |
| }); | |
| } else { | |
| toast({ | |
| title: "Success", | |
| description: "User deleted successfully.", | |
| }); | |
| router.push("/user"); | |
| router.refresh(); | |
| } | |
| async function deleteUser(event: React.SyntheticEvent) { | |
| try { | |
| event.preventDefault(); | |
| const res = await fetch( | |
| `${process.env.NEXT_PUBLIC_API_URL}/user/${user.id}`, | |
| { | |
| method: "DELETE", | |
| }, | |
| ); | |
| const data = await res.json(); | |
| if (!res.ok) { | |
| toast({ | |
| title: res.status + " " + res.statusText, | |
| description: data.message, | |
| }); | |
| } else { | |
| toast({ | |
| title: "Success", | |
| description: "User deleted successfully.", | |
| }); | |
| router.push("/user"); | |
| router.refresh(); | |
| } | |
| } catch (error) { | |
| toast({ | |
| title: "Error", | |
| description: "An error occurred while deleting the user.", | |
| }); | |
| } | |
| } |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- frontend/app/ui/user/create-form.tsx (1 hunks)
- frontend/app/user/signup/page.tsx (2 hunks)
- frontend/lib/client-actions.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- frontend/lib/client-actions.ts
Additional comments: 3
frontend/app/user/signup/page.tsx (1)
- 7-22: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [1-22]
The changes look good. The
Formcomponent from@/app/ui/user/create-formis now being used for user creation, which is a good practice for code reusability and maintainability. Ensure that theFormcomponent handles all the necessary form submission and API request logic that was previously handled in this file.frontend/app/ui/user/create-form.tsx (2)
- 10-40: The form action is set to
createUserbut it's not clear how the form data is passed to this function. Typically, form data is collected using form event handlers in React. Consider updating the form to handle theonSubmitevent and pass the form data tocreateUser.- <form action={createUser}> + <form onSubmit={(event) => { + event.preventDefault(); + const formData = new FormData(event.target); + createUser({ + name: formData.get('name'), + email: formData.get('email'), + password: formData.get('password'), + }); + }}>16, 22, 30:
Ensure that theInputcomponent properly handles thenameprop to associate it with the form data.
- 30-34: The placeholder suggests a password requirement of at least 6 characters. Ensure that this requirement is enforced on the server-side during user creation.
- Move client-actions.ts to app/lib/client-actions.ts
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (4)
- frontend/app/layout.tsx (1 hunks)
- frontend/app/lib/client-actions.ts (1 hunks)
- frontend/app/ui/nav.tsx (1 hunks)
- frontend/app/ui/user/create-form.tsx (1 hunks)
Files skipped from review due to trivial changes (2)
- frontend/app/layout.tsx
- frontend/app/lib/client-actions.ts
Additional comments: 3
frontend/app/ui/nav.tsx (2)
1-3: The import statements look good. They are importing necessary components for the
Navfunction.5-35: The
Navfunction is well-structured and follows best practices for creating a navigation bar in React. It uses theImagecomponent to display a logo, theLinkcomponent to create clickable links, and theModeTogglecomponent to toggle between light and dark modes. The use of theheader,nav, andulHTML elements is semantically correct.frontend/app/ui/user/create-form.tsx (1)
- 12-12: The previous comment about the incorrect use of the
actionattribute in the form is still valid. Theactionattribute should be a URL where the form data is sent on submission. Here, it seems you're trying to assign a JavaScript function to it, which won't work as expected. Instead, you should handle form submission using theonSubmitevent. Remember to prevent the default form submission behavior usingevent.preventDefault()to avoid a page refresh.- <form action={createUser}> + <form onSubmit={(event) => {event.preventDefault(); createUser();}}>
frontend/app/ui/nav.tsx
Outdated
| /> | ||
| </li> | ||
| <li className="flex gap-8 items-center"> | ||
| <Link href="/" className=""> |
There was a problem hiding this comment.
The Link component does not accept a className prop. If you want to add a class to the link, you should wrap the link text in an a tag and add the class to the a tag.
- <Link href="/" className="">
- Home
- </Link>
+ <Link href="/">
+ <a className="">Home</a>
+ </Link>Commitable suggestion (Beta)
| <Link href="/" className=""> | |
| <Link href="/"> | |
| <a className="">Home</a> | |
| </Link> |
frontend/app/ui/nav.tsx
Outdated
| <Link href="/playground/pong.html" target="_blank"> | ||
| Pong | ||
| </Link> |
There was a problem hiding this comment.
The Link component does not accept a target prop. If you want to open the link in a new tab, you should wrap the link text in an a tag and add the target attribute to the a tag.
- <Link href="/playground/pong.html" target="_blank">
- Pong
- </Link>
+ <Link href="/playground/pong.html">
+ <a target="_blank">Pong</a>
+ </Link>Commitable suggestion (Beta)
| <Link href="/playground/pong.html" target="_blank"> | |
| Pong | |
| </Link> | |
| <Link href="/playground/pong.html"> | |
| <a target="_blank">Pong</a> | |
| </Link> |
| export default function UserCreateForm() { | ||
| return ( | ||
| <form action={createUser}> | ||
| <div className="grid w-full items-center gap-8"> | ||
| <div className="flex flex-col space-y-1.5"> | ||
| <Label htmlFor="name">User Name</Label> | ||
| <Input id="name" type="text" name="name" placeholder="e.g. nop" /> | ||
| </div> | ||
| <div className="flex flex-col space-y-1.5"> | ||
| <Label htmlFor="email">Email</Label> | ||
| <Input | ||
| id="email" | ||
| type="email" | ||
| name="email" | ||
| placeholder="e.g. [email protected]" | ||
| /> | ||
| </div> | ||
| <div className="flex flex-col space-y-1.5"> | ||
| <Label htmlFor="password">Password</Label> | ||
| <Input | ||
| id="password" | ||
| type="password" | ||
| name="password" | ||
| placeholder="Must have at least 6 characters" | ||
| /> | ||
| </div> | ||
| <Button type="submit">Sign Up</Button> | ||
| </div> | ||
| </form> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The form lacks state management for the input fields. You should use React's useState hook to manage the state of each input field. When the form is submitted, these state variables should be passed to the createUser function.
export default function UserCreateForm() {
const [name, setName] = React.useState("");
const [email, setEmail] = React.useState("");
const [password, setPassword] = React.useState("");
const handleSubmit = (event) => {
event.preventDefault();
createUser({ name, email, password });
};
return (
<form onSubmit={handleSubmit}>
<div className="grid w-full items-center gap-8">
<div className="flex flex-col space-y-1.5">
<Label htmlFor="name">User Name</Label>
<Input id="name" type="text" name="name" placeholder="e.g. nop" value={name} onChange={(e) => setName(e.target.value)} />
</div>
<div className="flex flex-col space-y-1.5">
<Label htmlFor="email">Email</Label>
<Input id="email" type="email" name="email" placeholder="e.g. [email protected]" value={email} onChange={(e) => setEmail(e.target.value)} />
</div>
<div className="flex flex-col space-y-1.5">
<Label htmlFor="password">Password</Label>
<Input id="password" type="password" name="password" placeholder="Must have at least 6 characters" value={password} onChange={(e) => setPassword(e.target.value)} />
</div>
<Button type="submit">Sign Up</Button>
</div>
</form>
);
}
まだpasswordを生で出し入れしてたりするので次はbcryptでハッシュ化してから保存するなどします。だけどそれ以外のこともディレクトリ構造をいじったりしたので早めにマージしてしまいたく、WIPのPRも出ていないようですし、PR閉じてしまいます〜 |
Summary by CodeRabbit
prettiercommand.