-
Notifications
You must be signed in to change notification settings - Fork 8
feat: 채팅방 신고 기능 추가 #483
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: 채팅방 신고 기능 추가 #483
Conversation
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
25-33: 데이터 무결성 예외 매핑 추가 안내
Report 엔티티와 마이그레이션에서 이미
(reporter_id, target_type, target_id)에 대한 유니크 제약이 선언되어 있습니다.
└ 엔티티:src/main/java/com/example/solidconnection/report/domain/Report.java@table(uniqueConstraints… 라인 19
└ 마이그레이션:V25__create_report_table.sqlconstraintuk_report_reporter_id_target_type_target_id라인 10그럼에도 불구하고 동시성 상황에서 동시 삽입 경합이 발생할 수 있으므로,
reportRepository.save(...)호출부를try-catch로 감싸DataIntegrityViolationException를 처리하고
CustomException(ErrorCode.ALREADY_REPORTED_BY_CURRENT_USER)로 매핑하는 예외 핸들링을 추가해야 합니다.아래와 같이
ReportService#createReport메서드를 수정해 주세요.
org.springframework.dao.DataIntegrityViolationException임포트 추가reportRepository.save(report)호출부를try { … } catch (DataIntegrityViolationException e) { … }블록으로 래핑@@ src/main/java/com/example/solidconnection/report/service/ReportService.java - import com.example.solidconnection.chat.repository.ChatMessageRepository; + import com.example.solidconnection.chat.repository.ChatMessageRepository; + import org.springframework.dao.DataIntegrityViolationException; @@ public void createReport(long reporterId, ReportRequest request) { Report report = new Report(reporterId, request.reportType(), request.targetType(), request.targetId()); - reportRepository.save(report); + try { + reportRepository.save(report); + } catch (DataIntegrityViolationException e) { + // (reporter_id, target_type, target_id) 유니크 제약 위반 시 + throw new CustomException(ErrorCode.ALREADY_REPORTED_BY_CURRENT_USER); + }
🧹 Nitpick comments (4)
src/main/java/com/example/solidconnection/report/domain/TargetType.java (1)
6-6: 2) 명칭 정합성 점검: CHAT은 '채팅방'인지 '채팅메시지'인지 확인이 필요합니다.
PR 제목과 이슈는 ‘채팅방 신고’로 되어 있으나, 실제 로직/테스트는 ChatMessage 기준으로 동작합니다.
혼선을 줄이려면 TargetType을 CHAT_MESSAGE로 명시하거나, JavaDoc에 "CHAT은 ChatMessage를 의미"라고 문서화하는 것이 좋습니다.필요 시 아래 최소 변경안을 제안드립니다.
- POST, - CHAT + POST, + CHAT_MESSAGE서비스/테스트의 참조도 동일하게 변경하면 됩니다.
src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java (2)
60-60: 2) 필드 네이밍 구체화 제안: chatMessage → reportTargetChatMessage.
테스트 가독성을 위해 ‘신고 타겟’임을 드러내면 의도가 더 분명해집니다.
테스트에서 유사 엔티티가 늘어날 때 혼동을 줄여줍니다.
67-69: 3) 불리언 파라미터의 의미를 드러내기: 불리언 트랩 회피.
chatRoomFixture.채팅방(false)에서 false의 의미가 즉시 파악되지 않습니다.
메서드 네이밍을 채팅방_비공개(false)처럼 바꾸거나, 빌더/정적 팩터리로 의도를 노출하는 것을 권합니다.src/main/java/com/example/solidconnection/report/service/ReportService.java (1)
41-46: 2) 스위치 식 기반 존재 검증: 현재 스코프에서는 가장 단순하고 명확합니다.
열거형 기반이라 새로운 타입이 추가되면 컴파일 타임에 누락을 잡을 수 있습니다.
다만 타입이 늘어날 가능성이 높다면 전략 패턴(타입→existence 체크 함수 매핑)으로의 리팩터도 고려할 수 있습니다.예시(개념 스케치):
// Map<TargetType, LongPredicate> existences = Map.of( // POST, postRepository::existsById, // CHAT, chatMessageRepository::existsById // );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/example/solidconnection/report/domain/TargetType.java(1 hunks)src/main/java/com/example/solidconnection/report/service/ReportService.java(3 hunks)src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/main/java/com/example/solidconnection/report/domain/TargetType.java (1)
5-7: 1) CHAT 타겟 타입 추가: 구현 방향과 호환성 모두 양호합니다.
컴파일 타임에 스위치 식이 열거형을 모두 소진하도록 강제되므로 추후 누락 리스크가 낮습니다.
현재 변경으로 기존 POST 흐름과 충돌이 없고, 서비스/테스트와도 일관되게 동작합니다.src/test/java/com/example/solidconnection/report/service/ReportServiceTest.java (2)
6-9: 1) 채팅 픽스처 주입: 테스트 구성 명확하고 가독성 좋습니다.
ChatRoomFixture와 ChatMessageFixture로 도메인 맥락을 분리해, POST/CHAT 테스트를 독립적으로 유지한 점이 좋습니다.
중복 설정을 최소화하고 시나리오 확장이 용이합니다.Also applies to: 52-57
113-153: 4) 채팅 신고 시나리오 3종 테스트: 커버리지와 기대 동작 모두 적합합니다.
정상, 대상 부재, 중복 신고 예외까지 핵심 경로가 잘 검증되어 있습니다.
중복 신고 케이스는 저장소에 중복 레코드가 남지 않음을 추가로 검증하면 더 견고합니다.아래 보강 테스트를 고려해 주세요.
- 목적: 중복 신고 예외 후에도 동일 키 조합의 레코드 수가 1건임을 검증.
가능하다면 ReportRepository에 카운트 쿼리가 있다 가정하고 예시를 첨부합니다.
@Test void 이미_신고한_경우_예외_발생하며_중복_저장은_없다() { // given reportFixture.신고(siteUser.getId(), TargetType.CHAT, chatMessage.getId()); ReportRequest request = new ReportRequest(ReportType.INSULT, TargetType.CHAT, chatMessage.getId()); // when & then assertThatCode(() -> reportService.createReport(siteUser.getId(), request)) .isInstanceOf(CustomException.class) .hasMessageContaining(ErrorCode.ALREADY_REPORTED_BY_CURRENT_USER.getMessage()); // then // 예: countBy... API가 없으면, findBy... 로 목록 크기 검증으로 대체 // assertThat(reportRepository.countByReporterIdAndTargetTypeAndTargetId(...)).isEqualTo(1); }또한 공통 검증으로 “신고자 미존재” 케이스를 한 번 추가하면 방어력이 올라갑니다.
@Nested class 공통_검증 { @Test void 신고자가_존재하지_않으면_예외가_발생한다() { long notExistingUserId = 999_999L; ReportRequest request = new ReportRequest(ReportType.INSULT, TargetType.CHAT, chatMessage.getId()); assertThatCode(() -> reportService.createReport(notExistingUserId, request)) .isInstanceOf(CustomException.class) .hasMessageContaining(ErrorCode.USER_NOT_FOUND.getMessage()); } }src/main/java/com/example/solidconnection/report/service/ReportService.java (2)
23-24: 1) 의존성 주입 확장: ChatMessageRepository 추가는 타당합니다.
타겟 존재 여부 검증 경로에 필요한 최소 의존성만 주입되어 SRP 관점에서도 깔끔합니다.
테스트에서 실제로 해당 경로가 실행되므로 배선도 정상으로 보입니다.
41-46: 4) 소프트 삭제/차단 정책과 존재 검증의 일치성 확인.
Post/ChatMessage가 소프트 삭제되거나 차단 상태일 때도 existsById가 true를 반환한다면, 신고가 허용되는지가 정책적으로 모호할 수 있습니다.
필요 시 existsByIdAndDeletedFalse 같은 조건형 존재 검증으로 맞춰 주세요.
whqtker
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.
확인했습니다 !
관련 이슈
작업 내용
특이 사항
리뷰 요구사항 (선택)