Skip to content

Code Review PR#14

Open
ToKyun02 wants to merge 5 commits intocode/reviewfrom
main
Open

Code Review PR#14
ToKyun02 wants to merge 5 commits intocode/reviewfrom
main

Conversation

@ToKyun02
Copy link
Owner

@ToKyun02 ToKyun02 commented Mar 2, 2025

✍️ PR 내용 설명

Code Review에 관한 PR입니다.

  1. /post 페이지에서 게시글 생성 및 페이지네이션 기능 구현했습니다.
  2. ORM은 Prisma 사용하였습니다.
  3. 로그인 및 회원가입도 기능 구현하였고, 소셜 로그인도 구현했습니다.
  4. /map 페이지에서 카카오맵 API를 구현하긴 했지만, API 이용약관에서 DB에 데이터 저장을 허용해주지 않기에, 제외시킬 예정입니다.

✅ 체크리스트

PR

  • Branch Convention 확인
  • Base Branch 확인
  • 적절한 Label 지정
  • 이슈 체크리스트 작성 완료

Test

  • 로컬 작동 확인

Additional Notes

  • (없음)

ToKyun02 and others added 5 commits February 25, 2025 19:20
* ✨ feat : db 연동

* ✨ feat : 회원가입 기능 구현

* ✨ feat : 로그인 기능 구현

* ✨ feat : 소셜 로그인

* 🐛 fix : Prisma Client Vercel 캐싱 오류 해결
* ✨ feat : kakao map 생성

* ✨ feat : 카카오맵 키워드 별 마커 생성

* ✨ feat : 키워드 검색 기능 구현

* ✨ 카카오맵 키워드 별 리스트 구현
* ♻️ refactor : 유저 생성 Dao 작성

* ✨ feat : 게시판 관련 model 추가 및 게시글 리스트 조회, 생성 구현

* ✨ feat : 글 생성 모달 디자인

* ✨ feat : 게시글 추가, 리스트 조회 구현

* ✨ feat : 게시판 페이지네이션 기능
* ✨ feat : 토스트 컴포넌트 구현

* ♻️ refactor : 게시글 연속 중복 생성 방지 및 로딩 UI 추가

* ✨ feat : pagination skeleton UI
@ToKyun02 ToKyun02 added the 📝 Docs 문서 작업 label Mar 2, 2025
@ToKyun02 ToKyun02 self-assigned this Mar 2, 2025
@vercel
Copy link

vercel bot commented Mar 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
local-commu ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 2, 2025 9:18am

Copy link

@Lanace Lanace left a comment

Choose a reason for hiding this comment

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

생각보다 많아져서 한번 끊어서 리뷰할게여...ㅠㅠㅠ

근데 깔끔하게 잘 짜고계신데요ㅋㅋㅋ?

Comment on lines +37 to +43
- Next.js(v15), React(v19), App Router
- Next-Auth V5
- Prisma (NeonDB로 이용합니다.)
- Tanstack Query
- TypeScript
- Style : Tailwind CSS
- UI Lib : Shadcn
Copy link

Choose a reason for hiding this comment

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

일반적으로 많이 사용하는거 잘 고르셨네여ㅎㅎ

특히 Shadcn 은 저도 안써보긴 했는데 요즘 엄청 뜨고있더라구요

