[Feat] 사용자 푸시알림 수신여부 조회 #299
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough유저 푸시 알림 수신 여부를 조회하는 신규 API를 추가하고, 이를 위한 애플리케이션/퍼시스턴스 계층의 조회 흐름과 Swagger 응답 서술을 확장했다. FirebaseAdapter의 활성 프로필 조건식을 수정했고, 불필요한 DummyUseCase를 제거했으며 일부 import를 정리했다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant NQC as NotificationQueryController
participant SVC as NotificationShowEnableStateService
participant PORT as FcmTokenPersistencePort
participant REP as FcmTokenJpaRepository
note over C,NQC: GET /users/notification-settings?deviceId=...
C->>NQC: 요청(userId, deviceId)
NQC->>SVC: getNotificationShowEnableState(userId, deviceId)
SVC->>PORT: getByDeviceIdAndUserIdOrThrow(deviceId, userId)
PORT->>REP: findByDeviceIdAndUserId(deviceId, userId)
REP-->>PORT: Optional<FcmToken>
PORT-->>SVC: FcmToken (exists) / throws (없음)
SVC-->>NQC: boolean isEnabled
NQC-->>C: BaseResponse.ok(NotificationShowEnableStateResponse)
note over NQC,C: 오류 시 Swagger: NOTIFICATION_GET_ENABLE_STATE(FCM_TOKEN_NOT_FOUND)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (11)
✨ 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. Comment |
Test Results457 tests 457 ✅ 42s ⏱️ Results for commit b3fbac0. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
src/main/java/konkuk/thip/user/application/port/in/UserShowNotificationSettingsInfoUseCase.java (1)
4-4: 메소드 매개변수에 공백 누락메소드 매개변수 목록에서 콤마 뒤에 공백이 없습니다.
- boolean getUserNotificationSettingsInfo(Long userId,String deviceId); + boolean getUserNotificationSettingsInfo(Long userId, String deviceId);src/main/java/konkuk/thip/user/application/service/UserShowNotificationSettingInfoService.java (1)
18-18: 메소드 매개변수에 공백 누락메소드 매개변수 목록에서 콤마 뒤에 공백이 없습니다.
- public boolean getUserNotificationSettingsInfo(Long userId,String deviceId) { + public boolean getUserNotificationSettingsInfo(Long userId, String deviceId) {src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java (2)
31-31: Import 개선명시적 import를 wildcard import로 변경한 것은 코드를 간결하게 만들지만, 가독성과 유지보수성 측면에서는 트레이드오프가 있습니다.
가능하다면 사용하는 상수들만 명시적으로 import하는 것을 권장합니다:
import static konkuk.thip.common.swagger.SwaggerResponseDescription.GET_USER_FOLLOW; import static konkuk.thip.common.swagger.SwaggerResponseDescription.USER_SEARCH; import static konkuk.thip.common.swagger.SwaggerResponseDescription.USER_GET_NOTIFICATION_INFO;
185-185: 메소드 매개변수에 공백 누락메소드 호출에서 매개변수 목록의 콤마 뒤에 공백이 없습니다.
- UserNotificationSettingsInfoResponse.of(userShowNotificationSettingsInfoUseCase.getUserNotificationSettingsInfo(userId,deviceId))); + UserNotificationSettingsInfoResponse.of(userShowNotificationSettingsInfoUseCase.getUserNotificationSettingsInfo(userId, deviceId)));src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java (2)
16-16: [P3] 표기 간소화 니트픽.배열 리터럴 없이 단일 문자열로 쓰는 편이 관용적입니다. 기능 차이는 없습니다.
-@Profile({"!test & !local"}) +@Profile("!test & !local")
22-24: [P2] 마스킹 정책의 환경 판단을 Spring Profile로 통일하는 것을 권장.
server.profile커스텀 프로퍼티 대신 SpringEnvironment의acceptsProfiles("dev")를 사용하면 설정 드리프트를 줄일 수 있습니다. 아래와 같이 리팩터링을 제안합니다.@@ -import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.env.Environment; +import org.springframework.core.env.Profiles; @@ - @Value("${server.profile}") - private String profile; + private final Environment environment; @@ - private boolean isDev() { - return profile != null && profile.trim().equalsIgnoreCase("dev"); - } + private boolean isDev() { + return environment != null && environment.acceptsProfiles(Profiles.of("dev")); + }주의: 생성자 주입(@requiredargsconstructor)으로
Environment가 주입되도록 필드를final로 두는 것도 검토하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java(1 hunks)src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java(3 hunks)src/main/java/konkuk/thip/user/adapter/in/web/response/UserNotificationSettingsInfoResponse.java(1 hunks)src/main/java/konkuk/thip/user/application/port/in/UserShowNotificationSettingsInfoUseCase.java(1 hunks)src/main/java/konkuk/thip/user/application/service/UserShowNotificationSettingInfoService.java(1 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 (6)
src/main/java/konkuk/thip/common/swagger/SwaggerResponseDescription.java (1)
44-47: 적절한 에러 코드 매핑새로운 사용자 푸시알림 수신여부 조회 API에 대한 적절한 에러 코드들이 매핑되어 있습니다.
FCM_TOKEN_NOT_FOUND와FCM_TOKEN_ACCESS_FORBIDDEN은 해당 기능의 예상되는 에러 시나리오와 잘 맞습니다.src/main/java/konkuk/thip/user/adapter/in/web/response/UserNotificationSettingsInfoResponse.java (1)
3-9: 간결하고 명확한 응답 모델Record를 사용한 간결한 응답 모델과 정적 팩토리 메소드가 잘 구현되어 있습니다. boolean 값을 감싸는 단순한 구조가 API 요구사항에 적합합니다.
src/main/java/konkuk/thip/user/application/service/UserShowNotificationSettingInfoService.java (1)
20-24: 비즈니스 로직 구현 확인FCM 토큰 조회, 소유권 검증, 활성화 상태 반환의 흐름이 명확하고 적절합니다.
@Transactional(readOnly = true)어노테이션도 조회 전용 작업에 맞게 설정되어 있습니다.src/main/java/konkuk/thip/user/adapter/in/web/UserQueryController.java (1)
174-186: API 엔드포인트 구현 검토새로운 푸시알림 수신여부 조회 API가 잘 구현되어 있습니다. 메소드 시그니처, 어노테이션, 응답 처리가 모두 적절합니다. deviceId를 필수 파라미터로 받는 것도 비즈니스 로직에 맞습니다.
src/main/java/konkuk/thip/message/adapter/out/firebase/FirebaseAdapter.java (2)
16-16: [P1] 프로파일 표현식 수정이 정확합니다 (OR → AND 고침).기존
@Profile({"!test", "!local"})는 OR로 평가되어local만 활성이어도 빈이 활성화되는 문제가 있었는데,@Profile("!test & !local")로 바뀌며 test/local 중 하나라도 활성화되면 비활성화되어 의도대로 동작합니다. 굿 픽스입니다.
16-18: FakeFirebaseAdapter가 test/local 프로파일에서 FirebaseMessagingPort를 구현 — 문제 없음src/main/java/konkuk/thip/message/adapter/out/firebase/FakeFirebaseAdapter.java의 @Profile("local | test")가 FirebaseMessagingPort 구현체로 확인되었고, FirebaseAdapter는 @Profile({"!test & !local"})로 분리되어 있어 test 또는 local 프로파일에서 빈 부재로 인한 컨텍스트 로드 실패 우려가 없습니다.
| @Slf4j | ||
| @Component | ||
| @Profile({"!test", "!local"}) | ||
| @Profile({"!test & !local"}) |
There was a problem hiding this comment.
엇 이 부분은 제가 수정한 부분과 겹치는거 같습니다!!
There was a problem hiding this comment.
아마 ci 통과하려면 수정해야해서 같이 고치신 것 같습니다! 저도 뒷 pr에서 고쳤습니다
| @Operation( | ||
| summary = "사용자 푸시알림 수신여부 조회 (마이페이지 -> 알림설정)", | ||
| description = "알림설정 페이지에서 사용자의 푸시알림 수신여부 정보를 조회합니다." | ||
| ) | ||
| @ExceptionDescription(USER_GET_NOTIFICATION_INFO) | ||
| @GetMapping("/users/notification-settings") | ||
| public BaseResponse<UserNotificationSettingsInfoResponse> showUserNotificationSettingsInfo( | ||
| @Parameter(hidden = true) @UserId final Long userId, | ||
| @Parameter(description = "디바이스 고유 ID", example = "device12345") | ||
| @RequestParam("deviceId") final String deviceId) { | ||
| return BaseResponse.ok( | ||
| UserNotificationSettingsInfoResponse.of(userShowNotificationSettingsInfoUseCase.getUserNotificationSettingsInfo(userId,deviceId))); | ||
| } |
There was a problem hiding this comment.
엇 그 NotificationQueryController에 api 핸들러를 정의하는게 어떨까요? 알림 관련 조회 API에 더 가깝다고 생각합니다!
There was a problem hiding this comment.
path 또한 푸시 알림 수신 여부 설정 변경 api와 동일하게 GET /notifications/enable-state로 해도 괜찮을 것 같아요!
| @Override | ||
| @Transactional(readOnly = true) | ||
| public boolean getUserNotificationSettingsInfo(Long userId,String deviceId) { | ||
|
|
||
| FcmToken fcmToken = fcmTokenPersistencePort.getByDeviceIdOrThrow(deviceId); | ||
|
|
||
| fcmToken.validateFcmOwner(userId); | ||
|
|
||
| return fcmToken.isEnabled(); | ||
| } |
There was a problem hiding this comment.
음 FcmToken을 가져올때 getByDeviceIdAndUserIdOrThrow(deviceId, userId) 이런식으로 가져오는 것은 어떤가요? 해당 FcmToken 자체에 fk로 매핑되어 있기 때문에 만약 같은 유저가 같은 디바이스로 요청을 보내는 경우의 예외처리가 '토큰이 존재하지 않다'로 한번에 가능할 것 같습니다.
| private final NotificationShowEnableStateUseCase notificationShowEnableStateUseCase; | ||
|
|
||
| @Operation( | ||
| summary = "사용자 푸시알림 수신여부 조회 (마이페이지 -> 알림설정)", | ||
| description = "알림설정 페이지에서 사용자의 푸시알림 수신여부 정보를 조회합니다." | ||
| ) | ||
| @ExceptionDescription(NOTIFICATION_GET_ENABLE_STATE) | ||
| @GetMapping("/users/notification-settings") | ||
| public BaseResponse<NotificationShowEnableStateResponse> showNotificationEnableState( | ||
| @Parameter(hidden = true) @UserId final Long userId, | ||
| @Parameter(description = "디바이스 고유 ID", example = "device12345") | ||
| @RequestParam("deviceId") final String deviceId) { | ||
| return BaseResponse.ok( | ||
| NotificationShowEnableStateResponse.of(notificationShowEnableStateUseCase.getNotificationShowEnableState(userId,deviceId))); | ||
| } |
|
|
||
| import static konkuk.thip.common.swagger.SwaggerResponseDescription.NOTIFICATION_GET_ENABLE_STATE; | ||
|
|
||
| @RestController |
#️⃣ 연관된 이슈
📝 작업 내용
📸 스크린샷
💬 리뷰 요구사항
📌 PR 진행 시 이러한 점들을 참고해 주세요
Summary by CodeRabbit