Conversation
WalkthroughAdds block-status handling: moves block/unblock endpoints to Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatController
participant ChatService
participant ChatRepository
participant BlockListRepository
Client->>ChatController: GET /chats/{chatId}/block (user-id header)
ChatController->>ChatService: getBlockStatus(me, chatId)
ChatService->>ChatRepository: validate participant & resolve partnerId
ChatRepository-->>ChatService: partnerId
ChatService->>BlockListRepository: query isBlocked(me, partnerId)
BlockListRepository-->>ChatService: iBlockedPartner
ChatService->>BlockListRepository: query isBlocked(partnerId, me)
BlockListRepository-->>ChatService: partnerBlockedMe
ChatService-->>ChatController: BlockStatusResponse(iBlockedPartner, partnerBlockedMe)
ChatController-->>Client: 200 ApiResponse<BlockStatusResponse>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/java/com/salemale/domain/chat/dto/BlockStatusResponse.java (1)
6-12: Consider using@Getterand@Builderfor consistency.The
@Dataannotation generates setters that are unnecessary for a response DTO. For consistency with other response DTOs inChatDtos.java(e.g.,ChatSummaryResponse,ChatResponse) and to ensure immutability, consider using@Getterwith@Builderinstead.Apply this diff:
-@Data +@Getter +@Builder @AllArgsConstructor +@NoArgsConstructor public class BlockStatusResponse { - private boolean iBlockedPartner; // 내가 상대를 차단했는지 private boolean partnerBlockedMe; // 상대가 나를 차단했는지 }src/main/java/com/salemale/domain/chat/service/ChatService.java (1)
61-63: Use imported types instead of fully qualified names.Lines 63 uses fully qualified names
java.util.Setandjava.util.HashSet, but these types are already imported at lines 29 and 31. Use the short names for consistency.Apply this diff:
// 내가 차단한 사용자 id 목록을 한 번에 조회 List<Long> blockedUserIds = blockListRepository.findBlockedUserIds(me); - java.util.Set<Long> blockedSet = new java.util.HashSet<>(blockedUserIds); + Set<Long> blockedSet = new HashSet<>(blockedUserIds);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/salemale/domain/chat/controller/ChatController.java(4 hunks)src/main/java/com/salemale/domain/chat/dto/BlockStatusResponse.java(1 hunks)src/main/java/com/salemale/domain/chat/dto/ChatDtos.java(2 hunks)src/main/java/com/salemale/domain/chat/service/ChatService.java(7 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/salemale/domain/chat/dto/ChatDtos.java (1)
57-60: LGTM!The
iBlockedPartnerfield is correctly placed inChatSummaryResponseand aligns with the block status feature implementation in the service layer.src/main/java/com/salemale/domain/chat/controller/ChatController.java (1)
114-114: LGTM!The route mappings follow RESTful conventions:
POST /chats/{chatId}/blockcreates a blockPOST /chats/{chatId}/unblockremoves a blockGET /chats/{chatId}/blockqueries block statusSpring MVC correctly distinguishes these endpoints by HTTP method, so there is no ambiguity.
Also applies to: 128-128, 137-145
src/main/java/com/salemale/domain/chat/service/ChatService.java (3)
114-115: LGTM!The implementation efficiently checks block status using a preloaded
Setfor O(1) lookup, avoiding N+1 queries when building chat summaries.Also applies to: 123-123
309-311: LGTM!The participant validation checks are correctly applied to both
blockPartnerandunblockPartner. Additionally, returningblocked=truewhen a user is already blocked (line 321) is more semantically accurate than the previousfalsevalue.Also applies to: 317-321, 346-349
372-393: LGTM!The
getBlockStatusmethod correctly validates chat participation, resolves the partner ID, and queries block status in both directions. The implementation follows the established patterns in this service.
src/main/java/com/salemale/domain/chat/controller/ChatController.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/salemale/domain/chat/dto/ChatDtos.java (1)
53-56: Previous critical issue resolved; consider idiomatic naming.The field is now correctly declared inside the
ChatSummaryResponseclass body, fixing the compilation error flagged in the previous review.The
iBlockedPartnernaming is unconventional—Java boolean fields typically use prefixes likeis,has, orcan(e.g.,hasBlockedPartnerorisPartnerBlocked). However, the current naming is functional and the comments clearly explain the semantics.If you prefer a more idiomatic convention, consider renaming to
hasBlockedPartner:- // 내가 이 대화 상대를 차단했는지 여부 - // true -> 이미 차단됨 - // false -> 차단 안 됨 - private boolean iBlockedPartner; + // 내가 이 대화 상대를 차단했는지 여부 + // true -> 이미 차단됨 + // false -> 차단 안 됨 + private boolean hasBlockedPartner;Note: This would require updating references in the service layer and API responses.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/salemale/domain/chat/dto/ChatDtos.java(2 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 (1)
src/main/java/com/salemale/domain/chat/dto/ChatDtos.java (1)
28-28: LGTM: formatting adjustment.The whitespace change has no functional impact.
2 participants
🎋 이슈 및 작업중인 브랜치
#142
🔑 주요 내용
-refactor로 기존 채팅 목록 조회에 추가하는 방식으로 하려했으나 새로 api를 추가하는 방식으로 하여 사실상 feat
-차단 여부 목록 조회 방식 (이전 pr 코드래빗 피드백 반영)
상대방 차단일때 true
상대방 차단 해제일때 false 반환
Check List
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.