8000 [Fix] 알림 수신 설정을 Device 기준으로 변경 및 테스트용 알림 API 구현 by qkrcodus · Pull Request #53 · If-u-Lab/If-u-server · GitHub
[go: up one dir, main page]
Skip to content

[Fix] 알림 수신 설정을 Device 기준으로 변경 및 테스트용 알림 API 구현 - #53

Merged
qkrcodus merged 6 commits into
devfrom
fix/device-notification-schema
Jan 2, 2026
Merged

[Fix] 알림 수신 설정을 Device 기준으로 변경 및 테스트용 알림 API 구현#53
qkrcodus merged 6 commits into
devfrom
fix/device-notification-schema

Conversation

@qkrcodus
@qkrcodus qkrcodus commented Dec 27, 2025
Copy link
Copy Markdown
Member

🔗 Issue Link

#52

🎯 What I Did

  • 알림 수신 설정 관리 단위를 User에서 Device로 변경
  • 테스트 알림 API(POST /api/v1/notifications/test) 구현 ( 인증 필요 없이 호출 가능 )
  • User 도메인의 구버전 알림 설정 DTO 삭제 및 필드 삭제

🧪 API Test Scenarios

테스트한 엔드포인트

  • /api/v1/notifications/test

테스트 시나리오

  1. 정상 플로우
스크린샷 2025-12-27 오전 10 27 16 스크린샷 2025-12-27 오전 10 34 57
  1. 에러 케이스
스크린샷 2025-12-27 오전 10 57 08

✅ Checklist

  • Swagger 문서화 완료 (@Operation, @ApiResponse, @ApiErrorExample(s) 등)
  • 필요시 Notion 문서화 완료

@qkrcodus qkrcodus self-assigned this Dec 27, 2025
@qkrcodus qkrcodus added the enhancement New feature or request label Dec 27, 2025
@qkrcodus qkrcodus linked an issue Dec 27, 2025 that may be closed by this pull request
[Fix] 알림 수신 설정을 Device 기준으로 변경 및 테스트용 알림 API 구현 #52
Closed
@gemini-code-assist
gemini-code-assist Bot commented Dec 27, 2025
Copy link
Copy Markdown

Summary of Changes

Hello @qkrcodus, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

이 PR은 알림 시스템의 유연성과 테스트 용이성을 향상시키기 위해 알림 수신 설정의 관리 단위를 사용자에서 개별 기기로 전환하고, 개발 및 디버깅을 위한 테스트 알림 전송 API를 도입합니다. 이를 통해 사용자는 자신의 여러 기기 중 특정 기기에서만 알림을 받거나, 알림을 완전히 비활성화하는 등 더욱 세분화된 제어가 가능해지며, 개발자는 알림 기능을 효율적으로 테스트할 수 있게 됩니다.

Highlights

  • 알림 설정 단위 변경: 알림 수신 설정을 사용자(User) 단위에서 기기(Device) 단위로 변경하여, 각 기기별로 알림을 세밀하게 제어할 수 있도록 개선했습니다.
  • 테스트용 알림 API 구현: POST /api/v1/notifications/test 엔드포인트를 구현하여 특정 사용자에게 테스트 푸시 알림을 전송하고 결과를 확인할 수 있도록 했습니다. 이 API는 인증 없이 호출 가능합니다.
  • 사용자 알림 설정 관련 코드 제거: 사용자 도메인에 존재하던 구버전 알림 설정 DTO 및 관련 필드, API 엔드포인트를 모두 삭제하여 코드를 정리했습니다.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@claude
claude Bot commented Dec 27, 2025
Copy link
Copy Markdown

PR 리뷰

알림 설정 관리를 User 단위에서 Device 단위로 개선한 좋은 변경입니다. 테스트 알림 API 추가도 유용해 보입니다.

P1 (꼭 반영해주세요)

  • DB 마이그레이션 누락 - Device.java:53-54 에 notification_enabled 컬럼이 추가되었고, User.java 에서 동일한 이름의 컬럼이 삭제되었습니다. 기존 데이터를 처리하고 스키마를 변경하는 마이그레이션 스크립트가 필요합니다.

  • SecurityConfig.java:47 - 테스트 알림 API를 인증 없이 공개(permitAll)한 것은 보안상 위험합니다. 악의적 사용자가 임의의 userId로 반복 호출하여 알림 폭탄 공격이 가능합니다. 최소한 인증이 필요하거나, 관리자 권한(hasRole("ADMIN"))으로 제한해야 합니다.

P2 (적극적으로 고려해주세요)

  • PushNotificationService.java:188 - 주석 오타: "디바이스사" → "디바이스가"

  • DeviceNotificationSettingsResponse.java:16 - 파일 끝에 개행 문자 누락

