refactor : SkillExchange의 알림 read flag 컬럼 방식에서 Notification 도메인 연결 전환#157
Conversation
- GET /exchange/request/notification 엔드포인트 제거 (→ /notifications/unread-count 사용) - SkillExchangeNotificationResponseDto 삭제 - SkillExchangeRepository bulkUpdateRequesterReadStatus/ReceiverReadStatus, existsBy*IsReadFalse 제거 - SentDetailQuery.isRead, ReceivedDetailQuery.isRead 필드 제거 (JPQL 포함) - acceptSkillExchange/rejectSkillExchange/cancelSkillExchange updateRequesterReadToFalse/ReadToFalse 호출 제거 - NotificationRepository.findUnreadRefIdsByUserIdAndTypes() 배치 쿼리 추가 - NotificationService.getUnreadSentRequestRefIds(), getUnreadReceivedRequestRefIds() 추가 - getSentRequests/getReceivedRequests : 읽음 처리 전 미읽음 refId 배치 조회 후 isNew per-item 계산 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the skill exchange notification system by migrating read-status tracking from the SkillExchange entity to the Notification domain. Key changes include removing the dedicated notification endpoint, updating DTOs to accept externally calculated isNew flags, and implementing batch retrieval of unread notification IDs via the NotificationService. Review feedback identifies critical inconsistencies in NotificationServiceImpl, noting that methods like markReceivedRequestAsRead still target single notification types rather than the newly defined RECEIVED_REQUEST_TYPES constant, which could lead to status-change notifications remaining unread.
| private static final List<NotificationType> RECEIVED_REQUEST_TYPES = List.of( | ||
| NotificationType.REQUEST_RECEIVED, | ||
| NotificationType.REQUEST_STATUS_CHANGED | ||
| ); |
There was a problem hiding this comment.
RECEIVED_REQUEST_TYPES 상수가 정의되었으나, 기존의 markReceivedRequestAsRead 메서드와 getUnreadCounts 메서드(본 diff에는 포함되지 않음)에서는 여전히 NotificationType.REQUEST_RECEIVED를 직접 사용하고 있는 것으로 보입니다. 이로 인해 REQUEST_STATUS_CHANGED 타입의 알림이 "받은 요청" 탭의 미읽음 카운트에 반영되지 않거나, 탭 진입 시 읽음 처리가 누락되는 현상이 발생할 수 있습니다. SENT_REQUEST_TYPES를 사용하는 방식과 일관성을 유지하도록 해당 메서드들이 RECEIVED_REQUEST_TYPES를 사용하도록 수정이 필요합니다.
| Set<Long> unreadRefIds = notificationService.getUnreadReceivedRequestRefIds(userId); | ||
|
|
||
| // 5. Notification 도메인 읽음 처리 (REQUEST_RECEIVED) | ||
| // 4. Notification 도메인 읽음 처리 (REQUEST_RECEIVED) | ||
| notificationService.markReceivedRequestAsRead(userId); |
There was a problem hiding this comment.
getUnreadReceivedRequestRefIds는 RECEIVED_REQUEST_TYPES(REQUEST_RECEIVED + REQUEST_STATUS_CHANGED)를 사용하여 미읽음 ID를 조회하는 반면, markReceivedRequestAsRead는 REQUEST_RECEIVED 타입만 읽음 처리하고 있습니다. 이로 인해 상태 변경 알림(REQUEST_STATUS_CHANGED)이 목록에서는 isNew=true로 표시되지만, 실제 알림 엔티티는 읽음 처리가 되지 않아 탭의 배지 카운트가 줄어들지 않는 문제가 발생할 수 있습니다. markReceivedRequestAsRead 메서드가 RECEIVED_REQUEST_TYPES 전체를 처리하도록 NotificationService 구현부의 수정을 검토해 주세요.
- RECEIVED_REQUEST_TYPES에 REQUEST_STATUS_CHANGED 포함 시 받은 요청 탭 진입만으로 보낸 요청 탭의 상태 변경 알림까지 읽음 처리되는 부작용 발생 - RECEIVED_REQUEST_TYPES = [REQUEST_RECEIVED] 으로 수정 - markReceivedRequestAsRead(), getUnreadCounts() receivedRequestCount 모두 RECEIVED_REQUEST_TYPES 상수를 통해 일관성 있게 처리 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the skill exchange notification system by migrating the 'read' status management from the SkillExchange entity to the Notification domain. Key changes include removing read-status flags from the SkillExchange repository and DTOs, and instead utilizing the NotificationService to fetch unread reference IDs for determining the 'isNew' status of items in request lists. Feedback indicates a logic issue in the classification of notification types: since REQUEST_STATUS_CHANGED is exclusively mapped to sent requests, receivers will not see updated status indicators or correct unread counts when a requester cancels an exchange. It is recommended to distinguish between status changes for sent and received requests to ensure UI consistency.
| // REQUEST_STATUS_CHANGED 는 보낸 요청(SENT) 버킷에만 속함 | ||
| // 받은 요청 탭에서 REQUEST_STATUS_CHANGED 까지 읽음 처리하면 | ||
| // 보낸 요청 탭의 미읽음 배지가 잘못 감소하는 부작용 발생 | ||
| private static final List<NotificationType> RECEIVED_REQUEST_TYPES = List.of( | ||
| NotificationType.REQUEST_RECEIVED | ||
| ); |
There was a problem hiding this comment.
The current classification of REQUEST_STATUS_CHANGED exclusively into SENT_REQUEST_TYPES is problematic. When a requester cancels a skill exchange, a REQUEST_STATUS_CHANGED notification is sent to the receiver. Since this type is missing from RECEIVED_REQUEST_TYPES, the receiver will not see a red dot (isNew) for that item in their "Received Requests" tab, and the unread count for that tab will be incorrect.
To fix this while avoiding the "cross-tab clearing" issue mentioned in the comments, you should distinguish between status changes for sent requests and received requests. Consider splitting REQUEST_STATUS_CHANGED into two distinct types: SENT_REQUEST_STATUS_CHANGED and RECEIVED_REQUEST_STATUS_CHANGED.
|
|
||
| // 받은 요청 탭 | ||
| long receivedRequestCount = notificationRepository.countUnreadByUserIdAndType(userId, NotificationType.REQUEST_RECEIVED); | ||
| long receivedRequestCount = notificationRepository.countUnreadByUserIdAndTypes(userId, RECEIVED_REQUEST_TYPES); |
There was a problem hiding this comment.
As noted in the previous comment, receivedRequestCount will be inaccurate because it excludes REQUEST_STATUS_CHANGED notifications that are intended for the receiver (e.g., when a requester cancels). This leads to a discrepancy where the main notification badge shows a count that doesn't match the sum of the tab badges, or a tab badge that doesn't reflect actual unread changes.
- REQUEST_STATUS_CHANGED 단일 타입으로는 수락/거절(→요청자)과 요청자취소(→수신자)를 서로 다른 탭 버킷에 분류하는 것이 불가능한 문제 해결 - SENT_REQUEST_STATUS_CHANGED: 수락/거절/수신자취소/만료 시 요청자에게 발송 - RECEIVED_REQUEST_STATUS_CHANGED: 요청자취소/만료 시 수신자에게 발송 - SENT_REQUEST_TYPES = [REQUEST_SENT, SENT_REQUEST_STATUS_CHANGED] - RECEIVED_REQUEST_TYPES = [REQUEST_RECEIVED, RECEIVED_REQUEST_STATUS_CHANGED] - 기존 REQUEST_STATUS_CHANGED 는 DB 호환성을 위해 enum에 @deprecated 유지 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
🔥ISSUE
스킬교환 알림 시스템의 미읽음 상태 관리를 SkillExchange 엔티티의 read flag 컬럼 방식에서 Notification 테이블 단일 소스 방식으로 완전 전환
📒TODO
existsByRequester_IdAndIsRequesterReadFalse, existsByReceiver_IdAndIsReceiverReadFalse 4개 메서드 제거
📋REF