Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[feat] post 상세보기 api #27
[feat] post 상세보기 api #27
Changes from 2 commits
8f54ea4
3fc8e68
6dda02c
9d8b0a0
53480f8
e3d0a76
6fb0355
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
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.
Post라고 한정 짓는거 보단 NotFoundException으로 클래스명을 변경하고 전역에서 사용하는게 더 좋을 것 같습니당
여기도 EOL이 안지켜졌어요~!
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.
contentService라고 되어있네요?~
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.
record 클래스에 대해 찾아보시고 적용하시면 더 깔끔할 것 같습니당
그리고 DatailResponse보단 PostDetailResponse가 ㅇ어떤 엔티티에 대한 Response인지 더 명확할 것 같습니다!
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.
JPA 변경감지 기능을 사용하시면 따로 save하지 않아도 괜찮습니다~
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.
service 레이어에서 Response를 반환해주는것 보다 Controller 레이어에서 DetailResponse에게 post 객체를 넘겨주고 내부적으로 처리하는게 더 맞는것 같아요 ~!
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.
repository 테스트랑 service 테스트랑 다른게 없는 것 같아요 service 테스트만 있어도 될 것 같습니다~!
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.
옴.. 테스트는 촘촘하면 좋지만 repository 테스트는 일반적으로 작성하지 않아서 드린 말씀이었어요 !!
service 테스트만으로도 충분한 것 같아서 드린 말씀이었습니다 ㅎㅎ
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.
저는 필수적이진 않지만 repository도 테스트를 작성하는게 좋다고 생각합니다. 그런데 이렇게 단순한 로직의 경우는 필요 없을 수도 있겠네요!