images: {
remotePatterns: [
{
protocol: 'http',
Copy link

Choose a reason for hiding this comment

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

kakaocdn 은 http 로 들어오나보네요...?ㄸ

Copy link
Owner Author

Choose a reason for hiding this comment

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

imageUrl 응답 확인해보니 프로토콜이 http이더라고욤..

Comment on lines +17 to +21
name String?
email String? @unique
emailVerified DateTime? @map("email_verified")
image String?
password String?
Copy link

Choose a reason for hiding this comment

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

name이나 email이나 password 가 optional 인건 이유가 있을까요??

Copy link

Choose a reason for hiding this comment

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

아 일단 email에 unique인데 optional인건 잘 이해가 안가서요ㅠ

Copy link
Owner Author

Choose a reason for hiding this comment

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

name, email의 경우 optional하지 않아도 될 것 같네요..
next auth를 처음 써봤는데 스키마 작성 예시를 그대로 가져와서 사용했어서 이렇게 된 것 같슴다.. 수정하겠슴다!

password의 경우 oauth일 때 비밀번호 안받아서 optional로 해놨슴다!

Comment on lines +40 to +41
refresh_token String? @db.Text
access_token String? @db.Text
Copy link

Choose a reason for hiding this comment

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

refresh_token, access_token, id_token 같은 토큰값을 보통 DB에 저장하지 않고 사용하는데, 따로 저장하신 이유가 있을까요?

아 그리구 타입도 보통은 토큰 길이는 정해져있어서 Text보단 String(255) 같이 사용하는게 좋긴 해요ㅎㅎ

Copy link
Owner Author

Choose a reason for hiding this comment

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

무의식적으로 활용 안하면서 DB에 넣은 것 같습니다...
말씀 주신 의견 참고해서 수정하겠습니다!

String알려주신 것도 참고하겠습니다!

Comment on lines +84 to +85
parentId String?
parent Comment? @relation("CommentToComment", fields: [parentId], references: [id], onDelete: SetNull)
Copy link

Choose a reason for hiding this comment

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

DB 컨벤션도 맞추면 좋을것같아요!

위쪽에서는 expires_at나 session_state 처럼 _ 를 사용하다가 카멜케이스로 쓰길래여...

보통 DB에서는 스네이크 케이스를 쓰고 js에선 카멜케이스를 쓰니 @@Map을 활용하시면 좋을것같네요ㅎㅎ

Copy link
Owner Author

Choose a reason for hiding this comment

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

컨벤션 맞추겠슴다!


return (
<div>
{JSON.stringify(session)}
Copy link

Choose a reason for hiding this comment

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

테스트 코드일텐데, 가급적이면 삭제하고, 나중을 위해서 // TODO: 같은 주석을 추가해두시면 좋을것같아요!

간혹 이런 코드가 production에 나가서 곤란한 경우가 있거든요ㅠㅠㅠ
경험담이라...하ㅠㅠㅠ

Copy link
Owner Author

Choose a reason for hiding this comment

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

앗 TODO 주석을 깜빡했네요...

말씀 감사합니다!

Comment on lines +9 to +10
const page = parseInt(searchParams.get('page') || '1');
const limit = parseInt(searchParams.get('limit') || '10');
Copy link

Choose a reason for hiding this comment

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

저라면...

    const page = parseInt(searchParams.get('page') || '1');

    const page = parseInt(searchParams.get('page')) || 1;

로 했을것같긴 한데,
image

보니까 NaN이 나올 수 있을것같아서요...!ㅎㅎ

NaN || 1 은 1이니까 위와같이 예외처리하면 좀더 안전할것같네여~

Copy link
Owner Author

Choose a reason for hiding this comment

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

음... 저는 NaN || '1' 의 결과값도 '1'로 나오니 괜찮다고 생각했었는데, 말씀 주신 의견 방식이 더 좋은 것 같습니다!
적용해보겠습니다.

},
});

return NextResponse.json(post, { status: 201 });
Copy link

Choose a reason for hiding this comment

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

상태코드 잘 처리하셨네요!

const user = await UserDao.getUserByEmail(email);
if (!user || !user.password) return null;

const passwordsMatch = await bcrypt.compare(password, user.password);
Copy link

Choose a reason for hiding this comment

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

굳굳 👍

Comment on lines +28 to +31
const urlError =
searchParams.get('error') === 'OAuthAccountNotLinked'
? '다른 소셜 계정에서 사용된 이메일입니다.'
: '';
Copy link

Choose a reason for hiding this comment

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

이런 에러코드는 dictionary 를 만들어서 처리하면 좋아요ㅎㅎ

나중에 error code가 많이 추가될 수 있으니까요...!

const errorParamsMap: Record<string, string> = {
  OAuthAccountNotLinked: "다른 소셜 계정에서 사용된 이메일입니다",
  ...: ...
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

감사합니다! error Code에 관한 확장성을 고려하지 않았네요... 말씀 주신 의견 반영하겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📝 Docs 문서 작업

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants