-
Notifications
You must be signed in to change notification settings - Fork 2
feat: 견적 제안 및 반려 알림 연동 #70
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5f3e0f1
c89d21f
3932820
e5d1292
a706475
a696105
4ca1653
fec5a68
f49f7d3
d20451b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import assert from "node:assert/strict"; | ||
| import { describe, it } from "node:test"; | ||
|
|
||
| import { getRejectionNotificationExpiresAt } from "./estimate.notification-policy"; | ||
|
|
||
| describe("반려 알림 만료 정책", () => { | ||
| it("알림 생성 시점부터 7일 후에 만료합니다.", () => { | ||
| const createdAt = new Date("2026-07-31T06:30:00.000Z"); | ||
|
|
||
| assert.equal( | ||
| getRejectionNotificationExpiresAt(createdAt).toISOString(), | ||
| "2026-08-07T06:30:00.000Z", | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| const REJECTION_NOTIFICATION_VISIBILITY_DAYS = 7; | ||
| const DAY_MS = 24 * 60 * 60 * 1000; | ||
|
|
||
| export function getRejectionNotificationExpiresAt(createdAt: Date): Date { | ||
| return new Date(createdAt.getTime() + REJECTION_NOTIFICATION_VISIBILITY_DAYS * DAY_MS); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,12 @@ import { AppError } from "../../lib/app-error"; | |
| import { buildPagination } from "../../utils/pagination.util"; | ||
| import { runTransaction } from "../../utils/transaction"; | ||
| import { notificationService } from "../notification/notification.service"; | ||
| import { moverEstimateRequestRepository, receivedEstimateRepository } from "./estimate.repository"; | ||
| import { getRejectionNotificationExpiresAt } from "./estimate.notification-policy"; | ||
| import { | ||
| moverEstimateRequestRepository, | ||
| moverSentEstimateRepository, | ||
| receivedEstimateRepository, | ||
| } from "./estimate.repository"; | ||
| import type { | ||
| ConfirmReceivedEstimateParams, | ||
| GetReceivedEstimateDetailParams, | ||
|
|
@@ -15,6 +20,7 @@ import type { | |
| MoverEstimateRejectionListQuery, | ||
| MoverEstimateRejectionListItem, | ||
| MoverEstimateRejectionListResult, | ||
| MoverSentEstimateListQuery, | ||
| PendingEstimateQuery, | ||
| RejectEstimateParams, | ||
| SendEstimateParams, | ||
|
|
@@ -36,6 +42,11 @@ import type { | |
| */ | ||
|
|
||
| const KST_OFFSET_MS = 9 * 60 * 60 * 1000; | ||
| const MOVE_TYPE_LABEL: Record<MoveType, string> = { | ||
| SMALL: "소형이사", | ||
| HOME: "가정이사", | ||
| OFFICE: "사무실이사", | ||
| }; | ||
|
|
||
| // ============================================================================= | ||
| // 기사: 고객의 견적 요청 목록 조회 | ||
|
|
@@ -197,7 +208,7 @@ export const moverEstimateRequestService = { | |
|
|
||
| //견적 제안 | ||
| async sendEstimate({ estimateRequestId, moverId, input }: SendEstimateParams) { | ||
| return runTransaction(async (tx) => { | ||
| const result = await runTransaction(async (tx) => { | ||
| const profile = await moverEstimateRequestRepository.findMoverProfile(moverId, tx); | ||
|
|
||
| //기사 프로필 존재 확인 | ||
|
|
@@ -262,8 +273,8 @@ export const moverEstimateRequestService = { | |
|
|
||
| const isDesignated = estimateRequest.designatedMovers.length > 0; | ||
|
|
||
| //견적 셍성 | ||
| return moverEstimateRequestRepository.createEstimate( | ||
| //견적 생성 | ||
| const estimate = await moverEstimateRequestRepository.createEstimate( | ||
| { | ||
| estimateRequestId, | ||
| moverId, | ||
|
|
@@ -273,12 +284,31 @@ export const moverEstimateRequestService = { | |
| }, | ||
| tx, | ||
| ); | ||
|
|
||
| return { | ||
| estimate, | ||
| customerId: estimateRequest.customerId, | ||
| moverNickname: profile.nickname, | ||
| moveType: estimateRequest.moveType, | ||
| expiresAt: estimateRequest.expiresAt, | ||
| }; | ||
| }); | ||
|
|
||
| await notificationService.createNotification({ | ||
| userId: result.customerId, | ||
| type: "ESTIMATE_RECEIVED", | ||
| title: "견적 도착", | ||
| content: `${result.moverNickname} 기사님의 ${MOVE_TYPE_LABEL[result.moveType]} 견적`, | ||
| linkUrl: null, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ 확인 질문 프론트 경로가 확정되지 않아 프론트에서는
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 리뷰 확인했습니다. 해당 부분은 영미님께서 이미 linkUrl 존재 여부에 따라 링크와 버튼을 분기하고 있는 것으로 확인됩니다. linkUrl이 없는 알림은 페이지 이동 없이 읽음 처리만 수행하고 이미 읽은 알림은 비활성화되어 안전하게 처리되고 있는 것 같습니다! 현재는 별도 이슈나 TODO로 관리하고 있지는 않습니다. 경로 확정 후 linkUrl을 연결하는 작업만 별도로 진행하면 될 것 같습니다. |
||
| expiresAt: result.expiresAt, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ 정책 확인 견적 도착·반려 알림의 견적 도착 알림은 요청 만료와 함께 숨기는 것이 자연스러울 수 있지만,
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 앗 해당 부분은 고려하지 못했던 부분입니다... 견적 도착 알림은 요청 만료 이후 활용도가 낮다고 판단해 견적 요청의 만료 시각을 동일하게 적용했습니다. 반면 반려 알림은 처리 결과를 안내하는 성격이므로, 슬기님께서 말씀해 주신 것처럼 견적 요청과 동일한 만료 시각을 적용할 필요는 없다고 생각합니다. 반려 알림은 다음 두 가지 방안을 고려할 수 있을 것 같습니다. expiresAt: null: 무기한 노출 두 방식 중 어떤 정책이 적절할지 여쭤보고 싶습니다.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반려 알림은 견적 요청의 만료 시각과 관계없이, 알림 생성 시점부터 7일간 노출하는 정책으로 적용하면 좋을 것 같습니다. 반려 알림은 요청이 유효한 동안 행동을 유도하는 알림이 아니라 처리 결과를 안내하는 성격이므로, 요청 만료 이후에도 일정 기간 확인할 수 있어야 합니다. 다만 영구 보관이 필요한 정보는 아니고 오래된 알림이 계속 목록에 쌓이는 것을 방지하기 위해 expiresAt: addDays(new Date(), 7);
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 넵 바로 반영하겠습니다. 감사합니다! |
||
| }); | ||
|
Comment on lines
+297
to
304
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift 커밋 후 알림 실패를 별도 재시도 경로로 분리해 주세요.
As per path instructions, Also applies to: 386-393 🤖 Prompt for AI AgentsSource: Path instructions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 슬기님 코멘트 아래에 답변하겠습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You are interacting with an AI system. |
||
|
|
||
| return result.estimate; | ||
| }, | ||
|
|
||
| // 견적 요청 반려 | ||
| async rejectEstimate({ estimateRequestId, moverId, input }: RejectEstimateParams) { | ||
| return runTransaction(async (tx) => { | ||
| const result = await runTransaction(async (tx) => { | ||
| //기사 프로필 | ||
| const profile = await moverEstimateRequestRepository.findMoverProfile(moverId, tx); | ||
|
|
||
|
|
@@ -342,15 +372,33 @@ export const moverEstimateRequestService = { | |
| } | ||
|
|
||
| //데이터 생성 | ||
| return moverEstimateRequestRepository.createEstimateRejection( | ||
| const rejection = await moverEstimateRequestRepository.createEstimateRejection( | ||
| { | ||
| estimateRequestId, | ||
| moverId, | ||
| reason: input.reason, | ||
| }, | ||
| tx, | ||
| ); | ||
|
|
||
| return { | ||
| rejection, | ||
| customerId: estimateRequest.customerId, | ||
| moverNickname: profile.nickname, | ||
| }; | ||
| }); | ||
|
|
||
| const notificationCreatedAt = new Date(); | ||
| await notificationService.createNotification({ | ||
| userId: result.customerId, | ||
| type: "ESTIMATE_REQUEST_REJECTED", | ||
| title: "견적 요청 반려", | ||
| content: result.moverNickname, | ||
| linkUrl: null, | ||
| expiresAt: getRejectionNotificationExpiresAt(notificationCreatedAt), | ||
|
Comment on lines
+391
to
+398
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 커밋 후 알림 실패를 API 오류로 전파하지 않도록 처리해 주세요.
알림을 부가 기능으로 유지하려면 🤖 Prompt for AI AgentsSource: Path instructions |
||
| }); | ||
|
|
||
| return result.rejection; | ||
| }, | ||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨 수정이 필요한 부분
현재 견적 생성 트랜잭션이 커밋된 이후 알림 생성을
await하고 있어,알림 저장에 실패하면 견적은 이미 생성됐지만 API는 실패 응답을 반환하게 됩니다.
이 경우 사용자가 재시도하면 이미 견적을 보낸 상태라
CONFLICT가 발생할 수 있어실제 처리 결과와 API 응답이 달라질 것 같습니다.
알림을 견적 처리의 필수 데이터로 볼지, 부가 기능으로 볼지 정책을 먼저 정한 뒤
다음 중 한 방향으로 처리하는 것이 필요해 보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 확인했습니다.
현재 핵심 작업은 기사의 견적 제안 및 반려 처리이며, 알림 생성은 부가 기능으로 보는 것이 적절하다고 판단했습니다.
따라서 알림 생성에 실패하더라도 이미 완료된 견적 제안/반려 처리에는 영향을 주지 않고, 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리뷰 확인 감사합니다!
말씀해주신 내용 확인해보면서 저도 알림의 성격을 다시 고민해봤습니다.
현재는 기사의 견적 제안 및 반려 처리가 핵심 기능이고, 알림은 그 결과를 사용자에게 전달하는 부가 기능으로 보는 것이 적절하다고 생각했습니다. 그래서 알림 생성에 실패하더라도 이미 완료된 견적 제안/반려까지 실패 처리하는 것은 사용자 입장에서도 다소 어색할 수 있을 것 같습니다.
우선은 아래와 같이 트랜잭션 커밋 이후 알림을 별도로 생성하고, 실패 시에는 로그만 남기는 방향을 고려하고 있습니다.
이 방식이면 견적 API는 실제 처리 결과에 맞게 성공 응답을 반환할 수 있고, 구현 범위도 크게 늘어나지 않는 장점이 있습니다. 다만 말씀하신 것처럼 알림이 누락될 경우 자동으로 복구되지 않는다는 한계는 있습니다.
CodeRabbit이 제안한 Outbox Pattern도 확인해봤는데, 별도 테이블과 재시도 로직까지 함께 도입해야 해서 현재 PR 범위에서는 조금 큰 변경이라고 판단했습니다.
추가로 다른 알림 연동 코드도 확인해봤는데,
처럼 현재도 정책이 일관되어 있지는 않은 상태였습니다.
그래서 이번 PR에서는 제안/반려만 최소 수정하는 것이 좋을지, 아니면 프로젝트 전체의 알림 정책(필수 기능인지, 부가 기능인지)을 먼저 합의한 뒤 공통적으로 맞추는 것이 좋을지 의견을 여쭙고 싶습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이미 작업한 분들이 다시 작업하기 귀찮으시겠지만.. 제가 생각했을 때는 사용자를 위해서라도 공통으로 맞추는 과정이 필요하다고 판단됩니다.