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..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,6 +1802,17 @@ private DealQuotationDto approveAndIssuePricingRequestOrigin(DealQuotationDto qu quotations.lockTicket(quotation.ticketId()); PricingRequestSummaryDto summary = pricingRequests.findSummary(pricingRequestId) .orElseThrow(() -> new ApiException(HttpStatus.NOT_FOUND, "ไม่พบคำขอราคานี้")); + // 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 84d1905fd..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 @@ -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)); } @@ -1030,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 f0b7c3e6e..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, "คำขอราคาถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง"); @@ -609,9 +615,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(), @@ -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/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/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..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 @@ -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; @@ -70,6 +76,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 +754,285 @@ 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()); + } + + /** 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(); + } + + /** #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("ลูกค้ายกเลิกโครงการ"); + } + + /** 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) + .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/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.dealQuotationPricingRequestApproval.test.js b/frontend/src/api/mockApi.dealQuotationPricingRequestApproval.test.js index 4a9a67559..6951b2f5c 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 @@ -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); @@ -209,3 +211,67 @@ describe('mockApi dealQuotations approval (GLA-123 slice S2, reworked)', () => { } }); }); + +/** 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); + expect(quotation.docStatus).toBe('SUPERSEDED'); + await api.auth.login({ role: 'sales_manager' }); + await expect(api.dealQuotations.approve(quotationId, {})).rejects.toMatchObject({ status: 409 }); + }); + + 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('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, status); + 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'); + }); +}); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 441319732..ba0c164e0 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -6848,6 +6848,10 @@ function mockApproveDealQuotationPricingRequestOrigin(row, payload, user) { || dto.itemsRemovedFromCeoCount > 0; if (changed) fail('มีรายการที่เปลี่ยนจากราคา CEO — ต้องให้ CEO อนุมัติ', 403); } + // 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; @@ -6865,7 +6869,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'; @@ -16675,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) }); @@ -16697,6 +16707,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) }); },