feat: 관리자 확정 견적·견적 요청 수동 취소 API 추가 - #114
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough관리자가 확정 견적을 취소하는 Changes관리자 확정 견적 취소
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant 관리자
participant adminEstimateRouter
participant adminEstimatesController
participant adminEstimatesService
participant adminEstimatesRepository
관리자->>adminEstimateRouter: PATCH /api/admin/estimates/:estimateId/cancel
adminEstimateRouter->>adminEstimatesController: 검증된 견적 ID와 취소 본문 전달
adminEstimatesController->>adminEstimatesService: 견적 ID, 관리자 ID, 취소 정보 전달
adminEstimatesService->>adminEstimatesRepository: 견적 조회 및 행 잠금
adminEstimatesService->>adminEstimatesRepository: 상태, 이력, 알림, 채팅, 감사 로그 저장
adminEstimatesService-->>adminEstimatesController: 취소 결과 반환
adminEstimatesController-->>관리자: HTTP 200 응답
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
👍 좋았던 점
- 계정 정지와 확정 거래 취소를 별도 정책으로 분리한 방향이 명확합니다.
- 취소 가능 조건을 Estimate / EstimateRequest / confirmedEstimateId까지 함께 검증하고 있습니다.
- EstimateRequest를
FOR UPDATE로 잠근 뒤 최신 상태를 다시 조회합니다. - 실제 상태 변경도 조건부
updateMany로 처리해 동시 취소/완료에 대한 방어를 유지했습니다. - EstimateRequest와 Estimate 중 하나라도 상태 전이에 실패하면 transaction 전체가 rollback됩니다.
- 원 거래 취소 시 PENDING 수정 요청도 함께 종료합니다.
- EstimateRequestHistory와 ActivityLog를 모두 남겨 운영 추적이 가능합니다.
- 고객/기사 취소 알림에 sourceId를 사용해 재시도 시 중복 생성을 방지하도록 구성했습니다.
NotificationTypeenum과 migration도 함께 추가되었습니다.- 입력 validator와 OpenAPI 문서도 API 추가 범위에 맞게 포함되어 있습니다.
🚨 확인이 필요한 부분
채팅 SYSTEM 메시지 처리에서 두 가지를 꼭 확인하고 싶습니다.
첫 번째는 SYSTEM 메시지의 senderId입니다.
현재 관리자 ID를 ChatMessage.senderId로 저장하고 있는데,
관리자가 해당 ChatRoom participant가 아니라면
기존 메시지 조회/Socket 응답이 sender를 CUSTOMER/MOVER participant로 가정하는 경우
발신자 정보가 깨질 수 있습니다.
기존 SYSTEM 메시지가 senderId=null 또는 별도 시스템 발신 규칙을 사용하는지 확인이 필요합니다.
To Reviewer 내용 기준으로 동시 취소와 상태 정합성을 중점적으로 확인했습니다.
EstimateRequest 행을 FOR UPDATE로 잠근 뒤
최신 견적/요청 상태를 다시 조회하고,
- Estimate = CONFIRMED
- EstimateRequest = CONFIRMED
- isActive = true
- confirmedEstimateId 일치
조건을 모두 만족하는지 검증한 뒤 취소하도록 되어 있습니다.
그 이후에도 EstimateRequest와 Estimate 각각 조건부 updateMany를 수행하고
둘 중 하나라도 count=0이면 transaction을 실패시키기 때문에
여러 관리자가 동시에 취소하거나
다른 상태 변경과 경합하는 경우의 정합성 방어는 잘 구성되어 있습니다.
이력, ActivityLog, 수정 요청 취소, 알림 생성도 같은 transaction 안에서 처리되어
본 상태 변경만 반영되고 side effect 일부만 빠지는 문제를 줄이고 있습니다.
다만 채팅 SYSTEM 메시지는 별도 확인이 필요해 보입니다.
현재 관리자 ID를 senderId로 저장하고 있는데
관리자가 ChatRoom participant가 아닌 구조라면
기존 메시지 sender 처리와 충돌할 수 있습니다.
또한 estimateRequest.chatRooms 전체에 SYSTEM 메시지를 생성하고 있어
한 요청에 여러 기사 채팅방이 존재할 경우
취소 대상이 아닌 다른 기사 채팅방에도 동일한 취소 안내가 들어갈 가능성이 있습니다.
이 부분이 현재 ChatRoom/ChatMessage 도메인 정책에 맞는지 확인 부탁드립니다.
Notification의 경우 동일 sourceId를 고객/기사 모두 사용하고 있으므로
복합 unique에 userId가 포함되어 있는지도 함께 확인하면 좋겠습니다.
두 번째는 SYSTEM 메시지를 생성하는 ChatRoom 범위입니다.
현재 estimate.estimateRequest.chatRooms 전체에 취소 메시지를 생성합니다.
견적 요청 하나에 여러 기사와의 채팅방이 존재할 수 있다면,
관리자가 확정한 특정 견적을 취소했는데
다른 SENT 견적 기사와의 채팅방까지 동일한 “확정 거래 취소” 메시지를 받게 될 수 있습니다.
관리자 취소 대상이 특정 estimateId이므로
확정 견적과 연결된 ChatRoom만 대상으로 해야 하는지 확인이 필요해 보입니다.
| sourceId: notificationSourceId, | ||
| })); | ||
| const chatRoomIds = estimate.estimateRequest.chatRooms.map((room) => room.id); | ||
| const systemMessages: Prisma.ChatMessageCreateManyInput[] = chatRoomIds.map((roomId) => ({ |
There was a problem hiding this comment.
🚨 확인이 필요합니다.
현재 관리자 취소 SYSTEM 메시지를 만들 때
{
roomId,
senderId: adminId,
type: "SYSTEM",
content: CANCELLATION_MESSAGE,
}형태로 관리자 ID를 senderId에 저장하고 있습니다.
다만 해당 채팅방의 실제 participant는 CUSTOMER / MOVER이고,
관리자는 해당 ChatRoom 참여자가 아닐 가능성이 높습니다.
기존 메시지 조회나 Socket payload에서 senderId를 기준으로
참여자 정보나 이름/role을 조회하는 구조라면
SYSTEM 메시지에 adminId를 넣었을 때 발신자 해석이 깨질 수 있어 보입니다.
SYSTEM 메시지는 기존 도메인에서
senderId = null을 허용하거나 별도의 시스템 발신자 규칙을 사용하는지 확인 부탁드립니다.
관리자가 채팅 participant가 아닌 구조라면
현재 방식은 수정이 필요할 수 있습니다.
There was a problem hiding this comment.
리뷰 감사합니다! 말씀해주신대로 관리자는 해당 채팅방의 참여자가 아니기 때문에, senderId = null을 허용하고 시스템 메시지의 senderId과 sender를 null으로 반환하도록 수정했습니다.
| expiresAt: null, | ||
| sourceId: notificationSourceId, | ||
| })); | ||
| const chatRoomIds = estimate.estimateRequest.chatRooms.map((room) => room.id); |
There was a problem hiding this comment.
🔍 확인 및 제안
현재 estimateRequest.chatRooms 전체를 대상으로 SYSTEM 메시지를 생성하고 있습니다.
견적 요청 하나에 여러 ChatRoom이 생길 수 있는 구조라면
이 중 실제 확정된 estimate.id의 채팅방에만 취소 안내를 보내야 하는지,
아니면 요청에 연결된 모든 채팅방에 알려야 하는지 정책 확인이 필요해 보입니다.
현재 select는 ChatRoom id만 가져오고 있어
어떤 estimate의 채팅방인지 구분하지 않습니다.
관리자 취소 대상이 “확정된 특정 견적 거래”라면
확정 견적과 연결된 room만 대상으로 하는 것이 더 자연스러울 수 있습니다.
There was a problem hiding this comment.
확인 감사합니다! 관리자 확정 거래 취소 안내는 견적 요청의 모든 채팅방이 아닌 취소 대상 estimateId와 연결된 채팅방에만 생성되도록 수정했습니다.
d6570ce to
30556ea
Compare
Obebe-creator
left a comment
Obebe-creator
left a comment
There was a problem hiding this comment.
저 역시 SYSTEM 메시지를 특정 사용자의 발화가 아닌 자동 안내로 보고 senderId/sender를 null로 통일한 방향은 적절해 보입니다. 관리자는 채팅방 참여자가 아니기 때문에, 관리자 ID를 senderId로 저장하면 추후 FE에서 일반 참여자 메시지처럼 렌더링될 수 있다는 판단도 타당하다고 생각합니다
Notification content 컨벤션과 ChatMessage sender nullable 변경에 따른 삭제 정책 영향은 한 번 더 확인하면 좋을 것 같습니다.
또한 FE에서도 sender:null이 오면 깨질 수 있어서 일반 말풍선 로직이 아닌 따로 구현을 해야할 것 같은데 계획에 있으신지 궁금합니다
ba6c10f to
3c9cfa2
Compare
3 participants
📌 작업 내용
관리자가 계정 정지 시 확정(
CONFIRMED) 견적·견적 요청은 자동으로 취소되지 않습니다. 관리자가 확정 거래(CONFIRMED)를 수동으로 취소할 수 있는 API를 추가했습니다.계정 정지는 향후 서비스 이용을 제한하는 조치이고, 확정 거래 취소는 이미 성립한 고객·기사 간 약속을 해제하는 조치이므로 분리했습니다.
✅ 변경 사항
관리자 확정 견적 수동 취소 API
PATCH /api/admin/estimates/:estimateId/cancel추가CONFIRMED이고, 요청의confirmedEstimateId가 대상 견적과 일치할 때만 취소 가능SENT견적은 본 API의 대상이 아니며, 고객 요청 취소 또는 계정 정지 시 견적 요청 단위로 함께 취소됨404 ESTIMATE_NOT_FOUND, 확정 거래가 아니거나 이미 종료된 거래는409 ADMIN_ESTIMATE_CANCEL_NOT_ALLOWED반환동시성 및 상태 정합성 처리
FOR UPDATE로 잠근 뒤 최신 상태를 재확인updateMany로 처리해 동시 취소·완료 등으로 상태가 바뀐 거래를 다시 취소하지 않도록 처리취소 시 처리
CONFIRMED → CANCELED,isActive: false,canceledAt기록CONFIRMED → CANCELED,canceledAt기록PENDING견적 수정 요청 취소EstimateRequestHistory,ActivityLog생성estimateId와 연결된 채팅방에만 SYSTEM 메시지 생성 및lastMessageAt갱신sourceId와 복합 유니크 제약으로 재시도 시 알림 중복 생성 방지알림
content를 완성 문장 대신 강조할 대상어를 전달하도록 수정contentESTIMATE_CANCELED_BY_ADMIN"확정 견적 거래"관리자 확인으로 {content}가 취소되었습니다.ESTIMATE_REQUEST_CANCELED_BY_ACCOUNT_SUSPENSION"견적 요청"고객의 이용 제한으로 {content}이 취소되었습니다.ESTIMATE_CANCELED_BY_ACCOUNT_SUSPENSION"견적"기사의 이용 제한으로 {content}이 취소되었습니다.프론트
NotificationType및notificationMessages에 아래 타입을 추가해야 합니다.ESTIMATE_REQUEST_CANCELED_BY_ACCOUNT_SUSPENSIONESTIMATE_CANCELED_BY_ACCOUNT_SUSPENSIONESTIMATE_CANCELED_BY_ADMIN채팅 SYSTEM 메시지 처리
senderId: null,sender: null으로 반환lastMessageAt을 갱신해 채팅 목록 최신순 정렬에 반영Prisma schema 및 migration 변경
NotificationType에ESTIMATE_CANCELED_BY_ADMIN추가ChatMessage.senderId를 nullable로 변경🧪 테스트
npx tsc --noEmit,npm run lint테스트 방법
CANCELED처리EstimateRequestHistory,ActivityLog생성PENDING견적 수정 요청 취소lastMessageAt갱신ESTIMATE_CANCELED_BY_ADMIN알림 생성409 ADMIN_ESTIMATE_CANCEL_NOT_ALLOWEDSENT,CANCELED,EXPIRED상태 견적을 취소하려는 경우CONFIRMED지만, 연결된 견적 요청이CONFIRMED가 아니거나confirmedEstimateId가 대상 견적과 일치하지 않는 경우404 ESTIMATE_NOT_FOUNDestimateId로 요청한 경우422 VALIDATION_ERRORestimateId가 양의 정수가 아닌 경우reason이 누락됐거나 비어 있는 경우, 또는 500자를 초과한 경우internalNote가 1000자를 초과한 경우401 UNAUTHORIZED403 FORBIDDEN📷 스크린샷 (선택)
관리자에 의해 확정 견적 취소 알림
🔥 체크리스트
🙏 To Reviewer
NotificationTypeenum이 변경되었습니다. (ESTIMATE_CANCELED_BY_ADMIN추가)senderId·sendernullable 응답 변경이 채팅 API 및 프론트 처리에 적절한지도 함께 확인 부탁드립니다.NotificationType추가 및ChatMessage.senderIdnullable 변경 migration이 포함되어 있어 적용 범위를 확인 부탁드립니다.Summary by CodeRabbit
새 기능
개선
문서