-
Notifications
You must be signed in to change notification settings - Fork 1
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
user-crud 생성 #5
user-crud 생성 #5
Conversation
현광이처럼 멋진 코드 잘 봤습니다 review + 1 |
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.
고생했어! 리뷰가 좀 많네 🤣
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.
Controller, Service에 중복되는 코드들이 많이 보이는데 Middleware를 하나 추가하면 좋을 것 같아.
공통적으로 줄일 수 있는 것들
- req.body로 받아온 deviceID로 User Collection 조회해서 있는 유저인지 => 있으면 user / 없으면 next()에 에러 던지기
- req.body로 받아온 memeID로 Meme Collection 조회해서 있는 밈인지 => 있으면 meme / 없으면 next()에 에러 던지기
이렇게 하게되면 Controller, Service 단까지 에러 처리를 신경쓰지않아도 돼
Meme은 내가 이전 PR에서 작업해두어서 코드 참고하면 좋을 것 같아!
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.
확인 :) 고생했어요!
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.
어떤건 boolean, 어떤건 Meme을 반환하도록 되어있어서, 반환 형식을 정하고 통일해야할 것 같아
서버 회의때 논의해보자
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.
이거 약간 사용하는 화면에 따라 달라질 것 같아서 고민 좀 많이 해봐야할 것 같드라.. 일단 나는 바로 화면에 영향 없는 것들은 boolean으로 주고 아닌 것들은 바뀐 값들 return 한 것 같아.
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.
굿굿. 토욜에 논의하고 boolean으로 가거나, 아니면 다 통일하거나 합시다!
Co-authored-by: Seohyun Yoon <[email protected]>
Co-authored-by: Seohyun Yoon <[email protected]>
Co-authored-by: Seohyun Yoon <[email protected]>
Co-authored-by: Seohyun Yoon <[email protected]>
…nto feature/user-crud
유저 관련 crud 생성했어용