[REFACTOR] notification transaction flow - #79
Merged
Merged
Conversation
📝 WalkthroughWalkthrough견적 확정, 견적 전송·반려, 리뷰 생성 과정에서 알림 생성을 트랜잭션 내부로 이동했습니다. 트랜잭션이 완료된 뒤 알림을 전송하고, 서비스 응답에는 기존 도메인 객체만 반환합니다. Changes트랜잭션 기반 알림 처리
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Service
participant Transaction
participant Notification
Service->>Transaction: 도메인 작업 및 알림 생성
Transaction-->>Service: 도메인 객체와 알림 반환
Service->>Notification: 트랜잭션 완료 후 알림 전송
Service-->>Service: 응답에서 알림 제외
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🤖 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/modules/estimate/customer/customer-estimate.service.ts`:
- Line 448: Update the notification content near the estimate creation flow to
be a complete sentence, including the customer name and a clear statement that
the estimate has been confirmed, rather than ending with the possessive particle
in isolation.
- Around line 443-453: customer-estimate.service.ts 443-453,
mover-estimate.service.ts 259-269 및 359-369, review.service.ts 256-266의
createNotification 호출에 동일한 tx를 전달하세요. NotificationService.createNotification이
DbClient를 받아 notificationRepository.create에 전달하고 도메인 변경과 같은 트랜잭션으로 저장하도록 수정하며,
SSE 전송은 커밋 이후 수행하세요. 공유 notificationService export에 호출부가 사용하는 sendNotification
메서드도 포함하세요.
In `@src/modules/review/review.service.ts`:
- Line 261: Update the notification payload’s content near the review creation
flow so it stores the complete user-facing notification sentence rather than
only the fragment "고객님이". Include all wording and dynamic values required for
the client to display a complete message, using the surrounding payload data and
existing notification conventions.
- Around line 256-279: Update the review transaction around createdReview to
call createNotification with only the CreateNotificationInput argument, removing
the unsupported tx parameter. Use the supported notification creation and
post-commit delivery APIs instead of notificationService.sendNotification,
ensuring the notification is persisted within the review transaction and its SSE
event is emitted only after the transaction commits.
🪄 Autofix (Beta)
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.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8cf0c63a-304a-48d8-bb9b-ee2b0c6544e5
📒 Files selected for processing (3)
src/modules/estimate/customer/customer-estimate.service.tssrc/modules/estimate/mover/mover-estimate.service.tssrc/modules/review/review.service.ts
Comment on lines
+443
to
+453
| const notification = await notificationService.createNotification( | ||
| { | ||
| userId: confirmedEstimate.mover.id, | ||
| type: "ESTIMATE_CONFIRMED", | ||
| title: "견적 확정", | ||
| content: `${estimate.estimateRequest.customer.name}님의`, | ||
| linkUrl: "/estimate/received-requests", | ||
| expiresAt: getKstEndOfDay(estimate.estimateRequest.moveDate), | ||
| }, | ||
| tx, | ||
| ); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -a '^notification\.service\.ts$' src -x ast-grep outline {} --items all
fd -a '^notification\.service\.ts$' src -x rg -n -C 8 'createNotification|sendNotification' {}
rg -n -C 6 'createNotification\(' \
src/modules/estimate/customer/customer-estimate.service.ts \
src/modules/estimate/mover/mover-estimate.service.tsRepository: 4roro-moving/moving-backend
Length of output: 5511
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- notification service ---'
sed -n '1,230p' src/modules/notification/notification.service.ts
printf '%s\n' '--- notification repository ---'
sed -n '1,180p' src/modules/notification/notification.repository.ts
printf '%s\n' '--- transaction utility and notification types ---'
sed -n '1,220p' src/utils/transaction.ts
rg -n -C 5 'CreateNotificationInput|NotificationItem|PrismaClient|TransactionClient' \
src/modules/notification src/modules/estimate
printf '%s\n' '--- all createNotification call shapes ---'
rg -n -U -P 'createNotification\s*\([^;]*?\)' src --glob '*.ts' || trueRepository: 4roro-moving/moving-backend
Length of output: 21480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- estimate and review transaction contexts ---'
sed -n '220,280p' src/modules/estimate/mover/mover-estimate.service.ts
sed -n '330,380p' src/modules/estimate/mover/mover-estimate.service.ts
sed -n '410,465p' src/modules/estimate/customer/customer-estimate.service.ts
sed -n '230,275p' src/modules/review/review.service.ts
printf '%s\n' '--- notification repository create implementation ---'
sed -n '235,275p' src/modules/notification/notification.repository.ts
printf '%s\n' '--- static call/signature verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
service = Path("src/modules/notification/notification.service.ts").read_text()
match = re.search(
r"const\s+createNotification\s*=\s*async\s*\((.*?)\)\s*:\s*Promise",
service,
re.S,
)
if not match:
raise SystemExit("createNotification declaration not found")
params = [p.strip() for p in match.group(1).split(",") if p.strip()]
print(f"notificationService.createNotification declared parameters: {len(params)}")
print("\n".join(f" {i + 1}: {p}" for i, p in enumerate(params)))
for path in Path("src/modules").rglob("*.service.ts"):
text = path.read_text()
for m in re.finditer(r"notificationService\.createNotification\s*\(", text):
start = m.start()
depth = 0
end = None
for i in range(text.find("(", start), len(text)):
if text[i] == "(":
depth += 1
elif text[i] == ")":
depth -= 1
if depth == 0:
end = i + 1
break
if end is None:
continue
args = text[text.find("(", start) + 1:end - 1]
# Top-level commas count arguments without executing repository code.
level = 0
commas = 0
for ch in args:
if ch in "({[":
level += 1
elif ch in ")}]":
level -= 1
elif ch == "," and level == 0:
commas += 1
print(f"{path}:{text[:start].count(chr(10)) + 1}: call arguments={commas + 1}")
PYRepository: 4roro-moving/moving-backend
Length of output: 7560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- estimate and review transaction contexts ---'
sed -n '220,280p' src/modules/estimate/mover/mover-estimate.service.ts
sed -n '330,380p' src/modules/estimate/mover/mover-estimate.service.ts
sed -n '410,465p' src/modules/estimate/customer/customer-estimate.service.ts
sed -n '230,275p' src/modules/review/review.service.ts
printf '%s\n' '--- notification repository create implementation ---'
sed -n '235,275p' src/modules/notification/notification.repository.ts
printf '%s\n' '--- static call/signature verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
service = Path("src/modules/notification/notification.service.ts").read_text()
match = re.search(
r"const\s+createNotification\s*=\s*async\s*\((.*?)\)\s*:\s*Promise",
service,
re.S,
)
if not match:
raise SystemExit("createNotification declaration not found")
params = [p.strip() for p in match.group(1).split(",") if p.strip()]
print(f"notificationService.createNotification declared parameters: {len(params)}")
print("\n".join(f" {i + 1}: {p}" for i, p in enumerate(params)))
for path in Path("src/modules").rglob("*.service.ts"):
text = path.read_text()
for m in re.finditer(r"notificationService\.createNotification\s*\(", text):
start = m.start()
depth = 0
end = None
for i in range(text.find("(", start), len(text)):
if text[i] == "(":
depth += 1
elif text[i] == ")":
depth -= 1
if depth == 0:
end = i + 1
break
if end is None:
continue
args = text[text.find("(", start) + 1:end - 1]
level = 0
commas = 0
for ch in args:
if ch in "({[":
level += 1
elif ch in ")}]":
level -= 1
elif ch == "," and level == 0:
commas += 1
print(f"{path}:{text[:start].count(chr(10)) + 1}: call arguments={commas + 1}")
PYRepository: 4roro-moving/moving-backend
Length of output: 7560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- notification service exports and send calls ---'
rg -n -C 4 'sendNotification|export const notificationService|notificationService' \
src/modules/notification/notification.service.ts \
src/modules/estimate/mover/mover-estimate.service.ts \
src/modules/estimate/customer/customer-estimate.service.ts \
src/modules/review/review.service.ts
printf '%s\n' '--- imports of notification service ---'
rg -n -C 3 'notificationService' \
src/modules/estimate/mover/mover-estimate.service.ts \
src/modules/estimate/customer/customer-estimate.service.ts \
src/modules/review/review.service.tsRepository: 4roro-moving/moving-backend
Length of output: 16649
NotificationService의 트랜잭션 및 SSE 계약을 함께 반영하세요.
createNotification은 현재 인자 하나만 받지만, 다음 네 위치는 tx를 전달하므로 컴파일 오류가 발생합니다.
src/modules/estimate/customer/customer-estimate.service.ts:443src/modules/estimate/mover/mover-estimate.service.ts:259src/modules/estimate/mover/mover-estimate.service.ts:359src/modules/review/review.service.ts:256
notificationRepository.create가 지원하는 DbClient를 NotificationService까지 전달하세요. 알림 저장은 도메인 변경과 같은 트랜잭션에서 수행해야 합니다. SSE 전송은 트랜잭션 커밋 후 수행하세요.
또한 호출부의 notificationService.sendNotification(...) 사용에 맞게 공유 서비스가 해당 메서드를 export해야 합니다. 현재 notificationService export에는 이 메서드가 없습니다.
📍 Affects 2 files
src/modules/estimate/customer/customer-estimate.service.ts#L443-L453(this comment)src/modules/estimate/mover/mover-estimate.service.ts#L259-L269src/modules/estimate/mover/mover-estimate.service.ts#L359-L369
🤖 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/modules/estimate/customer/customer-estimate.service.ts` around lines 443
- 453, customer-estimate.service.ts 443-453, mover-estimate.service.ts 259-269 및
359-369, review.service.ts 256-266의 createNotification 호출에 동일한 tx를 전달하세요.
NotificationService.createNotification이 DbClient를 받아
notificationRepository.create에 전달하고 도메인 변경과 같은 트랜잭션으로 저장하도록 수정하며, SSE 전송은 커밋 이후
수행하세요. 공유 notificationService export에 호출부가 사용하는 sendNotification 메서드도 포함하세요.
Source: Path instructions
| userId: confirmedEstimate.mover.id, | ||
| type: "ESTIMATE_CONFIRMED", | ||
| title: "견적 확정", | ||
| content: `${estimate.estimateRequest.customer.name}님의`, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
알림 내용을 완전한 문장으로 설정하세요.
현재 알림 본문은 홍길동님의처럼 조사로 끝납니다. 수신자는 알림 본문만으로 이벤트를 이해할 수 없습니다. 예를 들어 "${estimate.estimateRequest.customer.name}님의 견적이 확정되었습니다."처럼 완료된 내용을 저장하세요.
🤖 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/modules/estimate/customer/customer-estimate.service.ts` at line 448,
Update the notification content near the estimate creation flow to be a complete
sentence, including the customer name and a clear statement that the estimate
has been confirmed, rather than ending with the possessive particle in
isolation.
Comment on lines
+256
to
+279
| const notification = await notificationService.createNotification( | ||
| { | ||
| userId: estimate.moverId, | ||
| type: "REVIEW_RECEIVED", | ||
| title: "리뷰 도착", | ||
| content: "고객님이", | ||
| linkUrl: null, | ||
| expiresAt: null, | ||
| }, | ||
| tx, | ||
| ); | ||
|
|
||
| return { | ||
| review: createdReview, | ||
| notification, | ||
| }; | ||
| }, | ||
| { | ||
| // 같은 기사님에게 여러 리뷰가 동시에 등록될 때 통계 재계산 결과가 덮어써지는 것을 방지 | ||
| isolationLevel: Prisma.TransactionIsolationLevel.Serializable, | ||
| }, | ||
| ); | ||
|
|
||
| await notificationService.createNotification({ | ||
| userId: estimate.moverId, | ||
| type: "REVIEW_RECEIVED", | ||
| title: "리뷰 도착", | ||
| content: "고객님이", | ||
| linkUrl: null, | ||
| expiresAt: null, | ||
| }); | ||
| notificationService.sendNotification(estimate.moverId, result.notification); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# NotificationService 정의와 호출 계약을 확인합니다.
fd -t f '^notification\.service\.ts$' src | while IFS= read -r file; do
ast-grep outline "$file" --items all
rg -n -C 8 '\b(createNotification|sendNotification)\s*\(' "$file"
done
# 서비스 호출부와 전달 인자를 확인합니다.
rg -n -C 6 '\b(createNotification|sendNotification)\s*\(' srcRepository: 4roro-moving/moving-backend
Length of output: 11215
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- notification.service.ts ---'
cat -n src/modules/notification/notification.service.ts | sed -n '1,235p'
printf '%s\n' '--- notification types and repository ---'
fd -t f . src/modules/notification | sort
rg -n -C 5 'CreateNotificationInput|NotificationItem|notificationService|sendNotification|createNotification' src/modules/notification src/modules/review/review.service.ts
printf '%s\n' '--- review imports and transaction context ---'
cat -n src/modules/review/review.service.ts | sed -n '1,40p;210,290p'Repository: 4roro-moving/moving-backend
Length of output: 27822
NotificationService 호출 계약을 수정하세요.
createNotification은 CreateNotificationInput 하나만 받으며 tx를 지원하지 않습니다. notificationService에는 sendNotification도 없습니다. 현재 코드는 컴파일되지 않습니다. 또한 알림을 리뷰 트랜잭션에 포함하고 커밋 후 SSE를 전송하도록 호출 구조를 수정해야 합니다.
🤖 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/modules/review/review.service.ts` around lines 256 - 279, Update the
review transaction around createdReview to call createNotification with only the
CreateNotificationInput argument, removing the unsupported tx parameter. Use the
supported notification creation and post-commit delivery APIs instead of
notificationService.sendNotification, ensuring the notification is persisted
within the review transaction and its SSE event is emitted only after the
transaction commits.
| userId: estimate.moverId, | ||
| type: "REVIEW_RECEIVED", | ||
| title: "리뷰 도착", | ||
| content: "고객님이", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
완전한 알림 본문을 저장하세요.
Line 261의 content는 "고객님이"만 저장합니다. 이 payload에는 클라이언트가 문장을 완성할 값이 없습니다. 사용자는 불완전한 알림을 받습니다.
수정 예시
- content: "고객님이",
+ content: "고객님이 리뷰를 남겼습니다.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| content: "고객님이", | |
| content: "고객님이 리뷰를 남겼습니다.", |
🤖 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/modules/review/review.service.ts` at line 261, Update the notification
payload’s content near the review creation flow so it stores the complete
user-facing notification sentence rather than only the fragment "고객님이". Include
all wording and dynamic values required for the client to display a complete
message, using the surrounding payload data and existing notification
conventions.
wkdalswn11
reviewed
Aug 3, 2026
Contributor
There was a problem hiding this comment.
견적 확정과 알림 DB 저장을 동일 트랜잭션에 포함하고,
생성된 알림을 반환한 뒤 커밋 이후 sendNotification()으로 SSE를 전송하도록
의도한 구조에 맞게 잘 수정된 것 같습니다.
고생하셨습니다¡
Contributor
There was a problem hiding this comment.
견적 제안·반려 모두 알림 저장을 핵심 작업과 같은 트랜잭션에 포함하고,
커밋 이후 sendNotification()으로 SSE를 전송하도록
의도한 구조에 맞게 잘 수정된 것 같습니다.
고생하셨어요¡
Contributor
There was a problem hiding this comment.
알림 저장과 SSE 전송 구조는 의도한 방식대로 잘 적용된 것 같습니다.
리뷰 생성·기사 리뷰 통계 갱신·알림 DB 저장을 동일 트랜잭션으로 처리하고,
커밋 이후 sendNotification()으로 SSE를 전송하도록 정상적으로 분리되어 있습니다.
고생하셨어용¡
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 작업 내용
견적/리뷰 도메인의 알림 호출부를 변경 예정인 NotificationService 구조에 맞춰 선반영했습니다.
✅ 변경 사항
notificationService.sendNotification()으로 SSE 전송을 호출하도록 변경🧪 테스트
테스트 방법
📷 스크린샷 (선택)
🔥 체크리스트
🙏 To Reviewer
notification.service.ts를 수정하지 않고, 견적/리뷰 도메인 호출부만 변경합니다.Summary by CodeRabbit