Feature/43 jwt #67
Feature/43 jwt #67
Conversation
WalkthroughThe project underwent a comprehensive update, enhancing various API endpoints and integrating new features. Changes include adjustments to JWT handling, the introduction of new API routes for diary management, emotion analysis, and image uploads, as well as updates to error handling and response formatting. The integration of state management with Recoil and session management improvements with NextAuth are also notable. These updates aim to refine the application's functionality and user experience. Changes
Assessment against linked issues (Beta)
Poem
TipsChat with CodeRabbit Bot (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| const hashPassword = crypto.createHash('sha512').update(body.password + salt).digest('hex'); | ||
| // 비밀번호 뒤에 salt를 붙여서 암호화. | ||
| const hashPassword = crypto.createHash('sha512').update(password + salt).digest('hex'); |
Check failure
Code scanning / CodeQL
Use of password hash with insufficient computational effort
| return NextResponse.json({result:'뭔가가 이상함.'}); | ||
| } | ||
| const salt = chk[0].user_salt; | ||
| const db_pw = chk[0].user_password; |
Check failure
Code scanning / CodeQL
Use of password hash with insufficient computational effort
![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: 19
Configuration used: CodeRabbit UI
Files ignored due to filter (7)
- package-lock.json
- package.json
- public/imgs/1699932284800_yi862zgmllcb4h7KzeLzGq3LFaGkwIxP.png
- public/imgs/1699932377524_q28ucKw2qv7zceIO.png
- public/imgs/1699932690871_6ziy3nprERU56YTy.png
- public/imgs/스크린샷 2023-09-11 100559.png
- public/imgs/스크린샷 2023-09-15 085650.png
Files selected for processing (27)
- .prettierrc.js (1 hunks)
- src/app/api/advice/route.ts (1 hunks)
- src/app/api/auth/[...nextauth]/route.ts (2 hunks)
- src/app/api/diary/route.ts (1 hunks)
- src/app/api/emotion/route.ts (2 hunks)
- src/app/api/login/route.ts (3 hunks)
- src/app/api/summary/route.ts (1 hunks)
- src/app/api/test/route.ts (1 hunks)
- src/app/api/trans/route.ts (1 hunks)
- src/app/api/user/route.ts (1 hunks)
- src/app/api/user/token/route.ts (1 hunks)
- src/app/calendar/page.tsx (1 hunks)
- src/app/components/Providers.tsx (1 hunks)
- src/app/components/RefreshToken.tsx (2 hunks)
- src/app/diary/_components/DiaryLayout.tsx (1 hunks)
- src/app/diary/page.tsx (2 hunks)
- src/app/hooks/hooks.ts (1 hunks)
- src/app/icontest/contenttest/page.tsx (1 hunks)
- src/app/icontest/formtest/page.tsx (1 hunks)
- src/app/icontest/icon.css (1 hunks)
- src/app/icontest/page.tsx (1 hunks)
- src/app/lib/atoms/atom.ts (1 hunks)
- src/app/lib/jwt.ts (1 hunks)
- src/app/signin2/_components/LoginPage.tsx (1 hunks)
- src/app/signin2/page.tsx (1 hunks)
- src/app/signin2/test/page.tsx (1 hunks)
- src/app/write/page.tsx (1 hunks)
Files skipped from review due to trivial changes (8)
- .prettierrc.js
- src/app/api/auth/[...nextauth]/route.ts
- src/app/api/summary/route.ts
- src/app/api/trans/route.ts
- src/app/diary/_components/DiaryLayout.tsx
- src/app/lib/atoms/atom.ts
- src/app/signin2/_components/LoginPage.tsx
- src/app/write/page.tsx
Additional comments: 21
src/app/hooks/hooks.ts (1)
- 5-8: Reducing the length of the random string from 32 to 16 characters will decrease the entropy of the generated string, which could potentially affect the security if these strings are used in sensitive contexts such as tokens, session identifiers, etc. Ensure that this change does not compromise the security requirements of the system.
src/app/components/Providers.tsx (1)
- 2-23: The integration of
RecoilRootwithinSessionProvideris correctly implemented. This setup ensures that the Recoil state management is available throughout the application while still maintaining the session context provided bynext-auth. It's important to ensure that the rest of the application is adapted to use Recoil where state management is needed.src/app/api/user/token/route.ts (3)
- 4-5: The console log here seems to be for debugging purposes. Ensure that it is intended to be part of the production code. If not, it should be removed to avoid unnecessary logging in production.
- console.log('fetched...')
15-15: The comment indicates that the current time is being set to 1 minute earlier. This could be for token expiration leeway. Verify that this adjustment aligns with the intended token expiration policy.
18-18: The check for
decodedbeing equal to 'signout' is a specific business logic. Ensure that this behavior is documented and that other parts of the system are aware of this special 'signout' token case.src/app/icontest/page.tsx (1)
- 1-19: The React component
Pageis correctly using theuseStatehook to manage the toggle state and a click event handler to update it. The use of template literals to conditionally apply the 'toggle' class is also correct. The import of the CSS file is standard practice for styling components in React.However, the
"use client"directive at the top of the file is not standard. If this is intended to be a Next.js specific directive for client-side only components, ensure that it is supported by the version of Next.js being used and that it is correctly implemented.// Correct usage of useState and event handling const [toggle, setToggle] = useState(false); // ... <div className={`btn ${toggle ? 'toggle' : ''}`} onClick={() => setToggle(!toggle)}>src/app/api/login/route.ts (3)
1-4: The import of
NextResponseand the use ofNextResponse.jsonis a good practice for handling HTTP responses in Next.js API routes. It ensures consistency and leverages built-in helpers for JSON responses.15-15: The error message is in a language other than English. While this may be intentional for localization, ensure that this aligns with the application's language policy and that appropriate localization mechanisms are in place if the application is intended for an international audience.
31-31: The error message for an incorrect password is clear and user-friendly. However, ensure that the language used is consistent with the rest of the application.
src/app/calendar/page.tsx (1)
- 6-31: The removal of the date formatting logic before the axios post request could potentially change the behavior of the API call. Ensure that the backend API endpoint at
/api/testis expecting an empty object and that no date information is required for the request to be processed correctly. If the date is still needed, the removed code should be restored or the backend should be updated accordingly.- // const dateValue = Array.isArray(value) ? (value as Date[])[0] : (value as Date); - const response = await axios.post(`${process.env.NEXT_PUBLIC_BASE_URL}/api/test`, - { - data: { - } - }); + const dateValue = Array.isArray(value) ? value[0] : value; + const response = await axios.post(`${process.env.NEXT_PUBLIC_BASE_URL}/api/test`, { date: dateValue });src/app/diary/page.tsx (1)
- 63-69: The removal of the class name binding from the JSX elements is a visual change. Ensure that this change is intentional and that the corresponding CSS rules are updated to match the new class names if necessary. Also, verify that the removal of the class name binding does not affect the styling and functionality of the pagination elements.
src/app/api/user/route.ts (1)
- 119-123: The
OPTIONSmethod seems to be for testing purposes and logs the user ID. Ensure that this method does not expose any sensitive information and is protected or removed before deploying to production.src/app/api/emotion/route.ts (3)
4-8: The
reqBodytype has been updated to expect an array of strings for thetextproperty. This change will require all client-side code that sends requests to this endpoint to send an array of strings instead of a single string. Ensure that all such instances are updated to match this new expectation.19-25: The use of an environment variable
process.env.MODEL_URLis a good practice for maintaining flexibility and configurability of the application. However, ensure that this environment variable is properly set in all deployment configurations to prevent runtime errors.25-25: The response is being directly passed as
result.resultwhich assumes that the axios response data has a propertyresultcontaining the desired data. Ensure that the external service is indeed returning data in this format. Additionally, consider adding error handling for cases whereres.datadoes not match the expected structure to prevent runtime errors.src/app/icontest/icon.css (1)
- 1-46: The CSS changes introduced here seem to be for a toggle button, where the
.circleelement moves and changes color when the.toggleclass is applied. The removal of the::afterpseudo-element in the toggled state suggests a visual change in the button's appearance when activated. Ensure that these changes align with the design specifications and that they have been tested across different browsers for compatibility.
- The use of absolute positioning for
.circle(line 18) and the transformation on line 25 to vertically center the element is a common technique, but it's important to test this in the context of the full UI to ensure it works as expected.- The transition on line 26 indicates a smooth animation effect when the toggle state changes. Verify that the duration and easing of the transition are consistent with the UX design.
- The
.toggleclass on line 40-42 changes the position and color of the.circle, which is a clear indication of a state change. This is a good use of CSS to reflect dynamic states in the UI.- The
display: none;on line 45 for the::afterpseudo-element when.toggleis applied suggests that some visual element is removed from the circle when toggled. This should be confirmed to match the intended design.Overall, the CSS changes seem appropriate for a toggle button. However, it's important to ensure that these styles are scoped appropriately to avoid conflicts with other elements in the application and that they are tested for cross-browser compatibility.
src/app/signin2/page.tsx (1)
- 1-78: - The
use clientdirective at the top of the file is not a standard directive in Next.js or React. If it's meant to be a custom directive or comment, it should be clearly marked as such to avoid confusion.
src/app/components/RefreshToken.tsx (3)
1-7: The use of
"use client";is not standard and might be a typo or a custom directive. If it's meant to be a directive for client-side rendering, Next.js handles this automatically, and such a directive is unnecessary. Verify if this is intended and remove it if it's not serving any purpose.31-36: The use of
setIntervalfor refreshing the access token is a good approach. However, ensure that the interval is cleared when the component is unmounted to prevent memory leaks. The current implementation correctly clears the interval, which is good practice.37-37: The dependency array for
useEffectonly includessession, which is correct if therefreshAccessfunction does not depend on any other variables that could change over time. If there are other dependencies, they should be included in the array to ensure the effect is run when they change.src/app/signin2/test/page.tsx (1)
- 1-90: The component seems to be handling user registration with image upload. Here are some points to consider:
- The
use clientdirective at the top is non-standard and should be removed unless it's part of a custom DSL or framework that requires it.- The
console.log(session);on line 15 should be removed or replaced with a more robust logging mechanism before deploying to production to avoid exposing session details.- The
handleImgfunction on lines 16-23 creates a new object URL every time a file is selected but only revokes the previous URL if a new file is selected. This could potentially lead to memory leaks if the user selects files multiple times without submitting the form.- The
handleSubmitfunction on lines 32-58 does not handle errors from theaxios.putcall. It's important to add error handling to inform the user if the registration fails and to handle any exceptions that may occur.- The
inputfields for the password on line 63 should usetype="password"instead oftype="text"to hide the input from view.- The
classNamevalues like'border'and'w-[60px] h-[60px] rounded-[50%]'suggest that Tailwind CSS is being used. Ensure that these classes are defined in your Tailwind configuration.- The
Imagecomponent fromnext/imageon lines 77-83 is used for displaying the profile image preview. Ensure that thesrcattribute is properly sanitized to prevent any security vulnerabilities.- The
imgResetfunction on lines 24-30 revokes the object URL and clears the image preview, which is good for resetting the state, but it should also handle any necessary cleanup if the component is unmounted to prevent memory leaks.- The
pagefunction should be named with a capital letter (Page) to follow React component naming conventions and to avoid potential issues with case-sensitive file systems or tooling.- The
export default pageon line 90 should be updated to match the corrected component name.Here's a diff with some suggested changes:
-"use client" + import axios from 'axios'; import { useSession } from 'next-auth/react'; import Image from 'next/image'; import React, { useRef, useState } from 'react' -const page = () => { +const Page = () => { - console.log(session); + // Consider using a more robust logging mechanism or remove this line. - <input type="text" ref={pwRef} className='border' /><br /> + <input type="password" ref={pwRef} className='border' /><br /> </blockquote></details></blockquote></details> </details>
src/app/diary/page.tsx
Outdated
| import 'react-datepicker/dist/react-datepicker.css'; | ||
| import { ko } from 'date-fns/esm/locale' | ||
| import DiaryLayout from './_components/DiaryLayout'; | ||
| import { useRecoilState } from 'recoil' | ||
| import { textState } from '@/app/lib/atoms/atom' | ||
|
|
||
|
|
||
| const Diary = () => { | ||
| const [startDate, setStartDate] = useState<Date>(new Date()); | ||
| const [endDate, setEndDate] = useState<Date>(new Date()); | ||
| const [test, setText] = useRecoilState(textState); | ||
| console.log(test); |
There was a problem hiding this comment.
The introduction of Recoil for state management is a significant change. Ensure that the textState atom is properly defined and that its usage here aligns with the intended application state management strategy. Also, consider removing the console.log statement before merging to production to avoid exposing application state unnecessarily.
- console.log(test);Commitable 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.
| import 'react-datepicker/dist/react-datepicker.css'; | |
| import { ko } from 'date-fns/esm/locale' | |
| import DiaryLayout from './_components/DiaryLayout'; | |
| import { useRecoilState } from 'recoil' | |
| import { textState } from '@/app/lib/atoms/atom' | |
| const Diary = () => { | |
| const [startDate, setStartDate] = useState<Date>(new Date()); | |
| const [endDate, setEndDate] = useState<Date>(new Date()); | |
| const [test, setText] = useRecoilState(textState); | |
| console.log(test); | |
| import 'react-datepicker/dist/react-datepicker.css'; | |
| import { ko } from 'date-fns/esm/locale' | |
| import DiaryLayout from './_components/DiaryLayout'; | |
| import { useRecoilState } from 'recoil' | |
| import { textState } from '@/app/lib/atoms/atom' | |
| const Diary = () => { | |
| const [startDate, setStartDate] = useState<Date>(new Date()); | |
| const [endDate, setEndDate] = useState<Date>(new Date()); | |
| const [test, setText] = useRecoilState(textState); |
| `; | ||
|
|
||
| // 쿼리 실행 | ||
| await queryPromise(sql, [user_id, emotion_img, user_pw, user_name, provider]); | ||
|
|
||
| const result = await queryPromise(sql, [user_id, emotion_img, user_pw, user_name, provider]); | ||
| console.log('test------------------------') | ||
| // 성공적인 응답 반환 | ||
| const result = { emotion_img }; | ||
| console.log(result); | ||
|
|
||
| // NextResponse 생성자를 사용하여 응답 반환 | ||
| return new NextResponse(JSON.stringify(result), { |
There was a problem hiding this comment.
The code correctly logs the result of the database query. However, it's generally not a good practice to leave console logs in production code as it can lead to unintentional information disclosure and clutter the server logs. Consider removing the console log or using a more sophisticated logging mechanism that can be toggled for development and production environments.
- console.log('test------------------------')
- console.log(result);Commitable 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.
| `; | |
| // 쿼리 실행 | |
| await queryPromise(sql, [user_id, emotion_img, user_pw, user_name, provider]); | |
| const result = await queryPromise(sql, [user_id, emotion_img, user_pw, user_name, provider]); | |
| console.log('test------------------------') | |
| // 성공적인 응답 반환 | |
| const result = { emotion_img }; | |
| console.log(result); | |
| // NextResponse 생성자를 사용하여 응답 반환 | |
| return new NextResponse(JSON.stringify(result), { | |
| `; | |
| // 쿼리 실행 | |
| const result = await queryPromise(sql, [user_id, emotion_img, user_pw, user_name, provider]); | |
| // 성공적인 응답 반환 | |
| // NextResponse 생성자를 사용하여 응답 반환 | |
| return new NextResponse(JSON.stringify(result), { |
| accessToken, | ||
| refreshToken | ||
| }; | ||
| return new Response(JSON.stringify(rst)) |
There was a problem hiding this comment.
The response for a successful login attempt should also use NextResponse.json for consistency with the error handling responses.
- return new Response(JSON.stringify(rst))
+ return NextResponse.json(rst)Commitable 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.
| return new Response(JSON.stringify(rst)) | |
| return NextResponse.json(rst) |
| export const verifyJwt = (token: string) => { | ||
| try { | ||
| const secret_key = process.env.SECRET_KEY | ||
| const decoded = jwt.verify(token, secret_key!) | ||
| const decoded = jwt.verify(token, secret_key!); | ||
| const currentTime = Math.floor(Date.now() / 1000) - 60000; // 갱신할 시간. | ||
| return decoded as JwtPayload | ||
| } catch(err) { | ||
| console.log(err); | ||
| return null; | ||
| console.log('에러에러에러'); | ||
| return "signout"; | ||
| } | ||
| } |
There was a problem hiding this comment.
The verifyJwt function has been updated to include a currentTime variable, which seems to be intended for token refresh logic. However, the variable is not used within the function, which could be an oversight. If the intention was to check the token's expiration against the current time, this logic is missing. Additionally, returning a string "signout" on error is a significant change in the function's contract and could lead to unexpected behavior if the calling code expects a JwtPayload object or null to indicate failure. The calling code should be reviewed to ensure it can handle this new return type properly.
Consider refactoring to use the currentTime variable for expiration checks and to return a more consistent type, such as null or an object with an error property, to indicate failure.
- const currentTime = Math.floor(Date.now() / 1000) - 60000; // 갱신할 시간.
+ const currentTime = Math.floor(Date.now() / 1000);
+ if (decoded.exp && currentTime > decoded.exp) {
+ // Token has expired
+ return null; // or { error: "Token has expired" }
+ }Also, the comment // 갱신할 시간. is not in English, which may not be consistent with the rest of the codebase if English is the standard language for comments. It should be translated to maintain consistency.
Commitable 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.
| export const verifyJwt = (token: string) => { | |
| try { | |
| const secret_key = process.env.SECRET_KEY | |
| const decoded = jwt.verify(token, secret_key!) | |
| const decoded = jwt.verify(token, secret_key!); | |
| const currentTime = Math.floor(Date.now() / 1000) - 60000; // 갱신할 시간. | |
| return decoded as JwtPayload | |
| } catch(err) { | |
| console.log(err); | |
| return null; | |
| console.log('에러에러에러'); | |
| return "signout"; | |
| } | |
| } | |
| export const verifyJwt = (token: string) => { | |
| try { | |
| const secret_key = process.env.SECRET_KEY | |
| const decoded = jwt.verify(token, secret_key!); | |
| const currentTime = Math.floor(Date.now() / 1000); | |
| if (decoded.exp && currentTime > decoded.exp) { | |
| // Token has expired | |
| return null; // or { error: "Token has expired" } | |
| } | |
| return decoded as JwtPayload | |
| } catch(err) { | |
| console.log('에러에러에러'); | |
| return "signout"; | |
| } | |
| } |
src/app/api/user/route.ts
Outdated
| export const POST = async(req: Request) => { | ||
| const body: reqBody = await req.json(); | ||
| // 쿼리 가져옴. | ||
|
|
||
| try { | ||
| let sql = 'SELECT user_id from tb_user WHERE user_id = ?' | ||
| let sql = 'SELECT user_id from tb_user WHERE user_id = ?'; | ||
| const chk = await queryPromise(sql, [body.user_id]); | ||
| if(chk.length >= 1) { | ||
| // 아이디가 이미 있다. | ||
| return new Response(JSON.stringify({"result":"exists"})) | ||
| // 아이디 존재. | ||
| return NextResponse.json({result:'이미 있는 아이디예요.'}); | ||
| } | ||
| sql = 'INSERT INTO tb_user VALUES(?,?,?,?,?,?,?,?,?)' | ||
| return NextResponse.json({result:'가입할 수 있는 아이디예요.'}) | ||
| } catch(err: any) { | ||
| console.log(err); | ||
| return NextResponse.json({result:'에러에러에러.'}) | ||
| } |
There was a problem hiding this comment.
The POST method is correctly checking for the existence of a user ID and returning appropriate JSON responses. However, the error message in line 30 is generic and not informative for debugging purposes. Consider logging more specific error details or using a structured error response.
| export const PUT = async(req: Request) => { | ||
| const data = await req.formData(); | ||
| const images: any[] = []; | ||
| data.forEach((v, k) => { | ||
| images.push(v); | ||
| }) | ||
| const imgs: any[] = []; | ||
|
|
||
| const uploadImages = async () => { | ||
| for (const v of images) { | ||
| const fb = new FormData(); | ||
| fb.append('image', v); | ||
|
|
||
| try { | ||
| const result = await axios.post( | ||
| 'https://api.imgur.com/3/upload', | ||
| fb, | ||
| { | ||
| headers: { | ||
| 'Authorization': `Client-ID ${process.env.IMGUR_KEY}`, | ||
| 'Accept': 'application/json' | ||
| } | ||
| } | ||
| ); | ||
| imgs.push(result.data.data.link); | ||
| } catch (error) { | ||
| console.log(error); | ||
| return 0; | ||
| } | ||
| } | ||
| return imgs; | ||
| }; | ||
|
|
||
| const result = await uploadImages(); | ||
| if(result === 0) { | ||
| return NextResponse.json({result:'error'}); | ||
| } | ||
| console.log(result); | ||
|
|
||
| return NextResponse.json({result:imgs}) | ||
| } No newline at end of file |
There was a problem hiding this comment.
- The PUT function is designed to upload multiple images to Imgur, but there is no check to ensure that the images are valid or that the upload was successful before returning the URLs.
- The function
uploadImagesis defined within thePUTfunction, which could be extracted to a separate utility function for reusability and clarity. - The error handling in the
uploadImagesfunction returns0in case of an error, which is not a standard way to handle errors in asynchronous functions. It would be better to throw an error and handle it appropriately. - The
FormDataobject is being used without checking if the incoming data contains files, which could lead to errors if the data is not as expected. - The
console.logstatements should be replaced with a proper logging mechanism.
| } else if(result.result === 'signout') { | ||
| alert('다시 로그인해 주세요.'); | ||
| signOut(); |
There was a problem hiding this comment.
Using alert for notifying the user is not recommended in a production environment as it is considered disruptive. Consider using a more user-friendly notification system that is integrated into your application's UI.
- alert('다시 로그인해 주세요.');
+ // Replace with a user-friendly notification systemCommitable 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.
| } else if(result.result === 'signout') { | |
| alert('다시 로그인해 주세요.'); | |
| signOut(); | |
| } else if(result.result === 'signout') { | |
| // Replace with a user-friendly notification system | |
| signOut(); |
| "use client"; | ||
|
|
||
| import React, { useState, useRef } from 'react' | ||
| import Image from 'next/image' | ||
| import axios from 'axios' | ||
| // 이미지 여러 개 올리기 하자... | ||
| const page = () => { | ||
| const [images, setImages] = useState<File[]>([]); | ||
| const [previews, setPreviews] = useState<string[]>([]); | ||
| const imgRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| const handleImageView = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const newImages = [...images]; | ||
| const newPreviews = [...previews]; | ||
| for (let i=0; i < e.target.files!.length; i++) { | ||
| if (newImages.length < 3) { | ||
| const file = e.target.files![i]; | ||
| newImages.push(file) | ||
| const reader = new FileReader(); | ||
| reader.onload = (e) => { | ||
| newPreviews.push(e.target!.result as string); | ||
| setPreviews(newPreviews); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
| } | ||
| } | ||
| setImages(newImages); | ||
| }; | ||
|
|
||
| const handleDeletePreview = (index: number) => { | ||
| const newImages = [...images]; | ||
| const newPreviews = [...previews]; | ||
| newImages.splice(index, 1); | ||
| newPreviews.splice(index, 1); | ||
| setImages(newImages); | ||
| setPreviews(newPreviews); | ||
| }; | ||
|
|
||
| const send = async() => { | ||
| const formData = new FormData(); | ||
| images.forEach((img) => { | ||
| if( img instanceof File && img.size > 0) { | ||
| formData.append('img', img) | ||
| }; | ||
| }); | ||
| const result = await axios.put( | ||
| '/api/diary', | ||
| formData, | ||
| { | ||
| headers: { | ||
| 'Content-Type':'multipart/form-data' | ||
| } | ||
| } | ||
| ); | ||
| console.log(result); | ||
| }; | ||
| return ( | ||
| <div> | ||
| { | ||
| previews?.map((prev, index) =>( | ||
| <div key={index}> | ||
| <Image | ||
| src={prev} | ||
| width={200} | ||
| height={300} | ||
| alt={`${prev}-${index}`} | ||
| /> | ||
| <button | ||
| onClick={() => handleDeletePreview(index)} | ||
| > | ||
| X | ||
| </button> | ||
| </div> | ||
| )) | ||
| } | ||
| <input | ||
| ref={imgRef} | ||
| type='file' | ||
| multiple | ||
| accept='image/*' | ||
| className='border' | ||
| onChange={(e) => handleImageView(e)} | ||
| /> | ||
| <button onClick={send}> | ||
| gogo | ||
| </button> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| export default page |
There was a problem hiding this comment.
The component for uploading and previewing images seems to be well-structured and functional. However, there are a few areas that could be improved for better performance and code quality:
- The
handleImageViewfunction currently reads all the selected files into memory to create previews. This could be a performance issue if the user selects very large images. Consider limiting the size of the images that can be uploaded or handling the previews in a way that does not require reading the entire file into memory. - The
sendfunction usesaxios.putto send the form data to the server. Ensure that the server endpoint at/api/diaryis expecting a PUT request with form data and that it is properly authenticated and authorized to handle image uploads. - The
sendfunction does not handle errors from theaxiosrequest. It would be good to add error handling to provide feedback to the user if the upload fails. - The
sendfunction logs the result to the console, which is fine for debugging but should be removed or replaced with a more user-friendly notification when the feature goes into production. - The
pagecomponent is not following the naming convention for React components, which should be capitalized. Consider renamingpagetoPageor something more descriptive. - The
use clientdirective at the beginning of the file is not standard. If it's meant to indicate that this component should only run in the client environment, consider using dynamic imports or other Next.js features to ensure this.
Here's a diff with some suggested changes:
- const page = () => {
+ const ImageUploadPreviewPage = () => {
...
- const send = async() => {
+ const sendImages = async() => {
try {
const formData = new FormData();
images.forEach((img) => {
if( img instanceof File && img.size > 0) {
formData.append('img', img)
};
});
const result = await axios.put(
'/api/diary',
formData,
{
headers: {
'Content-Type':'multipart/form-data'
}
}
);
// Handle success, e.g., show a notification to the user
} catch (error) {
// Handle error, e.g., show an error message to the user
}
};
...
- <button onClick={send}>
+ <button onClick={sendImages}>
gogo
</button>
...
- export default page
+ export default ImageUploadPreviewPageCommitable 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.
| "use client"; | |
| import React, { useState, useRef } from 'react' | |
| import Image from 'next/image' | |
| import axios from 'axios' | |
| // 이미지 여러 개 올리기 하자... | |
| const page = () => { | |
| const [images, setImages] = useState<File[]>([]); | |
| const [previews, setPreviews] = useState<string[]>([]); | |
| const imgRef = useRef<HTMLInputElement>(null); | |
| const handleImageView = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| const newImages = [...images]; | |
| const newPreviews = [...previews]; | |
| for (let i=0; i < e.target.files!.length; i++) { | |
| if (newImages.length < 3) { | |
| const file = e.target.files![i]; | |
| newImages.push(file) | |
| const reader = new FileReader(); | |
| reader.onload = (e) => { | |
| newPreviews.push(e.target!.result as string); | |
| setPreviews(newPreviews); | |
| }; | |
| reader.readAsDataURL(file); | |
| } | |
| } | |
| setImages(newImages); | |
| }; | |
| const handleDeletePreview = (index: number) => { | |
| const newImages = [...images]; | |
| const newPreviews = [...previews]; | |
| newImages.splice(index, 1); | |
| newPreviews.splice(index, 1); | |
| setImages(newImages); | |
| setPreviews(newPreviews); | |
| }; | |
| const send = async() => { | |
| const formData = new FormData(); | |
| images.forEach((img) => { | |
| if( img instanceof File && img.size > 0) { | |
| formData.append('img', img) | |
| }; | |
| }); | |
| const result = await axios.put( | |
| '/api/diary', | |
| formData, | |
| { | |
| headers: { | |
| 'Content-Type':'multipart/form-data' | |
| } | |
| } | |
| ); | |
| console.log(result); | |
| }; | |
| return ( | |
| <div> | |
| { | |
| previews?.map((prev, index) =>( | |
| <div key={index}> | |
| <Image | |
| src={prev} | |
| width={200} | |
| height={300} | |
| alt={`${prev}-${index}`} | |
| /> | |
| <button | |
| onClick={() => handleDeletePreview(index)} | |
| > | |
| X | |
| </button> | |
| </div> | |
| )) | |
| } | |
| <input | |
| ref={imgRef} | |
| type='file' | |
| multiple | |
| accept='image/*' | |
| className='border' | |
| onChange={(e) => handleImageView(e)} | |
| /> | |
| <button onClick={send}> | |
| gogo | |
| </button> | |
| </div> | |
| ); | |
| } | |
| export default page | |
| "use client"; | |
| import React, { useState, useRef } from 'react' | |
| import Image from 'next/image' | |
| import axios from 'axios' | |
| // 이미지 여러 개 올리기 하자... | |
| const ImageUploadPreviewPage = () => { | |
| const [images, setImages] = useState<File[]>([]); | |
| const [previews, setPreviews] = useState<string[]>([]); | |
| const imgRef = useRef<HTMLInputElement>(null); | |
| const handleImageView = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| const newImages = [...images]; | |
| const newPreviews = [...previews]; | |
| for (let i=0; i < e.target.files!.length; i++) { | |
| if (newImages.length < 3) { | |
| const file = e.target.files![i]; | |
| newImages.push(file) | |
| const reader = new FileReader(); | |
| reader.onload = (e) => { | |
| newPreviews.push(e.target!.result as string); | |
| setPreviews(newPreviews); | |
| }; | |
| reader.readAsDataURL(file); | |
| } | |
| } | |
| setImages(newImages); | |
| }; | |
| const handleDeletePreview = (index: number) => { | |
| const newImages = [...images]; | |
| newPreviews.splice(index, 1); | |
| setImages(newImages); | |
| setPreviews(newPreviews); | |
| }; | |
| const sendImages = async() => { | |
| try { | |
| const formData = new FormData(); | |
| images.forEach((img) => { | |
| if( img instanceof File && img.size > 0) { | |
| formData.append('img', img) | |
| }; | |
| }); | |
| const result = await axios.put( | |
| '/api/diary', | |
| formData, | |
| { | |
| headers: { | |
| 'Content-Type':'multipart/form-data' | |
| } | |
| } | |
| ); | |
| // Handle success, e.g., show a notification to the user | |
| } catch (error) { | |
| // Handle error, e.g., show an error message to the user | |
| } | |
| }; | |
| return ( | |
| <div> | |
| { | |
| previews?.map((prev, index) =>( | |
| <div key={index}> | |
| <Image | |
| src={prev} | |
| width={200} | |
| height={300} | |
| alt={`${prev}-${index}`} | |
| /> | |
| <button | |
| onClick={() => handleDeletePreview(index)} | |
| > | |
| X | |
| </button> | |
| </div> | |
| )) | |
| } | |
| <input | |
| ref={imgRef} | |
| type='file' | |
| multiple | |
| accept='image/*' | |
| className='border' | |
| onChange={(e) => handleImageView(e)} | |
| /> | |
| <button onClick={sendImages}> | |
| gogo | |
| </button> | |
| </div> | |
| ); | |
| } | |
| export default ImageUploadPreviewPage |
| 'use client' | ||
|
|
||
| import React, { useState, useRef } from 'react' | ||
| import Image from 'next/image' | ||
| import axios from 'axios' | ||
| import { useSession } from 'next-auth/react' | ||
| const page = () => { | ||
| const { data: session } = useSession(); | ||
| // 제목 | ||
| const titleRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| // 내용 | ||
| const contentRef = useRef<HTMLTextAreaElement>(null); | ||
|
|
||
| // 날씨 | ||
| const weatherRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| // 이미지 제목 | ||
| const imgTitRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| // 이미지 | ||
| const imgRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| // 이미지 주소 (미리보기 보여주기 위함) | ||
| const [imgUrl, setImgUrl] = useState(""); | ||
|
|
||
| const handleImageView = (e: React.ChangeEvent<{files: FileList | null}>) => { | ||
| if (e.target.files && e.target.files.length > 0) { | ||
| const file = e.target.files[0]; | ||
| URL.revokeObjectURL(imgUrl); | ||
| // 이전 미리보기를 지움. | ||
|
|
||
| setImgUrl(prev => URL.createObjectURL(file)); | ||
| } | ||
| } | ||
| const imgReset = () => { | ||
| // 미리보기 안 보이게. | ||
| if (imgRef.current) { | ||
| imgRef.current.value = ''; | ||
| URL.revokeObjectURL(imgUrl); | ||
| setImgUrl(prev => ''); | ||
| } | ||
| } | ||
| const send = async() => { | ||
| if(!session) return; | ||
| // 작성. | ||
| if(!titleRef.current) { | ||
| alert("제목 입력."); | ||
| return; | ||
| } | ||
| if(!contentRef.current) { | ||
| alert("내용 입력."); | ||
| return; | ||
| } | ||
| if(!weatherRef.current) { | ||
| alert('날씨 입력.'); | ||
| return; | ||
| } | ||
| const formData = new FormData(); | ||
| formData.append("title", titleRef.current.value); | ||
| formData.append("content", contentRef.current.value); | ||
| formData.append('id', session?.user?.id as string); | ||
| formData.append('name', session.user?.name as string); | ||
| formData.append('weather', weatherRef.current.value); | ||
| if(imgRef.current && imgRef.current.files && imgRef.current.files.length > 0 && imgTitRef.current) { | ||
| formData.append('img', imgRef.current.files[0]) | ||
| formData.append('imgTit', imgTitRef.current.value) | ||
| }; | ||
| const result = await axios.post( | ||
| "/api/diary", | ||
| formData, | ||
| { | ||
| headers: { | ||
| "Content-Type":"multipart/form-data", | ||
| "Authorization":`mlru ${session?.accessToken}` | ||
| } | ||
| } | ||
| ); | ||
| console.log(result.data); | ||
| imgReset(); | ||
| }; | ||
| return ( | ||
| <div> | ||
| 일기 작성 테스트<br /> | ||
|
|
||
| <input type='text' className='border' ref={titleRef} /><br /> | ||
| <br /> | ||
| <textarea ref={contentRef} name="diary" id="diary" className='border'></textarea> | ||
| <br /> | ||
| <input ref={weatherRef} type='text' className='border' /> | ||
| <br /> | ||
| <input ref={imgTitRef} type='text' className='border' /> | ||
| <br /> | ||
| <input ref={imgRef} type='file' className='border' onChange={(e) => handleImageView(e)}/> | ||
| <br /> | ||
| { | ||
| imgUrl && ( | ||
| <> | ||
| <Image | ||
| src={imgUrl} | ||
| alt='preview' | ||
| width={200} | ||
| height={300} | ||
| /> | ||
| <button onClick={imgReset} className='border'>지우기</button> | ||
| <br /> | ||
| </> | ||
| ) | ||
| } | ||
| <br /> | ||
| <button className='border' onClick={send}>gogogogogo</button> | ||
| </div> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The component seems well-structured and includes proper handling of image previews and form data submission. However, there are a few areas that could be improved:
- The use of
alertfor user notifications is not considered a best practice for user experience. Consider using a more user-friendly way to display messages, such as a modal or a toast notification. - The
sendfunction could be improved by adding error handling for theaxios.postcall. This would allow you to handle any errors that occur during the API request and provide feedback to the user. - The
Authorizationheader in theaxios.postcall is using a custom schememlru. Verify that this is the intended scheme and that the backend is configured to handle it. - The
imgResetfunction revokes the object URL but does not check ifimgUrlis a non-empty string before callingURL.revokeObjectURL(imgUrl). This could potentially lead to an unnecessary call ifimgUrlis already an empty string. - The
handleImageViewfunction creates a new object URL but does not handle the cleanup of this URL if the component unmounts before the user resets the image. Consider adding a cleanup effect usinguseEffectto revoke any created object URLs when the component unmounts. - The
sendfunction directly accesses the DOM elements to get their values. While this works, it's not the React way of handling form inputs. Consider using state hooks to manage form input values and changes. - The
sendfunction checks for the existence ofsessionbut does not handle the case wheresessionis null. This could lead to unexpected behavior if the session is not established. - The
sendfunction assumes thatsession.user.idandsession.user.nameare present. Ensure that these values are always available in the session object, or add checks to handle cases where they might be undefined.
Here's an example of how you might refactor the send function to include error handling:
const send = async () => {
if (!session) {
// Handle the case where there is no session
return;
}
// ... rest of the validation logic
try {
const result = await axios.post("/api/diary", formData, {
headers: {
"Content-Type": "multipart/form-data",
"Authorization": `Bearer ${session.accessToken}`, // Assuming Bearer token is used
},
});
console.log(result.data);
imgReset();
// Handle success (e.g., show success message to user)
} catch (error) {
console.error(error);
// Handle error (e.g., show error message to user)
}
};| const handleSubmit = async (e: React.MouseEvent<HTMLButtonElement, MouseEvent>) => { | ||
| e.preventDefault(); | ||
| if (idRef.current && pwRef.current && nameRef.current) { | ||
| const formData = new FormData(); | ||
| formData.append('user_id', idRef.current.value); | ||
| formData.append('password', pwRef.current.value); | ||
| formData.append('user_name', nameRef.current.value); | ||
| if (imgRef.current && imgRef.current.files && imgRef.current.files.length > 0) { | ||
| formData.append('img', imgRef.current.files[0]); | ||
| } | ||
| const result = await axios.put( | ||
| "/api/user", | ||
| formData, | ||
| { | ||
| headers: { | ||
| "Content-Type": "multipart/form-data" | ||
| } | ||
| } | ||
| ); | ||
| if (result.statusText === 'OK') { | ||
| console.log('가입에 성공했어요.'); | ||
| imgReset(); | ||
| } else { | ||
| console.log(result.data.result) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
-
try { const result = await axios.put( "/api/user", formData, { headers: { "Content-Type": "multipart/form-data" } } ); if (result.statusText === 'OK') { console.log('가입에 성공했어요.'); imgReset(); } else { console.log(result.data.result) } -
} catch (error) { -
console.error('Registration failed:', error); -
// Handle the error appropriately here -
}
-export default page
+export default Page
<!-- This is an auto-generated comment by CodeRabbit -->
Issues 번호 :
Closes #50
변경, 추가된 코드(설명 등)
코드 주의점
Summary by CodeRabbit
New Features
useSessionandRecoilstate management.RefreshTokencomponent to handle token refresh every 30 minutes.textStateatom for state management with Recoil.Enhancements
Bug Fixes
Style
Documentation
Refactor
useRefimport in the login component for better code organization.Chores
Revert
NavBarimport from the write page component.