Skip to content

fix : NoticeMessageDto ID 형식 String으로 변경#348

Merged
rlagkswn00 merged 1 commit intodevelopfrom
fix/set-string-id-to-message
Feb 26, 2026
Merged

fix : NoticeMessageDto ID 형식 String으로 변경#348
rlagkswn00 merged 1 commit intodevelopfrom
fix/set-string-id-to-message

Conversation

@rlagkswn00
Copy link
Member

@rlagkswn00 rlagkswn00 commented Feb 26, 2026

#️⃣ 이슈

📌 요약

  • 알림 전송 간 data에 Long값 사용으로 알림이 발송되지 않는 문제를 발견하여 수정했습니다.

🛠️ 상세

  • FCM Message는 Map<String, String>만 지원합니다.
  • Jackson을 사용하면 Map<String, Object>로 반환됩니다.
  • 이에 Jackson을 사용하여 만든 data Map을 FCM Message Builder의 putAllData로 넣으면 예외 발생하여 알림이 보내지지 않는 것으로 원인을 진단했습니다.

💬 기타

@rlagkswn00 rlagkswn00 self-assigned this Feb 26, 2026
@rlagkswn00 rlagkswn00 added the 🐛 Bug Something isn't working label Feb 26, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 26, 2026

Walkthrough

NoticeMessageDto의 id 필드 타입을 Long에서 String으로 변경했습니다. 빌더 생성자 매개변수와 팩토리 메서드(from)에서 id 값을 String으로 변환하도록 업데이트했으며, 임포트 문을 명시적으로 지정했습니다.

Changes

Cohort / File(s) Summary
DTO 타입 변경
src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java
id 필드 타입을 Long에서 String으로 변경. 빌더 생성자 매개변수 업데이트, from(Notice)과 from(DepartmentNotice) 팩토리 메서드에서 id를 String.valueOf()로 변환하도록 수정. 임포트문을 와일드카드에서 명시적 형식으로 변경.

Possibly related PRs

Poem

🐰 긴 숫자들이 글씨로 변신했네,
String의 옷을 입은 id,
빌더가 맞춰주고 공장에서 만들어져,
타입의 춤을 춘다, 우아하게!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR 제목은 NoticeMessageDto의 ID 필드 타입을 String으로 변경하는 주요 변경사항을 명확하게 설명하고 있습니다.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/set-string-id-to-message

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java (1)

32-33: String 타입에 적합한 유효성 검증 사용 권장

id 타입이 String으로 변경되었으므로, Assert.notNull 대신 Assert.hasText를 사용하는 것이 더 적절합니다. 현재 검증은 빈 문자열("")을 허용하며, 에러 메시지("must not be empty")와 실제 검증 로직이 일치하지 않습니다.

♻️ 권장 수정안
     `@Builder`
     private NoticeMessageDto(String id, String articleId, String postedDate, String subject, String category, String categoryKorName, String baseUrl) {
-        Assert.notNull(id, "id must not be empty");
+        Assert.hasText(id, "id must not be empty");
         Assert.notNull(articleId, "articleId must not be null");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java`
around lines 32 - 33, The constructor NoticeMessageDto(String id, String
articleId, String postedDate, String subject, String category, String
categoryKorName, String baseUrl) currently uses Assert.notNull(id, "id must not
be empty") which allows empty strings; replace that check with
Assert.hasText(id, "id must not be empty") (and similarly update any other
String validations in this constructor if they use notNull) so the validation
rejects blank/empty id values and the message aligns with the check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java`:
- Around line 32-33: The constructor NoticeMessageDto(String id, String
articleId, String postedDate, String subject, String category, String
categoryKorName, String baseUrl) currently uses Assert.notNull(id, "id must not
be empty") which allows empty strings; replace that check with
Assert.hasText(id, "id must not be empty") (and similarly update any other
String validations in this constructor if they use notNull) so the validation
rejects blank/empty id values and the message aligns with the check.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6e5845e and 359170c.

📒 Files selected for processing (1)
  • src/main/java/com/kustacks/kuring/message/application/port/out/dto/NoticeMessageDto.java

@github-actions
Copy link

Unit Test Results

  81 files    81 suites   1m 35s ⏱️
570 tests 563 ✔️ 7 💤 0
573 runs  566 ✔️ 7 💤 0

Results for commit 359170c.

@rlagkswn00 rlagkswn00 merged commit e5e2d49 into develop Feb 26, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant