-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[조형민] sprint7 #5
The head ref may contain hidden characters: "express-\uC870\uD615\uBBFC-sprint7"
[조형민] sprint7 #5
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
형민님 고생많으셨습니다!
전반적으로 깔끔하게 작성해주신 것 같습니다!
주석에는 이후에 작업할 내용들인 것 같아서 따로 코멘트는 안했습니다 ㅎㅎ
추가로 express 작업을 하니, route 파일로 분리하는 것도 한번 시도해보셔도 좋을듯 합니다!
product.route.js 이런식으로요 ㅎㅎ controller, repository도 같이 봐보시면 좋을듯 해요
수고하셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기는 node.js 쪽이라 react 관련된 lint 는 필요없어서 해당 부분 지워주시면 좋을것 같습니다 ㅎㅎ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다음시간에 환경변수 관련되서 말씀드리겠지만, 보통 .env는 github에 올리지는 않고 따로 관리를 합니다 ㅎㅎ
@@ -0,0 +1,23 @@ | |||
-- CreateTable | |||
CREATE TABLE "Product" ( | |||
"id" TEXT NOT NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보통 mysql이나 다른 db에서는 TEXT가 아닌 VARCHAR를 쓰게끔 하는데요
postgresql을 사용하셔서 TEXT를 사용하셔도 성능상 이슈는 없지만, 보통 길이가 엄청 길지 않는 이상 (product.description 등) VARCHAR을 사용하긴 합니다 ㅎㅎ
const comment = await prisma.comment.create({ data: req.body }); | ||
|
||
// 해당 게시글에 댓글을 연결 | ||
const { id: commentId } = comment; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 이거 깔끔하게 잘 빼주셧네요 ㅎㅎ
assert(req.body, PatchComment); | ||
const { id } = req.params; | ||
const comment = await prisma.comment.update({ where: { id }, data: { ...req.body } }); | ||
res.send(comment); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 통일성 있게 res.status(200).send(comment)로 해주는게 더 좋을듯 합니다.
app.patch( | ||
'/comments/:id', | ||
asyncHandler(async (req, res) => { | ||
assert(req.body, PatchComment); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오.. 이런 assert 로 validation 추가해주시다니 좋네요 ㅎㅎ
const nextCursor = lastComment.id; | ||
const isLastPage = limit > comments.length; | ||
// 최종적으로 cursor와 comments 전달 | ||
const finalData = isLastPage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이런 로직은 위에도 중복코드가 있어서 따로 함수를 빼서 범용적으로 사용할수 있게 하면 더 좋을듯 합니다~
요구사항
기본 요구사항
중고마켓
공통
자유게시판
댓글
심화 요구사항
주요 변경사항
스크린샷
멘토에게