-
Notifications
You must be signed in to change notification settings - Fork 0
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
[PR][#30] Internal API module 추가 #33
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.
👍
}); | ||
|
||
if (!user) { | ||
throw new NotFoundException(`User with ID: ${userId} not Found.`); |
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.
저는 개인적으로 이런 케이스에 500 내지는 400을 사용하는 것을 선호하긴 하는데요, 다른 분들 의견도 궁금하네요. (404는 endpoint가 없으면 띄우는 편)
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.
prisma 공식 문서에서 탐색할 레코드가 없을 때 NotFoundError를 반환함을 명시하고 있습니다만.. 저도 무슨 코드를 사용해야할지는 잘 모르겠네요. @redjen8 @ammerss
https://www.prisma.io/docs/reference/api-reference/prisma-client-reference#finduniqueorthrow
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.
저건 prisma의 NotFoundError 라서, 저희가 핸들링 하면 될 것 같고, 핸들링 과정에서 500으로 내릴지 400 혹은 404 내릴지만 정해보면 될 것 같습니다.
개인적으로는 유저의 요청 자체가 잘못된 경우 (유저 정보를 찾아오는 요청인데 유저가 없을 경우) 400이
맞다고 생각하고, 요청은 괜찮은데 저희 내부 로직 처리 중간 결과로 DB에서 조회를 하는데 정보가 없는 경우 (유저가 쓴 공고의 내용을 가져오려 하는데, 공고의 id를 조회했으나 정합성에 문제가 생겨 실제로 공고가 없는 경우) 500이 맞가고 생각합니다.
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.
유저 요청이 부정확한 경우(id에 number 형태가 들어오지 않은 경우) 400, db에서 유저 데이터가 없을 경우 500을 반환하도록 수정하겠습니다.
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.
400 -> userId 자체가 유효하지 않은 경우 (ex, 100자 혹은 2자, format이 맞지 않는 다던가)
404 -> 해당 userId의 사용자가 존재하지 않는 경우
500 -> 서버 아파요
요렇게 분리하면 좀 더 명확한 의사 소통이 가능할 것 같은데, 다른 분들 의견도 궁금합니다~
const {id, manners, intro, profileURL}: GetUserResponseDto = user; | ||
responseUsers.push({ id, manners, intro, profileURL }); | ||
const responseUsers = users.map((user) => { | ||
const { id, manners, intro, profileURL }: GetUserResponseDto = user; |
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.
조금 더 응집도를 높인다고 가정을 하면,
도메인으로부터 Response DTO를 생성하는 static factory method를 사용해볼 수도 있을 것 같아요.
하지만 지금 도메인이 엄격히 분리돼 있지 않은 상태이니, 이대로도 좋습니다.
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.
이견 없습니다, 다만 response status 관련하여 comment 드린 내용은 확실하게 정하고 갔으면 좋겠습니다!
status 관련한 사항은 이번 회의 discuss 파트에 달아놓겠습니다. |
서버 간 통신을 위한 내부 API를 추가합니다.
infra [ path : 'internal/api/v1' ]
ㄴ internal-user [ path : 'user' ]
req: http://localhost:3000/api/v1/user
res:
req: http://localhost:3000/internal/api/v1/user
res:
기타 추가 수정 사항
User 모듈에서 선언적 함수를 사용하도록 리팩토링 했습니다. (push -> map)
User DB 내의 leg3nd id (universalAccountId) 필드에 @ unique 속성을 추가했습니다.
User module 내 사용하지 않는 테스트 파일을 삭제했습니다.
User 요청이 잘 못 들어온 경우 코드 400, DB에 데이터가 없는 경우 (기존 404) -> 500을 반환합니다.