파싱 완료 알림을 제목=아이템 이름 / 본문=상태 문구로 분리 - #924
Conversation
- OS 푸시 제목은 줄바꿈 없이 뒤가 잘린다. 이름과 상태를 한 줄에 담으면 이름이 길 때 정작 무슨 일인지가 사라져서, 지금은 이름을 10글자로 잘라 막고 있었다 - 길이를 줄이려는 게 아니라 잘리는 자리를 고르려는 것이었고, 대가로 실제 상품명 대부분이 일찍 잘렸다 - 이름을 제목에, 상태를 본문에 둔다. 제목이 이름뿐이면 OS 가 잘라도 잃는 게 없고 body 는 두 줄까지 보인다. 그래서 파싱 완료의 표시 글자 절단을 없앴다(char 안전망만 유지) - body 문구는 등록 출처로 갈린다 - 위시에 직접 담은 것과 토너먼트에 직접 올린 것은 다른 사건이다(토너먼트에 올려도 위시리스트에 안 들어간다). 판정은 라우팅이 이미 하고 있어 그대로 재사용했고 새 조회가 없다 - 문장을 통째로 변수로 채운다. notification_templates 가 타입당 한 행(PK=type)이라 위시용·토너먼트용 body 를 따로 둘 자리가 없고, 두 문장이 구조도 달라 공통 뼈대 + 변수로도 안 쪼개진다. 대가로 이 body 는 백오피스(#252)에서 편집할 수 없다 - title 은 여전히 템플릿이 소유한다 - 절단 유틸을 ItemDisplayName 으로 공유하고 캡을 인자로 받게 했다. 두 알림의 요구가 다르다: 파싱 완료는 제목이 이름뿐이라 절단 없음, 아이템 삭제는 "OO님이 '{이름}'을(를) 삭제했어요" 라 이름 뒤에 문장이 붙어 캡이 필요하다 - TournamentItemDeletedHandler 의 절단 누락도 함께 고쳤다. 상품명이 512자까지 허용되는데 캡이 없어, 긴 이름이 엔티티 불변식(require(title.length <= 255))에 걸리면 dispatcher 의 runCatching 이 예외를 삼켜 삭제 알림이 전 수신자에게 조용히 누락됐다 - 문서·example 갱신: "body 는 전 타입 빈 문자열" 서술 폐기, 출처별 문구 설명 추가. example 의 상태 문구는 리터럴 대신 핸들러 상수를 끌어와 문구가 바뀌면 따라오게 했다 - 테스트: ItemDisplayNameTest 로 이관하며 절단 없음/있음 양쪽 계약을 고정(이모지·ZWJ·조합부호·공백 정규화), 출처별 completionMessage 통합테스트 2건 추가 클라 대응(TeamPiKi/client#460)이 선행 또는 동반돼야 한다 - 지금 카드는 title 만 렌더해서 서버만 먼저 나가면 상품명만 보인다. API 계약은 안 바뀌어(body 필드 이미 존재) 클라를 먼저 배포해도 안전하다.
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
Walkthrough파싱 완료 알림의 제목을 아이템 이름으로 분리하고, 본문에 고정 완료 문구를 설정했습니다. 공통 아이템 표시명 처리기를 추가하고 삭제 알림에 적용했습니다. 템플릿 마이그레이션, API 문서, 통합 테스트를 갱신했습니다. Changes알림 템플릿 분리
Estimated code review effort: 3 (Moderate) | ~20 minutes Assessment against linked issues
Sequence Diagram(s)sequenceDiagram
participant ParsingEvent
participant ItemParsingCompletedHandler
participant NotificationTemplate
ParsingEvent->>ItemParsingCompletedHandler: parsing completed event
ItemParsingCompletedHandler->>NotificationTemplate: itemName
NotificationTemplate-->>ParsingEvent: title itemName and fixed body message
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/kotlin/com/depromeet/piki/notification/handler/TournamentItemDeletedHandler.kt (1)
34-35: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win삭제 알림 handler의 회귀 테스트를 추가하세요.
제공된 테스트는 파싱 완료 알림의
completionMessage만 검증합니다.TournamentItemDeletedHandler.resolveActorContext가 10 grapheme으로 절단된 값을itemName에 넣는지는 검증하지 않습니다. 긴 상품명, 이모지 또는 결합 문자가 포함된 상품명, 누락된 snapshot을 각각 사용해context.variables["itemName"]을 검증하세요.As per path instructions: 핵심 비즈니스 규칙, 예외 케이스, 경계값 검증을 우선합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/depromeet/piki/notification/handler/TournamentItemDeletedHandler.kt` around lines 34 - 35, 파싱 완료 알림만 검증하는 테스트에 TournamentItemDeletedHandler.resolveActorContext의 itemName 회귀 테스트를 추가하세요. 긴 상품명은 10 grapheme으로 절단되는지, 이모지와 결합 문자가 포함된 이름은 grapheme 단위로 보존되는지, snapshot이 없으면 해당 기본값이 사용되는지를 각각 검증하고 context.variables["itemName"]을 확인하세요.Source: Path instructions
src/main/kotlin/db/migration/V20260811010101__split_item_parsing_completed_template_title_body.kt (1)
27-29: 🗄️ Data Integrity & Integration | 🔵 Trivial클라이언트가
body를 렌더링한 뒤 마이그레이션을 배포하세요.현재
production,staging,dev의NotificationContent는notification.title만 표시합니다. 이 마이그레이션 후에는 제목이 아이템 이름만 포함하므로, 클라이언트 배포 전에 적용하면 파싱 완료 상태가 사라집니다. 클라이언트에서notification.body를 함께 표시하거나, 호환 기간에는 기존 완료 문구를 제목에도 유지하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/db/migration/V20260811010101__split_item_parsing_completed_template_title_body.kt` around lines 27 - 29, Update the migration’s notification template around statement.setString(1) and statement.setString(2) so clients remain compatible: either ensure production, staging, and dev clients render notification.body before applying this migration, or preserve the existing completion text in the title during the compatibility period. Do not deploy the title-only template until the client rendering path supports body.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.kt`:
- Around line 36-38: Update the API documentation text in NotificationHistoryApi
so it no longer states that every body except ITEM_PARSING_COMPLETED is empty.
Document that ITEM_PARSING_COMPLETED uses a source-specific status message in
body, while ANNOUNCEMENT provides the administrator-entered announcement content
in body; preserve the existing title behavior and client handling guidance.
In
`@src/main/kotlin/com/depromeet/piki/notification/handler/ItemParsingCompletedHandler.kt`:
- Around line 40-42: ItemParsingCompletedHandler의 수신자 처리에서 단일 resolveRouting 및
completionMessageOf 결과를 전체 수신자에게 재사용하지 마세요. 각 수신자에 대한 userId와
NotificationRouting을 함께 구성한 뒤 라우팅별로 그룹화하고, 그룹별로 해당 본문과 알림을 생성해 위시 수신자에게 위시
라우팅·문구가, 토너먼트 등록자에게 토너먼트 라우팅·문구가 전달되도록 수정하세요. 동일한 snapshotId를 공유하는 위시와 토너먼트의
dispatch 통합 테스트에서 수신자별 body와 routing을 검증하세요.
---
Nitpick comments:
In
`@src/main/kotlin/com/depromeet/piki/notification/handler/TournamentItemDeletedHandler.kt`:
- Around line 34-35: 파싱 완료 알림만 검증하는 테스트에
TournamentItemDeletedHandler.resolveActorContext의 itemName 회귀 테스트를 추가하세요. 긴 상품명은
10 grapheme으로 절단되는지, 이모지와 결합 문자가 포함된 이름은 grapheme 단위로 보존되는지, snapshot이 없으면 해당
기본값이 사용되는지를 각각 검증하고 context.variables["itemName"]을 확인하세요.
In
`@src/main/kotlin/db/migration/V20260811010101__split_item_parsing_completed_template_title_body.kt`:
- Around line 27-29: Update the migration’s notification template around
statement.setString(1) and statement.setString(2) so clients remain compatible:
either ensure production, staging, and dev clients render notification.body
before applying this migration, or preserve the existing completion text in the
title during the compatibility period. Do not deploy the title-only template
until the client rendering path supports body.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b889c20-a872-4178-88de-980d3f6d12aa
📒 Files selected for processing (10)
src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.ktsrc/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApiExamples.ktsrc/main/kotlin/com/depromeet/piki/notification/handler/ItemDisplayName.ktsrc/main/kotlin/com/depromeet/piki/notification/handler/ItemParsingCompletedHandler.ktsrc/main/kotlin/com/depromeet/piki/notification/handler/TournamentItemDeletedHandler.ktsrc/main/kotlin/com/depromeet/piki/notification/service/NotificationTemplateVariables.ktsrc/main/kotlin/db/migration/V20260811010101__split_item_parsing_completed_template_title_body.ktsrc/test/kotlin/com/depromeet/piki/notification/handler/ItemDisplayNameTest.ktsrc/test/kotlin/com/depromeet/piki/notification/handler/ItemParsingCompletedHandlerTest.ktsrc/test/kotlin/com/depromeet/piki/notification/handler/NotificationEventHandlerIntegrationTest.kt
💤 Files with no reviewable changes (1)
- src/test/kotlin/com/depromeet/piki/notification/handler/ItemParsingCompletedHandlerTest.kt
CodeRabbit 리뷰 대응. - 문서가 "body 는 ITEM_PARSING_COMPLETED 만 값이 있다" 고 단정했는데 ANNOUNCEMENT 도 body 변수를 갖는다(관리자가 입력한 공지 본문). 클라가 공지 본문을 숨기거나 잘못 처리할 수 있어 두 타입을 함께 명시한다. - TournamentItemDeletedHandler 가 이번에 도입한 10자 절단은 ItemDisplayName 단위 테스트가 규칙을 망라하지만, 핸들러가 그 규칙에 이름을 실제로 통과시키는지는 검증되지 않았다. 긴 이름·이모지 두 케이스로 위임을 고정한다. 이모지 케이스는 입력 설계에 두 가지를 반영했다. - 단순 이모지(1 grapheme = 2 char)를 써서 절단 결과가 char 안전망(MAX_CHARS) 아래에 남게 했다. 가족 이모지처럼 1 grapheme 이 11 char 인 입력은 안전망에 걸리는데, 그 안전망의 코드 유닛 절단은 "조합 부호를 쌓은 비정상 입력에서 글자 깨짐보다 알림 누락 방지를 택한다" 는 의도된 트레이드오프라 검증 대상이 아니다. - 앞에 1 char 를 둬 절단 경계를 홀수로 밀었다. 이모지만 있으면 코드 유닛으로 잘라도 짝이 맞아떨어져 회귀가 드러나지 않는다. grapheme 절단을 코드 유닛 절단으로 바꾸면 실패하는 것을 실측 확인했다.
CodeRabbit nitpick 2건 처리① 삭제 핸들러 회귀 테스트 — 반영 (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/kotlin/com/depromeet/piki/notification/handler/NotificationRecipientResolutionIntegrationTest.kt`:
- Around line 147-149: Update the deletion-notification assertions around
itemName and ItemDisplayName to assertEquals the exact contract: 10 grapheme
units with the expected ellipsis handling, rather than only checking prefix and
shorter length. In the surrogate-pair test, extract the input into sourceName
and assert that the resolved itemName is shorter than sourceName while retaining
the existing surrogate-integrity checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 95ec1c44-9614-4146-8d38-e6ca6b919bce
📒 Files selected for processing (2)
src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.ktsrc/test/kotlin/com/depromeet/piki/notification/handler/NotificationRecipientResolutionIntegrationTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.kt
출처별로 문구를 가르려 했는데(위시 "위시 저장이 성공했어요" / 토너먼트 "아이템이 등록됐어요"), 지금 구조에선 위시 수신자에게 토너먼트 문구가 갈 수 있다. dispatcher 는 라우팅과 title/body 렌더를 수신자 루프 밖에서 한 번만 해석해 전 수신자에게 같은 값을 박는다. 그런데 한 snapshot 에 서로 다른 출처의 수신자가 함께 붙을 수 있다 - 공유 정체성(#825)의 "진행 중 합류" 경로다. A 가 URL 을 위시에 담아 파싱이 도는 중에 B 가 같은 URL 을 토너먼트에 올리면, resolveAttachment 가 그 진행 중 snapshot 을 그대로 물려줘 tournament_item(B) 와 wish(A) 가 같은 버전을 가리킨다. 파싱이 끝나면 수신자는 둘인데 라우팅은 firstOrNull 이 고른 토너먼트 하나라, A 가 토너먼트 문구를 받는다. 딥링크가 어긋나는 것 자체는 이 PR 이전부터 있었고 resolveRouting 주석이 알면서 수용한다고 적어 뒀다. 다만 문구까지 그 라우팅에서 파생시키면 이 PR 이 "거짓 문구" 를 새로 만드는 셈이라, 분기를 걷어내고 단일 문구로 되돌린다. - body_template 을 변수 없는 고정 문구("파싱이 완료되었어요")로 둔다. 부수 효과로 백오피스(#252) 편집 손실이 사라졌다 - 문장을 통째로 변수에 담느라 잃었던 것이라, 분기를 접으니 title·body 둘 다 다시 템플릿이 온전히 소유한다 - 변수 카탈로그에서 completionMessage 를 제거하고 itemName 만 남긴다 - example 의 두 파싱 항목이 같은 body 를 쓰는 게 계약이라 상수로 묶고, 문구 소유자가 DB 템플릿임을 주석으로 남긴다 (referenceItem 의 title 과 같은 방식) - 회귀 가드: 출전 pin 이 있어도 문구 변수가 itemName 하나뿐임을 단언한다. 수신자별 해석 없이 분기를 되살리면 여기서 깨진다 출처별 문구는 수신자별 라우팅 해석·wishId 딥링크와 함께 #933 에서 다룬다. 삭제 알림 테스트 단언도 함께 조였다. "짧아졌나" 와 "짝 잃은 surrogate 가 없나" 만 보고 있어서, 핸들러가 캡을 10 대신 20 으로 바꾸거나 이모지 이름을 아예 안 잘라도 통과했다. ItemDisplayNameTest 는 캡을 인자로 받아 검증하므로 핸들러가 고른 캡 값은 이 테스트에서만 고정된다. 정확한 기대값으로 못 박았다.
…tle-body-split' into feat/913-parsing-notification-title-body-split
|
@coderabbitai review body 의 nitpick 2건도 확인했습니다. 이미 반영됨 - 삭제 알림 핸들러 회귀 테스트
이 nitpick 은 그 커밋 이전 상태( 정정 필요 - 배포 순서결론(클라 먼저)에는 동의하지만 영향 범위 서술이 실제보다 넓습니다. 클라 코드를 직접 확인했습니다.
즉 "파싱 완료 상태가 사라집니다" 는 히스토리 카드 한 곳에 한정됩니다. 푸시와 토스트는 클라 수정 없이 바로 두 줄이 되며, 이게 이 PR 의 본래 목적입니다. 그래서 "호환 기간에 제목에 완료 문구를 유지" 하는 제안은 채택하지 않습니다. 그렇게 하면 이 PR 이 고치려는 문제(제목 한 줄에 이름과 상태가 함께 들어가 이름이 길면 상태가 잘려 사라짐)가 그대로 남고, 이름 절단(10글자)도 되살려야 합니다. 퇴행이 실시간 경로가 아니라 되짚기 경로 한 곳이고 레이아웃·딥링크는 그대로라, 그 대가를 치를 만큼은 아니라고 봤습니다. 클라 대응은 TeamPiKi/client#460 으로 별도 추적 중이며, 이 PR 머지 시점은 그쪽 진행에 맞춥니다. 한 가지 단서: 위 클라 확인은 로컬 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.kt (1)
36-39: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
body렌더링 클라이언트 배포를 릴리스 조건으로 고정하세요.현재 클라이언트가
title만 렌더링하면 서버가body에 저장한"파싱이 완료되었어요"가 사용자에게 표시되지 않습니다.TeamPiKi/client#460을 선행 또는 동반 배포하고, 파싱 완료 알림 카드가 비어 있지 않은body를 표시하는 검증을 릴리스 체크리스트에 추가하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.kt` around lines 36 - 39, NotificationHistoryApi의 알림 body 계약 변경과 함께 body를 렌더링하는 클라이언트 변경(TeamPiKi/client#460)을 선행 또는 동반 배포로 고정하세요. 파싱 완료 알림 카드가 비어 있지 않은 body, 특히 “파싱이 완료되었어요”를 표시하는지 릴리스 체크리스트의 검증 항목에 추가하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.kt`:
- Around line 36-39: NotificationHistoryApi의 알림 body 계약 변경과 함께 body를 렌더링하는 클라이언트
변경(TeamPiKi/client#460)을 선행 또는 동반 배포로 고정하세요. 파싱 완료 알림 카드가 비어 있지 않은 body, 특히 “파싱이
완료되었어요”를 표시하는지 릴리스 체크리스트의 검증 항목에 추가하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1663dbd-94e5-463f-be51-8391e578d558
📒 Files selected for processing (7)
src/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApi.ktsrc/main/kotlin/com/depromeet/piki/notification/controller/NotificationHistoryApiExamples.ktsrc/main/kotlin/com/depromeet/piki/notification/handler/ItemParsingCompletedHandler.ktsrc/main/kotlin/com/depromeet/piki/notification/service/NotificationTemplateVariables.ktsrc/main/kotlin/db/migration/V20260811010101__split_item_parsing_completed_template_title_body.ktsrc/test/kotlin/com/depromeet/piki/notification/handler/NotificationEventHandlerIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/notification/handler/NotificationRecipientResolutionIntegrationTest.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/kotlin/com/depromeet/piki/notification/handler/NotificationRecipientResolutionIntegrationTest.kt
- src/main/kotlin/db/migration/V20260811010101__split_item_parsing_completed_template_title_body.kt
|
Tip For best results, initiate chat on the files or code changes.
삭제 알림 회귀 테스트 nitpick은 배포 순서에 대한 기존 설명은 영향 범위를 과도하게 넓게 표현했습니다. 정정합니다.
✏️ Learnings added
🧠 Learnings usedYou are interacting with an AI system. |
Situation
{상품명} 파싱이 완료되었어요를 다 담고 본문은 비어 있다.TournamentItemService는Wish행을 만들지 않는다). 두 경로가 다른 사건인데 같은 문구를 쓰고 있었다.Task
Action
왜 제목에 이름만 두면 절단이 사라지나
제목이 이름 하나뿐이면 OS 가 뒤를 잘라도 잃는 정보가 없다. 무슨 일인지는 본문이 들고 있고, 본문은 두 줄까지 보이며 펼치면 전부 보인다. 그래서 파싱 완료 경로의 표시 글자 절단을 없앴다.
{10글자로 자른 이름} 파싱이 완료되었어요{이름}(절단 없음){상태 문구}절단을 없애도 글자 수 안전망은 남긴다. 조합 부호를 쌓으면 눈에 보이는 글자 1개가 수백 char 이 될 수 있고, 그대로 두면 알림 엔티티의 불변식(
require(title.length <= 255))에 걸린다. 그 예외는 dispatcher 가 삼켜서 알림이 전 수신자에게 조용히 누락된다.본문 문구를 출처로 나눈 이유
위시 저장이 성공했어요아이템이 등록됐어요출처 판정은 새로 만들지 않았다. 알림 라우팅이 이미 "그 버전을 pin 한 출전이 있나"로 같은 판정을 하고 있어서, 그 결과를 그대로 재사용한다. 조회가 늘지 않는다.
실패 알림은 손대지 않았다. 실패한 버전은 이름이 비어 있어(추출 자체가 실패) 제목에 넣을 이름이 없다. 논의 중 성공과 같은 모양(
{이름} 파싱이 실패했어요)도 검토했지만, 이름이 없어 기본값상품으로 채워지면 "상품 파싱이 실패했어요" 라는 어색한 문구가 된다. 현행 유지.문장을 통째로 변수에 담은 트레이드오프
${completionMessage}한 변수로 두고 문장을 통째로 채움{X}이 성공했어요식)제목 템플릿(
${itemName})은 여전히 백오피스가 소유한다. 편집 능력을 잃는 건 본문 한 문구뿐이고, 이 사실을 변수 카탈로그에 주석으로 남겼다.함께 고친 기존 버그
아이템 삭제 알림에 절단 캡이 없었다. 이 알림은 제목이
{닉네임}님이 '{이름}'을(를) 삭제했어요라 이름 뒤에 문장이 붙는다. 상품명은 512자까지 허용되는데 캡이 없어, 긴 이름이 들어오면 위에 적은 그 불변식에 걸려 삭제 알림이 전 수신자에게 조용히 누락됐다. 캡 10글자를 적용했다.이 발견이 절단 유틸을 공유하되 캡을 인자로 받게 만든 이유다. 두 알림의 요구가 정반대라 캡을 고정하면 한쪽이 틀린다.
문서
kind를 파생시키는 것과 같은 결).Result
연관 이슈
Summary by CodeRabbit
개선 사항
문서