[REFACTOR] 알람 중복 로직 제거 및 유저 이미지 추가#280
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough알림 생성 로직을 서비스별 직접 호출에서 중앙화된 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant S as 서비스 (예: FollowService)
participant F as NotificationFactory
participant U as UserEntityRepository
participant UI as UserImageService
participant N as NotificationService
S->>F: followed(followerId, targetUserId)
F->>U: findById(followerId)
U-->>F: UserEntity
F->>UI: createImageGetUrlOptional(followerId)
UI-->>F: Optional<String> imageUrl
F->>N: createNotification(targetUserId, FOLLOWED, payload(with image info))
N-->>F: void
F-->>S: void
Note over F, N: 중앙화된 페이로드 생성 및 전송 흐름
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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: 3
🧹 Nitpick comments (2)
src/test/java/hanium/modic/backend/domain/follow/service/FollowMockingServiceTest.java (1)
25-26: 팔로우 알림 호출에 대한 검증 추가를 고려해 주세요
followSuccess테스트에서notificationFactory.followed(anyLong(), anyLong())만 stub 하고verify는 하지 않아, 알림 호출 여부가 깨져도 테스트가 잡아내지 못합니다. 예를 들어 아래 정도를 추가하면 리팩터링 시 회귀를 막는 데 도움이 될 것 같습니다.verify(notificationFactory, times(1)).followed(me.getId(), target.getId());기존 동작에는 영향 없고 테스트 신뢰도만 올라가는 수준이라, 여유 있을 때 반영해 보셔도 좋겠습니다.
Also applies to: 48-49, 75-77
src/main/java/hanium/modic/backend/domain/notification/factory/NotificationFactory.java (1)
39-42: 발신자 이미지 처리 로직이 Factory에서 아직 사용되지 않습니다
UserImageService와getUserImageUrl(...)헬퍼를 두셨고,basePayload주석에도 “보내는 이의 이미지”라고 되어 있지만, 실제로는 이미지 정보를NotificationPayload에 넣지 않고 있습니다. 현재 구현 기준으로는:
UserImageService/getUserImageUrl/Optional반환값이 전혀 사용되지 않고,- 발신자 이미지는 다른 계층(알림 조회 시점 등)에서 따로 붙인다고 오해할 여지가 있습니다.
의도에 따라 두 가지 방향 중 하나를 추천드립니다.
- 여기서 이미지까지 포함시키려는 의도였다면:
basePayload안에서getUserImageUrl(senderId)를 사용해NotificationPayload에 이미지 관련 필드를 세팅하는 로직을 추가.- 조회 시점에만 이미지를 붙일 계획이라면: 이 Factory에서
UserImageService의존성과getUserImageUrl메서드를 제거하고 주석도 정리해서, 실제 책임 위치를 명확히 표현.지금도 기능적으로는 문제 없지만, PR 설명의 “발신자 이미지 추가”와 코드의 역할이 살짝 어긋나 보이는 부분이라 정리해 두면 나중에 읽는 사람이 덜 헷갈릴 것 같습니다.
Also applies to: 49-58
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/hanium/modic/backend/domain/ai/aiChat/service/AiImagePermissionService.java(4 hunks)src/main/java/hanium/modic/backend/domain/follow/service/FollowService.java(3 hunks)src/main/java/hanium/modic/backend/domain/notification/dto/GetNotificationsResponse.java(3 hunks)src/main/java/hanium/modic/backend/domain/notification/dto/NotificationPayload.java(3 hunks)src/main/java/hanium/modic/backend/domain/notification/factory/NotificationFactory.java(1 hunks)src/main/java/hanium/modic/backend/domain/notification/service/NotificationService.java(5 hunks)src/main/java/hanium/modic/backend/domain/post/service/AiDerivedPostService.java(3 hunks)src/main/java/hanium/modic/backend/domain/postLike/service/PostLikeService.java(3 hunks)src/main/java/hanium/modic/backend/domain/postReview/service/PostReviewService.java(3 hunks)src/main/java/hanium/modic/backend/domain/transaction/service/AccountService.java(3 hunks)src/test/java/hanium/modic/backend/domain/ai/service/AiImagePermissionServiceTest.java(4 hunks)src/test/java/hanium/modic/backend/domain/follow/service/FollowMockingServiceTest.java(3 hunks)src/test/java/hanium/modic/backend/domain/post/service/AiDerivedPostServiceTest.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 (9)
src/main/java/hanium/modic/backend/domain/notification/dto/NotificationPayload.java (1)
6-21: 코드 정리가 잘 되었습니다.포맷팅 변경만 있고 기능적 변경은 없습니다.
Also applies to: 39-43
src/main/java/hanium/modic/backend/domain/postReview/service/PostReviewService.java (1)
18-18: 팩토리 패턴으로의 리팩터링이 적절합니다.
NotificationFactory를 사용하여 알림 생성 로직을 중앙화하고 중복을 제거했습니다. 팩토리 메서드가 필요한 데이터를 내부적으로 가져오는 것으로 보입니다.Also applies to: 49-49, 78-78
src/main/java/hanium/modic/backend/domain/transaction/service/AccountService.java (1)
16-16:NotificationFactory로의 전환이 올바릅니다.코인 전송 알림이 간결한 팩토리 메서드로 처리됩니다.
Also applies to: 38-38, 81-81
src/main/java/hanium/modic/backend/domain/post/service/AiDerivedPostService.java (1)
15-15: 파생 포스트 알림 로직이 팩토리 패턴으로 깔끔하게 개선되었습니다.트랜잭션 커밋 후 비동기로 실행되는 알림 로직이 간소화되었습니다.
Also applies to: 50-50, 170-170
src/test/java/hanium/modic/backend/domain/post/service/AiDerivedPostServiceTest.java (1)
28-28: 테스트 코드가 프로덕션 코드의 팩토리 패턴 변경에 맞게 올바르게 업데이트되었습니다.
NotificationFactory사용으로 전환이 정확히 반영되었습니다.Also applies to: 74-74, 122-122
src/test/java/hanium/modic/backend/domain/ai/service/AiImagePermissionServiceTest.java (1)
22-22: 테스트가 코인 및 티켓 구매 알림의 팩토리 패턴 변경을 올바르게 반영합니다.두 결제 방식 모두에서
NotificationFactory사용이 정확히 적용되었습니다.Also applies to: 56-56, 74-74, 140-140
src/main/java/hanium/modic/backend/domain/notification/dto/GetNotificationsResponse.java (1)
18-39: 발신자 이미지 필드 추가 설계 👍
hasSenderImage/senderImageUrl필드 추가와of(...)팩토리 시그니처 확장은 자연스럽고, 호출부에서도 컴파일 타임에 새 인자를 강제하므로 안정적입니다. 응답 스키마가 바뀌었으니, 이 두 필드를 사용하는 쪽(특히 프론트)이 정상적으로 파싱·표시하는지만 한 번만 확인해 주세요.src/main/java/hanium/modic/backend/domain/ai/aiChat/service/AiImagePermissionService.java (1)
13-49: NotificationFactory로 위임한 구조 적절합니다기존의
NotificationService직접 호출을notificationFactory.postPurchasedByCoin(...),postPurchasedByTicket(...)로 치환한 부분이 자연스럽고,buyerId/postId인자도 의도에 맞게 전달되고 있습니다. 알림 생성 책임을 공통 Factory로 모아서, 이 서비스는 도메인 로직에만 집중하게 된 점이 좋습니다.Also applies to: 71-72, 88-89
src/main/java/hanium/modic/backend/domain/follow/service/FollowService.java (1)
18-19: 팔로우 알림 위임 로직 일관성 좋습니다
followOrUnfollow에서 팔로우 시notificationFactory.followed(me.getId(), targetId)로 위임한 부분이 Factory 정의(followed(followerId, targetUserId))와 정확히 맞고, 서비스 레벨에서 알림 세부 구현을 감추는 구조도 깔끔합니다. Follow 관련 중복 알림 로직을 잘 정리하신 것 같습니다.Also applies to: 39-39, 61-61
src/main/java/hanium/modic/backend/domain/notification/factory/NotificationFactory.java
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/domain/notification/service/NotificationService.java
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/domain/postLike/service/PostLikeService.java
Outdated
Show resolved
Hide resolved
1 participant
개요
작업사항
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.