P3 (웬만하면 반영해주세요)

  • Device.java:74 - 새로 등록되는 디바이스의 notificationEnabled 기본값이 true인데, 사용자가 명시적으로 동의하지 않은 상태에서 알림을 활성화하는 것은 UX 측면에서 개선이 필요할 수 있습니다. 초기 등록 시 사용자 선택을 받는 것을 권장합니다.

  • PushNotificationTestRequest.java:16 - dryRun 필드를 nullable Boolean 대신 @Builder.Default를 활용한 primitive boolean(기본값 false)으로 변경하면 더 명확합니다.

P4 (반영해도 좋고 넘어가도 좋습니다)

  • PushNotificationService.java:185 - 주석 끝 공백 제거
  • NotificationController.java:46 - 파일 끝 개행 문자 누락
  • PushNotificationTestResponse.java:26 - 파일 끝 개행 문자 누락

gemini-code-assist[bot]
gemini-code-assist Bot reviewed Dec 27, 2025
View reviewed changes
@gemini-code-assist gemini-code-assist Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

이번 PR은 알림 설정을 사용자 단위에서 기기 단위로 변경하고, 테스트용 알림 API를 추가하는 중요한 리팩토링 및 기능 추가를 포함하고 있습니다. 전반적으로 변경 사항은 명확하고 올바르게 구현되었습니다. 특히 기기별 알림 설정 기능 추가와 테스트 API 구현은 서비스 운영에 큰 도움이 될 것입니다. 몇 가지 코드 개선점에 대한 의견을 리뷰 코멘트로 남겼습니다. 주로 코드 효율성 및 중복 제거에 관한 내용으로, 확인 후 반영을 고려해 주시면 좋겠습니다.

Comment thread src/main/java/com/ifu/ifu_server/domain/notification/service/PushNotificationService.java Resolved
Comment thread src/main/java/com/ifu/ifu_server/domain/notification/service/PushNotificationService.java Resolved
@claude
claude Bot commented Jan 2, 2026
Copy link
Copy Markdown

PR 리뷰

알림 설정을 User에서 Device 단위로 변경한 것은 좋은 설계 개선입니다. 다만 DB 마이그레이션과 관련된 중요한 이슈가 있습니다.

P1 (꼭 반영해주세요)

  • DB 마이그레이션 스크립트 누락 - Device.java:53-54와 User.java에서 스키마 변경이 발생했으나 마이그레이션 스크립트가 없습니다. 기존 User 테이블의 notification_enabled 컬럼 삭제와 Device 테이블의 notification_enabled 컬럼 추가를 처리하는 스크립트가 필요합니다. 기존 데이터의 알림 설정 값을 어떻게 마이그레이션할지 정책도 명확히 해야 합니다.

P2 (적극적으로 고려해주세요)

  • PushNotificationService.java:236-262 - sendNotificationsInBatches() 메서드가 기존 sendToAllUsersInternal() 메서드(73-120줄)와 거의 동일한 로직을 중복 구현하고 있습니다. 두 메서드 모두 배치 전송과 무효 토큰 처리를 수행하므로, sendToAllUsersInternal()에서도 sendNotificationsInBatches()를 활용하도록 리팩토링하면 코드 중복을 제거할 수 있습니다.

  • DeviceRepository.java:17 - findAllActiveDevices() 쿼리가 notificationEnabled = true 조건을 포함하도록 변경되었으나, 메서드명이 이를 명시하지 않습니다. findAllActiveDevicesWithNotificationEnabled() 또는 findAllNotifiableDevices() 같은 이름으로 변경하면 의도가 더 명확해집니다.

P3 (웬만하면 반영해주세요)

  • PushNotificationService.java:176-226 - sendTestNotification() 메서드에 대한 단위 테스트 추가를 권장합니다. dryRun 모드, 정상 전송, 디바이스 없음, 알림 비활성화 등 다양한 시나리오를 검증하면 좋겠습니다.

  • DeviceNotificationSettingsResponse.java:16 - 파일 끝에 개행문자가 누락되었습니다.

P4 (반영해도 좋고 넘어가도 좋습니다)

  • PushNotificationService.java:38-39 - Javadoc 주석이 한 줄로 너무 길게 작성되어 가독성이 떨어집니다. 여러 줄로 분리하는 것을 권장합니다.

@claude
claude Bot commented Jan 2, 2026
Copy link
Copy Markdown

코드 리뷰

전반적으로 알림 설정을 User에서 Device로 이동시킨 구조 개선이 잘 이루어졌습니다. 테스트 API도 깔끔하게 구현되었네요.

P1 (꼭 반영해주세요)

  • Device.java:53-54 - 새로운 notification_enabled 컬럼 추가에 대한 DB 마이그레이션 스크립트가 누락되었습니다. 기존 Device 레코드의 기본값 설정이 필요합니다.
  • DeviceRepository.java:17 - findAllActiveDevices() 메서드가 findAllNotifiableDevices()로 이름이 변경되었으나, 실제로 이 메서드를 호출하는 코드가 없습니다. PushNotificationService.java:74에서는 여전히 findAllActiveDevices()를 호출하고 있어 컴파일 에러가 발생할 것으로 보입니다.

