-
Notifications
You must be signed in to change notification settings - Fork 4
CLAP-76 담당자 조회 api 구현 #83
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
Conversation
|
@joowojr 담당자 조회 api pull request 했습니다! 확인해주시면 감사하겠습니다! |
|
커밋컨벤션 지켜서 커밋올려주시면 좋을 것 같습니다. 추가로 중복된 커밋내용은 squash로 합친 후 적절한 커밋명으로 수정하여 업데이트 해주시면 좋을 것 같습니다~ |
| @GetMapping | ||
| public List<FindManagersResponse> findManagers() { | ||
| return findManagersUsecase.execute(); | ||
|
|
||
| List<FindManagersResponse> managers = findManagersUsecase.execute(); | ||
|
|
||
| if (managers.isEmpty()) { | ||
| return FindManagersResponse.emptyListResponse(); | ||
| } | ||
|
|
||
| return managers; |
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.
http 상태코드와 함께 반환해주시면 좋을 것 같습니다.
예시) ResponseEntity.ok<managers>
| spring: | ||
| jpa: | ||
| hibernate: | ||
| ddl-auto: none | ||
| config: |
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.
혹시 ddl-auto 설정 아예 지우신 걸까요??
joowojr
left a 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.
수고하셨습니다!
코드 컨벤션이랑 안맞는 부분들이 좀 있어서, 노션 문서 꼼꼼히 보시고 다시 수정 부탁드립니다:)
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.
dto의 자료형은 불변성 보장을 위해 record로 수정 부탁드립니다!
| private final FindManagersUsecase findManagersUsecase; | ||
|
|
||
| @GetMapping | ||
| public List<FindManagersResponse> findManagers() { |
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.
ResponseEntity로 감싸서 보내주셔야합니다!
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.
uscase의 구현체는 application/service 하위로 이동 부탁드리고 클래스명도 수정 부탁드립니다!
| List<Member> managers = memberService.findActiveManagers(); | ||
|
|
||
| if (managers.isEmpty()) { | ||
| return FindManagersResponse.emptyListResponse(); // 빈 리스트 반환 |
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.
managers.isEmpty()일 때 빈 리스트를 반환하기 보다는, managers를 FindManagersResponse로 매핑을 먼저 하여 반환해야 할것 같습니다.
그러면 managers가 empty여도 FindManagersResponse도 empty로 반환 됩니다!
| return FindManagersResponse.emptyListResponse(); // 빈 리스트 반환 | ||
| } | ||
|
|
||
| return managers.stream().map(manager -> { |
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.
매핑 부분은 mapper class로 옮겨서 부탁드립니다!
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.
@joowojr 넵 수정했습니다 확인해주시면 감사하겠습니다!
- 담당자 조회 API를 구현하였으며, 매니저의 정보를 조회할 수 있도록 처리 - 필요한 데이터는 담당자의 닉네임, 이미지 URL, 남은 작업 개수 등을 포함 <footer> - 관련: #35
- http 상태코드 반환 - dto의 자료형 record로 수정 - ResponseEntity로 감싸서 반환 - usecase의 구현체를 application/service 하위로 이동, 클래스명 수정 - managers를 FindManagersResponse로 매핑을 먼저 하여 빈 리스트를 반환 - 매핑 부분은 mapper class 이동(manager mapper 생성) <footer> - 관련: #35
2df8728 to
0633d0d
Compare
33446f1 to
f28650d
Compare
📄 요약(Summary)
✍🏼 상세(More)
PR Desciption
Requirements for Reviewer
PR Log
새롭게 배운 것
고민 중인 사항
첨부 자료
Requirements for Reviewer
✅ 체크리스트(Checklist)
🚪 이슈 번호(Issue numbers)
Closes #35