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

user-crud 생성 #5

Merged
merged 9 commits into from
Jun 20, 2024
Merged

user-crud 생성 #5

merged 9 commits into from
Jun 20, 2024

Conversation

Hyun-git
Copy link
Member

유저 관련 crud 생성했어용

@Hyun-git Hyun-git changed the base branch from main to develop June 17, 2024 09:27
@Hyun-git Hyun-git requested a review from pjw5521 June 17, 2024 09:28
@Hyun-git Hyun-git self-assigned this Jun 17, 2024
@Hyun-git Hyun-git requested a review from seohyun0120 June 17, 2024 09:28
@pjw5521
Copy link
Contributor

pjw5521 commented Jun 18, 2024

현광이처럼 멋진 코드 잘 봤습니다 review + 1

Copy link
Collaborator

@seohyun0120 seohyun0120 left a comment

Choose a reason for hiding this comment

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

고생했어! 리뷰가 좀 많네 🤣

src/controller/user.controller.ts Outdated Show resolved Hide resolved
src/model/user.ts Outdated Show resolved Hide resolved
src/service/user.service.ts Outdated Show resolved Hide resolved
src/service/user.service.ts Outdated Show resolved Hide resolved
src/service/user.service.ts Outdated Show resolved Hide resolved
src/service/user.service.ts Outdated Show resolved Hide resolved
src/service/user.service.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

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에서 작업해두어서 코드 참고하면 좋을 것 같아!

Copy link
Collaborator

Choose a reason for hiding this comment

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

확인 :) 고생했어요!

Copy link
Collaborator

Choose a reason for hiding this comment

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

어떤건 boolean, 어떤건 Meme을 반환하도록 되어있어서, 반환 형식을 정하고 통일해야할 것 같아
서버 회의때 논의해보자

Copy link
Member Author

Choose a reason for hiding this comment

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

이거 약간 사용하는 화면에 따라 달라질 것 같아서 고민 좀 많이 해봐야할 것 같드라.. 일단 나는 바로 화면에 영향 없는 것들은 boolean으로 주고 아닌 것들은 바뀐 값들 return 한 것 같아.

Copy link
Collaborator

Choose a reason for hiding this comment

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

굿굿. 토욜에 논의하고 boolean으로 가거나, 아니면 다 통일하거나 합시다!

src/service/user.service.ts Outdated Show resolved Hide resolved
@Hyun-git Hyun-git requested a review from seohyun0120 June 19, 2024 00:32
@Hyun-git Hyun-git merged commit 903ce6b into develop Jun 20, 2024
1 check passed
@seohyun0120 seohyun0120 deleted the feature/user-crud branch June 23, 2024 08:37
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.

3 participants