P2 (적극적으로 고려해주세요)

  • PushNotificationService.java:167 - findByUserIdAndNotificationEnabledTrue() 호출 시 삭제된 사용자(deletedAt != null)의 디바이스도 조회될 수 있습니다. 164줄에서 findValidUser()로 검증하지만, 레포지토리 쿼리에서도 필터링하는 것이 더 안전합니다.
  • DeviceNotificationSettingsResponse.java:16 - 파일 끝에 개행 문자가 누락되었습니다.

P3 (웬만하면 반영해주세요)

  • PushNotificationService.java:148-156 - Javadoc 첫 줄이 "테스트 알림 전송 특정 사용자의..." 로 붙어있습니다. "테스트 알림 전송." 으로 분리하거나 개행을 추가해주세요.
  • NotificationController.java:46 - 파일 끝에 개행 문자가 누락되었습니다.
  • DeviceService.java:88-110 - 새로 추가된 updateNotificationSettings() 메서드에 대한 단위 테스트 추가를 권장합니다.
  • NotificationController.java - 테스트 API가 인증 없이 호출 가능한데, 보안 이슈는 없는지 검토가 필요합니다. 운영 환경에서는 비활성화하거나 관리자 권한을 요구하는 것이 좋을 것 같습니다.

P4 (반영해도 좋고 넘어가도 좋습니다)

  • PushNotificationTestRequest.java:22 - 파일 끝에 개행 문자가 누락되었습니다.
  • PushNotificationTestResponse.java:26 - 파일 끝에 개행 문자가 누락되었습니다.
  • UserController.java:35 - 파일 끝에 개행 문자가 누락되었습니다.

@claude
claude Bot commented Jan 2, 2026
Copy link
Copy Markdown

PR 리뷰

알림 설정을 User에서 Device 단위로 변경한 리팩토링 작업입니다. 전반적으로 깔끔하게 구현되었으나, DB 마이그레이션과 몇 가지 개선 사항이 필요합니다.

P1 (꼭 반영해주세요)

  • DB 마이그레이션 스크립트 누락 - User 테이블의 notification_enabled 컬럼과 Device 테이블의 새 notification_enabled 컬럼에 대한 마이그레이션 스크립트가 필요합니다. 기존 사용자의 알림 설정 데이터를 어떻게 처리할지 명확한 전략이 필요합니다. (모든 기기에 기본값 true 적용? 기존 User 설정 상속?)

P2 (적극적으로 고려해주세요)

  • DeviceNotificationSettingsResponse.java:16 - 파일 끝에 개행 문자가 누락되어 있습니다.
  • NotificationController.java:46 - 파일 끝에 개행 문자가 누락되어 있습니다.
  • PushNotificationTestRequest.java:22 - 파일 끝에 개행 문자가 누락되어 있습니다.
  • PushNotificationTestResponse.java:26 - 파일 끝에 개행 문자가 누락되어 있습니다.
  • NotificationController.java:23-33 - 테스트용 API를 인증 없이 공개하는 것은 보안 위험이 있습니다. 최소한 관리자 권한(ADMIN role) 검증이나 별도의 API 키 인증을 고려해주세요.

P3 (웬만하면 반영해주세요)

  • DeviceService.java:88-110 - updateNotificationSettings 메서드의 JavaDoc이 명확하지만, 실제 업데이트 전후 로그 추가를 권장합니다.
  • PushNotificationService.java:158-208 - sendTestNotification 메서드에 대한 단위 테스트 추가를 권장합니다.
  • DeviceRepository.java:20-21 - 쿼리의 조건이 중복됩니다. u.deletedAt IS NULL은 두 메서드 모두 필요한가요? @Where 어노테이션으로 엔티티 레벨에서 처리하는 것도 고려해보세요.
  • SecurityConfig.java:42 - 테스트 API 경로를 permitAll로 열어두는 것에 대한 주석 설명 추가를 권장합니다.

P4 (반영해도 좋고 넘어가도 좋습니다)

  • PushNotificationService.java:38-39 - 주석의 줄바꿈이 어색합니다. 첫 줄을 한 줄로 정리하면 가독성이 좋아집니다.
  • DeviceNotificationSettingsRequest.java - 기존 NotificationSettingsRequest.java에서 이름만 변경되었는데, git history를 위해 git mv를 사용했는지 확인해주세요.

긍정적인 부분

  • Device 단위 알림 설정으로의 전환이 논리적으로 타당하며, 코드 구조도 기존 패턴을 잘 따르고 있습니다.
  • FcmBatchResult record를 활용한 리팩토링으로 코드 중복이 제거되어 유지보수성이 향상되었습니다.

@qkrcodus
qkrcodus merged commit 7bd5980 into dev Jan 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@qkrcodus qkrcodus

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Fix] 알림 수신 설정을 Device 기준으로 변경 및 테스트용 알림 API 구현

1 participant

@qkrcodus
0