[BE4] 강성욱 #6
Open
[BE4] 강성욱 #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 답변 생성 - 답변 조회
- 질문 생성 - 질문 및 답변 조회
… 해결. entityGraph로 명시적으로 모두 가져오게 변경.
1. 미인증 사용자 조회 시, 댓글 적기 disable 2. 댓글 작성자 표기
- 일회용으로, 로그인 후 삭제됨
farrar142
reviewed
Dec 24, 2024
Comment on lines
68
to
94
| public Integer modifyAnswer(Integer answerId, String loginUsername, String newContent) { | ||
| Answer findData = answerRepository.findByIdWithQuestion(answerId).orElseThrow( | ||
| () -> new DataNotFoundException("Answer not found")); | ||
|
|
||
| answerRepository.save(Answer.modifyAnswer(findData, newContent)); | ||
| return findData.getQuestion().getId(); | ||
| } | ||
|
|
||
| /** | ||
| * | ||
| * @param answerId | ||
| * @param loginUsername | ||
| * @return 연관된 Question ID 반환 | ||
| */ | ||
| @Transactional | ||
| @Override | ||
| public Integer deleteAnswer(Integer answerId, String loginUsername) { | ||
| Answer findData = answerRepository.findByIdWithQuestion(answerId).orElseThrow( | ||
| () -> new DataNotFoundException("answer not found")); | ||
|
|
||
| if (!findData.getSiteUser().getUsername().equals(loginUsername)) { | ||
| throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "삭제권한이 없습니다."); | ||
| } | ||
| Integer questionId = findData.getQuestion().getId(); | ||
| answerRepository.deleteById(answerId); | ||
| return questionId; | ||
| } |
There was a problem hiding this comment.
AnswerService와 QuestionService에서도 똑같이, modify와 delete에서 의 코드 스타일이 통일되지 않았네요,
하나는 함수 내부에서 권한체크를 하는데 한쪽은 아예 권한체크를 하지 않네요,
There was a problem hiding this comment.
권한체크를 어디서 해야될지 고민되어 통일이 안된 것 같습니다.
controller 혹은, service 로직상에서 권한 체크를 하도록 혼용되어 있거나 누락 되었는데, service로 통합되어 처리하였습니다.
감사합니다~!
farrar142
reviewed
Dec 24, 2024
Comment on lines
+37
to
+40
| private final AnswerRepository answerRepository; | ||
| private final UserRepository userRepository; | ||
| private final CommentRepository commentRepository; | ||
| private final CategoryRepository categoryRepository; |
There was a problem hiding this comment.
각 도메인에 대한 서비스가 아닌 리포지터리를 바로 불러온것은 이유가 있나요?
There was a problem hiding this comment.
@farrar142
서비스는 도메인에 종속적이다 라고 생각하고 있습니다.
만약, 서비스가 타 도메인의 서비스를 참조하게 된다면, 서비스간 강한 결합이 생겨 유지 보수가 더 어려워 진다고 생각해서 이렇게 진행하였습니다.
만약, 도메인 간 협력이 필요한 상황이 생긴다면, 레이어를 하나 추가할 것 같지만, 아직 그정도 수준의 개발은 되지 않았습니다.
이점은, 저도 계속 생각이 바뀌는 부분인데, 어떻게 생각하시는지 궁금합니다!
- controller 에서 삭제 권한 확인, service 에서 수정 권한 확인 되어있음. - service 에서 통합 관리 하도록 수정.
- 질문 수정 관련 권한 체크 없음 문제 수정
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.