-
Notifications
You must be signed in to change notification settings - Fork 4
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
신고 조치 기능 구현 #770
신고 조치 기능 구현 #770
Conversation
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 할께요 ~
@ApiResponse( | ||
responseCode = "400", | ||
description = """ | ||
1.신고 ID가 양의 정수가 아닌 경우 |
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가 존재하지 않은 경우에도 400
예외가 발생할 수 있을 것 같아요 !
List<ReportAggregateDto> findReportsGroupedByReportTypeAndTargetId(final Pageable pageable); | ||
List<ReportAggregateDto> findReportAggregateDtosByReportTypeAndTargetId(final Pageable pageable); | ||
|
||
Optional<ReportAggregateDto> findReportAggregateDtoByReportTypeAndTargetId(ReportType reportType, Long targetId); |
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.
final
키워드가 빠져있어요 !
@@ -13,6 +14,8 @@ public interface ReportStrategy { | |||
|
|||
String parseTarget(Long targetId); | |||
|
|||
void reportAction(ReportAggregateDto reportAggregateDto); |
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.
final
키워드가 빠져 있어요 !
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.
하나 궁금한 부분이 있어서 RC 남깁니다!
고생하셨습니다 :)
public void reportAction(final ReportActionRequest request) { | ||
final Report report = reportRepository.findById(request.id()) | ||
.orElseThrow(() -> new NotFoundException(ReportExceptionType.NOT_FOUND)); | ||
|
||
final ReportAggregateDto reportAggregateDto = reportRepository | ||
.findReportAggregateDtoByReportTypeAndTargetId(report.getReportType(), report.getTargetId()) | ||
.orElseThrow(() -> new NotFoundException(ReportExceptionType.NOT_FOUND)); |
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.
Q
이 두 상황에서 같은 예외를 사용하는 이유가 무엇인가요?
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.
먼저 저 두 단계를 거칠 필요가 있었습니다. 결국 reportType과 targetId로 group by 해서 가져와야 해서요.
그래서 이처럼 2번 가져오게 되었는데, 결국 id로든 reportType과 targetId 로든 결국 찾아오려고 하는데 못찾아오는 거면 NotFoundException이 맞다고 판단했습니다. 또한 둘 다 Report에 관련한 것들이니 ReportExceptionType을 사용하였습니다.
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.
그러게요. ReportAggregateDto에 관한 message를 따로 만들어볼게요.
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 하겠습니다!
고생하셨어요~
@Test | ||
@DisplayName("신고 조치 예정 정보를 조회한다") | ||
void getReportAggregateDto() { | ||
// given | ||
Member memberA = memberRepository.save(MemberFixtures.MALE_30.get()); | ||
Member memberB = memberRepository.save(MemberFixtures.MALE_20.get()); | ||
|
||
final ReportType reportType = ReportType.POST; | ||
final long targetId = 1L; | ||
|
||
final String reasonA = "reasonA"; | ||
reportTestPersister.builder() | ||
.member(memberA) | ||
.reportType(reportType) | ||
.reason(reasonA) | ||
.targetId(targetId) | ||
.save(); | ||
|
||
final String reasonB = "reasonB"; | ||
Report savedReportB = reportTestPersister.builder() | ||
.member(memberB) | ||
.reportType(reportType) | ||
.reason(reasonB) | ||
.targetId(targetId) | ||
.save(); | ||
|
||
// when | ||
ReportAggregateDto reportAggregateDto = reportCustomRepository | ||
.findReportAggregateDtoByReportTypeAndTargetId(reportType, targetId) | ||
.orElseThrow(() -> new NotFoundException(ReportExceptionType.NOT_FOUND)); | ||
|
||
// then | ||
assertSoftly(softly -> { | ||
softly.assertThat(reportAggregateDto.reportMaxId()).isEqualTo(savedReportB.getId()); | ||
softly.assertThat(reportAggregateDto.reportType()).isEqualTo(reportType); | ||
softly.assertThat(reportAggregateDto.targetId()).isEqualTo(targetId); | ||
softly.assertThat(reportAggregateDto.reasons()).contains(reasonA, reasonB); | ||
softly.assertThat(reportAggregateDto.createdAt().truncatedTo(ChronoUnit.SECONDS)) | ||
.isEqualTo(savedReportB.getCreatedAt().truncatedTo(ChronoUnit.SECONDS)); | ||
}); | ||
} | ||
|
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.
final이 있어요!
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.
굿굿 고생하셨습니다!~
🔥 연관 이슈
close: #757
📝 작업 요약
신고 조치 기능 구현
⏰ 소요 시간
7시간
🔎 작업 상세 설명
신고 조치 기능 구현할 때 조치 확정과 조치 기각으로 나뉜다.