From f4fc31087b0217597b86447cdf0be3a1d08585a5 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:03:00 +0700 Subject: [PATCH 01/19] =?UTF-8?q?test(sales):=20pin=20the=20=E0=B8=A1?= =?UTF-8?q?=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft=20from=20a=20new-e?= =?UTF-8?q?ngine=20pricing-request=20quotation=20(reproduces=20the=20409)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- .../DealQuotationOutcomeIntegrationTest.java | 67 ++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java index 3db12a414..f562b5f8c 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java @@ -33,6 +33,8 @@ import th.co.glr.hr.dealquotation.DealQuotationDtos.DealQuotationDto; import th.co.glr.hr.dealquotation.DealQuotationRequests.ApproveRequest; import th.co.glr.hr.dealquotation.DealQuotationRequests.RecordOutcomeRequest; +import th.co.glr.hr.deposit.DepositNoticeDraftRequest; +import th.co.glr.hr.deposit.DepositNoticeDto; import th.co.glr.hr.deposit.DepositNoticeRenderer; import th.co.glr.hr.deposit.DepositNoticeRepository; import th.co.glr.hr.deposit.DepositNoticeService; @@ -106,6 +108,7 @@ class DealQuotationOutcomeIntegrationTest extends AbstractPostgresIntegrationTes private CustomerQuotationRepository legacyQuotationRepository; private CustomerQuotationService legacyQuotationService; private OrderConfirmationService orderConfirmation; + private DepositNoticeService depositNoticeService; private TicketRepository tickets; private TicketService ticketService; @@ -184,7 +187,7 @@ decisionRepository, tickets, ticketService, customers, new QuotationRenderer(), new DiscountApprovalRepository(jdbc)); DepositNoticeRepository depositNoticeRepository = new DepositNoticeRepository(jdbc); - DepositNoticeService depositNoticeService = new DepositNoticeService(depositNoticeRepository, tickets, + depositNoticeService = new DepositNoticeService(depositNoticeRepository, tickets, notifications, new DepositNoticeRenderer(), new RemainingInvoiceRenderer(), customers, legacyQuotationRepository); orderConfirmation = new OrderConfirmationService( @@ -239,6 +242,68 @@ void recordOutcome_byOwningSales_accepted_setsPrStatusAndDocStatus() { .isEqualTo(PricingRequestStatus.QUOTATION_ACCEPTED); } + // ── Deposit notice from the pricing-request route when the accepted quotation is NEW-engine ── + // QA finding 2026-10-18 (develop QA stack): DealDepositPanel/DepositNoticePage call + // POST /pricing-requests/{id}/deposit-notice whenever a QUOTATION_ACCEPTED pricing request exists; + // that route read only legacy CustomerQuotation rows and answered 409 for every deal quoted + // through DealQuotationService.createFromPricingRequest. Direct quotations never take this route. + + @Test + void createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameDraftTheTicketRouteWould() { + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto viaPricingRequest = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + + // The reference is the accepted quotation's number — the one thing this route adds over the ticket route. + assertThat(viaPricingRequest.reference()).isEqualTo(accepted.number()); + assertThat(viaPricingRequest.items()).isNotEmpty(); + + // Items and percent are exactly what the ticket-level route produces for this deal (shared createDraft). + DepositNoticeDto viaTicket = depositNoticeService.createDraft(ticketId, + new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); + assertThat(viaPricingRequest.depositPercent()).isEqualByComparingTo(viaTicket.depositPercent()); + assertThat(viaPricingRequest.items()) + .usingRecursiveComparison().ignoringFields("id") + .isEqualTo(viaTicket.items()); + + assertThat(jdbc.queryForObject(""" + SELECT COUNT(*) FROM sales.pricing_request_event + WHERE pricing_request_id = :id AND event_kind = 'DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION' + """, Map.of("id", pricingRequestId), Long.class)).isEqualTo(1L); + } + + @Test + void createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409() { + DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = issued.pricingRequestId(); + + assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor)) + .isInstanceOf(ApiException.class) + .hasMessageContaining("ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isZero(); + } + + @Test + void createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach() { + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), otherSalesActor)) + .isInstanceOf(ApiException.class) + .hasMessageContaining("ไม่มีสิทธิ์เข้าถึงรายการนี้"); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isZero(); + } + @Test void recordOutcome_byNonOwningSales_refused_writesNothing() { DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); From d247e6a61822fc2a9d0e293b12fbdf8c577d9e4b Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 00:21:39 +0700 Subject: [PATCH 02/19] =?UTF-8?q?fix(sales):=20=E0=B8=A1=E0=B8=B1=E0=B8=94?= =?UTF-8?q?=E0=B8=88=E0=B8=B3=20draft=20from=20the=20pricing-request=20rou?= =?UTF-8?q?te=20accepts=20a=20new-engine=20ACCEPTED=20quotation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The route read only legacy CustomerQuotation rows, so every deal quoted through DealQuotationService.createFromPricingRequest got 409 at ขั้น 10. It now falls back to the pricing request's new-engine ACCEPTED quotation and lets createDraft source items and percent exactly as the ticket-level route does. ISSUED-only still 409s; owner gate unchanged. Co-Authored-By: Claude Opus 5.5 --- .../OrderConfirmationService.java | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index f7c000c25..032f0c3f5 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -428,6 +428,19 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * {@link CreateDepositNoticeFromQuotationRequest} itself unremoved (an older frontend build * may still send it) rather than making it a breaking DTO change for a value that is now a * no-op either way. + * + *

New-engine quotation (2026-10-19 fix): when the pricing request has no + * ACCEPTED legacy quotation, this falls back to its new-engine {@code PRICING_REQUEST}-origin + * quotation ({@code DealQuotationService#createFromPricingRequest}) when that one is ACCEPTED. + * Before this, every deal quoted through the new engine got the 409 below. For that branch + * {@code items} is passed as {@code null}, so {@link DepositNoticeService#createDraft} sources + * the items through the same fallback chain the ticket-level route uses, which for a + * new-engine deal ends at {@code DepositNoticeRepository#findDealQuotationItemsForDeposit(ticketId)}. + * That source is scoped to the ticket, not to the pricing request. It is still safe here because + * R8 in {@code DealQuotationService#recordOutcome} ({@code TicketRepository#hasAcceptedQuotation}) + * refuses a second ACCEPTED quotation on one ticket, so the ticket's accepted quotation is this + * pricing request's. The 409 semantics do not change: the customer must have accepted, and an + * ISSUED-only quotation still gets 409. */ @Transactional public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, @@ -439,23 +452,40 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, CustomerQuotationDto accepted = quotations.findByPricingRequest(pricingRequestId).stream() .filter(q -> QuotationStatus.ACCEPTED.equals(q.docStatus())) .findFirst() - .orElseThrow(() -> new ApiException(HttpStatus.CONFLICT, - "ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้")); - - // Mapping itself now lives on DepositNoticeService (this branch's fix), shared with that - // class's own new-chain item fallback in buildItemsFromRequest — see its Javadoc. - List items = DepositNoticeService.itemsFromQuotation(accepted.items()); + .orElse(null); + + String reference; + List items; + if (accepted != null) { + reference = accepted.number(); + // Mapping itself now lives on DepositNoticeService (this branch's fix), shared with that + // class's own new-chain item fallback in buildItemsFromRequest — see its Javadoc. + items = DepositNoticeService.itemsFromQuotation(accepted.items()); + } else { + // New-engine branch: see this method's Javadoc for why items stay null here. + th.co.glr.hr.dealquotation.DealQuotationDtos.DealQuotationDto newEngine = dealQuotations == null + ? null + : dealQuotations.findLatestForPricingRequest(pricingRequestId) + .flatMap(dealQuotations::findById) + .filter(q -> QuotationStatus.ACCEPTED.equals(q.docStatus())) + .orElse(null); + if (newEngine == null) { + throw new ApiException(HttpStatus.CONFLICT, "ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"); + } + reference = newEngine.number(); + items = null; + } // request.depositPercent() is deliberately still passed through here even though // createDraft now ignores it as the source — see this method's own Javadoc above. DepositNoticeDraftRequest draftRequest = new DepositNoticeDraftRequest( - null, null, null, null, accepted.number(), + null, null, null, null, reference, request != null ? request.depositPercent() : null, null, items); DepositNoticeDto draft = depositNotices.createDraft(summary.ticketId(), draftRequest, actor); pricingRequests.addEvent(pricingRequestId, summary.ticketId(), actor.id(), actor.name(), PricingRequestEventKind.DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION, summary.status(), summary.status(), - "สร้างร่างใบแจ้งยอดเงินรับมัดจำจากใบเสนอราคา " + accepted.number(), null); + "สร้างร่างใบแจ้งยอดเงินรับมัดจำจากใบเสนอราคา " + reference, null); return draft; } From 57498fa83aa68a791f52dbda62c32c4ec9c6cd07 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 02:15:53 +0700 Subject: [PATCH 03/19] =?UTF-8?q?test(sales):=20pin=20the=20pricing-reques?= =?UTF-8?q?t=20=E0=B8=A1=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft=20to?= =?UTF-8?q?=20THIS=20pricing=20request's=20accepted=20quotation=20(reprodu?= =?UTF-8?q?ces=20I-1/I-2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two red tests against d247e6a6: - createDepositNoticeFromQuotation_newEngineAcceptedWithAnOpenRevisionDraft_stillDraftsFromTheAcceptedOne (I-2): an open revision draft outranks the accepted quotation by id, so the "latest for this pricing request" lookup 409s although Q1 is accepted. - createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLines (I-1): with items = null, createDraft picks a sibling pricing request's legacy ISSUED quotation first, so the draft carries PR-A's lines under PR-B's number. Co-Authored-By: Claude Opus 5.5 --- .../DealQuotationOutcomeIntegrationTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java index f562b5f8c..22069dc40 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java @@ -304,6 +304,54 @@ void createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach() { Map.of("id", ticketId), Long.class)).isZero(); } + /** An open revision draft outranks the accepted quotation by id: recordOutcome has no + * open-revision guard, so "latest for this pricing request" is not "accepted". */ + @Test + void createDepositNoticeFromQuotation_newEngineAcceptedWithAnOpenRevisionDraft_stillDraftsFromTheAcceptedOne() { + DealQuotationDto q1 = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + DealQuotationDto q2 = quotationService.createRevision(q1.id(), salesActor); + assertThat(q2.id()).isGreaterThan(q1.id()); + assertThat(q2.docStatus()).isEqualTo(QuotationStatus.DRAFT); + quotationService.recordOutcome(q1.id(), + new RecordOutcomeRequest(QuotationStatus.ACCEPTED, "ลูกค้าโอเค", UUID.randomUUID().toString()), salesActor); + long pricingRequestId = q1.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + + assertThat(draft.reference()).isEqualTo(q1.number()); + assertThat(draft.items()).isNotEmpty(); + } + + /** Engines are exclusive per pricing request, not per ticket: a sibling pricing request's + * legacy ISSUED quotation must not supply the lines of this pricing request's draft. */ + @Test + void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLines() { + CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + + List thisQuotationsLines = jdbc.query(""" + SELECT qty, final_unit_price FROM sales.quotation_item WHERE quotation_id = :id ORDER BY seq + """, Map.of("id", accepted.id()), + (rs, i) -> line(rs.getBigDecimal("qty"), rs.getBigDecimal("final_unit_price"))); + List siblingLegacyLines = DepositNoticeService.itemsFromQuotation(legacyIssued.items()).stream() + .map(it -> line(it.qty(), it.netUnitPrice())).toList(); + // The fixture must make the two sources distinguishable, or the assertion below proves nothing. + assertThat(thisQuotationsLines).isNotEmpty().isNotEqualTo(siblingLegacyLines); + + assertThat(draft.reference()).isEqualTo(accepted.number()); + assertThat(draft.items().stream().map(it -> line(it.qty(), it.netUnitPrice())).toList()) + .isEqualTo(thisQuotationsLines); + } + @Test void recordOutcome_byNonOwningSales_refused_writesNothing() { DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); @@ -863,6 +911,11 @@ void recordOutcome_crossEngineClientRequestIdCollision_newThenLegacy_refused409( // Fixture helpers // ───────────────────────────────────────────────────────────────────────────────────── + private static String line(BigDecimal qty, BigDecimal netUnitPrice) { + return (qty == null ? "null" : qty.stripTrailingZeros().toPlainString()) + " @ " + + (netUnitPrice == null ? "null" : netUnitPrice.stripTrailingZeros().toPlainString()); + } + private DealQuotationDto acceptedNewOriginQuotation(String recipient, BigDecimal qty) { DealQuotationDto issued = issuedNewOriginQuotation(recipient, qty); return quotationService.recordOutcome(issued.id(), From 0d2548ee708b5c555c2754dc98f88be49d128ef2 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:20:28 +0700 Subject: [PATCH 04/19] =?UTF-8?q?fix(sales):=20pricing-request=20=E0=B8=A1?= =?UTF-8?q?=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft=20reads=20the=20ac?= =?UTF-8?q?cepted=20quotation=20of=20THIS=20pricing=20request,=20not=20the?= =?UTF-8?q?=20ticket's=20latest?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 1 on d247e6a6: - I-2: the new-engine lookup took the pricing request's LATEST quotation and filtered for ACCEPTED, so an open revision draft (higher id) hid an accepted Q1 and the route 409'd. New DealQuotationRepository#findAcceptedForPricingRequest selects the ACCEPTED row directly. - I-1: items = null let createDraft source lines ticket-wide, legacy first, so a sibling pricing request's legacy ISSUED quotation supplied the lines under this pricing request's number. Items now come from the accepted quotation itself (DepositNoticeService#itemsFromDealQuotation -> new DepositNoticeRepository#findDealQuotationItems, extracted unchanged from findDealQuotationItemsForDeposit, which now delegates to it). - The percent is still createDraft's ticket-scoped, legacy-first pick: a known limitation shared with the ticket route, stated in the Javadoc. - Javadoc corrections: m-1 (changelog prose dropped), m-2 (percent paragraph names the legacy branch), m-3 (PRICING_REQUEST origin does have revisions), m-4 (engine exclusivity is per pricing request, so legacy-first is a real preference on a mixed ticket). Tests (red on d247e6a6, green here): - createDepositNoticeFromQuotation_newEngineAcceptedWithAnOpenRevisionDraft_stillDraftsFromTheAcceptedOne - createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLines Co-Authored-By: Claude Opus 5.5 --- .../DealQuotationRepository.java | 31 +++++++++++++--- .../hr/deposit/DepositNoticeRepository.java | 19 +++++++--- .../glr/hr/deposit/DepositNoticeService.java | 6 ++++ .../OrderConfirmationService.java | 35 ++++++++++--------- 4 files changed, 65 insertions(+), 26 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRepository.java b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRepository.java index 06716828e..656cb6db8 100644 --- a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRepository.java +++ b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRepository.java @@ -282,10 +282,12 @@ public Optional findOpenDraftForPricingRequest(long pricingRequestId) { * most recent PRICING_REQUEST-origin quotation for this pricing request, REGARDLESS of * status — used by {@code DealQuotationService#createFromPricingRequest} to continue the SAME * {@code {base}-{n}} number family when re-creating after an EXPIRED quotation, rather than - * minting an unrelated fresh base number. There is at most one LIVE row at a time (M2/the - * one-quotation-per-request rule) and this origin has no revision chain (D12), so "most - * recent by id" and "the only row" coincide in every reachable case today; ORDER BY id DESC - * is still the honest query to write in case that ever stops being true. + * minting an unrelated fresh base number. This origin has a revision chain: {@code + * DealQuotationService#createRevision} copies {@code origin} and {@code pricing_request_id} + * from an ISSUED or REVISION_REQUESTED source, so one pricing request can hold several rows + * and the newest by id can be an open revision DRAFT. Newest by id is what continuing the + * number family needs; a caller that needs the customer-accepted row uses {@link + * #findAcceptedForPricingRequest} instead. */ public Optional findLatestForPricingRequest(long pricingRequestId) { try { @@ -300,6 +302,27 @@ public Optional findLatestForPricingRequest(long pricingRequestId) { } } + /** + * The customer-ACCEPTED {@code PRICING_REQUEST}-origin quotation for this pricing request, + * newest by id. {@code OrderConfirmationService#createDepositNoticeFromQuotation} uses this + * rather than {@link #findLatestForPricingRequest} because an open revision draft can outrank + * the accepted row by id: {@code DealQuotationService#recordOutcome} has no open-revision + * guard, so the customer can accept Q1 while its revision Q2 is still a DRAFT. + */ + public Optional findAcceptedForPricingRequest(long pricingRequestId) { + try { + return Optional.ofNullable(jdbc.queryForObject(""" + SELECT quotation_id FROM sales.quotation + WHERE pricing_request_id = :pricingRequestId AND origin = 'PRICING_REQUEST' + AND doc_status = 'ACCEPTED' + ORDER BY quotation_id DESC + LIMIT 1 + """, Map.of("pricingRequestId", pricingRequestId), Long.class)); + } catch (EmptyResultDataAccessException e) { + return Optional.empty(); + } + } + /** One accepted line's reconciliation-relevant numbers — {@link #qty} is {@code * quotation_item.qty} (see {@link #findAcceptedItemQuantitiesByPricingRequest}'s own Javadoc); * {@link #sqmPerPiece} is {@code quotation_item.sqm_per_piece} (V165), read from the SAME row diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java index 87663b6ac..6337ca9fa 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java @@ -113,10 +113,13 @@ public List findByTicket(long ticketId) { * single live quotation, preferring ACCEPTED, then ISSUED, then APPROVED (DEAL_DIRECT's * own only live terminal state), newest by id. * - * A given ticket's quotations are only ever ALL-legacy or ALL-origin-tagged (each engine - * stamps {@code origin} consistently for every row it writes), so at most one branch below - * ever matches — trying legacy first is a fixed, harmless order, not a real preference - * between the two lineages. Returns empty when there is no live (ACCEPTED/ISSUED/APPROVED) + * Engines are exclusive per pricing request, not per ticket ({@code + * CustomerQuotationService#create} and {@code DealQuotationService#createFromPricingRequest} + * each refuse a pricing request the other engine already quoted), so one ticket can carry a + * legacy quotation on one pricing request and an origin-tagged one on another. On such a mixed + * ticket both branches below can match, and trying legacy first IS a real preference: the + * percent comes from the legacy row. + * Returns empty when there is no live (ACCEPTED/ISSUED/APPROVED) * quotation for this ticket, or its {@code deposit_percent} is null — the caller ({@link * DepositNoticeService#createDraft}) falls back to 0.50 in that case, this document's * long-standing default. @@ -416,12 +419,18 @@ AND doc_status IN ('ACCEPTED', 'ISSUED', 'APPROVED') if (ids.isEmpty()) { return List.of(); } + return findDealQuotationItems(ids.get(0)); + } + + /** One deal quotation's lines as deposit-notice items, read the same way {@link + * #findDealQuotationItemsForDeposit} reads the quotation it selects for a ticket. */ + public List findDealQuotationItems(long quotationId) { return jdbc.query(""" SELECT seq, description, qty, raw_unit, unit_price, sales_discount, final_unit_price FROM sales.quotation_item WHERE quotation_id = :quotationId ORDER BY seq - """, Map.of("quotationId", ids.get(0)), (rs, i) -> { + """, Map.of("quotationId", quotationId), (rs, i) -> { String description = rs.getString("description"); if (description == null || description.isBlank()) { description = "รายการสินค้า"; diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java index abf49127e..e8bed4be5 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java @@ -1271,6 +1271,12 @@ private CustomerQuotationDto pickQuotation(List candidates return latestAccepted != null ? latestAccepted : latestIssued; } + /** The pricing-request route's new-engine item source, so the draft's rows come from the + * quotation its reference names. */ + public List itemsFromDealQuotation(long quotationId) { + return docs.findDealQuotationItems(quotationId); + } + /** * Maps a customer quotation's items to deposit-notice item requests. The single shared mapping * used by both {@link #buildItemsFromRequest} (this class's own ticket-chain fallback above) diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index 032f0c3f5..d4b4929be 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -422,25 +422,27 @@ private String mapUnitBasisToTicketItem(String unitBasis) { *

Deposit percent (owner ruling, 2026-09-29): {@code * request.depositPercent()} below is passed through but {@link * DepositNoticeService#createDraft} now IGNORES it unconditionally and re-derives the percent - * itself from {@code accepted}'s own {@code sales.quotation.deposit_percent} via {@code + * itself from {@code accepted}'s own {@code sales.quotation.deposit_percent} (legacy branch; + * the new-engine paragraph below covers the other branch) via {@code * DepositNoticeRepository#findDealQuotationDepositPercent(ticketId)} — this call site needs no * separate fix because it already funnels through that one shared method. Left the field on * {@link CreateDepositNoticeFromQuotationRequest} itself unremoved (an older frontend build * may still send it) rather than making it a breaking DTO change for a value that is now a * no-op either way. * - *

New-engine quotation (2026-10-19 fix): when the pricing request has no - * ACCEPTED legacy quotation, this falls back to its new-engine {@code PRICING_REQUEST}-origin - * quotation ({@code DealQuotationService#createFromPricingRequest}) when that one is ACCEPTED. - * Before this, every deal quoted through the new engine got the 409 below. For that branch - * {@code items} is passed as {@code null}, so {@link DepositNoticeService#createDraft} sources - * the items through the same fallback chain the ticket-level route uses, which for a - * new-engine deal ends at {@code DepositNoticeRepository#findDealQuotationItemsForDeposit(ticketId)}. - * That source is scoped to the ticket, not to the pricing request. It is still safe here because - * R8 in {@code DealQuotationService#recordOutcome} ({@code TicketRepository#hasAcceptedQuotation}) - * refuses a second ACCEPTED quotation on one ticket, so the ticket's accepted quotation is this - * pricing request's. The 409 semantics do not change: the customer must have accepted, and an - * ISSUED-only quotation still gets 409. + *

New-engine quotation: when the pricing request has no ACCEPTED legacy + * quotation, this falls back to its ACCEPTED new-engine {@code PRICING_REQUEST}-origin + * quotation ({@code DealQuotationService#createFromPricingRequest}), found by {@code + * DealQuotationRepository#findAcceptedForPricingRequest} rather than by "latest", because an + * open revision draft can outrank the accepted row by id. The items come from that quotation, + * the accepted quotation of THIS pricing request ({@link + * DepositNoticeService#itemsFromDealQuotation}), so a sibling pricing request's quotation on + * the same ticket can never supply them. The percent is still whatever {@code createDraft} + * derives, which is ticket-scoped and legacy-first: on a ticket that also carries a sibling + * pricing request's legacy ISSUED quotation, the percent comes from that legacy row. That is a + * known limitation the ticket-level route shares; this route does not introduce it. The 409 + * semantics do not change: the customer must have accepted, and an ISSUED-only quotation still + * gets 409. */ @Transactional public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, @@ -462,18 +464,17 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, // class's own new-chain item fallback in buildItemsFromRequest — see its Javadoc. items = DepositNoticeService.itemsFromQuotation(accepted.items()); } else { - // New-engine branch: see this method's Javadoc for why items stay null here. + // New-engine branch: the accepted quotation of THIS pricing request — see the Javadoc. th.co.glr.hr.dealquotation.DealQuotationDtos.DealQuotationDto newEngine = dealQuotations == null ? null - : dealQuotations.findLatestForPricingRequest(pricingRequestId) + : dealQuotations.findAcceptedForPricingRequest(pricingRequestId) .flatMap(dealQuotations::findById) - .filter(q -> QuotationStatus.ACCEPTED.equals(q.docStatus())) .orElse(null); if (newEngine == null) { throw new ApiException(HttpStatus.CONFLICT, "ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"); } reference = newEngine.number(); - items = null; + items = depositNotices.itemsFromDealQuotation(newEngine.id()); } // request.depositPercent() is deliberately still passed through here even though From 18aeabe3376c54f789fb253aae7b392002e8db60 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:32:55 +0700 Subject: [PATCH 05/19] =?UTF-8?q?test(mock):=20pin=20the=20pricing-request?= =?UTF-8?q?=20=E0=B8=A1=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft=20from?= =?UTF-8?q?=20a=20new-engine=20ACCEPTED=20quotation=20(reproduces=20the=20?= =?UTF-8?q?mock=20409)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 --- .../src/api/mockApi.depositNotices.test.js | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index 811cf846d..a595e59d8 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -20,7 +20,10 @@ function uuid() { return `77777777-7777-4777-8777-${String(seq).padStart(12, '0')}`; } -async function driveTicketToAcceptedQuotation() { +// Everything up to and including the CEO-approved pricing decision. `priceMode` is set only +// by the new-engine drive: dealQuotations.createFromPricingRequest refuses a decision without +// one, and the legacy drive below never set it. +async function driveTicketToApprovedDecision({ priceMode } = {}) { const n = seq + 1; await api.auth.login({ role: 'sales' }); @@ -114,10 +117,17 @@ async function driveTicketToAcceptedQuotation() { await api.auth.login({ role: 'ceo' }); const { decision } = await api.pricingRequests.startPricingDecision(prId, { defaultMarginPct: 0.2 }); await api.pricingRequests.updatePricingDecision(decision.id, { + ...(priceMode ? { priceMode } : {}), items: decision.items.map((item) => ({ pricingDecisionItemId: item.id, minimumSellingPrice: 50 })), }); await api.pricingRequests.approvePricingDecision(decision.id, {}); + return { ticketId, prId, customer, project }; +} + +async function driveTicketToAcceptedQuotation() { + const { ticketId, prId, customer, project } = await driveTicketToApprovedDecision(); + await api.auth.login({ role: 'sales' }); const { quotation: draftQuotation } = await api.pricingRequests.createCustomerQuotation(prId, {}); const { quotation: issued } = await api.pricingRequests.issueCustomerQuotation(draftQuotation.id, {}); @@ -127,6 +137,42 @@ async function driveTicketToAcceptedQuotation() { return { ticketId, customer, project, quotation: issued }; } +// DealQuotationOutcomeIntegrationTest#upsertWithValidityDays: the stored draft re-sent with only +// validityDays changed (recipientType is left out, as there — this origin refuses it). +function upsertWithValidityDays(q, days) { + const { + contactId, deptCode, unitCode, offerDate, depositPercent, remainderMode, creditDays, + customerNotes, priceMode, documentLanguage, currency, items, + } = q; + return { + contactId, deptCode, unitCode, offerDate, depositPercent, remainderMode, creditDays, + validityDays: days, customerNotes, priceMode, documentLanguage, currency, items, + }; +} + +// New-engine (PRICING_REQUEST-origin) quotation, issued by the sales manager — the same drive as +// DealQuotationOutcomeIntegrationTest#issuedNewOriginQuotation. +async function driveTicketToNewEngineIssuedQuotation() { + const { ticketId, prId } = await driveTicketToApprovedDecision({ priceMode: 'NET' }); + await api.auth.login({ role: 'sales' }); + const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); + await api.dealQuotations.update(draft.id, upsertWithValidityDays(draft, 30)); + await api.dealQuotations.submit(draft.id, {}); + await api.auth.login({ role: 'sales_manager' }); + const { quotation: issued } = await api.dealQuotations.approve(draft.id, {}); + await api.auth.login({ role: 'sales' }); + return { ticketId, prId, issued }; +} + +async function driveTicketToNewEngineAcceptedQuotation() { + const { ticketId, prId, issued } = await driveTicketToNewEngineIssuedQuotation(); + const { quotation: accepted } = await api.dealQuotations.recordOutcome(issued.id, { + outcome: 'ACCEPTED', customerNote: null, clientRequestId: uuid(), + }); + await api.pricingRequests.confirmOrder(prId, { clientRequestId: uuid() }); + return { ticketId, prId, accepted }; +} + describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notice autofill)', () => { it('sources items from the accepted quotation and autofills the header when no ticket item has an approved price', async () => { const { ticketId, customer, project, quotation } = await driveTicketToAcceptedQuotation(); @@ -173,6 +219,29 @@ describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notic }); }); +// Mirrors OrderConfirmationService.createDepositNoticeFromQuotation's new-engine branch: with no +// ACCEPTED legacy quotation, the draft comes from THIS pricing request's ACCEPTED new-engine +// quotation (DealQuotationRepository.findAcceptedForPricingRequest), and an ISSUED-only one +// still gets the 409. +describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engine quotation (mirrors OrderConfirmationService fix)', () => { + it('creates the draft with the accepted quotation number as reference and items from that quotation', async () => { + const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); + const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); + expect(depositNotice.reference).toBe(accepted.number); + expect(depositNotice.items.length).toBeGreaterThan(0); + expect(depositNotice.items[0].qty).toBe(accepted.items[0].piecesFinal); + expect(depositNotice.items[0].netUnitPrice).toBe(accepted.items[0].netUnitPrice); + }); + + it('still 409s while the new-engine quotation is only ISSUED (customer has not accepted)', async () => { + const { ticketId, prId } = await driveTicketToNewEngineIssuedQuotation(); + await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})) + .rejects.toMatchObject({ status: 409, message: 'ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้' }); + const { depositNotices } = await api.depositNotices.listByTicket(ticketId); + expect(depositNotices).toHaveLength(0); + }); +}); + // Issue #752: the mock must reproduce production's one-render gap. Per // DepositNoticeService.issue (backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java // ~L200-225), PR #721 moved the PDF/XLSX render into an afterCommit callback — so the DTO From 40d10c70478a90e792e04f2a1258f76cb732385a Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:35:21 +0700 Subject: [PATCH 06/19] chore(mock): pricing-request deposit route and ticket draft fallback mirror the new-engine quotation branch Co-Authored-By: Claude Opus 5.5 --- frontend/src/api/mockApi.js | 52 ++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index e6f6fa531..a64b52828 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1436,6 +1436,22 @@ function mockDepositNoticeItemsFromQuotation(items) { })); } +// Mirrors DepositNoticeRepository.findDealQuotationItems (the column pick that +// findDealQuotationItemsForDeposit and OrderConfirmationService.createDepositNoticeFromQuotation's +// new-engine branch share): one new-engine quotation's lines as deposit-notice items. Every money +// figure is read off the DTO, never recomputed. +function mockDepositNoticeItemsFromDealQuotation(row) { + return buildDealQuotationDto(row).items.map((item, idx) => ({ + seq: item.seq ?? idx + 1, + description: item.descriptionLine?.trim() || 'รายการสินค้า', + qty: item.piecesFinal, + unit: 'แผ่น', + unitPrice: item.unitPrice, + discountLabel: item.discountPct > 0 ? `ส่วนลด ${item.discountPct}% ต่อหน่วย` : null, + netUnitPrice: item.netUnitPrice, + })); +} + // Branch fix: mirrors DepositNoticeService.pickQuotation — the latest ACCEPTED revision for // this ticket, or (if none has been accepted yet) the latest ISSUED one. mockCustomerQuotations // carries no guaranteed ordering, so sort ascending by (quotationRevisionNo, id) first — same @@ -13263,12 +13279,22 @@ export const api = { // Branch fix (deposit-notice autofill): every deal created through the pricing-request // chain has approved_price = NULL on every ticket_item, so legacyItems above is always // empty for it — mirrors DepositNoticeService.buildItemsFromRequest's own new-chain - // fallback to the ticket's own customer quotation (latest ACCEPTED, else latest ISSUED). + // fallback to the ticket's own customer quotation (latest ACCEPTED, else latest ISSUED), + // then to its live new-engine quotation (mirrors + // DepositNoticeRepository.findDealQuotationItemsForDeposit: ACCEPTED > ISSUED > APPROVED, + // newest id). let items = payload.items?.length ? payload.items : legacyItems; if (!items.length) { const quotation = mockPickTicketQuotationForDepositNotice(ticketId); if (quotation) items = mockDepositNoticeItemsFromQuotation(quotation.items); } + if (!items.length) { + // The picker labels legacy rows 'PRICING_REQUEST' too, so the array tells new-engine apart. + const live = mockPickLiveQuotationForItemFallback(Number(ticketId)); + if (live && mockDealQuotations.includes(live.quotation)) { + items = mockDepositNoticeItemsFromDealQuotation(live.quotation); + } + } // Branch fix: header autofill (mirrors DepositNoticeService.createDraft) — a caller- // supplied non-blank value always wins; a ticket with no linked customer master row @@ -16163,15 +16189,21 @@ export const api = { const ticket = db.tickets.find((t) => t.id === pr.ticketId); if (ticket?.createdById !== user.id) fail('ไม่มีสิทธิ์เข้าถึงรายการนี้', 403); + // Mirrors OrderConfirmationService.createDepositNoticeFromQuotation: the ACCEPTED legacy + // quotation (mapped by DepositNoticeService.itemsFromQuotation), else THIS pricing request's + // ACCEPTED new-engine quotation (DealQuotationRepository.findAcceptedForPricingRequest, mapped + // by DepositNoticeRepository.findDealQuotationItems — the column pick + // findDealQuotationItemsForDeposit shares). Items/amounts trace to that quotation, never to a + // sales.ticket_item row or a sibling pricing request's quotation. const accepted = mockCustomerQuotations.find( (q) => q.pricingRequestId === pr.id && q.docStatus === 'ACCEPTED'); - if (!accepted) fail('ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้', 409); - - // Mirrors OrderConfirmationService.createDepositNoticeFromQuotation, which delegates the - // mapping to DepositNoticeService.itemsFromQuotation (shared with depositNotices - // .createDraft's own new-chain fallback below) — items/amounts trace to the quotation, - // never to any sales.ticket_item row. - const items = mockDepositNoticeItemsFromQuotation(accepted.items); + const newEngine = accepted ? null : mockDealQuotations.find( + (q) => q.pricingRequestId === pr.id && q.origin === 'PRICING_REQUEST' && q.docStatus === 'ACCEPTED'); + if (!accepted && !newEngine) fail('ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้', 409); + const reference = (accepted ?? newEngine).number; + const items = accepted + ? mockDepositNoticeItemsFromQuotation(accepted.items) + : mockDepositNoticeItemsFromDealQuotation(newEngine); // Requires quotation_issued (one of DepositNoticeService.requireApprovedTicket's three // accepted statuses) — mirrors what confirmOrder above already left in place; calling this @@ -16193,7 +16225,7 @@ export const api = { version: nextVer, docNumber: null, issueDate: null, status: 'DRAFT', customerName: ticket.customerName ?? '', customerTaxId: customerTaxId ?? '', customerAddress: customerAddress ?? '', - projectName: projectName ?? '', reference: accepted.number, + projectName: projectName ?? '', reference, depositPercent: payload.depositPercent ?? 0.5, vatPercent: 0.07, notes, items, issuedByName: null, preparerName: 'จินตนา หาญมนตรี', hasPdf: false, hasXlsx: false, @@ -16202,7 +16234,7 @@ export const api = { mockDepositNotices.push(doc); pushPricingRequestEvent(pr, user, 'DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION', pr.status, pr.status, - `สร้างร่างใบแจ้งยอดเงินรับมัดจำจากใบเสนอราคา ${accepted.number}`); + `สร้างร่างใบแจ้งยอดเงินรับมัดจำจากใบเสนอราคา ${reference}`); return delay({ depositNotice: structuredClone(doc) }); }, From ff7b555534e6da53612f18611bbc1299dd84cb55 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:39:15 +0700 Subject: [PATCH 07/19] chore(mock): deposit mapper mirrors findDealQuotationItems' own column picks; pin the ticket-level fallback Co-Authored-By: Claude Opus 5.5 --- .../src/api/mockApi.depositNotices.test.js | 30 +++++++++--------- frontend/src/api/mockApi.js | 31 +++++++++++-------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index a595e59d8..544049e5f 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -137,26 +137,14 @@ async function driveTicketToAcceptedQuotation() { return { ticketId, customer, project, quotation: issued }; } -// DealQuotationOutcomeIntegrationTest#upsertWithValidityDays: the stored draft re-sent with only -// validityDays changed (recipientType is left out, as there — this origin refuses it). -function upsertWithValidityDays(q, days) { - const { - contactId, deptCode, unitCode, offerDate, depositPercent, remainderMode, creditDays, - customerNotes, priceMode, documentLanguage, currency, items, - } = q; - return { - contactId, deptCode, unitCode, offerDate, depositPercent, remainderMode, creditDays, - validityDays: days, customerNotes, priceMode, documentLanguage, currency, items, - }; -} - // New-engine (PRICING_REQUEST-origin) quotation, issued by the sales manager — the same drive as // DealQuotationOutcomeIntegrationTest#issuedNewOriginQuotation. async function driveTicketToNewEngineIssuedQuotation() { const { ticketId, prId } = await driveTicketToApprovedDecision({ priceMode: 'NET' }); await api.auth.login({ role: 'sales' }); const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); - await api.dealQuotations.update(draft.id, upsertWithValidityDays(draft, 30)); + // Java's drive (DealQuotationOutcomeIntegrationTest) saves validityDays before submit; the + // mock's submit/approve/recordOutcome never require it, so that update is left out here. await api.dealQuotations.submit(draft.id, {}); await api.auth.login({ role: 'sales_manager' }); const { quotation: issued } = await api.dealQuotations.approve(draft.id, {}); @@ -229,10 +217,22 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); expect(depositNotice.reference).toBe(accepted.number); expect(depositNotice.items.length).toBeGreaterThan(0); - expect(depositNotice.items[0].qty).toBe(accepted.items[0].piecesFinal); + expect(depositNotice.items[0].qty).toBe(accepted.items[0].quantity); expect(depositNotice.items[0].netUnitPrice).toBe(accepted.items[0].netUnitPrice); }); + // Pin, not a reproduction: the ticket-level route with no items falls through the (empty) + // legacy sources to DepositNoticeRepository.findDealQuotationItemsForDeposit, which on a deal + // with one ACCEPTED new-engine quotation reads the same lines as the pricing-request route. + it('ticket-level createDraft with no items reads the same new-engine lines as the pricing-request route', async () => { + const { ticketId, prId } = await driveTicketToNewEngineAcceptedQuotation(); + const { depositNotice: fromTicket } = await api.depositNotices.createDraft(ticketId, {}); + const { depositNotice: fromPr } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); + expect(fromTicket.items.length).toBeGreaterThan(0); + const pick = (items) => items.map(({ qty, netUnitPrice }) => ({ qty, netUnitPrice })); + expect(pick(fromTicket.items)).toEqual(pick(fromPr.items)); + }); + it('still 409s while the new-engine quotation is only ISSUED (customer has not accepted)', async () => { const { ticketId, prId } = await driveTicketToNewEngineIssuedQuotation(); await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})) diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index a64b52828..07661b8f6 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1436,20 +1436,25 @@ function mockDepositNoticeItemsFromQuotation(items) { })); } -// Mirrors DepositNoticeRepository.findDealQuotationItems (the column pick that -// findDealQuotationItemsForDeposit and OrderConfirmationService.createDepositNoticeFromQuotation's -// new-engine branch share): one new-engine quotation's lines as deposit-notice items. Every money -// figure is read off the DTO, never recomputed. +// Mirrors DepositNoticeRepository.findDealQuotationItems — the column pick (seq, description, +// qty, raw_unit, unit_price, sales_discount, final_unit_price) that findDealQuotationItemsForDeposit +// and OrderConfirmationService.createDepositNoticeFromQuotation's new-engine branch share, with its +// own blank fallbacks. unitPrice/netUnitPrice are read off the DTO, never recomputed. function mockDepositNoticeItemsFromDealQuotation(row) { - return buildDealQuotationDto(row).items.map((item, idx) => ({ - seq: item.seq ?? idx + 1, - description: item.descriptionLine?.trim() || 'รายการสินค้า', - qty: item.piecesFinal, - unit: 'แผ่น', - unitPrice: item.unitPrice, - discountLabel: item.discountPct > 0 ? `ส่วนลด ${item.discountPct}% ต่อหน่วย` : null, - netUnitPrice: item.netUnitPrice, - })); + return buildDealQuotationDto(row).items.map((item, idx) => { + // Display label only: the mock stores no sales_discount, so this restates the column Java reads + // (DealQuotationRepository#itemParams writes it as unitPrice − netUnitPrice). + const salesDiscount = round2(item.unitPrice - item.netUnitPrice); + return { + seq: item.seq ?? idx + 1, + description: item.descriptionLine?.trim() ? item.descriptionLine : 'รายการสินค้า', + qty: item.quantity, + unit: item.unit?.trim() ? item.unit : 'หน่วย', + unitPrice: item.unitPrice, + discountLabel: salesDiscount > 0 ? `ส่วนลด ${salesDiscount} ต่อหน่วย` : null, + netUnitPrice: item.netUnitPrice, + }; + }); } // Branch fix: mirrors DepositNoticeService.pickQuotation — the latest ACCEPTED revision for From 9dcc7893e204d947d8a6a227da4ba7666712dd27 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:10:51 +0700 Subject: [PATCH 08/19] test(mock): pin the deposit mapper's column picks with a discounted line; fix the fallback-case comment Co-Authored-By: Claude Opus 5.5 --- .../src/api/mockApi.depositNotices.test.js | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index 544049e5f..ca07b7810 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -20,10 +20,10 @@ function uuid() { return `77777777-7777-4777-8777-${String(seq).padStart(12, '0')}`; } -// Everything up to and including the CEO-approved pricing decision. `priceMode` is set only -// by the new-engine drive: dealQuotations.createFromPricingRequest refuses a decision without -// one, and the legacy drive below never set it. -async function driveTicketToApprovedDecision({ priceMode } = {}) { +// Everything up to and including the CEO-approved pricing decision. `priceMode`/`discountPct` are +// set only by the new-engine drive: dealQuotations.createFromPricingRequest refuses a decision +// without a priceMode, and the legacy drive below never set either. +async function driveTicketToApprovedDecision({ priceMode, discountPct } = {}) { const n = seq + 1; await api.auth.login({ role: 'sales' }); @@ -118,7 +118,9 @@ async function driveTicketToApprovedDecision({ priceMode } = {}) { const { decision } = await api.pricingRequests.startPricingDecision(prId, { defaultMarginPct: 0.2 }); await api.pricingRequests.updatePricingDecision(decision.id, { ...(priceMode ? { priceMode } : {}), - items: decision.items.map((item) => ({ pricingDecisionItemId: item.id, minimumSellingPrice: 50 })), + items: decision.items.map((item) => ({ + pricingDecisionItemId: item.id, minimumSellingPrice: 50, ...(discountPct != null ? { discountPct } : {}), + })), }); await api.pricingRequests.approvePricingDecision(decision.id, {}); @@ -138,9 +140,12 @@ async function driveTicketToAcceptedQuotation() { } // New-engine (PRICING_REQUEST-origin) quotation, issued by the sales manager — the same drive as -// DealQuotationOutcomeIntegrationTest#issuedNewOriginQuotation. +// DealQuotationOutcomeIntegrationTest#issuedNewOriginQuotation. The line carries a 10% discount +// from the CEO decision, so the deposit mapper's baht discount label is exercised. The discount +// sits on the decision, not on a quotation update: a line that differs from the CEO price makes +// the mock's PRICING_REQUEST approve refuse sales_manager (403, CEO approval required). async function driveTicketToNewEngineIssuedQuotation() { - const { ticketId, prId } = await driveTicketToApprovedDecision({ priceMode: 'NET' }); + const { ticketId, prId } = await driveTicketToApprovedDecision({ priceMode: 'NET', discountPct: 10 }); await api.auth.login({ role: 'sales' }); const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); // Java's drive (DealQuotationOutcomeIntegrationTest) saves validityDays before submit; the @@ -217,13 +222,22 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); expect(depositNotice.reference).toBe(accepted.number); expect(depositNotice.items.length).toBeGreaterThan(0); + // qty is deliberately not distinguishable here: a mock TILE row sets quantity = piecesFinal. expect(depositNotice.items[0].qty).toBe(accepted.items[0].quantity); expect(depositNotice.items[0].netUnitPrice).toBe(accepted.items[0].netUnitPrice); + expect(depositNotice.items[0].unit).toBe(accepted.items[0].unit); + expect(depositNotice.items[0].description).toBe(accepted.items[0].descriptionLine); + expect(depositNotice.items[0].unitPrice).toBe(accepted.items[0].unitPrice); + // DepositNoticeRepository.findDealQuotationItems prints sales_discount (unitPrice − netUnitPrice) + // in baht, not the percent: ราคาตั้ง 120 at 10% off is net 108, so 12 baht. + expect([accepted.items[0].unitPrice, accepted.items[0].netUnitPrice]).toEqual([120, 108]); + expect(depositNotice.items[0].discountLabel).toBe('ส่วนลด 12 ต่อหน่วย'); }); - // Pin, not a reproduction: the ticket-level route with no items falls through the (empty) - // legacy sources to DepositNoticeRepository.findDealQuotationItemsForDeposit, which on a deal - // with one ACCEPTED new-engine quotation reads the same lines as the pricing-request route. + // Reproduces the fallback gap closed in 40d10c70: the ticket-level route with no items falls + // through the (empty) legacy sources to DepositNoticeRepository.findDealQuotationItemsForDeposit, + // which on a deal with one ACCEPTED new-engine quotation reads the same lines as the + // pricing-request route. Without that fallback the mock returned no items here. it('ticket-level createDraft with no items reads the same new-engine lines as the pricing-request route', async () => { const { ticketId, prId } = await driveTicketToNewEngineAcceptedQuotation(); const { depositNotice: fromTicket } = await api.depositNotices.createDraft(ticketId, {}); From daa5628720ce8ccea0efce1c1a5b01a84df257af Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:25:20 +0700 Subject: [PATCH 09/19] refactor(sales): correct two comments the deposit fix made false; trim narration Comment-only. The dealQuotations field and setter docs now name the deposit route's new-engine branch as a second user (and that an unwired instance answers 409 there). Drops two narration comments the calls already say, states the 409 rule directly, rewords findDealQuotationItems' circular Javadoc, and reflows one ragged wrap in findDealQuotationDepositPercent's Javadoc. Co-Authored-By: Claude Opus 5.5 --- .../hr/deposit/DepositNoticeRepository.java | 14 ++++++------- .../OrderConfirmationService.java | 20 +++++++++---------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java index 6337ca9fa..205d11f32 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java @@ -118,11 +118,10 @@ public List findByTicket(long ticketId) { * each refuse a pricing request the other engine already quoted), so one ticket can carry a * legacy quotation on one pricing request and an origin-tagged one on another. On such a mixed * ticket both branches below can match, and trying legacy first IS a real preference: the - * percent comes from the legacy row. - * Returns empty when there is no live (ACCEPTED/ISSUED/APPROVED) - * quotation for this ticket, or its {@code deposit_percent} is null — the caller ({@link - * DepositNoticeService#createDraft}) falls back to 0.50 in that case, this document's - * long-standing default. + * percent comes from the legacy row. Returns empty when there is no live + * (ACCEPTED/ISSUED/APPROVED) quotation for this ticket, or its {@code deposit_percent} is + * null — the caller ({@link DepositNoticeService#createDraft}) falls back to 0.50 in that + * case, this document's long-standing default. */ public Optional findDealQuotationDepositPercent(long ticketId) { Integer legacyPct = firstOrNull(jdbc.query(""" @@ -422,8 +421,9 @@ AND doc_status IN ('ACCEPTED', 'ISSUED', 'APPROVED') return findDealQuotationItems(ids.get(0)); } - /** One deal quotation's lines as deposit-notice items, read the same way {@link - * #findDealQuotationItemsForDeposit} reads the quotation it selects for a ticket. */ + /** One deal quotation's {@code sales.quotation_item} lines, by {@code seq}, as deposit-notice + * items: the resolved display columns the quotation prints. {@link + * #findDealQuotationItemsForDeposit} delegates to this for the quotation it selects. */ public List findDealQuotationItems(long quotationId) { return jdbc.query(""" SELECT seq, description, qty, raw_unit, unit_price, sales_discount, final_unit_price diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index d4b4929be..a7d9d7d1d 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -82,10 +82,11 @@ public class OrderConfirmationService { private final CustomerQuotationRepository quotations; private final DepositNoticeService depositNotices; private final NotificationRepository notifications; - // GLA-123 slice S3 BLOCKER 2 fix — read-only dependency onto the new engine, used ONLY by + // GLA-123 slice S3 BLOCKER 2 fix — read-only dependency onto the new engine, used by // #reconcileTicketItems to prefer an ACCEPTED PRICING_REQUEST-origin quotation's own line - // quantities over the PR item's authored ones. SETTER-injected (not a constructor parameter), - // matching DealQuotationService's own established device (see its + // quantities over the PR item's authored ones, and by #createDepositNoticeFromQuotation to + // find the pricing request's ACCEPTED new-engine quotation. SETTER-injected (not a + // constructor parameter), matching DealQuotationService's own established device (see its // #wirePricingRequestDependencies Javadoc): 12 existing hand-wired // `new OrderConfirmationService(...)` test call sites keep compiling unchanged, Spring wires // it automatically via the @Autowired setter in production, and #reconcileTicketItems @@ -106,8 +107,9 @@ public OrderConfirmationService(PricingRequestRepository pricingRequests, Ticket } /** GLA-123 slice S3 BLOCKER 2 fix — see {@link #dealQuotations}'s own Javadoc for why this is - * setter- rather than constructor-injected. {@code required = false} so a hand-wired test that - * never exercises a PRICING_REQUEST-origin confirmOrder is unaffected either way. */ + * setter- rather than constructor-injected. {@code required = false}: an unwired instance + * falls back to the PR item's own quantity in confirmOrder, and answers 409 on the new-engine + * branch of createDepositNoticeFromQuotation. */ @org.springframework.beans.factory.annotation.Autowired(required = false) public void wireDealQuotationRepository(th.co.glr.hr.dealquotation.DealQuotationRepository dealQuotations) { this.dealQuotations = dealQuotations; @@ -440,9 +442,8 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * the same ticket can never supply them. The percent is still whatever {@code createDraft} * derives, which is ticket-scoped and legacy-first: on a ticket that also carries a sibling * pricing request's legacy ISSUED quotation, the percent comes from that legacy row. That is a - * known limitation the ticket-level route shares; this route does not introduce it. The 409 - * semantics do not change: the customer must have accepted, and an ISSUED-only quotation still - * gets 409. + * known limitation the ticket-level route shares; this route does not introduce it. The + * customer must have accepted: an ISSUED-only quotation answers 409. */ @Transactional public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, @@ -460,11 +461,8 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, List items; if (accepted != null) { reference = accepted.number(); - // Mapping itself now lives on DepositNoticeService (this branch's fix), shared with that - // class's own new-chain item fallback in buildItemsFromRequest — see its Javadoc. items = DepositNoticeService.itemsFromQuotation(accepted.items()); } else { - // New-engine branch: the accepted quotation of THIS pricing request — see the Javadoc. th.co.glr.hr.dealquotation.DealQuotationDtos.DealQuotationDto newEngine = dealQuotations == null ? null : dealQuotations.findAcceptedForPricingRequest(pricingRequestId) From 39b826b409a53a7a6e0c59f49c5f99725691ca8b Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 05:27:07 +0700 Subject: [PATCH 10/19] refactor(mock): trim the deposit mirror comments to pointers; drop the commit sha from the test comment Co-Authored-By: Claude Opus 5.5 --- .../src/api/mockApi.depositNotices.test.js | 6 ++--- frontend/src/api/mockApi.js | 22 +++++++------------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index ca07b7810..cb7ce3e31 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -234,10 +234,8 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin expect(depositNotice.items[0].discountLabel).toBe('ส่วนลด 12 ต่อหน่วย'); }); - // Reproduces the fallback gap closed in 40d10c70: the ticket-level route with no items falls - // through the (empty) legacy sources to DepositNoticeRepository.findDealQuotationItemsForDeposit, - // which on a deal with one ACCEPTED new-engine quotation reads the same lines as the - // pricing-request route. Without that fallback the mock returned no items here. + // Covers the ticket-level new-engine fallback (DepositNoticeRepository.findDealQuotationItemsForDeposit): + // with no items and empty legacy sources, the route would return no items without it. it('ticket-level createDraft with no items reads the same new-engine lines as the pricing-request route', async () => { const { ticketId, prId } = await driveTicketToNewEngineAcceptedQuotation(); const { depositNotice: fromTicket } = await api.depositNotices.createDraft(ticketId, {}); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 07661b8f6..02942b431 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1436,10 +1436,8 @@ function mockDepositNoticeItemsFromQuotation(items) { })); } -// Mirrors DepositNoticeRepository.findDealQuotationItems — the column pick (seq, description, -// qty, raw_unit, unit_price, sales_discount, final_unit_price) that findDealQuotationItemsForDeposit -// and OrderConfirmationService.createDepositNoticeFromQuotation's new-engine branch share, with its -// own blank fallbacks. unitPrice/netUnitPrice are read off the DTO, never recomputed. +// Mirrors DepositNoticeRepository.findDealQuotationItems. Values are read off the DTO, never +// recomputed (label excepted — see below). function mockDepositNoticeItemsFromDealQuotation(row) { return buildDealQuotationDto(row).items.map((item, idx) => { // Display label only: the mock stores no sales_discount, so this restates the column Java reads @@ -13284,17 +13282,16 @@ export const api = { // Branch fix (deposit-notice autofill): every deal created through the pricing-request // chain has approved_price = NULL on every ticket_item, so legacyItems above is always // empty for it — mirrors DepositNoticeService.buildItemsFromRequest's own new-chain - // fallback to the ticket's own customer quotation (latest ACCEPTED, else latest ISSUED), - // then to its live new-engine quotation (mirrors - // DepositNoticeRepository.findDealQuotationItemsForDeposit: ACCEPTED > ISSUED > APPROVED, - // newest id). + // fallback to the ticket's own customer quotation (latest ACCEPTED, else latest ISSUED). let items = payload.items?.length ? payload.items : legacyItems; if (!items.length) { const quotation = mockPickTicketQuotationForDepositNotice(ticketId); if (quotation) items = mockDepositNoticeItemsFromQuotation(quotation.items); } if (!items.length) { - // The picker labels legacy rows 'PRICING_REQUEST' too, so the array tells new-engine apart. + // Then the live new-engine quotation: findDealQuotationItemsForDeposit, ordered as + // mockPickLiveQuotationForItemFallback's own comment says. The picker labels legacy rows + // 'PRICING_REQUEST' too, so the array tells new-engine apart. const live = mockPickLiveQuotationForItemFallback(Number(ticketId)); if (live && mockDealQuotations.includes(live.quotation)) { items = mockDepositNoticeItemsFromDealQuotation(live.quotation); @@ -16195,11 +16192,8 @@ export const api = { if (ticket?.createdById !== user.id) fail('ไม่มีสิทธิ์เข้าถึงรายการนี้', 403); // Mirrors OrderConfirmationService.createDepositNoticeFromQuotation: the ACCEPTED legacy - // quotation (mapped by DepositNoticeService.itemsFromQuotation), else THIS pricing request's - // ACCEPTED new-engine quotation (DealQuotationRepository.findAcceptedForPricingRequest, mapped - // by DepositNoticeRepository.findDealQuotationItems — the column pick - // findDealQuotationItemsForDeposit shares). Items/amounts trace to that quotation, never to a - // sales.ticket_item row or a sibling pricing request's quotation. + // quotation first, else THIS pricing request's ACCEPTED new-engine row + // (DealQuotationRepository.findAcceptedForPricingRequest). const accepted = mockCustomerQuotations.find( (q) => q.pricingRequestId === pr.id && q.docStatus === 'ACCEPTED'); const newEngine = accepted ? null : mockDealQuotations.find( From a32b02b62d3742bef010ec3d50175bd8cfd60b80 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:05:17 +0700 Subject: [PATCH 11/19] =?UTF-8?q?docs(plan):=20deposit=20notice=20from=20a?= =?UTF-8?q?=20new-engine=20pricing-request=20quotation=20=E2=80=94=20imple?= =?UTF-8?q?mentation=20plan?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plan the branch was executed from (subagent-driven). Its "items = null" shape for the new-engine branch was superseded during review by a pricing-request-scoped lookup with items from that quotation; the commits and their tests carry the final shape, the plan carries the reasoning and the coverage list. No test: docs-only. Co-Authored-By: Claude Fable 5.1 --- ...026-10-19-deposit-notice-new-engine-pcr.md | 321 ++++++++++++++++++ 1 file changed, 321 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md diff --git a/docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md b/docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md new file mode 100644 index 000000000..507df8f36 --- /dev/null +++ b/docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md @@ -0,0 +1,321 @@ +# Deposit Notice From a New-Engine Pricing-Request Quotation — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** `POST /api/pricing-requests/{id}/deposit-notice` creates the มัดจำ draft when the pricing request's accepted quotation came from the new deal-quotation engine, instead of answering 409. + +**Architecture:** One server-side change in `OrderConfirmationService.createDepositNoticeFromQuotation`: when no legacy (`origin IS NULL`) ACCEPTED quotation exists for the pricing request, accept the new-engine (`origin = 'PRICING_REQUEST'`) ACCEPTED quotation instead, pass its number as the draft's reference and let `DepositNoticeService.createDraft` source items and deposit percent exactly as the ticket-level route already does (`findDealQuotationItemsForDeposit` / `findDealQuotationDepositPercent`). No frontend change; the mock mirrors the same branch. No schema change. No authz change. + +**Tech Stack:** Spring Boot service (Java), `AbstractPostgresIntegrationTest` on real Postgres, vitest for the mock mirror. + +**Spec:** There is no separate design doc. The spec is the QA finding of 2026-10-18 on the develop QA stack plus the owner's confirmation on 2026-10-19: + +- A deal priced through a pricing request, quoted through the new engine (`DealQuotationService.createFromPricingRequest` → submit → approve → `recordOutcome(ACCEPTED)`), then order-confirmed, reaches ขั้น 10, and the UI's มัดจำ step (`DealDepositPanel`, `DepositNoticePage`) calls the pricing-request route, which answers `409 "ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"` because it only reads `CustomerQuotationRepository.findByPricingRequest` (legacy rows). +- The ticket-level route `POST /api/tickets/{id}/deposit-notice/draft` already handles the new engine; the UI never reaches it when a `QUOTATION_ACCEPTED` pricing request exists. +- Direct quotations (no pricing request) are unaffected and must stay so. +- The pricing-request route's own gate keeps its meaning: a quotation the customer has not accepted (ISSUED only) still yields the same 409. Owner-only access (`requireOwner`) is unchanged. +- Deposit percent is sourced from the quotation (owner ruling 2026-09-29), unchanged. + +## Global Constraints + +- Base branch `develop` at `c64a6e89`. Branch name `fix/deposit-notice-new-engine-pcr`. Never `gh pr merge --auto` into develop. +- Sales/CRM workflow logic may change (CLAUDE.md "Sales flow redesign"); every behaviour change is stated in the PR body. Payroll, tax, SSO, commission math untouched. +- Test-first: the failing test is written, shown red for the right reason, then the fix. Existing test files are revised; new files only where no file owns the behaviour (`tests-revise-dont-bloat`). +- Backend tests run on the local Postgres, one class at a time, with `-Dtest.fork.count=1` (exact property name) and a throwaway database; never the full `mvnw verify` locally. +- After any mutation-check revert of Java code, run `./mvnw -q clean` before trusting a re-run (stale `.class` files). +- `mockApi.js` keeps each `// Mirrors ` header accurate; the mock's authz is not evidence. +- The 409 message text `ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้` is unchanged (frontend shows `err.message`). +- Commit messages end with `Co-Authored-By: Claude Fable 5.1 `; PR body ends with `🤖 Generated with [Claude Code](https://claude.com/claude-code)`. + +## Review Focus + +1. **New-engine quotation ISSUED but not yet ACCEPTED** — the route must still answer the same 409 and write no `sales.deposit_notice` row. Pinned in Task 1 (`createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409`). +2. **Non-owning sales rep on a new-engine deal** — 403 before any quotation lookup, no row written. Pinned in Task 1 (`createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach`). +3. **A hand-wired `OrderConfirmationService` with no `DealQuotationRepository` wired** (`dealQuotations == null`, as in `OrderConfirmationIntegrationTest`) — the legacy path must behave exactly as before and the new branch must not NPE. Pinned by the existing `OrderConfirmationIntegrationTest.fullChain_quotationAcceptedThroughDepositPaid_composesWithoutShortcuts` staying green (Task 2 step 4) and by the null guard in Task 2. +4. **Calling the route twice** — creates a second DRAFT version, same as the legacy path today (`DepositNoticeService.createDraft` numbers versions). Not a new behaviour; not separately pinned. +5. **Items and percent of the new-engine draft must equal what the ticket-level route produces** — the two routes share `createDraft`, so the test compares the two drafts field by field (Task 1, happy path). A divergence here is wrong money on a customer document. + +--- + +### Task 1: Backend tests — the bug, and the two gates that must survive the fix + +**Files:** +- Modify: `backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java` (add three `@Test` methods after `recordOutcome_byOwningSales_accepted_setsPrStatusAndDocStatus`, ~line 240; reuse its wiring — it already constructs `DealQuotationService`, `DepositNoticeService`, `OrderConfirmationService` and calls `orderConfirmation.wireDealQuotationRepository(quotationRepository)`) + +**Interfaces:** +- Consumes (existing, in this test class): `acceptedNewOriginQuotation(String recipient, BigDecimal qty) → DealQuotationDto`, `issuedNewOriginQuotation(String recipient, BigDecimal qty) → DealQuotationDto`, `orderConfirmation.confirmOrder(long pricingRequestId, OrderConfirmationRequests.ConfirmOrderRequest, UserPrincipal)`, `orderConfirmation.createDepositNoticeFromQuotation(long pricingRequestId, OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest, UserPrincipal) → DepositNoticeDto`, the field `depositNoticeService` (make it a class field if it is still a local in `@BeforeEach`), `salesActor`, `otherSalesActor`, `ticketId`, `jdbc`. +- Consumes: `DealQuotationDto.pricingRequestId()`, `DealQuotationDto.number()`, `DepositNoticeDto.reference()`, `DepositNoticeDto.items()`, `DepositNoticeDto.depositPercent()`, `DepositNoticeItemDto(id, seq, description, qty, unit, unitPrice, discountLabel, netUnitPrice, amount)`, `DepositNoticeDraftRequest(customerName, customerTaxId, customerAddress, projectName, reference, depositPercent, notes, items)`. +- Produces: nothing new for later tasks; Task 2 turns the first test green. + +- [ ] **Step 1: Add the three tests** + +```java +// ── Deposit notice from the pricing-request route when the accepted quotation is NEW-engine ── +// QA finding 2026-10-18 (develop QA stack): DealDepositPanel/DepositNoticePage call +// POST /pricing-requests/{id}/deposit-notice whenever a QUOTATION_ACCEPTED pricing request exists; +// that route read only legacy CustomerQuotation rows and answered 409 for every deal quoted +// through DealQuotationService.createFromPricingRequest. Direct quotations never take this route. + +@Test +void createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameDraftTheTicketRouteWould() { + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto viaPricingRequest = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + + // The reference is the accepted quotation's number — the one thing this route adds over the ticket route. + assertThat(viaPricingRequest.reference()).isEqualTo(accepted.number()); + assertThat(viaPricingRequest.items()).isNotEmpty(); + + // Items and percent are exactly what the ticket-level route produces for this deal (shared createDraft). + DepositNoticeDto viaTicket = depositNoticeService.createDraft(ticketId, + new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); + assertThat(viaPricingRequest.depositPercent()).isEqualByComparingTo(viaTicket.depositPercent()); + assertThat(viaPricingRequest.items()) + .usingRecursiveComparison().ignoringFields("id") + .isEqualTo(viaTicket.items()); + + assertThat(jdbc.queryForObject(""" + SELECT COUNT(*) FROM sales.pricing_request_event + WHERE pricing_request_id = :id AND event_kind = 'DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION' + """, Map.of("id", pricingRequestId), Long.class)).isEqualTo(1L); +} + +@Test +void createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409() { + DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = issued.pricingRequestId(); + + assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor)) + .isInstanceOf(ApiException.class) + .hasMessageContaining("ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isZero(); +} + +@Test +void createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach() { + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), otherSalesActor)) + .isInstanceOf(ApiException.class) + .hasMessageContaining("ไม่มีสิทธิ์เข้าถึงรายการนี้"); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isZero(); +} +``` + +If `sales.pricing_request_event`'s column names differ (`kind` vs `event_kind`), copy the exact query `OrderConfirmationIntegrationTest.countPricingRequestEventsOfKind` uses (line ~774) rather than guessing. Add `import th.co.glr.hr.deposit.DepositNoticeDto;`, `import th.co.glr.hr.deposit.DepositNoticeDraftRequest;` if missing. + +- [ ] **Step 2: Create the throwaway DB once and run the class; verify the red is the 409** + +```bash +psql -h localhost -U "$(whoami)" -d postgres -c 'CREATE DATABASE glr_verify_it OWNER "'"$(whoami)"'";' 2>/dev/null || true +cd /Users/ploy_warit/Desktop/GL-R-ERP/backend && TEST_DB_URL=jdbc:postgresql://localhost:5432/glr_verify_it TEST_DB_USERNAME="$(whoami)" TEST_DB_PASSWORD="" \ + ./mvnw -B -q -Dtest.fork.count=1 -Dtest=DealQuotationOutcomeIntegrationTest -Dsurefire.failIfNoSpecifiedTests=false test 2>&1 | tail -40 +``` + +Expected: `createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameDraftTheTicketRouteWould` FAILS with `ApiException ... ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้` (the bug). The other two PASS already — they are the gates that must survive Task 2 and are mutation-checked there. Every pre-existing test in the class still passes. If the happy-path test fails for any other reason (compile error, `confirmOrder` 409, fixture), fix the test, not the service, until the red is the 409. + +- [ ] **Step 3: Commit the red tests** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP && git add backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +git commit -m "test(sales): pin the มัดจำ draft from a new-engine pricing-request quotation (reproduces the 409) + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +### Task 2: Backend fix — accept the new-engine ACCEPTED quotation on the pricing-request route + +**Files:** +- Modify: `backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java:433-459` (`createDepositNoticeFromQuotation`) and its Javadoc above it. + +**Interfaces:** +- Consumes: `dealQuotations` (field, may be null — setter-injected via `wireDealQuotationRepository`), `DealQuotationRepository.findLatestForPricingRequest(long) → Optional`, `DealQuotationRepository.findById(long) → Optional`, `QuotationStatus.ACCEPTED`, `DepositNoticeService.createDraft(long ticketId, DepositNoticeDraftRequest, UserPrincipal)`. +- Produces: unchanged public signature `createDepositNoticeFromQuotation(long, CreateDepositNoticeFromQuotationRequest, UserPrincipal) → DepositNoticeDto`. + +- [ ] **Step 1: Implement the two-branch resolution** + +Replace the `orElseThrow` lookup with: legacy ACCEPTED quotation via `quotations.findByPricingRequest(pricingRequestId)` as today, else — only when `dealQuotations != null` — `dealQuotations.findLatestForPricingRequest(pricingRequestId).flatMap(dealQuotations::findById).filter(q -> QuotationStatus.ACCEPTED.equals(q.docStatus()))`; if neither, throw the existing 409 with the unchanged message. ("Latest" is correct: ACCEPTED is terminal for a pricing request's quotation family — R8 in `recordOutcome` — so no newer row can exist once one is accepted.) + +Then build `DepositNoticeDraftRequest` with `reference = ` and `items = legacy ? DepositNoticeService.itemsFromQuotation(legacy.items()) : null`. A `null` items list makes `createDraft`'s `buildItemsFromRequest` fall through to `docs.findDealQuotationItemsForDeposit(ticketId)` — the same source the ticket-level route uses — and the percent already comes from `findDealQuotationDepositPercent(ticketId)`. Use the found number in the `DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION` event text as today. + +Update the method's Javadoc: one paragraph stating the new-engine branch, why `items` is `null` for it, and that the 409 semantics (customer must have accepted) are unchanged. Delete nothing else from the existing Javadoc. + +- [ ] **Step 2: Run the class; all green** + +Same command as Task 1 step 2. Expected: every test in `DealQuotationOutcomeIntegrationTest` passes, including the three new ones. + +- [ ] **Step 3: Mutation-check the ISSUED gate** + +Temporarily change the filter to `QuotationStatus.ISSUED.equals(q.docStatus()) || QuotationStatus.ACCEPTED.equals(q.docStatus())`, re-run the class. Expected: ONLY `createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409` fails. Revert, then `./mvnw -q clean` before the next run. + +- [ ] **Step 4: Run the legacy-path class to prove the unwired case is unchanged** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP/backend && TEST_DB_URL=jdbc:postgresql://localhost:5432/glr_verify_it TEST_DB_USERNAME="$(whoami)" TEST_DB_PASSWORD="" \ + ./mvnw -B -q -Dtest.fork.count=1 -Dtest='OrderConfirmationIntegrationTest,DepositNoticeIssueGuardIntegrationTest,DepositNoticeAccountNotificationIntegrationTest' -Dsurefire.failIfNoSpecifiedTests=false test 2>&1 | tail -20 +``` + +Expected: all pass (`OrderConfirmationIntegrationTest` never wires `DealQuotationRepository`, so this is the `dealQuotations == null` path). + +- [ ] **Step 5: Commit** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP && git add backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +git commit -m "fix(sales): มัดจำ draft from the pricing-request route accepts a new-engine ACCEPTED quotation + +The route read only legacy CustomerQuotation rows, so every deal quoted through +DealQuotationService.createFromPricingRequest got 409 at ขั้น 10. It now falls back +to the pricing request's new-engine ACCEPTED quotation and lets createDraft source +items and percent exactly as the ticket-level route does. ISSUED-only still 409s; +owner gate unchanged. + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +### Task 3: Mock mirror — same branch in `mockApi.js`, pinned in the existing mock deposit test + +**Files:** +- Modify: `frontend/src/api/mockApi.js` — `pricingRequests.createDepositNoticeFromQuotation` (~line 16160) and `depositNotices.createDraft`'s item fallback (~line 13266); add one mapper next to `mockDepositNoticeItemsFromQuotation` (~line 1427). +- Modify: `frontend/src/api/mockApi.depositNotices.test.js` — split the existing drive helper and add two cases. + +**Interfaces:** +- Consumes: `mockDealQuotations` rows (`pricingRequestId`, `origin`, `docStatus`, `number`, `ticketId`), `buildDealQuotationDto(row).items[]` (fields `descriptionLine`, `piecesFinal`, `unitPrice`, `discountPct`, `netUnitPrice` — confirm the names in `buildDealQuotationDto` before writing the mapper), `mockPickLiveQuotationForItemFallback(ticketId)` (already ranks ACCEPTED > ISSUED > APPROVED across both arrays, newest id wins), `api.dealQuotations.createFromPricingRequest / update / submit / approve / recordOutcome`, `api.pricingRequests.confirmOrder`. +- Produces: `function mockDepositNoticeItemsFromDealQuotation(row) → DepositNoticeItemRequest[]` (`{ seq, description, qty, unit, unitPrice, discountLabel, netUnitPrice }`, `qty = piecesFinal`, `unit = 'แผ่น'`, `discountLabel = discountPct > 0 ? \`ส่วนลด ${discountPct}% ต่อหน่วย\` : null`, `netUnitPrice` taken from the dto, never recomputed — the mock mirrors `DepositNoticeRepository.findDealQuotationItemsForDeposit`'s column pick, not a formula). + +- [ ] **Step 1: Revise the mock test — extract the drive to an approved decision, add two cases** + +In `mockApi.depositNotices.test.js`, split `driveTicketToAcceptedQuotation()` so the part up to and including the CEO-approved pricing decision becomes `driveTicketToApprovedDecision() → { ticketId, prId }`, and the existing helper calls it then continues on the legacy quotation path unchanged. Add: + +```js +async function driveTicketToNewEngineAcceptedQuotation() { + const { ticketId, prId } = await driveTicketToApprovedDecision(); + await api.auth.login({ role: 'sales' }); + const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); + // validity + submit + approve + accept, exactly as DealQuotationOutcomeIntegrationTest.acceptedNewOriginQuotation + await api.dealQuotations.update(draft.id, { ...payloadThatSetsValidityDays(draft, 30) }); + await api.dealQuotations.submit(draft.id, {}); + await api.auth.login({ role: 'sales_manager' }); + await api.dealQuotations.approve(draft.id, {}); + await api.auth.login({ role: 'sales' }); + const { quotation: accepted } = await api.dealQuotations.recordOutcome(draft.id, { outcome: 'ACCEPTED', customerNote: null, clientRequestId: uuid() }); + await api.pricingRequests.confirmOrder(prId, { clientRequestId: uuid() }); + return { ticketId, prId, accepted }; +} + +describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engine quotation (mirrors OrderConfirmationService fix)', () => { + it('creates the draft with the accepted quotation number as reference and items from that quotation', async () => { + const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); + const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); + expect(depositNotice.reference).toBe(accepted.number); + expect(depositNotice.items.length).toBeGreaterThan(0); + expect(depositNotice.items[0].qty).toBe(accepted.items[0].piecesFinal); + expect(depositNotice.items[0].netUnitPrice).toBe(accepted.items[0].netUnitPrice); + }); + + it('still 409s while the new-engine quotation is only ISSUED (customer has not accepted)', async () => { + const { ticketId, prId } = await driveTicketToApprovedDecision(); + // same drive, stop after approve (ISSUED) — factor the first half of the helper above if needed + … + await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})) + .rejects.toMatchObject({ status: 409, message: 'ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้' }); + const { depositNotices } = await api.depositNotices.listByTicket(ticketId); + expect(depositNotices).toHaveLength(0); + }); +}); +``` + +Resolve `payloadThatSetsValidityDays` by reading how `DealQuotationOutcomeIntegrationTest.upsertWithValidityDays` maps to the mock's `update` payload (the mock's own 400/409 messages name the missing field). The exact response envelope keys (`quotation`, `depositNotice`) are whatever the mock already returns for those calls. + +- [ ] **Step 2: Run the mock test; verify the red is the 409 on the happy case** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP/frontend && npx vitest run src/api/mockApi.depositNotices.test.js 2>&1 | tail -25 +``` + +Expected: the new happy-path case FAILS with the 409 message; the ISSUED case passes; every pre-existing case passes (the helper split changed no behaviour). + +- [ ] **Step 3: Implement the mock mirror** + +In `createDepositNoticeFromQuotation`: after the legacy `mockCustomerQuotations` lookup, if none, `const v2 = mockDealQuotations.find((q) => q.pricingRequestId === pr.id && q.origin === 'PRICING_REQUEST' && q.docStatus === 'ACCEPTED')`; 409 with the unchanged message if neither; `reference = (accepted ?? v2).number`; `items = accepted ? mockDepositNoticeItemsFromQuotation(accepted.items) : mockDepositNoticeItemsFromDealQuotation(v2)`. In `depositNotices.createDraft`'s fallback, after `mockPickTicketQuotationForDepositNotice` returns null, use `mockPickLiveQuotationForItemFallback(ticketId)` and the new mapper when its `origin` is not legacy — this mirrors `buildItemsFromRequest`'s last step, which the mock lacked. Update both `// Mirrors …` comments to name `OrderConfirmationService.createDepositNoticeFromQuotation` and `DepositNoticeRepository.findDealQuotationItemsForDeposit`. + +- [ ] **Step 4: Run the mock test and the contract test; lint** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP/frontend && npx vitest run src/api/mockApi.depositNotices.test.js src/api/contract.test.js src/features/deposits/DepositNoticePage.test.jsx src/features/tickets/TicketDetailPage.test.jsx 2>&1 | grep -E "Test Files|Tests " && npx eslint src/api/mockApi.js src/api/mockApi.depositNotices.test.js +``` + +Expected: all pass; eslint clean. + +- [ ] **Step 5: Commit** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP && git add frontend/src/api/mockApi.js frontend/src/api/mockApi.depositNotices.test.js +git commit -m "chore(mock): pricing-request deposit route and ticket draft fallback mirror the new-engine quotation branch + +Co-Authored-By: Claude Fable 5.1 " +``` + +--- + +### Task 4: Real-stack verification on the QA stack (the surface the bug was found on) + +**Files:** none changed. + +**Interfaces:** +- Consumes: the QA stack recipe — backend jar on `:8093` against DB `glr_qa_stages` (`SPRING_PROFILES_ACTIVE=demo SERVER_PORT=8093 SPRING_DATASOURCE_URL=jdbc:postgresql://localhost:5432/glr_qa_stages SPRING_DATASOURCE_USERNAME=ploy_warit APP_MAIL_PROVIDER=log SERVER_SESSION_COOKIE_SECURE=false APP_CORS_ALLOWED_ORIGINS=http://127.0.0.1:5173,http://localhost:5173`), vite `VITE_USE_MOCKS=false VITE_API_PROXY_TARGET=http://127.0.0.1:8093 npm run dev -- --port 5173 --strictPort`, seed deals titled `[SEED-10-18] ขั้น N …`. + +- [ ] **Step 1: Build the branch jar and start it on :8093** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP/backend && ./mvnw -B -q -DskipTests -Djacoco.skip=true package && cp target/*.jar "$SCRATCHPAD/deposit-fix-app.jar" +``` + +Stop whatever is on :8093 first (`lsof -ti:8093 | xargs kill`); start the new jar with the env above, in the background, and wait for `GET http://127.0.0.1:8093/actuator/health` → `{"status":"UP"}`. Restarting logs every cached browser session out. + +- [ ] **Step 2: Reproduce the finding against the fixed jar** + +With headless Playwright (or the Chrome extension) on `http://127.0.0.1:5173`: log in as the seed's sales persona, open the `[SEED-10-18] ขั้น 10` deal whose quotation is new-engine, press สร้างใบแจ้งยอดมัดจำ in the มัดจำ section. Expected: toast `สร้างร่างใบแจ้งยอดเงินรับมัดจำแล้ว`, navigation to `/tickets//deposit`, a DRAFT whose เลขที่อ้างอิง is the quotation number and whose rows match the quotation. Also open a `ขั้น 10` DIRECT-quotation seed deal and confirm its มัดจำ step still works (ticket-level route). Keep both screenshots in the scratchpad. + +If no ขั้น 10 new-engine seed exists in `glr_qa_stages`, run `SEED_BACKEND_URL=http://127.0.0.1:8093 ONLY=S10 node .design/sales-pipeline-review/real-backend/seed-stages.mjs` first. + +- [ ] **Step 3: Record the result** — the two screenshots' paths and the HTTP status of the create call go into the PR body (Task 5). No commit. + +--- + +### Task 5: Pre-PR gate, review, PR + +**Files:** none changed unless review finds something. + +- [ ] **Step 1: Pre-PR gate (targeted, per the standing rule)** + +```bash +cd /Users/ploy_warit/Desktop/GL-R-ERP/frontend && npm run lint && npx vitest run src/api/mockApi.depositNotices.test.js src/api/contract.test.js src/features/deposits src/features/tickets/DealDepositPanel.test.jsx 2>&1 | grep -E "Test Files|Tests " +cd /Users/ploy_warit/Desktop/GL-R-ERP && git rev-list --left-right --count HEAD...origin/develop # must be N 0 (not behind) +``` + +- [ ] **Step 2: Independent Opus review** (`sonnet-implements-opus-reviews-loop` / `parallel-agents-use-opus`): the reviewer re-runs `DealQuotationOutcomeIntegrationTest`, reverts `OrderConfirmationService` to develop's version to see the happy-path test go red, checks the mock mirrors the Java branch and not more, and checks the PR body's coverage list against the assertions. + +- [ ] **Step 3: Push and open the PR to develop** (on Ploy's say-so, per the standing rule). PR body sections: What (the behaviour change, stated as a sales-workflow change), Files changed, Commands run, Tests/build results (which ITs RAN on real Postgres), Authz evidence (`no authz change` — the owner gate is pre-existing; the wrong-way-round test pins it on the new path), Known risks (two DRAFT versions on a double call, same as before; the mock's ticket-level fallback gained the new-engine branch), Test coverage (the three Java cases, the two mock cases, red-before-green results, the mutation check, and what is NOT covered: no e2e-real spec drives the deposit step; the QA-stack walk in Task 4 is the browser evidence). + +--- + +## Self-review notes + +- **Spec coverage:** PCR new-engine → draft (Task 1/2); ISSUED-only → 409 (Task 1/2 + mutation); owner gate (Task 1); direct quotations unaffected (Task 4 step 2 second deal; the code path is untouched); mock fidelity (Task 3); real-stack reproduction (Task 4). +- **Types:** `DealQuotationDto.pricingRequestId()` is a `Long`; the tests unbox it where the accepted quotation is known to be PRICING_REQUEST-origin. `CreateDepositNoticeFromQuotationRequest(null)` is the controller's own default. `DepositNoticeItemDto` has `id` as the only per-document field, hence `ignoringFields("id")`. +- **Proportion:** the fix is ~15 lines of Java and ~25 of mock; the plan's length is in the tests, which are the deliverable the rules require. From e92741070367e947f9e1321f4d2a547b6f49ef19 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:16:40 +0700 Subject: [PATCH 12/19] =?UTF-8?q?test(sales):=20pin=20the=20pricing-reques?= =?UTF-8?q?t=20=E0=B8=A1=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft's=20p?= =?UTF-8?q?ercent=20to=20THIS=20pricing=20request's=20quotation=20(reprodu?= =?UTF-8?q?ces=20the=20sibling's=20percent)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent (renamed from ..._draftsThisQuotationsLines): the new-engine quotation carries 30%, the sibling pricing request's legacy ISSUED row carries 70%, and the draft must bill 0.30. Red on a32b02b6: 0.7000, because createDraft's ticket-scoped, legacy-first percent lookup reads the sibling's row. No code path writes deposit_percent on a legacy row today, so the test seeds it through jdbc; without the seed the lookup already falls through to this quotation's own percent. The issuedNewOriginQuotation/acceptedNewOriginQuotation/upsertWithValidityDays helpers gain a deposit-percent overload; the existing call shapes are unchanged. Co-Authored-By: Claude Opus 5.5 --- .../DealQuotationOutcomeIntegrationTest.java | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java index 22069dc40..16a76feec 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java @@ -326,11 +326,15 @@ void createDepositNoticeFromQuotation_newEngineAcceptedWithAnOpenRevisionDraft_s } /** Engines are exclusive per pricing request, not per ticket: a sibling pricing request's - * legacy ISSUED quotation must not supply the lines of this pricing request's draft. */ + * legacy ISSUED quotation must not supply the lines or the percent of this pricing request's + * draft. */ @Test - void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLines() { + void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent() { CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); - DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + // No code path writes deposit_percent on a legacy row; this stands in for one that carries it. + jdbc.update("UPDATE sales.quotation SET deposit_percent = 70 WHERE quotation_id = :id", + Map.of("id", legacyIssued.id())); + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10"), 30); long pricingRequestId = accepted.pricingRequestId(); orderConfirmation.confirmOrder(pricingRequestId, new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); @@ -350,6 +354,8 @@ void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingReque assertThat(draft.reference()).isEqualTo(accepted.number()); assertThat(draft.items().stream().map(it -> line(it.qty(), it.netUnitPrice())).toList()) .isEqualTo(thisQuotationsLines); + assertThat(accepted.depositPercent()).isEqualTo(30); + assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); } @Test @@ -917,15 +923,25 @@ private static String line(BigDecimal qty, BigDecimal netUnitPrice) { } private DealQuotationDto acceptedNewOriginQuotation(String recipient, BigDecimal qty) { - DealQuotationDto issued = issuedNewOriginQuotation(recipient, qty); + return acceptedNewOriginQuotation(recipient, qty, null); + } + + private DealQuotationDto acceptedNewOriginQuotation(String recipient, BigDecimal qty, Integer depositPercent) { + DealQuotationDto issued = issuedNewOriginQuotation(recipient, qty, depositPercent); return quotationService.recordOutcome(issued.id(), new RecordOutcomeRequest(QuotationStatus.ACCEPTED, "ลูกค้าโอเค", UUID.randomUUID().toString()), salesActor); } private DealQuotationDto issuedNewOriginQuotation(String recipient, BigDecimal qty) { + return issuedNewOriginQuotation(recipient, qty, null); + } + + /** {@code depositPercent} null keeps the percent the quotation was created with. */ + private DealQuotationDto issuedNewOriginQuotation(String recipient, BigDecimal qty, Integer depositPercent) { PricingDecisionDto decision = approvedDecision(recipient, qty); DealQuotationDto draft = quotationService.createFromPricingRequest(decision.pricingRequestId(), salesActor); - DealQuotationDto withValidity = quotationService.update(draft.id(), upsertWithValidityDays(draft, 30), salesActor); + DealQuotationDto withValidity = quotationService.update(draft.id(), upsertWithValidityDays(draft, 30, + depositPercent != null ? depositPercent : draft.depositPercent()), salesActor); DealQuotationDto submitted = quotationService.submit(withValidity.id(), salesActor); return quotationService.approve(submitted.id(), new ApproveRequest(null), salesManagerActor); } @@ -1036,9 +1052,14 @@ null, new BigDecimal("10"), new BigDecimal("0.36"), WastageCalculator.QUANTITY_M } private DealQuotationRequests.UpsertDealQuotationRequest upsertWithValidityDays(DealQuotationDto q, int days) { + return upsertWithValidityDays(q, days, q.depositPercent()); + } + + private DealQuotationRequests.UpsertDealQuotationRequest upsertWithValidityDays(DealQuotationDto q, int days, + Integer depositPercent) { List items = q.items().stream().map(this::asItemInput).toList(); return new DealQuotationRequests.UpsertDealQuotationRequest(q.contactId(), q.deptCode(), q.unitCode(), - q.offerDate(), q.depositPercent(), q.remainderMode(), q.creditDays(), days, q.customerNotes(), + q.offerDate(), depositPercent, q.remainderMode(), q.creditDays(), days, q.customerNotes(), q.priceMode(), q.documentLanguage(), q.currency(), items); } From 1c76cfb55a38bc136b20ddd41066e7ab67508b2d Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:19:36 +0700 Subject: [PATCH 13/19] =?UTF-8?q?test(mock):=20pin=20the=20pricing-request?= =?UTF-8?q?=20=E0=B8=A1=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft's=20pe?= =?UTF-8?q?rcent=20to=20the=20accepted=20new-engine=20quotation's=20own?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new-engine drive can now set the quotation's depositPercent before submit; the first new-engine case sets 30 and expects the draft to carry 0.3. Red on the current mock: 0.5, because the route takes payload.depositPercent ?? 0.5. Co-Authored-By: Claude Opus 5.5 --- .../src/api/mockApi.depositNotices.test.js | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index cb7ce3e31..b3ab8910b 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -144,12 +144,18 @@ async function driveTicketToAcceptedQuotation() { // from the CEO decision, so the deposit mapper's baht discount label is exercised. The discount // sits on the decision, not on a quotation update: a line that differs from the CEO price makes // the mock's PRICING_REQUEST approve refuse sales_manager (403, CEO approval required). -async function driveTicketToNewEngineIssuedQuotation() { +async function driveTicketToNewEngineIssuedQuotation({ depositPercent } = {}) { const { ticketId, prId } = await driveTicketToApprovedDecision({ priceMode: 'NET', discountPct: 10 }); await api.auth.login({ role: 'sales' }); const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); // Java's drive (DealQuotationOutcomeIntegrationTest) saves validityDays before submit; the // mock's submit/approve/recordOutcome never require it, so that update is left out here. + if (depositPercent != null) { + await api.dealQuotations.update(draft.id, { + priceMode: draft.priceMode, depositPercent, remainderMode: 'ON_DELIVERY', + items: draft.items.map((item) => ({ ...item, id: item.id })), + }); + } await api.dealQuotations.submit(draft.id, {}); await api.auth.login({ role: 'sales_manager' }); const { quotation: issued } = await api.dealQuotations.approve(draft.id, {}); @@ -157,8 +163,8 @@ async function driveTicketToNewEngineIssuedQuotation() { return { ticketId, prId, issued }; } -async function driveTicketToNewEngineAcceptedQuotation() { - const { ticketId, prId, issued } = await driveTicketToNewEngineIssuedQuotation(); +async function driveTicketToNewEngineAcceptedQuotation(options) { + const { ticketId, prId, issued } = await driveTicketToNewEngineIssuedQuotation(options); const { quotation: accepted } = await api.dealQuotations.recordOutcome(issued.id, { outcome: 'ACCEPTED', customerNote: null, clientRequestId: uuid(), }); @@ -217,10 +223,13 @@ describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notic // quotation (DealQuotationRepository.findAcceptedForPricingRequest), and an ISSUED-only one // still gets the 409. describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engine quotation (mirrors OrderConfirmationService fix)', () => { - it('creates the draft with the accepted quotation number as reference and items from that quotation', async () => { - const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); + it('creates the draft with the accepted quotation number as reference, and items and percent from that quotation', async () => { + const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation({ depositPercent: 30 }); const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); expect(depositNotice.reference).toBe(accepted.number); + // DepositNoticeService.createDraftFromQuotation: the chosen quotation's own whole-number percent. + expect(accepted.depositPercent).toBe(30); + expect(depositNotice.depositPercent).toBe(0.3); expect(depositNotice.items.length).toBeGreaterThan(0); // qty is deliberately not distinguishable here: a mock TILE row sets quantity = piecesFinal. expect(depositNotice.items[0].qty).toBe(accepted.items[0].quantity); From 316a37aff291ce08b769448aef1d4bed157fede3 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:42:19 +0700 Subject: [PATCH 14/19] =?UTF-8?q?fix(sales):=20pricing-request=20=E0=B8=A1?= =?UTF-8?q?=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20draft=20bills=20THIS=20p?= =?UTF-8?q?ricing=20request's=20quotation=20percent,=20not=20the=20ticket'?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloud review: the new-engine branch took reference and items from this pricing request's ACCEPTED quotation, but the percent still came from createDraft's ticket-scoped, legacy-first lookup, so a sibling pricing request's legacy row carrying a percent would be billed under this quotation's number. - DepositNoticeService#createDraftFromQuotation(ticketId, req, sourceDepositPercent, actor): a non-null source percent is the draft's percent; createDraft keeps its signature and delegates with null (per-ticket lookup, unchanged). - OrderConfirmationService#createDepositNoticeFromQuotation passes the chosen new-engine quotation's own depositPercent. The legacy branch passes null: CustomerQuotationDto carries no percent, so it keeps today's derivation. - mockApi pricingRequests.createDepositNoticeFromQuotation mirrors it: the new-engine branch bills newEngine.depositPercent / 100; the legacy branch and the ticket-level createDraft are unchanged. Test: createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent (red 0.7000, now 0.30) and the mock new-engine case (red 0.5, now 0.3). Co-Authored-By: Claude Opus 5.5 --- .../glr/hr/deposit/DepositNoticeService.java | 18 +++++++++++++++--- .../OrderConfirmationService.java | 18 ++++++++++++------ frontend/src/api/mockApi.js | 7 ++++++- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java index e8bed4be5..230085247 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java @@ -92,6 +92,15 @@ public DepositNoticeDto getById(long docId, UserPrincipal actor) { // Create a DRAFT from approved ticket items @Transactional public DepositNoticeDto createDraft(long ticketId, DepositNoticeDraftRequest req, UserPrincipal actor) { + return createDraftFromQuotation(ticketId, req, null, actor); + } + + /** {@link #createDraft} for a caller that has already chosen the source quotation: a non-null + * {@code sourceDepositPercent} (that quotation's own whole-number {@code deposit_percent}) is + * the draft's percent, instead of the per-ticket lookup. */ + @Transactional + public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDraftRequest req, + Integer sourceDepositPercent, UserPrincipal actor) { requireRole(actor, SALES_ROLES); TicketSummaryDto s = requireApprovedTicket(ticketId, actor); requireActiveLifecycle(s); @@ -128,9 +137,12 @@ public DepositNoticeDto createDraft(long ticketId, DepositNoticeDraftRequest req // document's long-standing 0.50 default only when the ticket has no live quotation to // read a percent from at all (e.g. a manually-approved legacy ticket with no quotation // chain) — see DepositNoticeRepository#findDealQuotationDepositPercent's own Javadoc for - // the full pick order. - BigDecimal effectiveDepositPercent = docs.findDealQuotationDepositPercent(ticketId) - .orElse(new BigDecimal("0.50")); + // the full pick order. The pricing-request route names its source quotation and passes + // that quotation's own percent; the ticket-level route passes null and still derives it + // per ticket. + BigDecimal effectiveDepositPercent = sourceDepositPercent != null + ? BigDecimal.valueOf(sourceDepositPercent, 2) + : docs.findDealQuotationDepositPercent(ticketId).orElse(new BigDecimal("0.50")); var effective = new DepositNoticeDraftRequest( req.customerName() != null ? req.customerName() : s.customerName(), diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index a7d9d7d1d..26467f1ea 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -439,11 +439,13 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * open revision draft can outrank the accepted row by id. The items come from that quotation, * the accepted quotation of THIS pricing request ({@link * DepositNoticeService#itemsFromDealQuotation}), so a sibling pricing request's quotation on - * the same ticket can never supply them. The percent is still whatever {@code createDraft} - * derives, which is ticket-scoped and legacy-first: on a ticket that also carries a sibling - * pricing request's legacy ISSUED quotation, the percent comes from that legacy row. That is a - * known limitation the ticket-level route shares; this route does not introduce it. The - * customer must have accepted: an ISSUED-only quotation answers 409. + * the same ticket can never supply them. The percent also comes from that quotation: its own + * {@code deposit_percent} goes to {@link DepositNoticeService#createDraftFromQuotation}, so a + * sibling pricing request's legacy row on the same ticket cannot supply it either (a + * new-engine sibling never could: R8 allows one ACCEPTED quotation per ticket and the lookup + * prefers ACCEPTED). The legacy branch passes no percent, because {@code CustomerQuotationDto} + * carries none, so it keeps {@code createDraft}'s per-ticket derivation; the ticket-level + * route does the same. The customer must have accepted: an ISSUED-only quotation answers 409. */ @Transactional public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, @@ -459,9 +461,11 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, String reference; List items; + Integer sourceDepositPercent; if (accepted != null) { reference = accepted.number(); items = DepositNoticeService.itemsFromQuotation(accepted.items()); + sourceDepositPercent = null; } else { th.co.glr.hr.dealquotation.DealQuotationDtos.DealQuotationDto newEngine = dealQuotations == null ? null @@ -473,6 +477,7 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, } reference = newEngine.number(); items = depositNotices.itemsFromDealQuotation(newEngine.id()); + sourceDepositPercent = newEngine.depositPercent(); } // request.depositPercent() is deliberately still passed through here even though @@ -480,7 +485,8 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, DepositNoticeDraftRequest draftRequest = new DepositNoticeDraftRequest( null, null, null, null, reference, request != null ? request.depositPercent() : null, null, items); - DepositNoticeDto draft = depositNotices.createDraft(summary.ticketId(), draftRequest, actor); + DepositNoticeDto draft = depositNotices.createDraftFromQuotation(summary.ticketId(), draftRequest, + sourceDepositPercent, actor); pricingRequests.addEvent(pricingRequestId, summary.ticketId(), actor.id(), actor.name(), PricingRequestEventKind.DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION, summary.status(), summary.status(), diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 02942b431..d511a29df 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -16225,7 +16225,12 @@ export const api = { customerName: ticket.customerName ?? '', customerTaxId: customerTaxId ?? '', customerAddress: customerAddress ?? '', projectName: projectName ?? '', reference, - depositPercent: payload.depositPercent ?? 0.5, vatPercent: 0.07, + // Mirrors DepositNoticeService.createDraftFromQuotation: the new-engine branch bills the + // chosen quotation's own whole-number percent; the legacy branch keeps the old stand-in. + depositPercent: newEngine?.depositPercent != null + ? newEngine.depositPercent / 100 + : payload.depositPercent ?? 0.5, + vatPercent: 0.07, notes, items, issuedByName: null, preparerName: 'จินตนา หาญมนตรี', hasPdf: false, hasXlsx: false, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), From 1d390bf23d65aad3cc5bf50f8cdeb3d36e05bf0a Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:17:59 +0700 Subject: [PATCH 15/19] test(sales): mixed-ticket deposit test asserts its legacy seed applied; qualify the percent Javadoc The 70% legacy seed now asserts it updated exactly one row, so the test cannot pass vacuously if the seed misses. The createDepositNoticeFromQuotation Javadoc now says the source quotation's percent wins only when it sets one; a null percent still falls back to the per-ticket lookup. Co-Authored-By: Claude Opus 5.5 --- .../co/glr/hr/orderconfirmation/OrderConfirmationService.java | 3 ++- .../hr/dealquotation/DealQuotationOutcomeIntegrationTest.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index 26467f1ea..3dd04539c 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -443,7 +443,8 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * {@code deposit_percent} goes to {@link DepositNoticeService#createDraftFromQuotation}, so a * sibling pricing request's legacy row on the same ticket cannot supply it either (a * new-engine sibling never could: R8 allows one ACCEPTED quotation per ticket and the lookup - * prefers ACCEPTED). The legacy branch passes no percent, because {@code CustomerQuotationDto} + * prefers ACCEPTED) when the quotation sets a percent; a null percent still falls back to the + * per-ticket lookup. The legacy branch passes no percent, because {@code CustomerQuotationDto} * carries none, so it keeps {@code createDraft}'s per-ticket derivation; the ticket-level * route does the same. The customer must have accepted: an ISSUED-only quotation answers 409. */ diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java index 16a76feec..a8f938c07 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java @@ -332,8 +332,8 @@ void createDepositNoticeFromQuotation_newEngineAcceptedWithAnOpenRevisionDraft_s void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent() { CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); // No code path writes deposit_percent on a legacy row; this stands in for one that carries it. - jdbc.update("UPDATE sales.quotation SET deposit_percent = 70 WHERE quotation_id = :id", - Map.of("id", legacyIssued.id())); + assertThat(jdbc.update("UPDATE sales.quotation SET deposit_percent = 70 WHERE quotation_id = :id", + Map.of("id", legacyIssued.id()))).isEqualTo(1); DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10"), 30); long pricingRequestId = accepted.pricingRequestId(); orderConfirmation.confirmOrder(pricingRequestId, From 12051a5a55d84ff0882452b2f4cc432ce2c5f1be Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:45:42 +0700 Subject: [PATCH 16/19] =?UTF-8?q?fix(sales):=20pricing-request=20=E0=B8=A1?= =?UTF-8?q?=E0=B8=B1=E0=B8=94=E0=B8=88=E0=B8=B3=20route=20refuses=20a=20ze?= =?UTF-8?q?ro-deposit=20new-engine=20quotation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 0% ("ไม่รับมัดจำ", full payment) quotation has nothing to bill. The new-engine branch of createDepositNoticeFromQuotation passed depositPercent 0 straight into the draft, producing a 0-baht notice that nothing downstream refuses to issue (issue() reads the ticket's deposit_policy, which no path syncs from the quotation). Now 409 with the quotation number. Mock mirrored. Test-first: DealQuotationOutcomeIntegrationTest .createDepositNoticeFromQuotation_newEngineAcceptedZeroDeposit_409AndNoDraft (real DB, red: "Expecting code to raise a throwable" → green, class 33/33) and mockApi.depositNotices.test.js "rejects a 0% (full-payment) new-engine quotation with 409 and creates no draft" (red: promise resolved with a draft → green, file 9/9). Co-Authored-By: Claude Fable 5.1 --- .../OrderConfirmationService.java | 8 ++++ .../DealQuotationOutcomeIntegrationTest.java | 39 +++++++++++++++++++ .../src/api/mockApi.depositNotices.test.js | 18 ++++++++- frontend/src/api/mockApi.js | 4 ++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index 3dd04539c..56a21a5a0 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -476,6 +476,14 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, if (newEngine == null) { throw new ApiException(HttpStatus.CONFLICT, "ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"); } + // A zero-deposit quotation (DealQuotationService#isZeroDeposit: "ไม่รับมัดจำ", paid in + // full under fullPaymentTerm) has nothing to bill. Refuse here rather than draft a + // 0-baht notice: DepositNoticeService#issue reads the ticket's deposit_policy, which no + // path syncs from the quotation, so nothing downstream would stop that draft. + if (Integer.valueOf(0).equals(newEngine.depositPercent())) { + throw new ApiException(HttpStatus.CONFLICT, "ใบเสนอราคา " + newEngine.number() + + " ระบุไม่มีมัดจำ (ชำระเต็มจำนวน) จึงออกใบแจ้งยอดเงินรับมัดจำไม่ได้"); + } reference = newEngine.number(); items = depositNotices.itemsFromDealQuotation(newEngine.id()); sourceDepositPercent = newEngine.depositPercent(); diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java index a8f938c07..08b841ab7 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java @@ -289,6 +289,28 @@ void createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409() { Map.of("id", ticketId), Long.class)).isZero(); } + /** A zero-deposit quotation ("ไม่รับมัดจำ", full payment) has no deposit to bill: the route must + * refuse rather than draft a 0-baht document, which nothing downstream would stop from being + * issued ({@code DepositNoticeService#issue} reads the ticket's deposit_policy, which no path + * syncs from a 0% quotation). */ + @Test + void createDepositNoticeFromQuotation_newEngineAcceptedZeroDeposit_409AndNoDraft() { + DealQuotationDto accepted = acceptedZeroDepositNewOriginQuotation(PricingRequestRecipient.DESIGNER, + new BigDecimal("10")); + long pricingRequestId = accepted.pricingRequestId(); + assertThat(accepted.depositPercent()).isZero(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor)) + .isInstanceOf(ApiException.class) + .hasFieldOrPropertyWithValue("status", HttpStatus.CONFLICT) + .hasMessageContaining("ไม่มีมัดจำ"); + assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", + Map.of("id", ticketId), Long.class)).isZero(); + } + @Test void createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach() { DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); @@ -932,6 +954,23 @@ private DealQuotationDto acceptedNewOriginQuotation(String recipient, BigDecimal new RecordOutcomeRequest(QuotationStatus.ACCEPTED, "ลูกค้าโอเค", UUID.randomUUID().toString()), salesActor); } + /** Like {@link #acceptedNewOriginQuotation} but with depositPercent 0, which {@code submit} + * only accepts together with a full-payment term ({@code resolveFullPaymentTerm}). */ + private DealQuotationDto acceptedZeroDepositNewOriginQuotation(String recipient, BigDecimal qty) { + PricingDecisionDto decision = approvedDecision(recipient, qty); + DealQuotationDto draft = quotationService.createFromPricingRequest(decision.pricingRequestId(), salesActor); + DealQuotationRequests.UpsertDealQuotationRequest zeroDeposit = new DealQuotationRequests.UpsertDealQuotationRequest( + draft.contactId(), draft.deptCode(), draft.unitCode(), draft.offerDate(), 0, draft.remainderMode(), + draft.creditDays(), 30, null, null, draft.customerNotes(), draft.priceMode(), draft.documentLanguage(), + draft.currency(), null, null, null, null, "BEFORE_DELIVERY", null, null, null, null, null, + draft.items().stream().map(this::asItemInput).toList()); + DealQuotationDto saved = quotationService.update(draft.id(), zeroDeposit, salesActor); + DealQuotationDto submitted = quotationService.submit(saved.id(), salesActor); + DealQuotationDto issued = quotationService.approve(submitted.id(), new ApproveRequest(null), salesManagerActor); + return quotationService.recordOutcome(issued.id(), + new RecordOutcomeRequest(QuotationStatus.ACCEPTED, "ลูกค้าโอเค", UUID.randomUUID().toString()), salesActor); + } + private DealQuotationDto issuedNewOriginQuotation(String recipient, BigDecimal qty) { return issuedNewOriginQuotation(recipient, qty, null); } diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index b3ab8910b..e0c3924ac 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -144,15 +144,16 @@ async function driveTicketToAcceptedQuotation() { // from the CEO decision, so the deposit mapper's baht discount label is exercised. The discount // sits on the decision, not on a quotation update: a line that differs from the CEO price makes // the mock's PRICING_REQUEST approve refuse sales_manager (403, CEO approval required). -async function driveTicketToNewEngineIssuedQuotation({ depositPercent } = {}) { +async function driveTicketToNewEngineIssuedQuotation({ depositPercent, fullPaymentTerm } = {}) { const { ticketId, prId } = await driveTicketToApprovedDecision({ priceMode: 'NET', discountPct: 10 }); await api.auth.login({ role: 'sales' }); const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); // Java's drive (DealQuotationOutcomeIntegrationTest) saves validityDays before submit; the // mock's submit/approve/recordOutcome never require it, so that update is left out here. + // fullPaymentTerm is only meaningful (and, for submit, required) when depositPercent is 0. if (depositPercent != null) { await api.dealQuotations.update(draft.id, { - priceMode: draft.priceMode, depositPercent, remainderMode: 'ON_DELIVERY', + priceMode: draft.priceMode, depositPercent, remainderMode: 'ON_DELIVERY', fullPaymentTerm, items: draft.items.map((item) => ({ ...item, id: item.id })), }); } @@ -261,6 +262,19 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin const { depositNotices } = await api.depositNotices.listByTicket(ticketId); expect(depositNotices).toHaveLength(0); }); + + // A zero-deposit ("ไม่รับมัดจำ", full payment) quotation has nothing to bill: Java refuses rather + // than drafting a 0-baht document that nothing downstream would stop from being issued. + it('rejects a 0% (full-payment) new-engine quotation with 409 and creates no draft', async () => { + const { ticketId, prId, accepted } = await driveTicketToNewEngineAcceptedQuotation({ + depositPercent: 0, fullPaymentTerm: 'BEFORE_DELIVERY', + }); + expect(accepted.depositPercent).toBe(0); + await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})) + .rejects.toMatchObject({ status: 409, message: expect.stringContaining('ไม่มีมัดจำ') }); + const { depositNotices } = await api.depositNotices.listByTicket(ticketId); + expect(depositNotices).toHaveLength(0); + }); }); // Issue #752: the mock must reproduce production's one-render gap. Per diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index d511a29df..230c258fa 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -16199,6 +16199,10 @@ export const api = { const newEngine = accepted ? null : mockDealQuotations.find( (q) => q.pricingRequestId === pr.id && q.origin === 'PRICING_REQUEST' && q.docStatus === 'ACCEPTED'); if (!accepted && !newEngine) fail('ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้', 409); + // Mirrors the zero-deposit refusal: a 0% ("ไม่รับมัดจำ") quotation has nothing to bill. + if (newEngine?.depositPercent === 0) { + fail(`ใบเสนอราคา ${newEngine.number} ระบุไม่มีมัดจำ (ชำระเต็มจำนวน) จึงออกใบแจ้งยอดเงินรับมัดจำไม่ได้`, 409); + } const reference = (accepted ?? newEngine).number; const items = accepted ? mockDepositNoticeItemsFromQuotation(accepted.items) From 06182e87b21c37126a92f464d12925c32d4d40dc Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:46:23 +0700 Subject: [PATCH 17/19] =?UTF-8?q?chore(docs):=20drop=20the=20committed=20i?= =?UTF-8?q?mplementation=20plan=20=E2=80=94=20the=20PR=20body=20is=20the?= =?UTF-8?q?=20handoff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md retired handoff docs in 2026-07; this was the first file under docs/superpowers and already described the superseded first design. No test: docs-only. Co-Authored-By: Claude Fable 5.1 --- ...026-10-19-deposit-notice-new-engine-pcr.md | 321 ------------------ 1 file changed, 321 deletions(-) delete mode 100644 docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md diff --git a/docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md b/docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md deleted file mode 100644 index 507df8f36..000000000 --- a/docs/superpowers/plans/2026-10-19-deposit-notice-new-engine-pcr.md +++ /dev/null @@ -1,321 +0,0 @@ -# Deposit Notice From a New-Engine Pricing-Request Quotation — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** `POST /api/pricing-requests/{id}/deposit-notice` creates the มัดจำ draft when the pricing request's accepted quotation came from the new deal-quotation engine, instead of answering 409. - -**Architecture:** One server-side change in `OrderConfirmationService.createDepositNoticeFromQuotation`: when no legacy (`origin IS NULL`) ACCEPTED quotation exists for the pricing request, accept the new-engine (`origin = 'PRICING_REQUEST'`) ACCEPTED quotation instead, pass its number as the draft's reference and let `DepositNoticeService.createDraft` source items and deposit percent exactly as the ticket-level route already does (`findDealQuotationItemsForDeposit` / `findDealQuotationDepositPercent`). No frontend change; the mock mirrors the same branch. No schema change. No authz change. - -**Tech Stack:** Spring Boot service (Java), `AbstractPostgresIntegrationTest` on real Postgres, vitest for the mock mirror. - -**Spec:** There is no separate design doc. The spec is the QA finding of 2026-10-18 on the develop QA stack plus the owner's confirmation on 2026-10-19: - -- A deal priced through a pricing request, quoted through the new engine (`DealQuotationService.createFromPricingRequest` → submit → approve → `recordOutcome(ACCEPTED)`), then order-confirmed, reaches ขั้น 10, and the UI's มัดจำ step (`DealDepositPanel`, `DepositNoticePage`) calls the pricing-request route, which answers `409 "ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"` because it only reads `CustomerQuotationRepository.findByPricingRequest` (legacy rows). -- The ticket-level route `POST /api/tickets/{id}/deposit-notice/draft` already handles the new engine; the UI never reaches it when a `QUOTATION_ACCEPTED` pricing request exists. -- Direct quotations (no pricing request) are unaffected and must stay so. -- The pricing-request route's own gate keeps its meaning: a quotation the customer has not accepted (ISSUED only) still yields the same 409. Owner-only access (`requireOwner`) is unchanged. -- Deposit percent is sourced from the quotation (owner ruling 2026-09-29), unchanged. - -## Global Constraints - -- Base branch `develop` at `c64a6e89`. Branch name `fix/deposit-notice-new-engine-pcr`. Never `gh pr merge --auto` into develop. -- Sales/CRM workflow logic may change (CLAUDE.md "Sales flow redesign"); every behaviour change is stated in the PR body. Payroll, tax, SSO, commission math untouched. -- Test-first: the failing test is written, shown red for the right reason, then the fix. Existing test files are revised; new files only where no file owns the behaviour (`tests-revise-dont-bloat`). -- Backend tests run on the local Postgres, one class at a time, with `-Dtest.fork.count=1` (exact property name) and a throwaway database; never the full `mvnw verify` locally. -- After any mutation-check revert of Java code, run `./mvnw -q clean` before trusting a re-run (stale `.class` files). -- `mockApi.js` keeps each `// Mirrors ` header accurate; the mock's authz is not evidence. -- The 409 message text `ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้` is unchanged (frontend shows `err.message`). -- Commit messages end with `Co-Authored-By: Claude Fable 5.1 `; PR body ends with `🤖 Generated with [Claude Code](https://claude.com/claude-code)`. - -## Review Focus - -1. **New-engine quotation ISSUED but not yet ACCEPTED** — the route must still answer the same 409 and write no `sales.deposit_notice` row. Pinned in Task 1 (`createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409`). -2. **Non-owning sales rep on a new-engine deal** — 403 before any quotation lookup, no row written. Pinned in Task 1 (`createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach`). -3. **A hand-wired `OrderConfirmationService` with no `DealQuotationRepository` wired** (`dealQuotations == null`, as in `OrderConfirmationIntegrationTest`) — the legacy path must behave exactly as before and the new branch must not NPE. Pinned by the existing `OrderConfirmationIntegrationTest.fullChain_quotationAcceptedThroughDepositPaid_composesWithoutShortcuts` staying green (Task 2 step 4) and by the null guard in Task 2. -4. **Calling the route twice** — creates a second DRAFT version, same as the legacy path today (`DepositNoticeService.createDraft` numbers versions). Not a new behaviour; not separately pinned. -5. **Items and percent of the new-engine draft must equal what the ticket-level route produces** — the two routes share `createDraft`, so the test compares the two drafts field by field (Task 1, happy path). A divergence here is wrong money on a customer document. - ---- - -### Task 1: Backend tests — the bug, and the two gates that must survive the fix - -**Files:** -- Modify: `backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java` (add three `@Test` methods after `recordOutcome_byOwningSales_accepted_setsPrStatusAndDocStatus`, ~line 240; reuse its wiring — it already constructs `DealQuotationService`, `DepositNoticeService`, `OrderConfirmationService` and calls `orderConfirmation.wireDealQuotationRepository(quotationRepository)`) - -**Interfaces:** -- Consumes (existing, in this test class): `acceptedNewOriginQuotation(String recipient, BigDecimal qty) → DealQuotationDto`, `issuedNewOriginQuotation(String recipient, BigDecimal qty) → DealQuotationDto`, `orderConfirmation.confirmOrder(long pricingRequestId, OrderConfirmationRequests.ConfirmOrderRequest, UserPrincipal)`, `orderConfirmation.createDepositNoticeFromQuotation(long pricingRequestId, OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest, UserPrincipal) → DepositNoticeDto`, the field `depositNoticeService` (make it a class field if it is still a local in `@BeforeEach`), `salesActor`, `otherSalesActor`, `ticketId`, `jdbc`. -- Consumes: `DealQuotationDto.pricingRequestId()`, `DealQuotationDto.number()`, `DepositNoticeDto.reference()`, `DepositNoticeDto.items()`, `DepositNoticeDto.depositPercent()`, `DepositNoticeItemDto(id, seq, description, qty, unit, unitPrice, discountLabel, netUnitPrice, amount)`, `DepositNoticeDraftRequest(customerName, customerTaxId, customerAddress, projectName, reference, depositPercent, notes, items)`. -- Produces: nothing new for later tasks; Task 2 turns the first test green. - -- [ ] **Step 1: Add the three tests** - -```java -// ── Deposit notice from the pricing-request route when the accepted quotation is NEW-engine ── -// QA finding 2026-10-18 (develop QA stack): DealDepositPanel/DepositNoticePage call -// POST /pricing-requests/{id}/deposit-notice whenever a QUOTATION_ACCEPTED pricing request exists; -// that route read only legacy CustomerQuotation rows and answered 409 for every deal quoted -// through DealQuotationService.createFromPricingRequest. Direct quotations never take this route. - -@Test -void createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameDraftTheTicketRouteWould() { - DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); - long pricingRequestId = accepted.pricingRequestId(); - orderConfirmation.confirmOrder(pricingRequestId, - new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); - - DepositNoticeDto viaPricingRequest = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, - new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); - - // The reference is the accepted quotation's number — the one thing this route adds over the ticket route. - assertThat(viaPricingRequest.reference()).isEqualTo(accepted.number()); - assertThat(viaPricingRequest.items()).isNotEmpty(); - - // Items and percent are exactly what the ticket-level route produces for this deal (shared createDraft). - DepositNoticeDto viaTicket = depositNoticeService.createDraft(ticketId, - new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); - assertThat(viaPricingRequest.depositPercent()).isEqualByComparingTo(viaTicket.depositPercent()); - assertThat(viaPricingRequest.items()) - .usingRecursiveComparison().ignoringFields("id") - .isEqualTo(viaTicket.items()); - - assertThat(jdbc.queryForObject(""" - SELECT COUNT(*) FROM sales.pricing_request_event - WHERE pricing_request_id = :id AND event_kind = 'DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION' - """, Map.of("id", pricingRequestId), Long.class)).isEqualTo(1L); -} - -@Test -void createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409() { - DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); - long pricingRequestId = issued.pricingRequestId(); - - assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, - new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor)) - .isInstanceOf(ApiException.class) - .hasMessageContaining("ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้"); - assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", - Map.of("id", ticketId), Long.class)).isZero(); -} - -@Test -void createDepositNoticeFromQuotation_newEngine_nonOwnerSales_cannotReach() { - DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); - long pricingRequestId = accepted.pricingRequestId(); - orderConfirmation.confirmOrder(pricingRequestId, - new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); - - assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, - new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), otherSalesActor)) - .isInstanceOf(ApiException.class) - .hasMessageContaining("ไม่มีสิทธิ์เข้าถึงรายการนี้"); - assertThat(jdbc.queryForObject("SELECT COUNT(*) FROM sales.deposit_notice WHERE ticket_id = :id", - Map.of("id", ticketId), Long.class)).isZero(); -} -``` - -If `sales.pricing_request_event`'s column names differ (`kind` vs `event_kind`), copy the exact query `OrderConfirmationIntegrationTest.countPricingRequestEventsOfKind` uses (line ~774) rather than guessing. Add `import th.co.glr.hr.deposit.DepositNoticeDto;`, `import th.co.glr.hr.deposit.DepositNoticeDraftRequest;` if missing. - -- [ ] **Step 2: Create the throwaway DB once and run the class; verify the red is the 409** - -```bash -psql -h localhost -U "$(whoami)" -d postgres -c 'CREATE DATABASE glr_verify_it OWNER "'"$(whoami)"'";' 2>/dev/null || true -cd /Users/ploy_warit/Desktop/GL-R-ERP/backend && TEST_DB_URL=jdbc:postgresql://localhost:5432/glr_verify_it TEST_DB_USERNAME="$(whoami)" TEST_DB_PASSWORD="" \ - ./mvnw -B -q -Dtest.fork.count=1 -Dtest=DealQuotationOutcomeIntegrationTest -Dsurefire.failIfNoSpecifiedTests=false test 2>&1 | tail -40 -``` - -Expected: `createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameDraftTheTicketRouteWould` FAILS with `ApiException ... ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้` (the bug). The other two PASS already — they are the gates that must survive Task 2 and are mutation-checked there. Every pre-existing test in the class still passes. If the happy-path test fails for any other reason (compile error, `confirmOrder` 409, fixture), fix the test, not the service, until the red is the 409. - -- [ ] **Step 3: Commit the red tests** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP && git add backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationOutcomeIntegrationTest.java -git commit -m "test(sales): pin the มัดจำ draft from a new-engine pricing-request quotation (reproduces the 409) - -Co-Authored-By: Claude Fable 5.1 " -``` - ---- - -### Task 2: Backend fix — accept the new-engine ACCEPTED quotation on the pricing-request route - -**Files:** -- Modify: `backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java:433-459` (`createDepositNoticeFromQuotation`) and its Javadoc above it. - -**Interfaces:** -- Consumes: `dealQuotations` (field, may be null — setter-injected via `wireDealQuotationRepository`), `DealQuotationRepository.findLatestForPricingRequest(long) → Optional`, `DealQuotationRepository.findById(long) → Optional`, `QuotationStatus.ACCEPTED`, `DepositNoticeService.createDraft(long ticketId, DepositNoticeDraftRequest, UserPrincipal)`. -- Produces: unchanged public signature `createDepositNoticeFromQuotation(long, CreateDepositNoticeFromQuotationRequest, UserPrincipal) → DepositNoticeDto`. - -- [ ] **Step 1: Implement the two-branch resolution** - -Replace the `orElseThrow` lookup with: legacy ACCEPTED quotation via `quotations.findByPricingRequest(pricingRequestId)` as today, else — only when `dealQuotations != null` — `dealQuotations.findLatestForPricingRequest(pricingRequestId).flatMap(dealQuotations::findById).filter(q -> QuotationStatus.ACCEPTED.equals(q.docStatus()))`; if neither, throw the existing 409 with the unchanged message. ("Latest" is correct: ACCEPTED is terminal for a pricing request's quotation family — R8 in `recordOutcome` — so no newer row can exist once one is accepted.) - -Then build `DepositNoticeDraftRequest` with `reference = ` and `items = legacy ? DepositNoticeService.itemsFromQuotation(legacy.items()) : null`. A `null` items list makes `createDraft`'s `buildItemsFromRequest` fall through to `docs.findDealQuotationItemsForDeposit(ticketId)` — the same source the ticket-level route uses — and the percent already comes from `findDealQuotationDepositPercent(ticketId)`. Use the found number in the `DEPOSIT_NOTICE_DRAFTED_FROM_QUOTATION` event text as today. - -Update the method's Javadoc: one paragraph stating the new-engine branch, why `items` is `null` for it, and that the 409 semantics (customer must have accepted) are unchanged. Delete nothing else from the existing Javadoc. - -- [ ] **Step 2: Run the class; all green** - -Same command as Task 1 step 2. Expected: every test in `DealQuotationOutcomeIntegrationTest` passes, including the three new ones. - -- [ ] **Step 3: Mutation-check the ISSUED gate** - -Temporarily change the filter to `QuotationStatus.ISSUED.equals(q.docStatus()) || QuotationStatus.ACCEPTED.equals(q.docStatus())`, re-run the class. Expected: ONLY `createDepositNoticeFromQuotation_newEngineIssuedNotAccepted_still409` fails. Revert, then `./mvnw -q clean` before the next run. - -- [ ] **Step 4: Run the legacy-path class to prove the unwired case is unchanged** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP/backend && TEST_DB_URL=jdbc:postgresql://localhost:5432/glr_verify_it TEST_DB_USERNAME="$(whoami)" TEST_DB_PASSWORD="" \ - ./mvnw -B -q -Dtest.fork.count=1 -Dtest='OrderConfirmationIntegrationTest,DepositNoticeIssueGuardIntegrationTest,DepositNoticeAccountNotificationIntegrationTest' -Dsurefire.failIfNoSpecifiedTests=false test 2>&1 | tail -20 -``` - -Expected: all pass (`OrderConfirmationIntegrationTest` never wires `DealQuotationRepository`, so this is the `dealQuotations == null` path). - -- [ ] **Step 5: Commit** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP && git add backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java -git commit -m "fix(sales): มัดจำ draft from the pricing-request route accepts a new-engine ACCEPTED quotation - -The route read only legacy CustomerQuotation rows, so every deal quoted through -DealQuotationService.createFromPricingRequest got 409 at ขั้น 10. It now falls back -to the pricing request's new-engine ACCEPTED quotation and lets createDraft source -items and percent exactly as the ticket-level route does. ISSUED-only still 409s; -owner gate unchanged. - -Co-Authored-By: Claude Fable 5.1 " -``` - ---- - -### Task 3: Mock mirror — same branch in `mockApi.js`, pinned in the existing mock deposit test - -**Files:** -- Modify: `frontend/src/api/mockApi.js` — `pricingRequests.createDepositNoticeFromQuotation` (~line 16160) and `depositNotices.createDraft`'s item fallback (~line 13266); add one mapper next to `mockDepositNoticeItemsFromQuotation` (~line 1427). -- Modify: `frontend/src/api/mockApi.depositNotices.test.js` — split the existing drive helper and add two cases. - -**Interfaces:** -- Consumes: `mockDealQuotations` rows (`pricingRequestId`, `origin`, `docStatus`, `number`, `ticketId`), `buildDealQuotationDto(row).items[]` (fields `descriptionLine`, `piecesFinal`, `unitPrice`, `discountPct`, `netUnitPrice` — confirm the names in `buildDealQuotationDto` before writing the mapper), `mockPickLiveQuotationForItemFallback(ticketId)` (already ranks ACCEPTED > ISSUED > APPROVED across both arrays, newest id wins), `api.dealQuotations.createFromPricingRequest / update / submit / approve / recordOutcome`, `api.pricingRequests.confirmOrder`. -- Produces: `function mockDepositNoticeItemsFromDealQuotation(row) → DepositNoticeItemRequest[]` (`{ seq, description, qty, unit, unitPrice, discountLabel, netUnitPrice }`, `qty = piecesFinal`, `unit = 'แผ่น'`, `discountLabel = discountPct > 0 ? \`ส่วนลด ${discountPct}% ต่อหน่วย\` : null`, `netUnitPrice` taken from the dto, never recomputed — the mock mirrors `DepositNoticeRepository.findDealQuotationItemsForDeposit`'s column pick, not a formula). - -- [ ] **Step 1: Revise the mock test — extract the drive to an approved decision, add two cases** - -In `mockApi.depositNotices.test.js`, split `driveTicketToAcceptedQuotation()` so the part up to and including the CEO-approved pricing decision becomes `driveTicketToApprovedDecision() → { ticketId, prId }`, and the existing helper calls it then continues on the legacy quotation path unchanged. Add: - -```js -async function driveTicketToNewEngineAcceptedQuotation() { - const { ticketId, prId } = await driveTicketToApprovedDecision(); - await api.auth.login({ role: 'sales' }); - const { quotation: draft } = await api.dealQuotations.createFromPricingRequest(prId); - // validity + submit + approve + accept, exactly as DealQuotationOutcomeIntegrationTest.acceptedNewOriginQuotation - await api.dealQuotations.update(draft.id, { ...payloadThatSetsValidityDays(draft, 30) }); - await api.dealQuotations.submit(draft.id, {}); - await api.auth.login({ role: 'sales_manager' }); - await api.dealQuotations.approve(draft.id, {}); - await api.auth.login({ role: 'sales' }); - const { quotation: accepted } = await api.dealQuotations.recordOutcome(draft.id, { outcome: 'ACCEPTED', customerNote: null, clientRequestId: uuid() }); - await api.pricingRequests.confirmOrder(prId, { clientRequestId: uuid() }); - return { ticketId, prId, accepted }; -} - -describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engine quotation (mirrors OrderConfirmationService fix)', () => { - it('creates the draft with the accepted quotation number as reference and items from that quotation', async () => { - const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); - const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); - expect(depositNotice.reference).toBe(accepted.number); - expect(depositNotice.items.length).toBeGreaterThan(0); - expect(depositNotice.items[0].qty).toBe(accepted.items[0].piecesFinal); - expect(depositNotice.items[0].netUnitPrice).toBe(accepted.items[0].netUnitPrice); - }); - - it('still 409s while the new-engine quotation is only ISSUED (customer has not accepted)', async () => { - const { ticketId, prId } = await driveTicketToApprovedDecision(); - // same drive, stop after approve (ISSUED) — factor the first half of the helper above if needed - … - await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})) - .rejects.toMatchObject({ status: 409, message: 'ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้' }); - const { depositNotices } = await api.depositNotices.listByTicket(ticketId); - expect(depositNotices).toHaveLength(0); - }); -}); -``` - -Resolve `payloadThatSetsValidityDays` by reading how `DealQuotationOutcomeIntegrationTest.upsertWithValidityDays` maps to the mock's `update` payload (the mock's own 400/409 messages name the missing field). The exact response envelope keys (`quotation`, `depositNotice`) are whatever the mock already returns for those calls. - -- [ ] **Step 2: Run the mock test; verify the red is the 409 on the happy case** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP/frontend && npx vitest run src/api/mockApi.depositNotices.test.js 2>&1 | tail -25 -``` - -Expected: the new happy-path case FAILS with the 409 message; the ISSUED case passes; every pre-existing case passes (the helper split changed no behaviour). - -- [ ] **Step 3: Implement the mock mirror** - -In `createDepositNoticeFromQuotation`: after the legacy `mockCustomerQuotations` lookup, if none, `const v2 = mockDealQuotations.find((q) => q.pricingRequestId === pr.id && q.origin === 'PRICING_REQUEST' && q.docStatus === 'ACCEPTED')`; 409 with the unchanged message if neither; `reference = (accepted ?? v2).number`; `items = accepted ? mockDepositNoticeItemsFromQuotation(accepted.items) : mockDepositNoticeItemsFromDealQuotation(v2)`. In `depositNotices.createDraft`'s fallback, after `mockPickTicketQuotationForDepositNotice` returns null, use `mockPickLiveQuotationForItemFallback(ticketId)` and the new mapper when its `origin` is not legacy — this mirrors `buildItemsFromRequest`'s last step, which the mock lacked. Update both `// Mirrors …` comments to name `OrderConfirmationService.createDepositNoticeFromQuotation` and `DepositNoticeRepository.findDealQuotationItemsForDeposit`. - -- [ ] **Step 4: Run the mock test and the contract test; lint** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP/frontend && npx vitest run src/api/mockApi.depositNotices.test.js src/api/contract.test.js src/features/deposits/DepositNoticePage.test.jsx src/features/tickets/TicketDetailPage.test.jsx 2>&1 | grep -E "Test Files|Tests " && npx eslint src/api/mockApi.js src/api/mockApi.depositNotices.test.js -``` - -Expected: all pass; eslint clean. - -- [ ] **Step 5: Commit** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP && git add frontend/src/api/mockApi.js frontend/src/api/mockApi.depositNotices.test.js -git commit -m "chore(mock): pricing-request deposit route and ticket draft fallback mirror the new-engine quotation branch - -Co-Authored-By: Claude Fable 5.1 " -``` - ---- - -### Task 4: Real-stack verification on the QA stack (the surface the bug was found on) - -**Files:** none changed. - -**Interfaces:** -- Consumes: the QA stack recipe — backend jar on `:8093` against DB `glr_qa_stages` (`SPRING_PROFILES_ACTIVE=demo SERVER_PORT=8093 SPRING_DATASOURCE_URL=jdbc:postgresql://localhost:5432/glr_qa_stages SPRING_DATASOURCE_USERNAME=ploy_warit APP_MAIL_PROVIDER=log SERVER_SESSION_COOKIE_SECURE=false APP_CORS_ALLOWED_ORIGINS=http://127.0.0.1:5173,http://localhost:5173`), vite `VITE_USE_MOCKS=false VITE_API_PROXY_TARGET=http://127.0.0.1:8093 npm run dev -- --port 5173 --strictPort`, seed deals titled `[SEED-10-18] ขั้น N …`. - -- [ ] **Step 1: Build the branch jar and start it on :8093** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP/backend && ./mvnw -B -q -DskipTests -Djacoco.skip=true package && cp target/*.jar "$SCRATCHPAD/deposit-fix-app.jar" -``` - -Stop whatever is on :8093 first (`lsof -ti:8093 | xargs kill`); start the new jar with the env above, in the background, and wait for `GET http://127.0.0.1:8093/actuator/health` → `{"status":"UP"}`. Restarting logs every cached browser session out. - -- [ ] **Step 2: Reproduce the finding against the fixed jar** - -With headless Playwright (or the Chrome extension) on `http://127.0.0.1:5173`: log in as the seed's sales persona, open the `[SEED-10-18] ขั้น 10` deal whose quotation is new-engine, press สร้างใบแจ้งยอดมัดจำ in the มัดจำ section. Expected: toast `สร้างร่างใบแจ้งยอดเงินรับมัดจำแล้ว`, navigation to `/tickets//deposit`, a DRAFT whose เลขที่อ้างอิง is the quotation number and whose rows match the quotation. Also open a `ขั้น 10` DIRECT-quotation seed deal and confirm its มัดจำ step still works (ticket-level route). Keep both screenshots in the scratchpad. - -If no ขั้น 10 new-engine seed exists in `glr_qa_stages`, run `SEED_BACKEND_URL=http://127.0.0.1:8093 ONLY=S10 node .design/sales-pipeline-review/real-backend/seed-stages.mjs` first. - -- [ ] **Step 3: Record the result** — the two screenshots' paths and the HTTP status of the create call go into the PR body (Task 5). No commit. - ---- - -### Task 5: Pre-PR gate, review, PR - -**Files:** none changed unless review finds something. - -- [ ] **Step 1: Pre-PR gate (targeted, per the standing rule)** - -```bash -cd /Users/ploy_warit/Desktop/GL-R-ERP/frontend && npm run lint && npx vitest run src/api/mockApi.depositNotices.test.js src/api/contract.test.js src/features/deposits src/features/tickets/DealDepositPanel.test.jsx 2>&1 | grep -E "Test Files|Tests " -cd /Users/ploy_warit/Desktop/GL-R-ERP && git rev-list --left-right --count HEAD...origin/develop # must be N 0 (not behind) -``` - -- [ ] **Step 2: Independent Opus review** (`sonnet-implements-opus-reviews-loop` / `parallel-agents-use-opus`): the reviewer re-runs `DealQuotationOutcomeIntegrationTest`, reverts `OrderConfirmationService` to develop's version to see the happy-path test go red, checks the mock mirrors the Java branch and not more, and checks the PR body's coverage list against the assertions. - -- [ ] **Step 3: Push and open the PR to develop** (on Ploy's say-so, per the standing rule). PR body sections: What (the behaviour change, stated as a sales-workflow change), Files changed, Commands run, Tests/build results (which ITs RAN on real Postgres), Authz evidence (`no authz change` — the owner gate is pre-existing; the wrong-way-round test pins it on the new path), Known risks (two DRAFT versions on a double call, same as before; the mock's ticket-level fallback gained the new-engine branch), Test coverage (the three Java cases, the two mock cases, red-before-green results, the mutation check, and what is NOT covered: no e2e-real spec drives the deposit step; the QA-stack walk in Task 4 is the browser evidence). - ---- - -## Self-review notes - -- **Spec coverage:** PCR new-engine → draft (Task 1/2); ISSUED-only → 409 (Task 1/2 + mutation); owner gate (Task 1); direct quotations unaffected (Task 4 step 2 second deal; the code path is untouched); mock fidelity (Task 3); real-stack reproduction (Task 4). -- **Types:** `DealQuotationDto.pricingRequestId()` is a `Long`; the tests unbox it where the accepted quotation is known to be PRICING_REQUEST-origin. `CreateDepositNoticeFromQuotationRequest(null)` is the controller's own default. `DepositNoticeItemDto` has `id` as the only per-document field, hence `ignoringFields("id")`. -- **Proportion:** the fix is ~15 lines of Java and ~25 of mock; the plan's length is in the tests, which are the deliverable the rules require. From 3f3c3d4fa4d4219490b4b04a099b2e51ccc3f72b Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:48:54 +0700 Subject: [PATCH 18/19] refactor(mock): deposit mapper reads the stored row; trim the new-engine deposit test fixtures mockDepositNoticeItemsFromDealQuotation built the whole deal-quotation DTO to read six fields that come straight off the stored row (mockDealQuotationItemDto only adds ceo* / priceChangedFromCeo on top of the spread). The test drops a redundant id after a spread, two conditional spreads the mock already gates on != null, and folds five single-field expects into one toMatchObject. No behaviour or message change. Co-Authored-By: Claude Fable 5.1 --- .../src/api/mockApi.depositNotices.test.js | 21 ++++++++----------- frontend/src/api/mockApi.js | 6 +++--- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index e0c3924ac..bc1a8e747 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -117,9 +117,9 @@ async function driveTicketToApprovedDecision({ priceMode, discountPct } = {}) { await api.auth.login({ role: 'ceo' }); const { decision } = await api.pricingRequests.startPricingDecision(prId, { defaultMarginPct: 0.2 }); await api.pricingRequests.updatePricingDecision(decision.id, { - ...(priceMode ? { priceMode } : {}), + priceMode, items: decision.items.map((item) => ({ - pricingDecisionItemId: item.id, minimumSellingPrice: 50, ...(discountPct != null ? { discountPct } : {}), + pricingDecisionItemId: item.id, minimumSellingPrice: 50, discountPct, })), }); await api.pricingRequests.approvePricingDecision(decision.id, {}); @@ -154,7 +154,7 @@ async function driveTicketToNewEngineIssuedQuotation({ depositPercent, fullPayme if (depositPercent != null) { await api.dealQuotations.update(draft.id, { priceMode: draft.priceMode, depositPercent, remainderMode: 'ON_DELIVERY', fullPaymentTerm, - items: draft.items.map((item) => ({ ...item, id: item.id })), + items: draft.items.map((item) => ({ ...item })), }); } await api.dealQuotations.submit(draft.id, {}); @@ -219,10 +219,7 @@ describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notic }); }); -// Mirrors OrderConfirmationService.createDepositNoticeFromQuotation's new-engine branch: with no -// ACCEPTED legacy quotation, the draft comes from THIS pricing request's ACCEPTED new-engine -// quotation (DealQuotationRepository.findAcceptedForPricingRequest), and an ISSUED-only one -// still gets the 409. +// Java picks the row with DealQuotationRepository.findAcceptedForPricingRequest. describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engine quotation (mirrors OrderConfirmationService fix)', () => { it('creates the draft with the accepted quotation number as reference, and items and percent from that quotation', async () => { const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation({ depositPercent: 30 }); @@ -233,11 +230,11 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin expect(depositNotice.depositPercent).toBe(0.3); expect(depositNotice.items.length).toBeGreaterThan(0); // qty is deliberately not distinguishable here: a mock TILE row sets quantity = piecesFinal. - expect(depositNotice.items[0].qty).toBe(accepted.items[0].quantity); - expect(depositNotice.items[0].netUnitPrice).toBe(accepted.items[0].netUnitPrice); - expect(depositNotice.items[0].unit).toBe(accepted.items[0].unit); - expect(depositNotice.items[0].description).toBe(accepted.items[0].descriptionLine); - expect(depositNotice.items[0].unitPrice).toBe(accepted.items[0].unitPrice); + const [line] = accepted.items; + expect(depositNotice.items[0]).toMatchObject({ + qty: line.quantity, netUnitPrice: line.netUnitPrice, unit: line.unit, + description: line.descriptionLine, unitPrice: line.unitPrice, + }); // DepositNoticeRepository.findDealQuotationItems prints sales_discount (unitPrice − netUnitPrice) // in baht, not the percent: ราคาตั้ง 120 at 10% off is net 108, so 12 baht. expect([accepted.items[0].unitPrice, accepted.items[0].netUnitPrice]).toEqual([120, 108]); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 230c258fa..d17302ee8 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1436,10 +1436,10 @@ function mockDepositNoticeItemsFromQuotation(items) { })); } -// Mirrors DepositNoticeRepository.findDealQuotationItems. Values are read off the DTO, never -// recomputed (label excepted — see below). +// Mirrors DepositNoticeRepository.findDealQuotationItems. Values are read off the stored row, +// never recomputed (label excepted — see below). function mockDepositNoticeItemsFromDealQuotation(row) { - return buildDealQuotationDto(row).items.map((item, idx) => { + return row.items.map((item, idx) => { // Display label only: the mock stores no sales_discount, so this restates the column Java reads // (DealQuotationRepository#itemParams writes it as unitPrice − netUnitPrice). const salesDiscount = round2(item.unitPrice - item.netUnitPrice); From 59f623ed9f2c06553890a539d7e767b33a8fa40d Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 12:49:30 +0700 Subject: [PATCH 19/19] refactor(sales): trim the deposit-route Javadoc to single statements of each reason Comment-only. The "accepted, not latest" reasoning now lives once, on DealQuotationRepository#findAcceptedForPricingRequest; the resolved-columns paragraph moves onto findDealQuotationItems, the method that actually reads them; createDraft's inline comment stops restating createDraftFromQuotation's Javadoc; one misplaced clause in createDepositNoticeFromQuotation's Javadoc moves next to the sentence it qualifies. Co-Authored-By: Claude Fable 5.1 --- .../co/glr/hr/deposit/DepositNoticeRepository.java | 12 +++++------- .../th/co/glr/hr/deposit/DepositNoticeService.java | 4 +--- .../orderconfirmation/OrderConfirmationService.java | 12 ++++++------ 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java index 205d11f32..7fe0ee83b 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java @@ -396,11 +396,7 @@ private void insertItems(long docId, List items) { * Owner-confirmable: that a deposit may autofill from an ISSUED-but-not-yet-ACCEPTED * PRICING_REQUEST quote (legacy parity) and from an APPROVED DEAL_DIRECT quote — this is item * autofill for a draft the user reviews before saving, not the deposit gate itself. - * - *

Reads the RESOLVED display columns the quotation itself prints - * ({@code qty}/{@code raw_unit}/{@code unit_price}/{@code final_unit_price}), NOT the legacy - * {@code requested_*}/{@code approved_unit_price} columns the V2 chain leaves null. The - * discount-label wording matches {@link DepositNoticeService#itemsFromQuotation}. + * The lines themselves come from {@link #findDealQuotationItems}. */ public List findDealQuotationItemsForDeposit(long ticketId) { List ids = jdbc.query(""" @@ -422,8 +418,10 @@ AND doc_status IN ('ACCEPTED', 'ISSUED', 'APPROVED') } /** One deal quotation's {@code sales.quotation_item} lines, by {@code seq}, as deposit-notice - * items: the resolved display columns the quotation prints. {@link - * #findDealQuotationItemsForDeposit} delegates to this for the quotation it selects. */ + * items. Reads the RESOLVED display columns the quotation itself prints + * ({@code qty}/{@code raw_unit}/{@code unit_price}/{@code final_unit_price}), NOT the legacy + * {@code requested_*}/{@code approved_unit_price} columns the V2 chain leaves null. The + * discount-label wording matches {@link DepositNoticeService#itemsFromQuotation}. */ public List findDealQuotationItems(long quotationId) { return jdbc.query(""" SELECT seq, description, qty, raw_unit, unit_price, sales_discount, final_unit_price diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java index 230085247..b2fa63925 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java @@ -137,9 +137,7 @@ public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDra // document's long-standing 0.50 default only when the ticket has no live quotation to // read a percent from at all (e.g. a manually-approved legacy ticket with no quotation // chain) — see DepositNoticeRepository#findDealQuotationDepositPercent's own Javadoc for - // the full pick order. The pricing-request route names its source quotation and passes - // that quotation's own percent; the ticket-level route passes null and still derives it - // per ticket. + // the full pick order. BigDecimal effectiveDepositPercent = sourceDepositPercent != null ? BigDecimal.valueOf(sourceDepositPercent, 2) : docs.findDealQuotationDepositPercent(ticketId).orElse(new BigDecimal("0.50")); diff --git a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java index 56a21a5a0..5ac3ab64e 100644 --- a/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java +++ b/backend/src/main/java/th/co/glr/hr/orderconfirmation/OrderConfirmationService.java @@ -435,15 +435,15 @@ private String mapUnitBasisToTicketItem(String unitBasis) { *

New-engine quotation: when the pricing request has no ACCEPTED legacy * quotation, this falls back to its ACCEPTED new-engine {@code PRICING_REQUEST}-origin * quotation ({@code DealQuotationService#createFromPricingRequest}), found by {@code - * DealQuotationRepository#findAcceptedForPricingRequest} rather than by "latest", because an - * open revision draft can outrank the accepted row by id. The items come from that quotation, + * DealQuotationRepository#findAcceptedForPricingRequest} (its Javadoc says why "latest" is + * the wrong pick). The items come from that quotation, * the accepted quotation of THIS pricing request ({@link * DepositNoticeService#itemsFromDealQuotation}), so a sibling pricing request's quotation on * the same ticket can never supply them. The percent also comes from that quotation: its own - * {@code deposit_percent} goes to {@link DepositNoticeService#createDraftFromQuotation}, so a - * sibling pricing request's legacy row on the same ticket cannot supply it either (a - * new-engine sibling never could: R8 allows one ACCEPTED quotation per ticket and the lookup - * prefers ACCEPTED) when the quotation sets a percent; a null percent still falls back to the + * {@code deposit_percent} goes to {@link DepositNoticeService#createDraftFromQuotation}, so + * when the quotation sets a percent a sibling pricing request's legacy row on the same ticket + * cannot supply it either (a new-engine sibling never could: R8 allows one ACCEPTED quotation + * per ticket and the lookup prefers ACCEPTED); a null percent still falls back to the * per-ticket lookup. The legacy branch passes no percent, because {@code CustomerQuotationDto} * carries none, so it keeps {@code createDraft}'s per-ticket derivation; the ticket-level * route does the same. The customer must have accepted: an ISSUED-only quotation answers 409.