Skip to content
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

[6주차] 기본 과제 제출 #5

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[6주차] 기본 과제 제출 #5

wants to merge 3 commits into from

Conversation

onpyeong
Copy link
Contributor

@onpyeong onpyeong commented Nov 24, 2022

SERVER PR


🐕 과제 구현 명세

  • [기본과제]: 세미나 코드를 정리하고, api 리팩토링을 진행했습니다.

🐥 이런 점이 새로웠어요 / 어려웠어요

  • express-validator를 이용해서 사용자가 보낸 데이터를 검증할 수 있다는 점이 신기했습니다.
  • error처리 관련해서 고민이 된 것이, user 삭제나 수정 등 존재하지 않는 userId를 보낸 경우,
    BAD_REQUEST가 맞는지, INTERNAL_SERVER_ERROR가 맞는지 궁금하고, 이 2가지 에러가 따로 구분되는 건지도 궁금합니다!!
    그리고 Controller에서는 userId가 있는지 없는지 알 수 없으므로 Service에서 판단을 해야할 것 같은데,
    update나 delete를 수행하면 error가 바로 던져지니까 이 error를 다시 Controller에서 catch해서
    어떻게 분기처리를 해야하는지 궁금해요!


await userService.deleteUser(+userId);
return res.status(200).json({ status: 200, message: "유저 삭제 성공"});
if (!userId) return res.status(sc.NOT_FOUND).send(fail(sc.NOT_FOUND, rm.NOT_FOUND));
Copy link
Member

Choose a reason for hiding this comment

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

잘 작성해주셨는데 PR에 작성해주신 코멘트에 대한 답변을 여기에 달아보자면 !
path 내에서 userId가 존재하지 않는다면 404 Not Found 를, userId가 db 내에 존재하지 않는 자원이라면 400 Bad Request 처리를 해주는 것이 좋아보입니다!
404 Not Found는 서버가 요청받은 리소스를 찾을 수 없다는 것을 의미인데 여기서 ‘리소스’는 DB 자원보다 API URI 요청 경로를 의미하는 바가 더 큰 것 같더라구요!
Bad Request를 던져주게 된다면 디테일한 메세지를 전달해주는 것이 중요해보입니다:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 제가 궁금한 부분에 대해 정확한 답변을 해주셨네요! 역시 최고👍

//* 유저 전체 조회
const getAllUser = async () => {
const data = await prisma.user.findMany();
const data = await prisma.user_Seminar.findMany();
Copy link
Member

Choose a reason for hiding this comment

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

이 코드 다음에 userId가 없을 경우에 대한 처리를 해주면 될거 같다는 생각을 했는데 수현님의 의견은 어떠신지 궁금합니다:)
예를 들면 이렇게 작성해볼 수 있을 것 같아요 !

if(!data) {
    return null
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

service 객체에서 하면 되겠네요! 감사합니다!

Copy link
Member

@m1njae m1njae left a comment

Choose a reason for hiding this comment

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

과제하느라 고생 많이 하셨습니다 ! 코드리뷰가 늦어진 점 양해 부탁드려요😢 세미나 마지막까지 화이팅해보도록 해요!!💪

body("password").notEmpty().isLength({ min: 6 }),
],
userController.signInUser
);

//* 전체 유저 조회 - GET api/user
router.get('/', userController.getAllUser);
Copy link
Member

Choose a reason for hiding this comment

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

서비스마다 다르겠지만 유저 정보 업데이트나 유저 삭제 시에는 auth를 미들웨어에 작성해주는게 좋을거 같다는 생각이 들었어요! 수현님의 생각은 어떠신지 궁금합니다😊

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 유저와 관련된 건 auth로 한 번 걸러주는 게 맞는거 같네요! 굿굿

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants