From a3ca8394507061ef795d05a6f2ef6f01a584917d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 16:16:08 +0000 Subject: [PATCH 1/5] test(sales): a cancelled request's PENDING_APPROVAL quotation is superseded and cannot be approved (RED) Cancelling a pricing request supersedes its open quotations, but the cancel's predicate skips PENDING_APPROVAL, so a PRICING_REQUEST-origin quotation waiting for approval stays in the approvals queue. Approving it issues a quotation on the cancelled request and supersedes the deal's live quotation. Real-DB tests (DealQuotationPricingRequestApprovalIntegrationTest): - cancel supersedes the request's PENDING_APPROVAL quotation and takes it out of the approvals queue; a sibling request's pending quotation stays; the audit is the request's one PRICING_REQUEST_CANCELLED event - approving after the cancel is refused and the deal's live quotation stays ISSUED - a PENDING_APPROVAL quotation already orphaned on a CANCELLED request is refused for the sales manager and the CEO, before any write - markLost's cascade supersedes the pending quotation too Mock parity (mockApi.dealQuotationPricingRequestApproval.test.js): cancel supersedes PENDING_APPROVAL and DRAFT quotations; approve refuses a quotation on a CANCELLED request. All seven fail on this commit; the fix follows. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BdQVhpg72sGQrzZVzfMP5U --- ...PricingRequestApprovalIntegrationTest.java | 122 ++++++++++++++++++ ...ealQuotationPricingRequestApproval.test.js | 36 +++++- 2 files changed, 157 insertions(+), 1 deletion(-) diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java index 809c127db..e8ba598fb 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java @@ -70,6 +70,7 @@ import th.co.glr.hr.pricingrequest.QuantityType; import th.co.glr.hr.support.AbstractPostgresIntegrationTest; import th.co.glr.hr.ticket.CreateTicketRequest; +import th.co.glr.hr.ticket.DealLostReason; import th.co.glr.hr.ticket.DealStage; import th.co.glr.hr.ticket.QuotationRenderer; import th.co.glr.hr.ticket.QuotationStatus; @@ -747,6 +748,127 @@ private List quotationIdsOnTicket() { java.util.Map.of("t", ticketId), Long.class); } + // ───────────────────────────────────────────────────────────────────────────────────── + // Cancelling the pricing request takes its PENDING_APPROVAL quotation with it. The cancel + // cascade (PricingRequestRepository#supersedeOpenPricingDecisionAndQuotation) retired DRAFT and + // every issued status but skipped PENDING_APPROVAL, so the quotation stayed in the approvals + // queue, and approving it issued an offer on a cancelled request and superseded the deal's live + // quotation through the B-4 sweep above. + // ───────────────────────────────────────────────────────────────────────────────────── + + /** The cascade, through PricingRequestService#cancel. The sibling request's quotation is the + * control: the supersede is scoped to the cancelled request, never to the deal. */ + @Test + void cancel_supersedesTheRequestsPendingApprovalQuotation_andTakesItOutOfTheApprovalsQueue() { + DealQuotationDto sibling = submittedQuotation(); + DealQuotationDto pending = submittedQuotation(); + long pricingRequestId = pending.pricingRequestId(); + assertThat(approvalQueueIds()).contains(sibling.id(), pending.id()); + + pricingRequestService.cancel(pricingRequestId, cancelRequest(), salesActor); + + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + assertThat(approvalQueueIds()).doesNotContain(pending.id()).contains(sibling.id()); + assertThat(statusOf(sibling.id())).isEqualTo("PENDING_APPROVAL"); + // The same record the cascade's other statuses get: the request's one cancel event, and no + // quotation event of its own. + assertThat(cancelEvents(pricingRequestId)) + .containsExactly("APPROVED_FOR_QUOTATION>CANCELLED:ลูกค้ายกเลิกโครงการ"); + assertThat(ticketEventCount(th.co.glr.hr.ticket.TicketEventKind.DEAL_QUOTATION_SUPERSEDED)).isZero(); + } + + /** The defect the 10-27 review found: approving the cancelled request's quotation from the + * approvals queue issued it and retired the deal's live offer. Either half of the fix refuses + * it on its own (the cascade leaves nothing PENDING_APPROVAL to approve; the guard refuses a + * CANCELLED request), so the tests either side of this one pin each half separately. */ + @Test + void approve_afterTheRequestWasCancelled_isRefused_andTheDealsLiveQuotationStaysLive() { + DealQuotationDto live = issuedRound(); + DealQuotationDto pending = submittedQuotation(); + pricingRequestService.cancel(pending.pricingRequestId(), cancelRequest(), salesActor); + + assertThatThrownBy(() -> quotationService.approve(pending.id(), new ApproveRequest(null), salesManagerActor)) + .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); + + assertThat(statusOf(live.id())).isEqualTo("ISSUED"); + assertThat(statusOf(pending.id())).isNotEqualTo("ISSUED"); + assertThat(supersededEventCount(ticketId, live.number(), pending.number())).isZero(); + assertThat(pricingRequestStatusOf(pending.pricingRequestId())).isEqualTo(PricingRequestStatus.CANCELLED); + } + + /** The approve guard on its own. A request cancelled before this fix kept its PENDING_APPROVAL + * quotation and nothing retires that row, so approve refuses any quotation whose request is + * CANCELLED, whatever the row's own status. Built through the real cancel; only the quotation + * is put back to the PENDING_APPROVAL the old cascade left. The CEO is refused too: the CEO's + * early return from the price-change gate must not double as a way past this one. */ + @Test + void approve_ofAPendingQuotationLeftOnACancelledRequest_isRefusedForEitherApprover_andWritesNothing() { + DealQuotationDto live = issuedRound(); + DealQuotationDto pending = submittedQuotation(); + pricingRequestService.cancel(pending.pricingRequestId(), cancelRequest(), salesActor); + jdbc.update("UPDATE sales.quotation SET doc_status = 'PENDING_APPROVAL' WHERE quotation_id = :id", + java.util.Map.of("id", pending.id())); + + for (UserPrincipal approver : List.of(salesManagerActor, ceoActor)) { + assertThatThrownBy(() -> quotationService.approve(pending.id(), new ApproveRequest(null), approver)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo("คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้"); + }); + } + + // Refused before the first write: this suite has no transaction to roll a late refusal back. + assertThat(statusOf(pending.id())).isEqualTo("PENDING_APPROVAL"); + assertThat(approverSnapshotCount(pending.id())).isZero(); + assertThat(statusOf(live.id())).isEqualTo("ISSUED"); + assertThat(supersededEventCount(ticketId, live.number(), pending.number())).isZero(); + } + + /** The deal-level cascade (TicketService#markLost -> PricingRequestService#cancelOpenForTicket) + * runs the same supersede, so a lost deal's pending quotation leaves the queue too. */ + @Test + void markLost_cancelsTheRequest_andSupersedesItsPendingApprovalQuotation() { + DealQuotationDto pending = submittedQuotation(); + + ticketService.markLost(ticketId, DealLostReason.PRICE, null, salesActor); + + assertThat(pricingRequestStatusOf(pending.pricingRequestId())).isEqualTo(PricingRequestStatus.CANCELLED); + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + assertThat(approvalQueueIds()).doesNotContain(pending.id()); + } + + private PricingRequestRequests.CancelPricingRequestRequest cancelRequest() { + return new PricingRequestRequests.CancelPricingRequestRequest("ลูกค้ายกเลิกโครงการ"); + } + + /** The approvals queue: every PENDING_APPROVAL quotation, as the sales manager lists them. */ + private List approvalQueueIds() { + return quotationService.search(List.of(QuotationStatus.PENDING_APPROVAL), false, salesManagerActor) + .stream().map(DealQuotationDto::id).toList(); + } + + private String pricingRequestStatusOf(long pricingRequestId) { + return jdbc.queryForObject("SELECT status FROM sales.pricing_request WHERE pricing_request_id = :id", + java.util.Map.of("id", pricingRequestId), String.class); + } + + private List cancelEvents(long pricingRequestId) { + return jdbc.queryForList(""" + SELECT from_status || '>' || to_status || ':' || message FROM sales.pricing_request_event + WHERE pricing_request_id = :id AND event_kind = 'PRICING_REQUEST_CANCELLED' + """, java.util.Map.of("id", pricingRequestId), String.class); + } + + private long ticketEventCount(String kind) { + return jdbc.queryForObject("SELECT COUNT(*) FROM sales.ticket_event WHERE ticket_id = :t AND kind = :k", + java.util.Map.of("t", ticketId, "k", kind), Long.class); + } + + private long approverSnapshotCount(long quotationId) { + return jdbc.queryForObject("SELECT COUNT(*) FROM sales.quotation_approver_snapshot WHERE quotation_id = :id", + java.util.Map.of("id", quotationId), Long.class); + } + // ───────────────────────────────────────────────────────────────────────────────────── // Helpers // ───────────────────────────────────────────────────────────────────────────────────── diff --git a/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js b/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js index 4a9a67559..b60076d11 100644 --- a/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js +++ b/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { api } from './mockApi.js'; +import { api, setPricingRequestStatusForTests } from './mockApi.js'; // GLA-123 slice S2, REWORKED (owner reversed the dual-approval design, 2026-09-20): mock coverage // for DealQuotationService#approveAndIssuePricingRequestOrigin/#requireCeoApprovalIfChanged — ONE @@ -209,3 +209,37 @@ describe('mockApi dealQuotations approval (GLA-123 slice S2, reworked)', () => { } }); }); + +// Mirrors PricingRequestService#cancel's quotation supersede (PricingRequestRepository +// #supersedeOpenPricingDecisionAndQuotation) and the cancelled-request guard in DealQuotationService +// #approveAndIssuePricingRequestOrigin. Mock plumbing only; the real-DB pins are the cancel/approve +// tests in DealQuotationPricingRequestApprovalIntegrationTest. +describe('mockApi pricingRequests.cancel takes the request\'s open quotation with it', () => { + it('supersedes a PENDING_APPROVAL quotation, which can then no longer be approved', async () => { + const { prId, quotationId } = await pendingApprovalPricingRequestQuotation(); + await api.pricingRequests.cancel(prId, { reason: 'ลูกค้ายกเลิกโครงการ' }); + const { quotation } = await api.dealQuotations.get(quotationId); + expect(quotation.docStatus).toBe('SUPERSEDED'); + await api.auth.login({ role: 'sales_manager' }); + await expect(api.dealQuotations.approve(quotationId, {})).rejects.toMatchObject({ status: 409 }); + }); + + it('supersedes a DRAFT quotation too', async () => { + const { prId, quotationId } = await draftPricingRequestQuotation(); + await api.pricingRequests.cancel(prId, { reason: 'ลูกค้ายกเลิกโครงการ' }); + const { quotation } = await api.dealQuotations.get(quotationId); + expect(quotation.docStatus).toBe('SUPERSEDED'); + }); + + it('approve refuses a PENDING_APPROVAL quotation left on a CANCELLED request, for either approver', async () => { + const { prId, quotationId } = await pendingApprovalPricingRequestQuotation(); + setPricingRequestStatusForTests(prId, 'CANCELLED'); + for (const role of ['sales_manager', 'ceo']) { + await api.auth.login({ role }); + await expect(api.dealQuotations.approve(quotationId, {})) + .rejects.toMatchObject({ status: 409, message: 'คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้' }); + } + const { quotation } = await api.dealQuotations.get(quotationId); + expect(quotation.docStatus).toBe('PENDING_APPROVAL'); + }); +}); From d0854d675c07f080f1a8e72f309de3d9439febde Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 16:16:09 +0000 Subject: [PATCH 2/5] fix(sales): cancelling a pricing request supersedes its PENDING_APPROVAL quotation; approve refuses a cancelled request - PricingRequestRepository#supersedeOpenPricingDecisionAndQuotation adds PENDING_APPROVAL to the statuses it supersedes. Both callers get it: PricingRequestService#cancel and the deal-kill cascade #cancelOpenForTicket. The audit stays the request's PRICING_REQUEST_CANCELLED event, as for DRAFT and the issued statuses. - DealQuotationService#approveAndIssuePricingRequestOrigin refuses (409) a quotation whose pricing request is CANCELLED, before any write. It covers a row orphaned before this change; there is no data migration. - mockApi mirrors both: the mock's cancel supersedes the request's open new-engine quotations, and the mock's approve refuses a cancelled request. - Comments that listed what an APPROVED_FOR_QUOTATION request can own now include a PENDING_APPROVAL quotation. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BdQVhpg72sGQrzZVzfMP5U --- .../co/glr/hr/dealquotation/DealQuotationService.java | 6 ++++++ .../hr/pricingrequest/PricingRequestRepository.java | 10 +++++++++- .../glr/hr/pricingrequest/PricingRequestService.java | 6 +++--- .../glr/hr/pricingrequest/PricingRequestStatus.java | 6 ++++-- frontend/src/api/mockApi.js | 11 ++++++++++- 5 files changed, 32 insertions(+), 7 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java index 639e87b5c..3dcfa5f18 100644 --- a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java +++ b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java @@ -1802,6 +1802,12 @@ private DealQuotationDto approveAndIssuePricingRequestOrigin(DealQuotationDto qu quotations.lockTicket(quotation.ticketId()); PricingRequestSummaryDto summary = pricingRequests.findSummary(pricingRequestId) .orElseThrow(() -> new ApiException(HttpStatus.NOT_FOUND, "ไม่พบคำขอราคานี้")); + // A cancelled request issues nothing. Its cancel supersedes a PENDING_APPROVAL quotation + // (PricingRequestRepository#supersedeOpenPricingDecisionAndQuotation); this refuses one + // orphaned before that status was in the cancel's list. Checked before the first write below. + if (PricingRequestStatus.CANCELLED.equals(summary.status())) { + throw new ApiException(HttpStatus.CONFLICT, "คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้"); + } // Same DATE-mode-with-special-pricing / DAYS-mode validity computation #approve already // uses for DEAL_DIRECT — reused verbatim rather than forked so the two origins can never // compute this differently by accident. diff --git a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java index 84d1905fd..054f544f1 100644 --- a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java +++ b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java @@ -961,6 +961,14 @@ AND status IN ('DRAFT', 'CALCULATED') * request transition, and put a live offer in front of a customer whose deal was cancelled. * Retiring the draft here is what makes that unreachable. * + *

{@code PENDING_APPROVAL} is in the predicate for the same reason. A + * PRICING_REQUEST-origin quotation ({@code th.co.glr.hr.dealquotation}) waits at + * PENDING_APPROVAL while the request is still APPROVED_FOR_QUOTATION, a status cancel admits. + * Left out of this list, it stayed in the approvals queue, and approving it issued an offer on + * the cancelled request and superseded the deal's live quotation through the one-live-offer + * sweep. {@code DealQuotationService#approveAndIssuePricingRequestOrigin} now also refuses a + * CANCELLED request, for a row orphaned before this status was added here. + * *

Not routed through {@code PricingDecisionRepository}/{@code CustomerQuotationRepository} * (this class's own dependency direction stays one-way, same as {@code cancelOpenStep2Children} * already reaching into {@code sales.factory_quote}/{@code sales.pricing_costing} directly) — @@ -973,7 +981,7 @@ public void supersedeOpenPricingDecisionAndQuotation(long pricingRequestId) { UPDATE sales.quotation SET doc_status = 'SUPERSEDED' WHERE pricing_request_id = :pricingRequestId - AND doc_status IN ('DRAFT', 'ISSUED', 'READY_TO_ISSUE', 'SENT', 'REVISION_REQUESTED') + AND doc_status IN ('DRAFT', 'PENDING_APPROVAL', 'ISSUED', 'READY_TO_ISSUE', 'SENT', 'REVISION_REQUESTED') """, Map.of("pricingRequestId", pricingRequestId)); } diff --git a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java index f0b7c3e6e..b34e0ca97 100644 --- a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java +++ b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java @@ -609,9 +609,9 @@ public PricingRequestDetailDto cancel(long id, CancelPricingRequestRequest reque // with an open pricing_decision (that needs READY_FOR_CEO_REVIEW+) or a quotation (that // needs APPROVED_FOR_QUOTATION). The widened cutoff makes it live on the main path: a // cancel from CEO_REVIEWING retires a DRAFT decision, and one from APPROVED_FOR_QUOTATION - // retires an APPROVED decision plus any DRAFT quotation built from it. The prediction the - // old comment made — "a future widening of that map cannot silently reintroduce design - // correction 1's bug" — is exactly what is being cashed in here. + // retires an APPROVED decision plus any DRAFT or PENDING_APPROVAL quotation built from it. + // The prediction the old comment made — "a future widening of that map cannot silently + // reintroduce design correction 1's bug" — is exactly what is being cashed in here. requests.supersedeOpenPricingDecisionAndQuotation(id); String metadataJson = toReasonMetadataJson(request.reason()); requests.addEvent(id, summary.ticketId(), actor.id(), actor.name(), diff --git a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestStatus.java b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestStatus.java index 44045a788..db68930df 100644 --- a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestStatus.java +++ b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestStatus.java @@ -99,10 +99,12 @@ public final class PricingRequestStatus { * CHILDREN, and cancelling a parent whose children stay live is worse than not cancelling at * all. The three statuses added above are exactly the ones that can own a READY_FOR_COSTING * factory quote, a SUBMITTED {@code pricing_costing}, a DRAFT/APPROVED - * {@code pricing_decision}, and a DRAFT {@code quotation}; {@code + * {@code pricing_decision}, and a DRAFT or PENDING_APPROVAL {@code quotation}; {@code * PricingRequestRepository#cancelOpenChildrenForDeadRequest} and {@code * #supersedeOpenPricingDecisionAndQuotation} were widened in the same change to reach all - * four. If a later change admits another status here, check what it can own first. + * four. (PENDING_APPROVAL joined later: GLA-123 S2 made it reachable here, and the quotation + * predicate missed it until 2026-10-27.) If a later change admits another status here, check + * what it can own first. */ // SUPERSEDED stays terminal here; the one exit is PricingRequestRepository#reviveParentOfCancelledRevision (direct SQL). private static final Map> ALLOWED = Map.ofEntries( diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 441319732..88a9bfa6b 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -6848,6 +6848,9 @@ function mockApproveDealQuotationPricingRequestOrigin(row, payload, user) { || dto.itemsRemovedFromCeoCount > 0; if (changed) fail('มีรายการที่เปลี่ยนจากราคา CEO — ต้องให้ CEO อนุมัติ', 403); } + // The service's cancelled-request guard: a cancelled request issues nothing. Read before any write. + const pr = findPricingRequestRaw(row.pricingRequestId); + if (pr.status === 'CANCELLED') fail('คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้', 409); const now = new Date().toISOString(); row.docStatus = 'ISSUED'; row.approvedById = user.employeeId ?? null; @@ -6865,7 +6868,6 @@ function mockApproveDealQuotationPricingRequestOrigin(row, payload, user) { : (row.validityDays ? addDaysIso(row.quotationDate, row.validityDays) : null); // Java writes the DEAL_QUOTATION_SUPERSEDED events BEFORE QUOTATION_ISSUED; keep that order. supersedeOtherLiveMockDealQuotations(row, user, now); - const pr = findPricingRequestRaw(row.pricingRequestId); const ticket = db.tickets.find((t) => t.id === row.ticketId); if (pr && pr.status === 'APPROVED_FOR_QUOTATION') { pr.status = 'QUOTATION_ISSUED'; @@ -16697,6 +16699,13 @@ export const api = { pr.status = 'CANCELLED'; pr.cancelledAt = now; pr.updatedAt = now; + // Mirrors the quotation statement of PricingRequestRepository#supersedeOpenPricingDecisionAndQuotation + // over this file's new-engine store, where a PENDING_APPROVAL row left behind stayed approvable. + // The rest of the real cascade (decision, legacy quotation, factory quote, costing) is not mirrored. + mockDealQuotations + .filter((q) => q.pricingRequestId === pr.id + && ['DRAFT', 'PENDING_APPROVAL', 'ISSUED', 'READY_TO_ISSUE', 'SENT', 'REVISION_REQUESTED'].includes(q.docStatus)) + .forEach((q) => { q.docStatus = 'SUPERSEDED'; }); pushPricingRequestEvent(pr, user, 'PRICING_REQUEST_CANCELLED', fromStatus, 'CANCELLED', payload.reason, JSON.stringify({ reason: payload.reason })); return delay({ pricingRequest: buildPricingRequestDetail(pr) }); }, From 97ca6dfe7e0e587f9e310bfffabde5a3b966c150 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 18:47:36 +0000 Subject: [PATCH 3/5] test(sales): cancel waits for approve's request lock; a customer-change revision retires the old request's pending quotation (RED) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner rulings 2026-10-27: one lock between cancel and approve, and a quotation still waiting for ผจก.ขาย/CEO approval on a request that a customer-change revision supersedes is retired, not left approvable. Real-DB tests (DealQuotationPricingRequestApprovalIntegrationTest): - cancel queues for the request's advisory lock that an approve holds, before its first write (a FOR UPDATE NOWAIT probe finds the request row free while it waits), and completes once the lock frees - a customer-change revision from APPROVED_FOR_QUOTATION supersedes the old request's PENDING_APPROVAL quotation; approving it is refused - from QUOTATION_ISSUED it supersedes a pending quotation revision while the issued offer stays ISSUED (2026-08-13 ruling) - approve refuses a PENDING_APPROVAL quotation left on a SUPERSEDED request, for the sales manager and the CEO, before any write Mock parity: a customer-change revision supersedes the old request's PENDING_APPROVAL and DRAFT quotations; approve refuses a SUPERSEDED request too (the CANCELLED case becomes a row of the same table). The new cases fail on this commit; the fix follows. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BdQVhpg72sGQrzZVzfMP5U --- ...PricingRequestApprovalIntegrationTest.java | 147 ++++++++++++++++++ ...ealQuotationPricingRequestApproval.test.js | 96 ++++++++---- 2 files changed, 211 insertions(+), 32 deletions(-) diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java index e8ba598fb..2d26a2236 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java @@ -1,6 +1,7 @@ package th.co.glr.hr.dealquotation; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.fasterxml.jackson.databind.ObjectMapper; @@ -10,6 +11,11 @@ import java.time.ZoneId; import java.util.List; import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import org.apache.poi.hssf.usermodel.HSSFPicture; import org.apache.poi.hssf.usermodel.HSSFSheet; import org.apache.poi.ss.usermodel.CellType; @@ -837,10 +843,151 @@ void markLost_cancelsTheRequest_andSupersedesItsPendingApprovalQuotation() { assertThat(approvalQueueIds()).doesNotContain(pending.id()); } + /** One lock (owner ruling 2026-10-27): cancel takes the request's advisory lock, the one + * approve takes first, so the two serialise instead of updating the quotation and request rows + * in opposite orders. Thread A stands in for an approve holding that lock. The cancel must queue + * for it while holding no row lock (the NOWAIT probe), then finish once the lock frees. */ + @Test + void cancel_waitsForTheRequestLockAnApproveHolds_beforeItsFirstWrite() throws Exception { + DealQuotationDto pending = submittedQuotation(); + long pricingRequestId = pending.pricingRequestId(); + ExecutorService pool = Executors.newFixedThreadPool(2); + CountDownLatch aHolds = new CountDownLatch(1); + CountDownLatch releaseA = new CountDownLatch(1); + try { + Future approveInFlight = pool.submit(() -> transactionTemplate.execute(s -> { + new th.co.glr.hr.customerquotation.CustomerQuotationRepository(jdbc).lockPricingRequest(pricingRequestId); + aHolds.countDown(); + try { + releaseA.await(30, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return null; + })); + assertThat(aHolds.await(10, TimeUnit.SECONDS)).isTrue(); + Future cancel = pool.submit(() -> transactionTemplate.execute(s -> { + jdbc.getJdbcTemplate().execute("SET LOCAL lock_timeout = '30s'"); + return pricingRequestService.cancel(pricingRequestId, cancelRequest(), salesActor); + })); + + assertThat(awaitWaiterOnRequestLock(pricingRequestId, cancel)) + .as("cancel queues for the request's advisory lock").isTrue(); + assertThatCode(() -> transactionTemplate.execute(s -> jdbc.queryForList(""" + SELECT pricing_request_id FROM sales.pricing_request + WHERE pricing_request_id = :id FOR UPDATE NOWAIT + """, java.util.Map.of("id", pricingRequestId), Long.class))) + .as("cancel queued before writing the request row").doesNotThrowAnyException(); + + releaseA.countDown(); + cancel.get(30, TimeUnit.SECONDS); + approveInFlight.get(10, TimeUnit.SECONDS); + } finally { + releaseA.countDown(); + pool.shutdown(); + pool.awaitTermination(30, TimeUnit.SECONDS); + } + assertThat(pricingRequestStatusOf(pricingRequestId)).isEqualTo(PricingRequestStatus.CANCELLED); + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + } + + // ───────────────────────────────────────────────────────────────────────────────────── + // A customer-change revision retires the old request's quotation that is still waiting for + // approval (owner ruling 2026-10-27), as it already retired a DRAFT. The new round's quotation + // then needs ผจก.ขาย/CEO approval like any other. An ISSUED offer stays live until its + // replacement is issued (reissue-through-CEO-chain ruling, 2026-08-13). + // ───────────────────────────────────────────────────────────────────────────────────── + + @Test + void customerChangeRevision_supersedesTheOldRequestsPendingApprovalQuotation_andApproveIsRefused() { + DealQuotationDto pending = submittedQuotation(); + + pricingRequestService.createCustomerChangeRevision(pending.pricingRequestId(), customerChangeRevision(), salesActor); + + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + assertThat(approvalQueueIds()).doesNotContain(pending.id()); + assertThatThrownBy(() -> quotationService.approve(pending.id(), new ApproveRequest(null), salesManagerActor)) + .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + } + + /** From QUOTATION_ISSUED: the pending quotation revision goes, the issued offer it would have + * replaced stays. */ + @Test + void customerChangeRevision_retiresAPendingQuotationRevision_andTheIssuedOfferStaysLive() { + DealQuotationDto issued = issuedRound(); + DealQuotationDto revision = quotationService.createRevision(issued.id(), salesActor); + quotationService.submit(revision.id(), salesActor); + assertThat(statusOf(revision.id())).isEqualTo("PENDING_APPROVAL"); + + pricingRequestService.createCustomerChangeRevision(issued.pricingRequestId(), customerChangeRevision(), salesActor); + + assertThat(statusOf(revision.id())).isEqualTo("SUPERSEDED"); + assertThat(statusOf(issued.id())).isEqualTo("ISSUED"); + assertThatThrownBy(() -> quotationService.approve(revision.id(), new ApproveRequest(null), salesManagerActor)) + .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); + assertThat(statusOf(issued.id())).isEqualTo("ISSUED"); + } + + /** The approve guard on its own for a SUPERSEDED request, the same way as the CANCELLED test + * above: built through the real revision, then the quotation put back to PENDING_APPROVAL. */ + @Test + void approve_ofAPendingQuotationLeftOnASupersededRequest_isRefusedForEitherApprover_andWritesNothing() { + DealQuotationDto live = issuedRound(); + DealQuotationDto pending = submittedQuotation(); + pricingRequestService.createCustomerChangeRevision(pending.pricingRequestId(), customerChangeRevision(), salesActor); + jdbc.update("UPDATE sales.quotation SET doc_status = 'PENDING_APPROVAL' WHERE quotation_id = :id", + java.util.Map.of("id", pending.id())); + + for (UserPrincipal approver : List.of(salesManagerActor, ceoActor)) { + assertThatThrownBy(() -> quotationService.approve(pending.id(), new ApproveRequest(null), approver)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()) + .isEqualTo("คำขอราคาของใบเสนอราคานี้ถูกแทนที่ด้วยคำขอราคาฉบับแก้ไขแล้ว จึงอนุมัติไม่ได้"); + }); + } + + assertThat(statusOf(pending.id())).isEqualTo("PENDING_APPROVAL"); + assertThat(approverSnapshotCount(pending.id())).isZero(); + assertThat(statusOf(live.id())).isEqualTo("ISSUED"); + assertThat(supersededEventCount(ticketId, live.number(), pending.number())).isZero(); + } + private PricingRequestRequests.CancelPricingRequestRequest cancelRequest() { return new PricingRequestRequests.CancelPricingRequestRequest("ลูกค้ายกเลิกโครงการ"); } + /** A customer-change revision of a request built by {@link #twoItemPricingRequest}: the same + * recipient and the same two lines. */ + private PricingRequestRequests.CustomerChangeRevisionRequest customerChangeRevision() { + return new PricingRequestRequests.CustomerChangeRevisionRequest( + "ลูกค้าขอเปลี่ยนแปลงรายละเอียด", UUID.randomUUID().toString(), PricingRequestRecipient.DESIGNER, null, + "Designer Co.", LocalDate.now().plusDays(14), new BigDecimal("1000.00"), "THB", "revision note", + null, null, null, null, null, null, null, null, + List.of(pricingItem("SCG", "Tile A-S2", "Factory A-S2", new BigDecimal("10")), + pricingItem("Cotto", "Tile B-S2", "Factory B-S2", new BigDecimal("5")))); + } + + /** Waits until a backend is queued for {@code pricingRequestId}'s advisory lock (the + * single-bigint form every request locker uses: classid 0, objid the id, objsubid 1), or until + * {@code caller} finishes without ever queueing. */ + private boolean awaitWaiterOnRequestLock(long pricingRequestId, Future caller) throws InterruptedException { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(20); + while (System.nanoTime() < deadline && !caller.isDone()) { + Long waiting = jdbc.queryForObject(""" + SELECT COUNT(*) FROM pg_locks + WHERE locktype = 'advisory' AND NOT granted + AND classid::bigint = 0 AND objid::bigint = :id AND objsubid = 1 + """, java.util.Map.of("id", pricingRequestId), Long.class); + if (waiting != null && waiting > 0) { + return true; + } + Thread.sleep(50); + } + return false; + } + /** The approvals queue: every PENDING_APPROVAL quotation, as the sales manager lists them. */ private List approvalQueueIds() { return quotationService.search(List.of(QuotationStatus.PENDING_APPROVAL), false, salesManagerActor) diff --git a/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js b/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js index b60076d11..6951b2f5c 100644 --- a/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js +++ b/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js @@ -15,6 +15,28 @@ function nextClientRequestId() { return crypto.randomUUID(); } +/** The fixture request's one line, shared with its customer-change revision below. */ +const REQUEST_ITEM = { + productId: 1, + brand: 'SCG', + model: 'Tile S2 Approval Mock', + factory: 'Panaria SpA', + color: 'ขาว', + texture: 'ด้าน', + size: '60x60', + thicknessMm: 10, + sqmPerPiece: 0.36, + quantityMode: 'PIECES', + piecesInput: 10, + wastageMode: 'NONE', + piecesPerBox: 4, + roundToFullBox: false, + originCountry: 'ไทย-สต็อก', + leadTimeMinDays: 3, + leadTimeMaxDays: 7, + quantityType: 'ESTIMATE', +}; + /** Drives a fresh PCR to an APPROVED NET decision, then createFromPricingRequest — the shared * starting point (a DRAFT quotation, unchanged from the CEO's decision) every test below builds * on. Mirrors mockApi.pricingRequestQuotation.test.js's own `approvedNetPricingRequest` helper, @@ -48,27 +70,7 @@ async function draftPricingRequestQuotation() { recipientType: 'DESIGNER', recipientLabel: 'ผู้ออกแบบทดสอบ S2', clientRequestId: nextClientRequestId(), - items: [{ - sourceTicketItemId: sourceItemId, - productId: 1, - brand: 'SCG', - model: 'Tile S2 Approval Mock', - factory: 'Panaria SpA', - color: 'ขาว', - texture: 'ด้าน', - size: '60x60', - thicknessMm: 10, - sqmPerPiece: 0.36, - quantityMode: 'PIECES', - piecesInput: 10, - wastageMode: 'NONE', - piecesPerBox: 4, - roundToFullBox: false, - originCountry: 'ไทย-สต็อก', - leadTimeMinDays: 3, - leadTimeMaxDays: 7, - quantityType: 'ESTIMATE', - }], + items: [{ sourceTicketItemId: sourceItemId, ...REQUEST_ITEM }], }); const prId = draftPr.summary.id; await api.pricingRequests.submit(prId); @@ -210,12 +212,24 @@ describe('mockApi dealQuotations approval (GLA-123 slice S2, reworked)', () => { }); }); -// Mirrors PricingRequestService#cancel's quotation supersede (PricingRequestRepository -// #supersedeOpenPricingDecisionAndQuotation) and the cancelled-request guard in DealQuotationService -// #approveAndIssuePricingRequestOrigin. Mock plumbing only; the real-DB pins are the cancel/approve -// tests in DealQuotationPricingRequestApprovalIntegrationTest. -describe('mockApi pricingRequests.cancel takes the request\'s open quotation with it', () => { - it('supersedes a PENDING_APPROVAL quotation, which can then no longer be approved', async () => { +/** A customer-change revision of the fixture's request: the same recipient and the same line. */ +function customerChangeRevision() { + return { + recipientType: 'DESIGNER', + recipientLabel: 'ผู้ออกแบบทดสอบ S2', + revisionReason: 'ลูกค้าขอเปลี่ยนแปลงรายละเอียด', + clientRequestId: nextClientRequestId(), + items: [{ ...REQUEST_ITEM }], + }; +} + +// Mirrors the quotation supersede of PricingRequestService#cancel (PricingRequestRepository +// #supersedeOpenPricingDecisionAndQuotation) and of #createCustomerChangeRevision +// (#supersedeUnissuedPricingRequestOriginQuotations), and the cancelled/superseded-request guard in +// DealQuotationService#approveAndIssuePricingRequestOrigin. Mock plumbing only; the real-DB pins are +// the cancel/revision/approve tests in DealQuotationPricingRequestApprovalIntegrationTest. +describe('mockApi: a cancelled or revised request takes its open quotation with it', () => { + it('cancel supersedes a PENDING_APPROVAL quotation, which can then no longer be approved', async () => { const { prId, quotationId } = await pendingApprovalPricingRequestQuotation(); await api.pricingRequests.cancel(prId, { reason: 'ลูกค้ายกเลิกโครงการ' }); const { quotation } = await api.dealQuotations.get(quotationId); @@ -224,20 +238,38 @@ describe('mockApi pricingRequests.cancel takes the request\'s open quotation wit await expect(api.dealQuotations.approve(quotationId, {})).rejects.toMatchObject({ status: 409 }); }); - it('supersedes a DRAFT quotation too', async () => { + it('cancel supersedes a DRAFT quotation too', async () => { const { prId, quotationId } = await draftPricingRequestQuotation(); await api.pricingRequests.cancel(prId, { reason: 'ลูกค้ายกเลิกโครงการ' }); const { quotation } = await api.dealQuotations.get(quotationId); expect(quotation.docStatus).toBe('SUPERSEDED'); }); - it('approve refuses a PENDING_APPROVAL quotation left on a CANCELLED request, for either approver', async () => { + it('a customer-change revision supersedes the old request\'s PENDING_APPROVAL quotation, which can then no longer be approved', async () => { + const { prId, quotationId } = await pendingApprovalPricingRequestQuotation(); + await api.pricingRequests.createCustomerChangeRevision(prId, customerChangeRevision()); + const { quotation } = await api.dealQuotations.get(quotationId); + expect(quotation.docStatus).toBe('SUPERSEDED'); + await api.auth.login({ role: 'sales_manager' }); + await expect(api.dealQuotations.approve(quotationId, {})).rejects.toMatchObject({ status: 409 }); + }); + + it('a customer-change revision supersedes the old request\'s DRAFT quotation too', async () => { + const { prId, quotationId } = await draftPricingRequestQuotation(); + await api.pricingRequests.createCustomerChangeRevision(prId, customerChangeRevision()); + const { quotation } = await api.dealQuotations.get(quotationId); + expect(quotation.docStatus).toBe('SUPERSEDED'); + }); + + it.each([ + ['CANCELLED', 'คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้'], + ['SUPERSEDED', 'คำขอราคาของใบเสนอราคานี้ถูกแทนที่ด้วยคำขอราคาฉบับแก้ไขแล้ว จึงอนุมัติไม่ได้'], + ])('approve refuses a PENDING_APPROVAL quotation left on a %s request, for either approver', async (status, message) => { const { prId, quotationId } = await pendingApprovalPricingRequestQuotation(); - setPricingRequestStatusForTests(prId, 'CANCELLED'); + setPricingRequestStatusForTests(prId, status); for (const role of ['sales_manager', 'ceo']) { await api.auth.login({ role }); - await expect(api.dealQuotations.approve(quotationId, {})) - .rejects.toMatchObject({ status: 409, message: 'คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้' }); + await expect(api.dealQuotations.approve(quotationId, {})).rejects.toMatchObject({ status: 409, message }); } const { quotation } = await api.dealQuotations.get(quotationId); expect(quotation.docStatus).toBe('PENDING_APPROVAL'); From 073bf84e74107e1670fc2e0b8ec051602b8aa8d5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 18:47:36 +0000 Subject: [PATCH 4/5] fix(sales): cancel takes the request lock approve takes; a customer-change revision retires the old request's pending quotation; approve refuses a superseded request - PricingRequestService#cancel takes PricingRequestRepository#lockPricingRequest, the advisory lock DealQuotationService#approveAndIssuePricingRequestOrigin takes first, before its first write. A cancel and an approve of the same request now serialise instead of updating its quotation and request rows in opposite orders. The deal-kill cascade is unchanged: mark lost / cancel deal update the deal row before it, so a request lock there could deadlock the other way. - PricingRequestRepository#supersedeOpenPricingRequestOriginDraft becomes #supersedeUnissuedPricingRequestOriginQuotations and supersedes PENDING_APPROVAL as well as DRAFT. An ISSUED offer stays live until its replacement is issued (2026-08-13 ruling); legacy (origin IS NULL) rows stay out of reach. - DealQuotationService#approveAndIssuePricingRequestOrigin refuses (409) a quotation whose request is SUPERSEDED, with its own message, beside CANCELLED. - mockApi mirrors the revision supersede and the SUPERSEDED guard. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BdQVhpg72sGQrzZVzfMP5U --- .../dealquotation/DealQuotationService.java | 11 ++++-- .../PricingRequestRepository.java | 38 +++++++++---------- .../pricingrequest/PricingRequestService.java | 20 ++++++---- ...ricingRequestQuotationIntegrationTest.java | 2 +- frontend/src/api/mockApi.js | 10 ++++- 5 files changed, 49 insertions(+), 32 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java index 3dcfa5f18..3c2d36d4e 100644 --- a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java +++ b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java @@ -1802,12 +1802,17 @@ private DealQuotationDto approveAndIssuePricingRequestOrigin(DealQuotationDto qu quotations.lockTicket(quotation.ticketId()); PricingRequestSummaryDto summary = pricingRequests.findSummary(pricingRequestId) .orElseThrow(() -> new ApiException(HttpStatus.NOT_FOUND, "ไม่พบคำขอราคานี้")); - // A cancelled request issues nothing. Its cancel supersedes a PENDING_APPROVAL quotation - // (PricingRequestRepository#supersedeOpenPricingDecisionAndQuotation); this refuses one - // orphaned before that status was in the cancel's list. Checked before the first write below. + // A cancelled or superseded request issues nothing (owner ruling 2026-10-27). Its cancel or + // customer-change revision supersedes a PENDING_APPROVAL quotation (PricingRequestRepository + // #supersedeOpenPricingDecisionAndQuotation, #supersedeUnissuedPricingRequestOriginQuotations); + // this refuses one orphaned before they did. Checked before the first write below. if (PricingRequestStatus.CANCELLED.equals(summary.status())) { throw new ApiException(HttpStatus.CONFLICT, "คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้"); } + if (PricingRequestStatus.SUPERSEDED.equals(summary.status())) { + throw new ApiException(HttpStatus.CONFLICT, + "คำขอราคาของใบเสนอราคานี้ถูกแทนที่ด้วยคำขอราคาฉบับแก้ไขแล้ว จึงอนุมัติไม่ได้"); + } // Same DATE-mode-with-special-pricing / DAYS-mode validity computation #approve already // uses for DEAL_DIRECT — reused verbatim rather than forked so the two origins can never // compute this differently by accident. diff --git a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java index 054f544f1..324cc8b0d 100644 --- a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java +++ b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestRepository.java @@ -1038,32 +1038,30 @@ AND status IN ('DRAFT', 'APPROVED') } /** - * MINOR fix (Opus review, 2026-09-20), GLA-123 slice S1 — a PRICING_REQUEST-origin (the NEW, - * {@code th.co.glr.hr.dealquotation} engine) DRAFT quotation orphaned by a customer-change - * revision, mirroring {@link #supersedeOpenPricingDecisionAndQuotation}'s own "DRAFT is in the - * predicate deliberately, and it closes a real hole" reasoning — but scoped MUCH narrower than - * that method, and NOT the same call: {@code createCustomerChangeRevision} deliberately never - * calls {@code supersedeOpenPricingDecisionAndQuotation} itself, because the owner's - * reissue-through-CEO-chain ruling keeps an ALREADY-ISSUED legacy (origin IS NULL) quotation - * live until the replacement is issued (see that method's own Javadoc and this call's own - * comment in {@code PricingRequestService#createCustomerChangeRevision}). That ruling is about - * an issued, customer-facing document — it has no bearing on this origin, which in S1 can - * NEVER reach ISSUED (S1 refused submit for this origin outright; since S2 only {@code createReorder} - * is still refused, via {@code DealQuotationService#requireReorderAllowedForOrigin}), - * so DRAFT is the ONLY state to ever clean up here, and there is no "stays live until the - * replacement issues" concept to preserve for it. Without this, a DRAFT quotation on this - * engine would be left attached to a now-SUPERSEDED parent forever — nothing else in S1 ever - * retires it (there is no S2 issue-time supersede for this origin yet). {@code origin = - * 'PRICING_REQUEST'} in the predicate is what keeps this from ever touching the legacy - * quotation the paragraph above is protecting. + * The PRICING_REQUEST-origin ({@code th.co.glr.hr.dealquotation}) quotations a customer-change + * revision retires on the request it supersedes: every one not yet issued, DRAFT or + * PENDING_APPROVAL. Sole caller is {@code PricingRequestService#createCustomerChangeRevision}. + * + *

DRAFT since GLA-123 S1 (Opus review, 2026-09-20), because nothing else would ever retire + * it. PENDING_APPROVAL by owner ruling 2026-10-27: the customer has changed what they want, so + * the old price must not still be approvable, and issuable, from the superseded request. The + * new round's quotation goes through ผจก.ขาย/CEO approval like any other, and {@code + * DealQuotationService#approveAndIssuePricingRequestOrigin} also refuses a SUPERSEDED request. + * + *

Narrower than {@link #supersedeOpenPricingDecisionAndQuotation}, and deliberately not that + * call: the reissue-through-CEO-chain ruling (2026-08-13) keeps an ISSUED offer live until its + * replacement is issued, so ISSUED and REVISION_REQUESTED stay out of this list (the + * replacement's issue retires them through the deal-wide one-live-offer sweep), and {@code + * origin = 'PRICING_REQUEST'} keeps the legacy (origin IS NULL) quotation that ruling protects + * out of reach. */ - public void supersedeOpenPricingRequestOriginDraft(long pricingRequestId) { + public void supersedeUnissuedPricingRequestOriginQuotations(long pricingRequestId) { jdbc.update(""" UPDATE sales.quotation SET doc_status = 'SUPERSEDED' WHERE pricing_request_id = :pricingRequestId AND origin = 'PRICING_REQUEST' - AND doc_status = 'DRAFT' + AND doc_status IN ('DRAFT', 'PENDING_APPROVAL') """, Map.of("pricingRequestId", pricingRequestId)); } diff --git a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java index b34e0ca97..9941d46bc 100644 --- a/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java +++ b/backend/src/main/java/th/co/glr/hr/pricingrequest/PricingRequestService.java @@ -598,6 +598,12 @@ public PricingRequestDetailDto cancel(long id, CancelPricingRequestRequest reque throw new ApiException(HttpStatus.CONFLICT, "ไม่สามารถยกเลิกคำขอราคาที่อยู่ในสถานะ '" + summary.status() + "' ได้"); } + // The request's lock, the one DealQuotationService#approveAndIssuePricingRequestOrigin takes + // first (owner ruling 2026-10-27): a cancel and an approve of this request serialise instead + // of updating its quotation and request rows in opposite orders. Taken before the first + // write. An approve that got there first leaves the request QUOTATION_ISSUED, so the + // compare-and-set below refuses this cancel. + requests.lockPricingRequest(id); int rows = requests.transition(id, summary.status(), PricingRequestStatus.CANCELLED, null, actor.id()); if (rows == 0) { throw new ApiException(HttpStatus.CONFLICT, "คำขอราคาถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง"); @@ -725,13 +731,13 @@ public PricingRequestDetailDto createCustomerChangeRevision(long id, CustomerCha // (sales.quotation_item), so superseding the decision cannot change what the customer was // quoted. requests.supersedeOpenPricingDecision(parent.id()); - // MINOR fix (Opus review, 2026-09-20) — see supersedeOpenPricingRequestOriginDraft's own - // Javadoc for why this is a SEPARATE, narrower call from the decision supersede just - // above, and why it is safe alongside the "legacy ISSUED quotation stays live" ruling - // documented in the block comment above: this only ever touches an origin = - // 'PRICING_REQUEST' DRAFT, never the legacy (origin IS NULL) quotation that ruling - // protects. - requests.supersedeOpenPricingRequestOriginDraft(parent.id()); + // The request's PRICING_REQUEST-origin quotations not yet issued go with it: DRAFT, and + // PENDING_APPROVAL by owner ruling 2026-10-27. See + // supersedeUnissuedPricingRequestOriginQuotations's own Javadoc for why this is a SEPARATE, + // narrower call from the decision supersede just above, and why it is safe alongside the + // "issued quotation stays live" ruling documented in the block comment above: it never + // touches an ISSUED offer, nor the legacy (origin IS NULL) quotation that ruling protects. + requests.supersedeUnissuedPricingRequestOriginQuotations(parent.id()); requests.addEvent(parent.id(), parent.ticketId(), actor.id(), actor.name(), PricingRequestEventKind.PRICING_REQUEST_REVISED, parent.status(), PricingRequestStatus.SUPERSEDED, request.revisionReason(), toRevisionMetadataJson(newId)); diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/PricingRequestQuotationIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/PricingRequestQuotationIntegrationTest.java index 8b63ef348..759abd3e1 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/PricingRequestQuotationIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/PricingRequestQuotationIntegrationTest.java @@ -1255,7 +1255,7 @@ void refreshDraftContactSnapshot_updatesAPricingRequestOriginDraftToo() { // DRAFT-supersede behaviour), or S1 would leave it orphaned forever (there is no S2 issue-time // supersede for this origin yet). Never touches a LEGACY quotation, which the owner's // reissue-through-CEO-chain ruling deliberately keeps live until the replacement is issued — - // see supersedeOpenPricingRequestOriginDraft's own Javadoc for the full reasoning. + // see supersedeUnissuedPricingRequestOriginQuotations's own Javadoc for the full reasoning. // ───────────────────────────────────────────────────────────────────────────────────── @Test diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 88a9bfa6b..ba0c164e0 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -6848,9 +6848,10 @@ function mockApproveDealQuotationPricingRequestOrigin(row, payload, user) { || dto.itemsRemovedFromCeoCount > 0; if (changed) fail('มีรายการที่เปลี่ยนจากราคา CEO — ต้องให้ CEO อนุมัติ', 403); } - // The service's cancelled-request guard: a cancelled request issues nothing. Read before any write. + // The service's guard: a cancelled or superseded request issues nothing. Read before any write. const pr = findPricingRequestRaw(row.pricingRequestId); if (pr.status === 'CANCELLED') fail('คำขอราคาของใบเสนอราคานี้ถูกยกเลิกแล้ว จึงอนุมัติไม่ได้', 409); + if (pr.status === 'SUPERSEDED') fail('คำขอราคาของใบเสนอราคานี้ถูกแทนที่ด้วยคำขอราคาฉบับแก้ไขแล้ว จึงอนุมัติไม่ได้', 409); const now = new Date().toISOString(); row.docStatus = 'ISSUED'; row.approvedById = user.employeeId ?? null; @@ -16677,6 +16678,13 @@ export const api = { .filter((q) => q.pricingRequestId === parent.id && ['ISSUED', 'READY_TO_ISSUE', 'SENT', 'REVISION_REQUESTED'].includes(q.docStatus)) .forEach((q) => { q.docStatus = 'SUPERSEDED'; }); + // Mirrors PricingRequestRepository#supersedeUnissuedPricingRequestOriginQuotations: the old + // request's new-engine quotations not yet issued go with it (PENDING_APPROVAL by owner ruling + // 2026-10-27). An ISSUED one stays live until its replacement is issued. + mockDealQuotations + .filter((q) => q.pricingRequestId === parent.id && q.origin === 'PRICING_REQUEST' + && ['DRAFT', 'PENDING_APPROVAL'].includes(q.docStatus)) + .forEach((q) => { q.docStatus = 'SUPERSEDED'; }); pushPricingRequestEvent(parent, user, 'PRICING_REQUEST_REVISED', parentStatus, 'SUPERSEDED', payload.revisionReason); pushPricingRequestEvent(revision, user, 'PRICING_REQUEST_CREATED', null, 'DRAFT', payload.revisionReason); return delay({ pricingRequest: buildPricingRequestDetail(revision) }); From b788261f94e616473448d5cd0034bc751bfc79bf Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:06:31 +0700 Subject: [PATCH 5/5] test(sales): cancelling a revision does not revive a parent whose pending quotation was retired (pins #1216 interaction) Co-Authored-By: Claude Sonnet 5.5 --- ...onPricingRequestApprovalIntegrationTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java index 2d26a2236..f8c5ba1cc 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationPricingRequestApprovalIntegrationTest.java @@ -954,6 +954,23 @@ void approve_ofAPendingQuotationLeftOnASupersededRequest_isRefusedForEitherAppro assertThat(supersededEventCount(ticketId, live.number(), pending.number())).isZero(); } + /** #1216's revive hands the parent back only when it still owns an ISSUED quotation. The + * revision retired the parent's PENDING_APPROVAL quotation, so cancelling the revision must NOT + * revive the parent into QUOTATION_ISSUED with no live quote. */ + @Test + void cancellingTheRevision_doesNotReviveAParentWhoseOnlyQuotationWasRetiredFromPendingApproval() { + DealQuotationDto pending = submittedQuotation(); + long parentId = pending.pricingRequestId(); + long childId = pricingRequestService.createCustomerChangeRevision(parentId, customerChangeRevision(), salesActor) + .summary().id(); + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + + pricingRequestService.cancel(childId, cancelRequest(), salesActor); + + assertThat(pricingRequestStatusOf(parentId)).isEqualTo(PricingRequestStatus.SUPERSEDED); + assertThat(statusOf(pending.id())).isEqualTo("SUPERSEDED"); + } + private PricingRequestRequests.CancelPricingRequestRequest cancelRequest() { return new PricingRequestRequests.CancelPricingRequestRequest("ลูกค้ายกเลิกโครงการ"); }