feat: 견적 제안 및 반려 알림 연동 - #70
Conversation
…into feature/received-estimate-requests
…into feature/received-estimate-requests
📝 WalkthroughWalkthrough견적 전송과 반려 처리에서 트랜잭션 결과를 사용해 고객 알림을 생성합니다. 기사 닉네임과 반려 알림 만료 정책을 추가합니다. 반려 목록 커서는 1 이상의 정수만 허용합니다. Changes견적 알림 처리
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant sendEstimate
participant runTransaction
participant findMoverProfile
participant Notification
sendEstimate->>runTransaction: 견적 생성 트랜잭션 실행
runTransaction->>findMoverProfile: 기사 프로필 조회
findMoverProfile-->>runTransaction: 기사 닉네임 반환
runTransaction-->>sendEstimate: 견적 및 알림 메타데이터 반환
sendEstimate->>Notification: 고객 알림 생성
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: 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/modules/estimate/estimate.service.ts`:
- Around line 291-298: estimate 처리 트랜잭션에서 createNotification을 커밋 후 동기 호출하지 않도록
변경하세요. 견적 수신 및 반려 처리와 알림 이벤트 기록을 동일한 runTransaction 안에서 원자적으로 저장하고, 커밋 후 별도 비동기
작업이 이벤트를 처리하며 실패 시 재시도하도록 분리하세요. 구현은 estimate 서비스의 해당 트랜잭션 흐름과 양쪽
createNotification 호출에만 적용하고 서비스가 비즈니스 규칙과 트랜잭션을 계속 담당하게 하세요.
🪄 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: b02484a1-0502-4a7c-807a-60c23f5af83d
📒 Files selected for processing (3)
src/modules/estimate/estimate.repository.tssrc/modules/estimate/estimate.service.tssrc/modules/estimate/estimate.validator.ts
| await notificationService.createNotification({ | ||
| userId: result.customerId, | ||
| type: "ESTIMATE_RECEIVED", | ||
| title: "견적 도착", | ||
| content: `${result.moverNickname} 기사님의 ${MOVE_TYPE_LABEL[result.moveType]} 견적`, | ||
| linkUrl: null, | ||
| expiresAt: result.expiresAt, | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
커밋 후 알림 실패를 별도 재시도 경로로 분리해 주세요.
runTransaction이 견적/반려를 이미 커밋한 뒤 createNotification을 await하므로, 알림 저장 실패 시 도메인 작업은 성공했지만 API는 오류로 끝납니다. 재시도하면 이미 처리된 요청으로 거절되어 고객 알림이 누락될 수 있습니다. 알림 이벤트(outbox)를 트랜잭션에 기록한 뒤 커밋 후 재시도하거나, 알림 실패를 비동기 재시도 작업으로 분리하세요.
As per path instructions, service는 비즈니스 규칙과 트랜잭션을 담당합니다.
Also applies to: 386-393
🤖 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/estimate.service.ts` around lines 291 - 298, estimate 처리
트랜잭션에서 createNotification을 커밋 후 동기 호출하지 않도록 변경하세요. 견적 수신 및 반려 처리와 알림 이벤트 기록을 동일한
runTransaction 안에서 원자적으로 저장하고, 커밋 후 별도 비동기 작업이 이벤트를 처리하며 실패 시 재시도하도록 분리하세요. 구현은
estimate 서비스의 해당 트랜잭션 흐름과 양쪽 createNotification 호출에만 적용하고 서비스가 비즈니스 규칙과 트랜잭션을 계속
담당하게 하세요.
Source: Path instructions
There was a problem hiding this comment.
슬기님 코멘트 아래에 답변하겠습니다.
There was a problem hiding this comment.
@soooob43, 네, 확인했습니다. 슬기님 코멘트 아래 답변을 확인한 뒤 해당 정합성 이슈를 다시 검토하겠습니다.
You are interacting with an AI system.
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 견적 제안과 반려가 성공한 경우에만 고객 알림을 생성하도록 비즈니스 흐름에 연결한 점이 좋았습니다.
- 알림 생성 책임을 Controller가 아닌 견적 Service에 두어 견적 처리 흐름을 한곳에서 확인할 수 있습니다.
- 알림 수신자로 견적 요청을 생성한 고객 ID를 사용해 대상이 올바르게 연결되어 있습니다.
- 기사 프로필 조회 결과에 닉네임을 포함해 알림 생성을 위한 별도의 추가 DB 조회를 만들지 않았습니다.
- 이사 유형 매핑에 Record<MoveType, string>을 사용해 Enum 값과 표시 문구의 누락을 타입으로 확인할 수 있도록 했습니다.
- 기존 견적 제안 및 반려 API의 응답 구조를 변경하지 않았습니다.
- 커서 검증을 1 이상의 정수로 변경해 에러 메시지와 실제 검증 조건을 일치시켰습니다.
🚨 수정이 필요한 부분
인라인 코멘트로 아래 내용을 남겼습니다.
- 견적 제안·반려 트랜잭션이 이미 커밋된 이후 알림 생성을 동기적으로 기다리고 있어, 알림 저장 실패 시 도메인 작업은 성공했지만 API가 실패 응답을 반환할 수 있습니다.
- 사용자가 재시도하면 이미 처리된 요청이라는 충돌이 발생할 수 있으므로, 알림 실패가 본 API 결과에 미치는 정책을 정하고 트랜잭션 포함 또는 실패 분리 처리가 필요합니다.
🔍 확인 및 제안
- 알림 만료 시각을 견적 요청 만료 시각과 동일하게 사용하고 있어, 만료 직전에 생성된 알림의 노출 시간이 매우 짧아질 수 있습니다. 특히 반려 알림에도 같은 정책을 적용하는 것이 의도된 것인지 확인이 필요합니다.
- linkUrl: null인 알림은 프론트에서 클릭 불가 상태로 처리되는지 확인하고, 경로 확정 후 연결 작업을 별도 이슈로 관리하면 좋겠습니다.
- PR에 함께 포함된 커서 Validator 변경도 변경 사항에 추가하면 이력을 이해하기 쉬울 것 같습니다.
전체적인 알림 대상과 생성 시점, 문구에 필요한 데이터 구성은 잘 연결되어 있습니다. 다만 현재 구조에서는 알림 저장 장애가 이미 성공한 견적 제안이나 반려 요청을 실패한 것처럼 응답하게 만들 수 있어, 이 부분은 병합 전에 정리하는 것이 안전해 보입니다. 수고하셨습니다! 😊
| //견적 제안 | ||
| async sendEstimate({ estimateRequestId, moverId, input }: SendEstimateParams) { | ||
| return runTransaction(async (tx) => { | ||
| const result = await runTransaction(async (tx) => { |
There was a problem hiding this comment.
🚨 수정이 필요한 부분
현재 견적 생성 트랜잭션이 커밋된 이후 알림 생성을 await하고 있어,
알림 저장에 실패하면 견적은 이미 생성됐지만 API는 실패 응답을 반환하게 됩니다.
이 경우 사용자가 재시도하면 이미 견적을 보낸 상태라 CONFLICT가 발생할 수 있어
실제 처리 결과와 API 응답이 달라질 것 같습니다.
알림을 견적 처리의 필수 데이터로 볼지, 부가 기능으로 볼지 정책을 먼저 정한 뒤
다음 중 한 방향으로 처리하는 것이 필요해 보입니다.
- 필수 데이터라면 견적/반려와 알림 DB 저장을 동일 트랜잭션에 포함
- 부가 기능이라면 알림 실패가 견적 API 실패로 전파되지 않도록 분리하고 로그·재시도 처리
There was a problem hiding this comment.
리뷰 확인했습니다.
현재 핵심 작업은 기사의 견적 제안 및 반려 처리이며, 알림 생성은 부가 기능으로 보는 것이 적절하다고 판단했습니다.
따라서 알림 생성에 실패하더라도 이미 완료된 견적 제안/반려 처리에는 영향을 주지 않고, API가 성공 응답을 반환하도록 두 작업을 분리하려고 합니다.
CodeRabbit이 제안한 Outbox은 실패한 알림 작업을 별도로 새로운 테이블을 생성하여 저장하고 재시도하는 방법인 것 같습니다. 현시점에서 신규 테이블과 재시도 처리 로직은 생각보다 구현 범위가 커질 것이라 판단했습니다.
우선 아래와 같이 알림 생성 과정에서 발생한 예외가 견적 API까지 영향이 가지 않도록 처리하고 실패 로그를 남기는 방향을 고려하고 있습니다.
const result = await runTransaction(/* 견적 제안 또는 반려 처리 */);
try {
await notificationService.createNotification(/* 알림 정보 */);
} catch (error) {
console.error("알림 생성 실패", error);
}
return result;
별도 테이블 없이 빠르게 적용할 수 있기도 하고 알림 생성 실패와 관계없이 견적 API가 실제 처리 결과에 맞는 성공 응답을 반환할 수 있습니다만 자동으로 재시도 하진 않아서 알림이 누락될 가능성이 있습니다...
두 가지 방향 중 어떤 게 나은 방향일지 또는 알림을 필수 기능으로 보고 알림 저장이 실패하면 견적 제안 및 반려도 같이 롤백되어야 할 지 여쭤보고 싶습니다.
추가로 다른 분들께서 알림 연동한 코드를 좀 살펴봤는데, 확인 결과 리뷰 작성과 견적 확정 알림도 핵심 트랜잭션 커밋 이후 알림 생성을 await하고 있어 동일한 실패 가능성이 있다고 판단됩니다. 반면에 견적 요청 생성과 기사 지정 알림은 동일 트랜잭션에 포함되어 있어 여기서는 알림을 필수 기능으로 판단하고 있는 것 같습니다.
이번 PR에서는 제안/반려만 최소 수정할지, 알림 정책을 공통으로 정리할지 범위 확인이 필요해 보입니다!
There was a problem hiding this comment.
리뷰 확인 감사합니다!
말씀해주신 내용 확인해보면서 저도 알림의 성격을 다시 고민해봤습니다.
현재는 기사의 견적 제안 및 반려 처리가 핵심 기능이고, 알림은 그 결과를 사용자에게 전달하는 부가 기능으로 보는 것이 적절하다고 생각했습니다. 그래서 알림 생성에 실패하더라도 이미 완료된 견적 제안/반려까지 실패 처리하는 것은 사용자 입장에서도 다소 어색할 수 있을 것 같습니다.
우선은 아래와 같이 트랜잭션 커밋 이후 알림을 별도로 생성하고, 실패 시에는 로그만 남기는 방향을 고려하고 있습니다.
const result = await runTransaction(/* 견적 제안 또는 반려 처리 */);
try {
await notificationService.createNotification(/* 알림 정보 */);
} catch (error) {
console.error("알림 생성 실패", error);
}
return result;
이 방식이면 견적 API는 실제 처리 결과에 맞게 성공 응답을 반환할 수 있고, 구현 범위도 크게 늘어나지 않는 장점이 있습니다. 다만 말씀하신 것처럼 알림이 누락될 경우 자동으로 복구되지 않는다는 한계는 있습니다.
CodeRabbit이 제안한 Outbox Pattern도 확인해봤는데, 별도 테이블과 재시도 로직까지 함께 도입해야 해서 현재 PR 범위에서는 조금 큰 변경이라고 판단했습니다.
추가로 다른 알림 연동 코드도 확인해봤는데,
- 리뷰 작성, 견적 확정 → 트랜잭션 커밋 이후 await notificationService.createNotification()
- 견적 요청 생성, 기사 지정 → 동일 트랜잭션 내에서 알림 생성
처럼 현재도 정책이 일관되어 있지는 않은 상태였습니다.
그래서 이번 PR에서는 제안/반려만 최소 수정하는 것이 좋을지, 아니면 프로젝트 전체의 알림 정책(필수 기능인지, 부가 기능인지)을 먼저 합의한 뒤 공통적으로 맞추는 것이 좋을지 의견을 여쭙고 싶습니다.
There was a problem hiding this comment.
이미 작업한 분들이 다시 작업하기 귀찮으시겠지만.. 제가 생각했을 때는 사용자를 위해서라도 공통으로 맞추는 과정이 필요하다고 판단됩니다.
| type: "ESTIMATE_RECEIVED", | ||
| title: "견적 도착", | ||
| content: `${result.moverNickname} 기사님의 ${MOVE_TYPE_LABEL[result.moveType]} 견적`, | ||
| linkUrl: null, |
There was a problem hiding this comment.
❓ 확인 질문
프론트 경로가 확정되지 않아 linkUrl: null로 처리한 것으로 확인했습니다.
프론트에서는 linkUrl이 없는 알림을 클릭 불가 상태로 안전하게 처리하고 있는지,
그리고 경로 확정 후 연결할 작업을 별도 이슈나 TODO로 관리하고 있는지 궁금합니다.
There was a problem hiding this comment.
리뷰 확인했습니다.
해당 부분은 영미님께서 이미 linkUrl 존재 여부에 따라 링크와 버튼을 분기하고 있는 것으로 확인됩니다. linkUrl이 없는 알림은 페이지 이동 없이 읽음 처리만 수행하고 이미 읽은 알림은 비활성화되어 안전하게 처리되고 있는 것 같습니다!
현재는 별도 이슈나 TODO로 관리하고 있지는 않습니다. 경로 확정 후 linkUrl을 연결하는 작업만 별도로 진행하면 될 것 같습니다.
| title: "견적 도착", | ||
| content: `${result.moverNickname} 기사님의 ${MOVE_TYPE_LABEL[result.moveType]} 견적`, | ||
| linkUrl: null, | ||
| expiresAt: result.expiresAt, |
There was a problem hiding this comment.
❓ 정책 확인
견적 도착·반려 알림의 expiresAt을 견적 요청 만료 시각과 동일하게 사용하고 있어,
만료 직전에 생성된 알림은 매우 짧은 시간만 노출될 수 있을 것 같습니다.
견적 도착 알림은 요청 만료와 함께 숨기는 것이 자연스러울 수 있지만,
반려 알림도 동일한 만료 정책을 적용하는 것이 의도된 것인지 확인하고 싶습니다.
There was a problem hiding this comment.
앗 해당 부분은 고려하지 못했던 부분입니다...
견적 도착 알림은 요청 만료 이후 활용도가 낮다고 판단해 견적 요청의 만료 시각을 동일하게 적용했습니다. 반면 반려 알림은 처리 결과를 안내하는 성격이므로, 슬기님께서 말씀해 주신 것처럼 견적 요청과 동일한 만료 시각을 적용할 필요는 없다고 생각합니다.
반려 알림은 다음 두 가지 방안을 고려할 수 있을 것 같습니다.
expiresAt: null: 무기한 노출
expiresAt: addDays(new Date(), 5): 생성 시점부터 일정 기간 노출
(5일은 예시이며 정책에 따라 조정)
두 방식 중 어떤 정책이 적절할지 여쭤보고 싶습니다.
There was a problem hiding this comment.
반려 알림은 견적 요청의 만료 시각과 관계없이, 알림 생성 시점부터 7일간 노출하는 정책으로 적용하면 좋을 것 같습니다.
반려 알림은 요청이 유효한 동안 행동을 유도하는 알림이 아니라 처리 결과를 안내하는 성격이므로, 요청 만료 이후에도 일정 기간 확인할 수 있어야 합니다.
다만 영구 보관이 필요한 정보는 아니고 오래된 알림이 계속 목록에 쌓이는 것을 방지하기 위해 expiresAt: null보다는 아래와 같이 생성 시점 기준 7일을 적용하는 방향으로 정리하겠습니다.
expiresAt: addDays(new Date(), 7);There was a problem hiding this comment.
넵 바로 반영하겠습니다. 감사합니다!
|
슬기님께서 리뷰 잘 달아주셔서 슬기님 리뷰 반영되면 머지하셔도 될 것 같습니다. |
…into feature/received-estimate-requests
…into feature/received-estimate-requests
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/modules/estimate/estimate.service.ts`:
- Around line 391-398: Wrap the post-transaction createNotification call in the
estimate rejection flow with try/catch so notification persistence or SSE
failures do not propagate as API errors after runTransaction has committed. Log
the caught error with logger.error, then continue returning the rejection
result; keep the existing notification payload and expiration behavior
unchanged.
🪄 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: 31ee3146-420e-48f3-8352-8227adb37aba
📒 Files selected for processing (1)
src/modules/estimate/estimate.service.ts
| const notificationCreatedAt = new Date(); | ||
| await notificationService.createNotification({ | ||
| userId: result.customerId, | ||
| type: "ESTIMATE_REQUEST_REJECTED", | ||
| title: "견적 요청 반려", | ||
| content: result.moverNickname, | ||
| linkUrl: null, | ||
| expiresAt: getRejectionNotificationExpiresAt(notificationCreatedAt), |
There was a problem hiding this comment.
커밋 후 알림 실패를 API 오류로 전파하지 않도록 처리해 주세요.
runTransaction이 성공한 뒤 createNotification을 await합니다. 알림 저장 또는 SSE 전송이 실패하면 견적 반려는 이미 커밋되었지만 API는 오류를 반환합니다. 재시도 시 CONFLICT가 발생하고 알림이 누락될 수 있습니다.
알림을 부가 기능으로 유지하려면 try/catch에서 logger.error로 기록한 뒤 반려 결과를 반환하세요. 재시도가 필요하면 알림 이벤트를 트랜잭션에 기록하고 별도 재시도 경로로 처리해야 합니다.
🤖 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/estimate.service.ts` around lines 391 - 398, Wrap the
post-transaction createNotification call in the estimate rejection flow with
try/catch so notification persistence or SSE failures do not propagate as API
errors after runTransaction has committed. Log the caught error with
logger.error, then continue returning the rejection result; keep the existing
notification payload and expiration behavior unchanged.
Source: Path instructions
|
트랜잭션 커밋 이후에 알림을 생성하도록 분리한 흐름이 잘 정리된 것 같습니다! 하나 확인해보시면 좋을 부분은, 제안 알림은 content에 "OO 기사님의 소형이사 견적"처럼 문구까지 저장하는데 반려 알림은 닉네임만 저장하고 있어서 두 알림의 저장 형식이 다릅니다. 프론트에서 고정 문구를 붙이는 방식에 문제는 없는지 체크해보시면 좋을 것 같습니다! 고생하셨습니다. |
4 participants
📌 작업 내용
기사님이 고객의 견적 요청에 견적을 제안하거나 요청을 반려했을 때 고객에게 알림 보낼 수 있도록 연동했습니다.
✅ 변경 사항
SMALL → 소형이사
HOME → 가정이사
OFFICE → 사무실이사
🧪 테스트
테스트 방법
📷 스크린샷 (선택)
🔥 체크리스트
🙏 To Reviewer
Summary by CodeRabbit
새로운 기능
버그 수정