-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 마이페이지 강사 정보 관련 api #8
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
Test Results27 files 27 suites 4s ⏱️ Results for commit 144cfaf. ♻️ This comment has been updated with latest results. |
🌻Test Coverage Report
|
Ho-Tea
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.
사소한 코멘트 몇개 남겼는데.. 이부분은 나중에 말해도 좋을 것 같아 우선 approve 할게욥
| List<@Size(max = 30) String> prizes, | ||
| @Size(min = 1) List<String> videoUrls | ||
| ) { | ||
| public TeacherUpdateCommand toCommand(long memberId) { |
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에서 인자로 넘겨줄때는 Long으로 넘겨주는데 여기서는 long으로 받는 의도가 있을까욥
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.
argumentresolver에서 null 넘어올 수 있어서 controller는 Long으로 받고 여기는 long으로 받도록 했는데
처리를 안해주면 당연히 npe나겠군요.. 앞으로 어떻게 통일하면 좋을지 조금 더 찾아볼게용
|
|
||
| public interface TeacherVideoRepository { | ||
| List<String> findAllByTeacherId(long teacherId); | ||
| void replace(long id, List<String> strings); |
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가 의미하는게 테이블의 프라이머리 키가아닌 teacher Id라 teacherId 변수명이 좋을 것 같아욥!
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.
반영했슴당
Related Issue 📌
close #7
Description ✔️
To Reviewers
테스트에 fixture 만들기 위해 도메인 객체와 repository를 사용하는 부분들이 있는데
이를 직접 jpa entity와 jparepository를 사용하도록 통일하는 작업이 필요할 것 같습니다.
이번 pr은 일단 빨리 머지해야 돼서 TeacherServiceTest에 repository, jparepository 의존성들 모두 넣어 놓을게요