Feat/#224 executive report#225
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive management system for reported posts, providing separate functionalities for administrators and executives. Key changes include the introduction of new controllers and services for handling reported post lists, details, activation, and deletion, alongside a transition to standardized DTO naming and centralized business exception handling. The review feedback highlights opportunities to optimize database performance by removing redundant queries in the service layer and improving the reliability of report-fetching logic in the repository. Additionally, it is recommended to ensure consistent state validation during post activation and to strictly adhere to the project's pattern of centralizing ownership validation within the dedicated validator component.
| ON r.reportedAt = ( | ||
| SELECT MAX(r2.reportedAt) | ||
| FROM Report r2 | ||
| WHERE r2.postId = p.id | ||
| ) |
There was a problem hiding this comment.
There was a problem hiding this comment.
select 절에서 id를 가져오고
order 절에서 id를 내림차순 정렬하는 방식으로 수정
| // 이미 활성 상태인지 체크하는 비즈니스 로직 예시 | ||
| if (post.getActiveStatus() == ActiveStatus.ACTIVE) { | ||
| throw new BusinessException(ErrorCode.ALREADY_ACTIVE_POST); | ||
| } |
There was a problem hiding this comment.
The check post.getActiveStatus() == ActiveStatus.ACTIVE is inconsistent with the logic in post.activate(), which requires the post to be in DELETED status. It is better to validate that the post is indeed deleted before attempting to activate it, utilizing the newly added ErrorCode.CANNOT_ACTIVATE_DELETED_POST for a more accurate business exception.
if (post.getActiveStatus() != ActiveStatus.DELETED) {
throw new BusinessException(ErrorCode.CANNOT_ACTIVATE_DELETED_POST);
}There was a problem hiding this comment.
추후 에러코드 추가하면 좋을 듯합니다.
| // 이미 활성 상태인지 체크하는 비즈니스 로직 예시 | ||
| if (post.getActiveStatus() == ActiveStatus.ACTIVE) { | ||
| throw new BusinessException(ErrorCode.ALREADY_ACTIVE_POST); | ||
| } |
There was a problem hiding this comment.
The status check should ensure the post is in a DELETED state before activation to remain consistent with the entity's internal state transition logic and to provide the correct error response.
if (post.getActiveStatus() != ActiveStatus.DELETED) {
throw new BusinessException(ErrorCode.CANNOT_ACTIVATE_DELETED_POST);
}There was a problem hiding this comment.
추후 에러코드 추가하면 좋을 듯합니다.
이슈 번호
작업 내용