From 8d068d0a26065e870a30241be7170701ac980507 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 15:45:08 +0700 Subject: [PATCH 01/32] test(sales): pin that a null-percent quotation's deposit notice bills the PDF's 30% A PRICING_REQUEST-origin quotation is created with deposit_percent NULL and can reach ACCEPTED without one. The customer PDF prints NULL as 30% (DealQuotationRenderAdapter), but both deposit routes fall back to 0.50. Red on purpose: the fix direction awaits an owner ruling. Co-Authored-By: Claude Fable 5.1 --- .../DealQuotationOutcomeIntegrationTest.java | 41 +++++++++++++++++++ .../src/api/mockApi.depositNotices.test.js | 10 +++++ 2 files changed, 51 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 1815e4db0..4d6abb472 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 @@ -408,6 +408,47 @@ void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingReque assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); } + // ── A quotation whose deposit percent was never set (NULL) ── + // createFromPricingRequest inserts deposit_percent NULL and nothing before ACCEPTED requires a + // value. The customer PDF prints NULL as 30%: DealQuotationRenderAdapter's remarkLines / + // nonTileRemarkLines (TH) and englishRemarkLines / englishNonTileRemarkLines (EN) all read + // `quotation.depositPercent() != null ? quotation.depositPercent() : 30`. The deposit notice + // must bill what the customer was shown, not DepositNoticeService's 0.50 fallback. + + @Test + void createDepositNoticeFromQuotation_newEngineNullPercent_billsThePdfPercent() { + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + assertDepositPercentIsNullAndThePdfPromises30(accepted); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + + assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); + } + + @Test + void createDraft_ticketRoute_newEngineNullPercent_billsThePdfPercent() { + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + assertDepositPercentIsNullAndThePdfPromises30(accepted); + orderConfirmation.confirmOrder(accepted.pricingRequestId(), + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto draft = depositNoticeService.createDraft(ticketId, + new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); + + assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); + } + + private void assertDepositPercentIsNullAndThePdfPromises30(DealQuotationDto accepted) { + assertThat(jdbc.queryForObject("SELECT deposit_percent IS NULL FROM sales.quotation WHERE quotation_id = :id", + Map.of("id", accepted.id()), Boolean.class)).isTrue(); + assertThat(DealQuotationRenderAdapter.toRenderModel(accepted, null, null).remarkLines()) + .anyMatch(line -> line.contains("มัดจำ 30%")); + } + @Test void recordOutcome_byNonOwningSales_refused_writesNothing() { DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index 9e497e2a7..24c379c60 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -278,6 +278,16 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin expect(depositNotices).toHaveLength(0); }); + // A never-set (null) percent: the customer PDF prints it as 30% (Java DealQuotationRenderAdapter, + // `depositPercent() != null ? depositPercent() : 30`; the mock's placeholder PDF prints no deposit + // line), so the notice must bill 0.3, not the 0.5 fallback. + it('bills the PDF percent (30%) for a new-engine quotation whose deposit percent was never set', async () => { + const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); + expect(accepted.depositPercent).toBeNull(); + const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); + expect(depositNotice.depositPercent).toBe(0.3); + }); + // 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 () => { From 32f4e873a279f084dc5564e63830731fa262eecc Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:23:39 +0700 Subject: [PATCH 02/32] fix(sales): blank deposit percent on a new-engine quotation bills the 30% the PDF prints Owner ruling (Ploy, 2026-10-22, option A): a NULL deposit_percent on a new-engine quotation means 30% everywhere, because DealQuotationRenderAdapter already prints it as 30%. Both deposit-notice routes billed 0.50 for it. - DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT replaces the four repeated 30 literals. - OrderConfirmationService#createDepositNoticeFromQuotation passes the constant for a null new-engine percent instead of handing the null to the per-ticket lookup (which tries a sibling pricing request's legacy row first). - DepositNoticeRepository#findDealQuotationDepositPercent reads a live v2 row with a NULL percent as the constant. The 0.50 fallback stays for a ticket with no live quotation (or only a legacy one with a null percent). - mockApi's pricing-request route mirrors it. Adds a sibling-legacy test that pins the OrderConfirmationService change on its own (without it the null falls through to the legacy sibling's 70%). Co-Authored-By: Claude Fable 5.1 --- .../DealQuotationRenderAdapter.java | 13 +++++++++---- .../hr/deposit/DepositNoticeRepository.java | 15 +++++++++------ .../OrderConfirmationService.java | 7 ++++--- .../DealQuotationOutcomeIntegrationTest.java | 19 +++++++++++++++++++ frontend/src/api/mockApi.js | 13 +++++++++---- 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRenderAdapter.java b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRenderAdapter.java index 52e6338f8..34e069bb0 100644 --- a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRenderAdapter.java +++ b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRenderAdapter.java @@ -32,6 +32,11 @@ private DealQuotationRenderAdapter() {} private static final ZoneId BANGKOK = ZoneId.of("Asia/Bangkok"); + /** The deposit percent the PDF prints when a quotation's {@code deposit_percent} is NULL. The + * deposit notice bills the same default (owner ruling 2026-10-22), so the customer is billed + * what the accepted PDF promised. */ + public static final int DEFAULT_DEPOSIT_PERCENT = 30; + /** Overload for callers with no configured bank block — the English document then prints the * proforma-invoice line instead. Kept so tests and any hand-wired caller need not thread a * config value they do not care about. */ @@ -635,7 +640,7 @@ private static String noReturnLine(int number) { * lead time (dropped entirely, not blanked, when nothing on the document carries one — same * {@link #dropLeadTimeLineAndRenumber} the tile set uses), and the no-exchange-or-return rule. */ private static List nonTileRemarkLines(DealQuotationDto quotation) { - int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : 30; + int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : DEFAULT_DEPOSIT_PERCENT; String remainderText = "CREDIT".equals(quotation.remainderMode()) ? "เครดิต " + (quotation.creditDays() != null ? quotation.creditDays() : 0) + " วัน" : "ขอรับก่อนส่งมอบสินค้าหรือเมื่อส่งมอบสินค้า"; @@ -652,7 +657,7 @@ private static List nonTileRemarkLines(DealQuotationDto quotation) { private static List remarkLines(DealQuotationDto quotation) { LocalDate offerDate = quotation.offerDate() != null ? quotation.offerDate() : LocalDate.now(BANGKOK); - int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : 30; + int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : DEFAULT_DEPOSIT_PERCENT; String remainderText = "CREDIT".equals(quotation.remainderMode()) ? "เครดิต " + (quotation.creditDays() != null ? quotation.creditDays() : 0) + " วัน" : "ขอรับก่อนส่งมอบสินค้าหรือเมื่อส่งมอบสินค้า"; @@ -902,7 +907,7 @@ private static String fullPaymentTermEnglishText(String code) { private static List englishRemarkLines(DealQuotationDto quotation, List bankBlockLines) { LocalDate offerDate = quotation.offerDate() != null ? quotation.offerDate() : LocalDate.now(BANGKOK); - int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : 30; + int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : DEFAULT_DEPOSIT_PERCENT; // Wording-scan fix 7 (2026-09-17): singular "day credit" at exactly 1 day. String remainderText; if ("CREDIT".equals(quotation.remainderMode())) { @@ -1029,7 +1034,7 @@ private static String englishNonTileLeadTimeLine(List item /** The English twin of {@link #nonTileRemarkLines}. */ private static List englishNonTileRemarkLines(DealQuotationDto quotation, List bankBlockLines) { - int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : 30; + int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : DEFAULT_DEPOSIT_PERCENT; // Wording-scan fix 7 (2026-09-17): singular "day credit" at exactly 1 day. String remainderText; if ("CREDIT".equals(quotation.remainderMode())) { 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 7fe0ee83b..1105e2179 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 @@ -18,6 +18,7 @@ import org.springframework.jdbc.support.GeneratedKeyHolder; import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; +import th.co.glr.hr.dealquotation.DealQuotationRenderAdapter; @Repository public class DepositNoticeRepository { @@ -118,10 +119,12 @@ 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. A live v2 row whose {@code deposit_percent} is null + * reads as {@link DealQuotationRenderAdapter#DEFAULT_DEPOSIT_PERCENT}, the percent its PDF + * printed (owner ruling 2026-10-22). Returns empty when there is no live + * (ACCEPTED/ISSUED/APPROVED) quotation for this ticket, or only a legacy one with a null + * {@code deposit_percent} — 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(""" @@ -138,13 +141,13 @@ AND doc_status IN ('ACCEPTED','ISSUED') } Integer v2Pct = firstOrNull(jdbc.query(""" - SELECT deposit_percent FROM sales.quotation + SELECT COALESCE(deposit_percent, :pdfDefault) AS deposit_percent FROM sales.quotation WHERE ticket_id = :ticketId AND origin IN ('PRICING_REQUEST','DEAL_DIRECT') AND doc_status IN ('ACCEPTED','ISSUED','APPROVED') ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 WHEN 'ISSUED' THEN 1 ELSE 2 END, quotation_id DESC LIMIT 1 - """, Map.of("ticketId", ticketId), + """, Map.of("ticketId", ticketId, "pdfDefault", DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT), (rs, i) -> (Integer) rs.getObject("deposit_percent") )); return v2Pct != null ? Optional.of(BigDecimal.valueOf(v2Pct, 2)) : Optional.empty(); 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 5ac3ab64e..6273e8ea8 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,8 +443,8 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * {@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} + * per ticket and the lookup prefers ACCEPTED); a null percent bills the PDF's default 30% + * instead of the 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. */ @@ -486,7 +486,8 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, } reference = newEngine.number(); items = depositNotices.itemsFromDealQuotation(newEngine.id()); - sourceDepositPercent = newEngine.depositPercent(); + sourceDepositPercent = newEngine.depositPercent() != null ? newEngine.depositPercent() + : th.co.glr.hr.dealquotation.DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT; } // request.depositPercent() is deliberately still passed through here even though 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 4d6abb472..b48092b94 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 @@ -449,6 +449,25 @@ private void assertDepositPercentIsNullAndThePdfPromises30(DealQuotationDto acce .anyMatch(line -> line.contains("มัดจำ 30%")); } + /** The pricing-request route must bill THIS quotation's defaulted 30%, not hand the null to the + * per-ticket lookup, which tries a sibling pricing request's legacy row first. */ + @Test + void createDepositNoticeFromQuotation_newEngineNullPercent_whenASiblingPricingRequestHasALegacyIssuedQuotation_billsThePdfPercentNotTheSiblings() { + CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); + 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")); + assertDepositPercentIsNullAndThePdfPromises30(accepted); + long pricingRequestId = accepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + + assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); + } + @Test void recordOutcome_byNonOwningSales_refused_writesNothing() { DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 6e76f3ec5..c65a6acb6 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -7153,6 +7153,10 @@ function resolveDealQuotationV3Header(payload, current = null) { return { priceMode, documentLanguage, currency }; } +// Mirrors DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT: the percent a quotation's PDF +// prints, and its deposit notice bills, when its depositPercent is null. +const MOCK_DEFAULT_DEPOSIT_PERCENT = 30; + /** * Item 4 (V181, "ไม่รับมัดจำ", owner ruling 2026-09-16) — mirrors * DealQuotationService#isZeroDeposit/#resolveFullPaymentTerm exactly: `depositPercent === 0` is @@ -16231,10 +16235,11 @@ export const api = { customerName: ticket.customerName ?? '', customerTaxId: customerTaxId ?? '', customerAddress: customerAddress ?? '', projectName: projectName ?? '', reference, - // 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 + // Mirrors OrderConfirmationService.createDepositNoticeFromQuotation: the new-engine branch + // bills the chosen quotation's own whole-number percent, a null one as the PDF's default; + // the legacy branch keeps the old stand-in. + depositPercent: newEngine + ? (newEngine.depositPercent ?? MOCK_DEFAULT_DEPOSIT_PERCENT) / 100 : payload.depositPercent ?? 0.5, vatPercent: 0.07, notes, items, issuedByName: null, preparerName: 'จินตนา หาญมนตรี', From 6b5a6974defdb2ad5bbbbfa6b75ce9d928b67f80 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:23:39 +0700 Subject: [PATCH 03/32] test(sales): pin that a live legacy quotation bills its PDF's 50%, not a sibling new-engine 30% Revises the legacy null-percent repository case to expect 0.50, adds a mixed-ticket repository case (live legacy null + sibling v2 null -> 0.50), a v2-only null case (-> 0.30, pins the v2 COALESCE at the SQL level), and one end-to-end case where both deposit routes on a legacy-ACCEPTED deal with a null-percent new-engine sibling bill 0.50 from the legacy lines. Co-Authored-By: Claude Opus 5.5 --- .../DealQuotationOutcomeIntegrationTest.java | 36 +++++++++++++++++++ ...positoryDepositPercentIntegrationTest.java | 32 +++++++++++++++-- 2 files changed, 66 insertions(+), 2 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 b48092b94..bbfcd6373 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 @@ -468,6 +468,42 @@ void createDepositNoticeFromQuotation_newEngineNullPercent_whenASiblingPricingRe assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); } + /** The reverse mix: an ACCEPTED legacy quotation's PDF prints a null percent as 50% + * ({@code ticket.QuotationRenderer}), so both routes bill 50% from the legacy row, never a + * sibling pricing request's null-percent new-engine 30%. */ + @Test + void createDepositNotice_legacyAcceptedNullPercent_whenASiblingPricingRequestHasANullPercentNewEngineQuotation_bothRoutesBillTheLegacyPdfsFifty() { + CustomerQuotationDto legacyAccepted = acceptedLegacyQuotation(PricingRequestRecipient.DESIGNER); + assertThat(jdbc.queryForObject("SELECT deposit_percent IS NULL FROM sales.quotation WHERE quotation_id = :id", + Map.of("id", legacyAccepted.id()), Boolean.class)).isTrue(); + DealQuotationDto siblingIssued = issuedNewOriginQuotation(PricingRequestRecipient.OWNER, new BigDecimal("10")); + assertDepositPercentIsNullAndThePdfPromises30(siblingIssued); + long pricingRequestId = legacyAccepted.pricingRequestId(); + orderConfirmation.confirmOrder(pricingRequestId, + new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); + + DepositNoticeDto viaPricingRequest = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + DepositNoticeDto viaTicket = depositNoticeService.createDraft(ticketId, + new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); + + List legacyLines = DepositNoticeService.itemsFromQuotation(legacyAccepted.items()).stream() + .map(it -> line(it.qty(), it.netUnitPrice())).toList(); + List siblingLines = jdbc.query(""" + SELECT qty, final_unit_price FROM sales.quotation_item WHERE quotation_id = :id ORDER BY seq + """, Map.of("id", siblingIssued.id()), + (rs, i) -> line(rs.getBigDecimal("qty"), rs.getBigDecimal("final_unit_price"))); + // The fixture must make the two sources distinguishable, or the item assertions prove nothing. + assertThat(legacyLines).isNotEmpty().isNotEqualTo(siblingLines); + + assertThat(viaPricingRequest.reference()).isEqualTo(legacyAccepted.number()); + for (DepositNoticeDto draft : List.of(viaPricingRequest, viaTicket)) { + assertThat(draft.depositPercent()).isEqualByComparingTo("0.50"); + assertThat(draft.items().stream().map(it -> line(it.qty(), it.netUnitPrice())).toList()) + .isEqualTo(legacyLines); + } + } + @Test void recordOutcome_byNonOwningSales_refused_writesNothing() { DealQuotationDto issued = issuedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java index 569cfddaa..10e44b251 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java @@ -117,18 +117,46 @@ void legacyChain_ignoresNonLiveStatusesLikeDraftOrRejected() { assertThat(result).isEmpty(); } + /** The legacy PDF prints a null percent as 50% ({@code ticket.QuotationRenderer}). */ @Test - void legacyChain_nullDepositPercentOnTheLiveQuotationYieldsEmpty() { + void legacyChain_nullDepositPercentOnTheLiveQuotationYieldsTheFiftyItsPdfPrints() { long ticketId = newTicketWithPricingRequestDraft(); insertLegacyQuotation(ticketId, "ACCEPTED", null); Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - assertThat(result).isEmpty(); + assertThat(result).isPresent(); + assertThat(result.get()).isEqualByComparingTo("0.50"); + } + + /** A live legacy row wins outright, so a sibling pricing request's v2 row (whose null reads as + * 30) is never consulted. */ + @Test + void mixedTicket_liveLegacyNullPercent_winsOverASiblingV2NullPercent() { + long ticketId = newTicketWithPricingRequestDraft(); + insertLegacyQuotation(ticketId, "ACCEPTED", null); + insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ISSUED", null); + + Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); + + assertThat(result).isPresent(); + assertThat(result.get()).isEqualByComparingTo("0.50"); } // ── V2 deal-quotation chain (origin IN ('PRICING_REQUEST','DEAL_DIRECT')) ───────────────── + /** The new-engine PDF prints a null percent as 30% ({@code DealQuotationRenderAdapter}). */ + @Test + void pricingRequestOrigin_nullDepositPercentOnTheLiveQuotationYieldsTheThirtyItsPdfPrints() { + long ticketId = newBareTicket(); + insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ACCEPTED", null); + + Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); + + assertThat(result).isPresent(); + assertThat(result.get()).isEqualByComparingTo("0.30"); + } + @Test void dealDirectOrigin_readsApprovedQuotationsDepositPercent() { long ticketId = newBareTicket(); From a33a80eac5be32e8de1a28b037ce72969be3e480 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:42:03 +0700 Subject: [PATCH 04/32] fix(sales): a live legacy quotation bills the 50% its PDF prints, never a sibling new-engine percent findDealQuotationDepositPercent tried the live legacy row first, but legacy rows never carry deposit_percent, so it fell through to the v2 query, which now reads a null as 30. A legacy ACCEPTED quotation (PDF prints 50% via ticket.QuotationRenderer) with a null-percent new-engine sibling therefore billed 30% on both deposit routes. COALESCE the legacy branch to 50, so a live legacy row always wins. Owner ruling (Ploy, 2026-10-22): bill what the PDF the customer received printed, per engine. Side effect, intended: a legacy null + v2 explicit sibling now bills 50 instead of the v2 percent. Co-Authored-By: Claude Opus 5.5 --- .../hr/deposit/DepositNoticeRepository.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 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 1105e2179..b9ea74ad3 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 @@ -119,16 +119,20 @@ 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. A live v2 row whose {@code deposit_percent} is null - * reads as {@link DealQuotationRenderAdapter#DEFAULT_DEPOSIT_PERCENT}, the percent its PDF - * printed (owner ruling 2026-10-22). Returns empty when there is no live - * (ACCEPTED/ISSUED/APPROVED) quotation for this ticket, or only a legacy one with a null - * {@code deposit_percent} — 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. A null {@code deposit_percent} reads as the percent that + * engine's PDF printed (owner ruling 2026-10-22): 50 on a legacy row ({@code + * ticket.QuotationRenderer#buildLegacyModel}), {@link + * DealQuotationRenderAdapter#DEFAULT_DEPOSIT_PERCENT} (30) on a v2 row. So a live legacy row + * always wins, and the v2 branch is reached only when the ticket has no live legacy quotation. + * Returns empty only when there is no live (ACCEPTED/ISSUED/APPROVED) quotation for this + * ticket — the caller ({@link DepositNoticeService#createDraft}) falls back to 0.50 then, + * this document's long-standing default. */ public Optional findDealQuotationDepositPercent(long ticketId) { + // 50 = the legacy PDF's own default: ticket.QuotationRenderer#buildLegacyModel prints + // `quotation.depositPercent() != null ? quotation.depositPercent() : 50`. Integer legacyPct = firstOrNull(jdbc.query(""" - SELECT deposit_percent FROM sales.quotation + SELECT COALESCE(deposit_percent, 50) AS deposit_percent FROM sales.quotation WHERE ticket_id = :ticketId AND pricing_request_id IS NOT NULL AND origin IS NULL AND doc_status IN ('ACCEPTED','ISSUED') ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 ELSE 1 END, quotation_id DESC From c8807ee19d27e3d7f057f67a0a2476439d899d57 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Thu, 22 Oct 2026 22:05:22 +0700 Subject: [PATCH 05/32] docs(sales): firstOrNull Javadoc no longer describes a null the queries cannot return no test: comment-only. Co-Authored-By: Claude Fable 5.1 --- .../th/co/glr/hr/deposit/DepositNoticeRepository.java | 8 +++----- 1 file changed, 3 insertions(+), 5 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 b9ea74ad3..0a647552b 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 @@ -157,11 +157,9 @@ AND doc_status IN ('ACCEPTED','ISSUED','APPROVED') return v2Pct != null ? Optional.of(BigDecimal.valueOf(v2Pct, 2)) : Optional.empty(); } - /** {@code list.stream().findFirst()} throws NPE the moment the single mapped row's own value - * is {@code null} ({@code Stream.findFirst}/{@code Optional.of} both reject a null element - * outright) — exactly the case a LIVE quotation with a not-yet-set {@code deposit_percent} - * hits. A plain index avoids that without changing what "no row" vs "row, null value" mean to - * the caller (both still read as "nothing usable here"). */ + /** First mapped row or {@code null} when the query matched nothing. Both queries above now + * COALESCE the percent, so a mapped value is never null; this stays index-based rather than + * {@code stream().findFirst()} so a future null column cannot turn into an NPE. */ private static T firstOrNull(List rows) { return rows.isEmpty() ? null : rows.get(0); } From 0762fd2ba11c26187c2b8d7311499d86adc6a70b Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:31:10 +0700 Subject: [PATCH 06/32] test(sales): fold the null-percent deposit cases into the existing route tests and pin the quotation's own percent - createsTheSameDraftTheTicketRouteWould: guard the NULL-percent fixture and pin both routes at 0.30, replacing the two single-route NullPercent tests on the same fixture. - Sibling-legacy test is now parameterized over (quotation percent -> billed): null -> 0.30, 50 -> 0.50. The old explicit 30 equalled the default, so nothing pinned that the pricing-request route bills the quotation's OWN percent; the null-percent copy is deleted. - Legacy-accepted test: the sibling new-engine quotation is at 40%, not NULL. With NULL/NULL it passed on develop (both routes fell back to 0.50); the repository-level mixedTicket_liveLegacyNullPercent_winsOverASiblingV2NullPercent keeps that case. - Drop assertDepositPercentIsNullAndThePdfPromises30; the renderer's 30 is pinned by DealQuotationRenderAdapterV3Test#thaiDocument_note2_nullDepositPercent_stillDefaultsToThirtyPercent_notTheZeroBranch. - Mock: fold the null-percent case into the two-route case; the own-percent case uses 50, since 30 equals MOCK_DEFAULT_DEPOSIT_PERCENT. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- .../DealQuotationOutcomeIntegrationTest.java | 88 ++++--------------- .../src/api/mockApi.depositNotices.test.js | 21 ++--- 2 files changed, 25 insertions(+), 84 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 bbfcd6373..fcd22d10d 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 @@ -11,6 +11,8 @@ import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.springframework.http.HttpStatus; import th.co.glr.hr.attachment.FileStorageService; import th.co.glr.hr.auth.EmployeeAuthRepository; @@ -251,6 +253,8 @@ void recordOutcome_byOwningSales_accepted_setsPrStatusAndDocStatus() { @Test void createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameDraftTheTicketRouteWould() { DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); + // The PDF prints a NULL percent as DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT (30). + assertThat(accepted.depositPercent()).isNull(); long pricingRequestId = accepted.pricingRequestId(); orderConfirmation.confirmOrder(pricingRequestId, new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); @@ -265,6 +269,7 @@ void createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameD // 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("0.30"); assertThat(viaPricingRequest.depositPercent()).isEqualByComparingTo(viaTicket.depositPercent()); assertThat(viaPricingRequest.items()) .usingRecursiveComparison().ignoringFields("id") @@ -377,14 +382,18 @@ 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 or the percent of this pricing request's - * draft. */ - @Test - void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent() { + * draft. The draft bills this quotation's own percent, a NULL one as the 30% its PDF prints. */ + @ParameterizedTest(name = "quotation percent {0} bills {1}") + @CsvSource({", 0.30", "50, 0.50"}) + void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent( + Integer quotationPercent, BigDecimal expectedDraftPercent) { CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); // No code path writes deposit_percent on a legacy row; this stands in for one that carries it. 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); + DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10"), + quotationPercent); + assertThat(accepted.depositPercent()).isEqualTo(quotationPercent); long pricingRequestId = accepted.pricingRequestId(); orderConfirmation.confirmOrder(pricingRequestId, new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); @@ -404,80 +413,19 @@ 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"); - } - - // ── A quotation whose deposit percent was never set (NULL) ── - // createFromPricingRequest inserts deposit_percent NULL and nothing before ACCEPTED requires a - // value. The customer PDF prints NULL as 30%: DealQuotationRenderAdapter's remarkLines / - // nonTileRemarkLines (TH) and englishRemarkLines / englishNonTileRemarkLines (EN) all read - // `quotation.depositPercent() != null ? quotation.depositPercent() : 30`. The deposit notice - // must bill what the customer was shown, not DepositNoticeService's 0.50 fallback. - - @Test - void createDepositNoticeFromQuotation_newEngineNullPercent_billsThePdfPercent() { - DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); - assertDepositPercentIsNullAndThePdfPromises30(accepted); - long pricingRequestId = accepted.pricingRequestId(); - orderConfirmation.confirmOrder(pricingRequestId, - new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); - - DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, - new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); - - assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); - } - - @Test - void createDraft_ticketRoute_newEngineNullPercent_billsThePdfPercent() { - DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10")); - assertDepositPercentIsNullAndThePdfPromises30(accepted); - orderConfirmation.confirmOrder(accepted.pricingRequestId(), - new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); - - DepositNoticeDto draft = depositNoticeService.createDraft(ticketId, - new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); - - assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); - } - - private void assertDepositPercentIsNullAndThePdfPromises30(DealQuotationDto accepted) { - assertThat(jdbc.queryForObject("SELECT deposit_percent IS NULL FROM sales.quotation WHERE quotation_id = :id", - Map.of("id", accepted.id()), Boolean.class)).isTrue(); - assertThat(DealQuotationRenderAdapter.toRenderModel(accepted, null, null).remarkLines()) - .anyMatch(line -> line.contains("มัดจำ 30%")); - } - - /** The pricing-request route must bill THIS quotation's defaulted 30%, not hand the null to the - * per-ticket lookup, which tries a sibling pricing request's legacy row first. */ - @Test - void createDepositNoticeFromQuotation_newEngineNullPercent_whenASiblingPricingRequestHasALegacyIssuedQuotation_billsThePdfPercentNotTheSiblings() { - CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); - 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")); - assertDepositPercentIsNullAndThePdfPromises30(accepted); - long pricingRequestId = accepted.pricingRequestId(); - orderConfirmation.confirmOrder(pricingRequestId, - new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); - - DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, - new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); - - assertThat(draft.depositPercent()).isEqualByComparingTo("0.30"); + assertThat(draft.depositPercent()).isEqualByComparingTo(expectedDraftPercent); } /** The reverse mix: an ACCEPTED legacy quotation's PDF prints a null percent as 50% * ({@code ticket.QuotationRenderer}), so both routes bill 50% from the legacy row, never a - * sibling pricing request's null-percent new-engine 30%. */ + * sibling pricing request's new-engine percent. */ @Test - void createDepositNotice_legacyAcceptedNullPercent_whenASiblingPricingRequestHasANullPercentNewEngineQuotation_bothRoutesBillTheLegacyPdfsFifty() { + void createDepositNotice_legacyAcceptedNullPercent_whenASiblingPricingRequestHasAFortyPercentNewEngineQuotation_bothRoutesBillTheLegacyPdfsFifty() { CustomerQuotationDto legacyAccepted = acceptedLegacyQuotation(PricingRequestRecipient.DESIGNER); assertThat(jdbc.queryForObject("SELECT deposit_percent IS NULL FROM sales.quotation WHERE quotation_id = :id", Map.of("id", legacyAccepted.id()), Boolean.class)).isTrue(); - DealQuotationDto siblingIssued = issuedNewOriginQuotation(PricingRequestRecipient.OWNER, new BigDecimal("10")); - assertDepositPercentIsNullAndThePdfPromises30(siblingIssued); + DealQuotationDto siblingIssued = issuedNewOriginQuotation(PricingRequestRecipient.OWNER, new BigDecimal("10"), 40); + assertThat(siblingIssued.depositPercent()).isEqualTo(40); long pricingRequestId = legacyAccepted.pricingRequestId(); orderConfirmation.confirmOrder(pricingRequestId, new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index 24c379c60..cc9553d43 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -222,12 +222,12 @@ describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notic // 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 }); + const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation({ depositPercent: 50 }); 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(accepted.depositPercent).toBe(50); + expect(depositNotice.depositPercent).toBe(0.5); expect(depositNotice.items.length).toBeGreaterThan(0); // qty is deliberately not distinguishable here: a mock TILE row sets quantity = piecesFinal. const [line] = accepted.items; @@ -244,9 +244,12 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin // 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 { ticketId, prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); + // The PDF prints a NULL percent as DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT (30). + expect(accepted.depositPercent).toBeNull(); const { depositNotice: fromTicket } = await api.depositNotices.createDraft(ticketId, {}); const { depositNotice: fromPr } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); + expect(fromPr.depositPercent).toBe(0.3); expect(fromTicket.items.length).toBeGreaterThan(0); const pick = (items) => items.map(({ qty, netUnitPrice }) => ({ qty, netUnitPrice })); expect(pick(fromTicket.items)).toEqual(pick(fromPr.items)); @@ -278,16 +281,6 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin expect(depositNotices).toHaveLength(0); }); - // A never-set (null) percent: the customer PDF prints it as 30% (Java DealQuotationRenderAdapter, - // `depositPercent() != null ? depositPercent() : 30`; the mock's placeholder PDF prints no deposit - // line), so the notice must bill 0.3, not the 0.5 fallback. - it('bills the PDF percent (30%) for a new-engine quotation whose deposit percent was never set', async () => { - const { prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); - expect(accepted.depositPercent).toBeNull(); - const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); - expect(depositNotice.depositPercent).toBe(0.3); - }); - // 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 () => { From 16ca96f73fc648d33b65363d413d81e3bda195af Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:05:16 +0700 Subject: [PATCH 07/32] test(sales): pin that the ticket-route deposit notice follows the latest answered quotation, lines and percent from one row (RED) Owner rulings 2026-10-23: the deposit follows the latest quotation the customer answered across both engines (ACCEPTED, or a DEAL_DIRECT quotation's APPROVED), else the newest ISSUED one; lines, percent and reference come from that one row; a legacy row bills its PDF's constant 50 and its stored column is never read. Revises the existing mixed-ticket, repository and service tests rather than adding parallel ones. DepositNoticeRepository#findGoverningQuotation is a compile-only scaffold here that reproduces today's legacy-first pick, so every new assertion fails on behaviour, not on compilation. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- .../hr/deposit/DepositNoticeRepository.java | 25 ++++ .../DealQuotationOutcomeIntegrationTest.java | 43 +++--- ...eDealQuotationAutofillIntegrationTest.java | 33 +++-- ...positoryDepositPercentIntegrationTest.java | 135 +++++++++--------- .../hr/deposit/DepositNoticeServiceTest.java | 93 +++++------- .../src/api/mockApi.depositNotices.test.js | 50 ++++++- 6 files changed, 216 insertions(+), 163 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 0a647552b..59dfd3255 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 @@ -157,6 +157,31 @@ AND doc_status IN ('ACCEPTED','ISSUED','APPROVED') return v2Pct != null ? Optional.of(BigDecimal.valueOf(v2Pct, 2)) : Optional.empty(); } + /** The quotation a ticket-level deposit notice bills; {@code depositPercent} is a fraction (0.30). */ + public record GoverningQuotation(long quotationId, boolean legacy, String number, BigDecimal depositPercent) {} + + /** RED-commit compile scaffold: today's two-step pick (legacy first), unchanged behaviour. */ + public Optional findGoverningQuotation(long ticketId) { + List legacy = jdbc.query(""" + SELECT quotation_id, number, COALESCE(deposit_percent, 50) AS pct FROM sales.quotation + WHERE ticket_id = :ticketId AND pricing_request_id IS NOT NULL AND origin IS NULL + AND doc_status IN ('ACCEPTED','ISSUED') + ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 ELSE 1 END, quotation_id DESC + LIMIT 1 + """, Map.of("ticketId", ticketId), (rs, i) -> new GoverningQuotation( + rs.getLong("quotation_id"), true, rs.getString("number"), BigDecimal.valueOf(rs.getInt("pct"), 2))); + if (!legacy.isEmpty()) return Optional.of(legacy.get(0)); + return jdbc.query(""" + SELECT quotation_id, number, COALESCE(deposit_percent, :pdfDefault) AS pct FROM sales.quotation + WHERE ticket_id = :ticketId AND origin IN ('PRICING_REQUEST','DEAL_DIRECT') + AND doc_status IN ('ACCEPTED','ISSUED','APPROVED') + ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 WHEN 'ISSUED' THEN 1 ELSE 2 END, quotation_id DESC + LIMIT 1 + """, Map.of("ticketId", ticketId, "pdfDefault", DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT), + (rs, i) -> new GoverningQuotation(rs.getLong("quotation_id"), false, rs.getString("number"), + BigDecimal.valueOf(rs.getInt("pct"), 2))).stream().findFirst(); + } + /** First mapped row or {@code null} when the query matched nothing. Both queries above now * COALESCE the percent, so a mapped value is never null; this stays index-based rather than * {@code stream().findFirst()} so a future null column cannot turn into an NPE. */ 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 fcd22d10d..d086eb5d0 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 @@ -71,7 +71,6 @@ import th.co.glr.hr.pricingdecision.PricingDecisionRequests.UpdatePricingDecisionItemRequest; import th.co.glr.hr.pricingdecision.PricingDecisionRequests.UpdatePricingDecisionRequest; import th.co.glr.hr.pricingdecision.PricingDecisionService; -import th.co.glr.hr.pricingrequest.PricingRequestDtos.PricingRequestSummaryDto; import th.co.glr.hr.pricingrequest.PricingRequestRecipient; import th.co.glr.hr.pricingrequest.PricingRequestRepository; import th.co.glr.hr.pricingrequest.PricingRequestRequests; @@ -262,14 +261,15 @@ void createDepositNoticeFromQuotation_newEngineAcceptedQuotation_createsTheSameD 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). + // Reference, items and percent are exactly what the ticket-level route produces for this deal. DepositNoticeDto viaTicket = depositNoticeService.createDraft(ticketId, new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); + assertThat(viaTicket.reference()).isEqualTo(viaPricingRequest.reference()); assertThat(viaPricingRequest.depositPercent()).isEqualByComparingTo("0.30"); + assertThat(viaTicket.depositPercent()).isEqualByComparingTo("0.30"); assertThat(viaPricingRequest.depositPercent()).isEqualByComparingTo(viaTicket.depositPercent()); assertThat(viaPricingRequest.items()) .usingRecursiveComparison().ignoringFields("id") @@ -381,11 +381,13 @@ 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 or the percent of this pricing request's - * draft. The draft bills this quotation's own percent, a NULL one as the 30% its PDF prints. */ + * older legacy ISSUED quotation must not supply the lines or the percent of the draft, on + * either route. The customer answered this quotation and not the sibling, so it governs the + * ticket route too (owner ruling 2026-10-23). The draft bills this quotation's own percent, a + * NULL one as the 30% its PDF prints. */ @ParameterizedTest(name = "quotation percent {0} bills {1}") - @CsvSource({", 0.30", "50, 0.50"}) - void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingRequestHasALegacyIssuedQuotation_draftsThisQuotationsLinesAndPercent( + @CsvSource({", 0.30", "20, 0.20"}) + void createDepositNotice_newEngineAccepted_whenASiblingPricingRequestHasAnOlderLegacyIssuedQuotation_bothRoutesDraftThisQuotationsLinesAndPercent( Integer quotationPercent, BigDecimal expectedDraftPercent) { CustomerQuotationDto legacyIssued = issuedLegacyQuotation(PricingRequestRecipient.OWNER); // No code path writes deposit_percent on a legacy row; this stands in for one that carries it. @@ -394,12 +396,15 @@ void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingReque DealQuotationDto accepted = acceptedNewOriginQuotation(PricingRequestRecipient.DESIGNER, new BigDecimal("10"), quotationPercent); assertThat(accepted.depositPercent()).isEqualTo(quotationPercent); + assertThat(accepted.id()).isGreaterThan(legacyIssued.id()); long pricingRequestId = accepted.pricingRequestId(); orderConfirmation.confirmOrder(pricingRequestId, new OrderConfirmationRequests.ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); - DepositNoticeDto draft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, + DepositNoticeDto viaPricingRequest = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, new OrderConfirmationRequests.CreateDepositNoticeFromQuotationRequest(null), salesActor); + DepositNoticeDto viaTicket = depositNoticeService.createDraft(ticketId, + new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); List thisQuotationsLines = jdbc.query(""" SELECT qty, final_unit_price FROM sales.quotation_item WHERE quotation_id = :id ORDER BY seq @@ -410,17 +415,20 @@ void createDepositNoticeFromQuotation_newEngineAccepted_whenASiblingPricingReque // 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); - assertThat(draft.depositPercent()).isEqualByComparingTo(expectedDraftPercent); + for (DepositNoticeDto draft : List.of(viaPricingRequest, viaTicket)) { + assertThat(draft.items().stream().map(it -> line(it.qty(), it.netUnitPrice())).toList()) + .isEqualTo(thisQuotationsLines); + assertThat(draft.depositPercent()).isEqualByComparingTo(expectedDraftPercent); + assertThat(draft.reference()).isEqualTo(accepted.number()); + } } - /** The reverse mix: an ACCEPTED legacy quotation's PDF prints a null percent as 50% - * ({@code ticket.QuotationRenderer}), so both routes bill 50% from the legacy row, never a - * sibling pricing request's new-engine percent. */ + /** The reverse mix: the customer answered the legacy quotation, and the sibling pricing + * request's newer new-engine quotation is only ISSUED. The answered legacy row governs both + * routes, and bills the 50% its PDF prints ({@code ticket.QuotationRenderer}), never the + * sibling's 40%. It wins because it was answered, not because it is legacy. */ @Test - void createDepositNotice_legacyAcceptedNullPercent_whenASiblingPricingRequestHasAFortyPercentNewEngineQuotation_bothRoutesBillTheLegacyPdfsFifty() { + void createDepositNotice_legacyAcceptedNullPercent_whenASiblingPricingRequestHasANewerFortyPercentIssuedQuotation_bothRoutesBillTheAnsweredLegacyQuotationsFifty() { CustomerQuotationDto legacyAccepted = acceptedLegacyQuotation(PricingRequestRecipient.DESIGNER); assertThat(jdbc.queryForObject("SELECT deposit_percent IS NULL FROM sales.quotation WHERE quotation_id = :id", Map.of("id", legacyAccepted.id()), Boolean.class)).isTrue(); @@ -444,11 +452,12 @@ void createDepositNotice_legacyAcceptedNullPercent_whenASiblingPricingRequestHas // The fixture must make the two sources distinguishable, or the item assertions prove nothing. assertThat(legacyLines).isNotEmpty().isNotEqualTo(siblingLines); - assertThat(viaPricingRequest.reference()).isEqualTo(legacyAccepted.number()); + assertThat(siblingIssued.id()).isGreaterThan(legacyAccepted.id()); for (DepositNoticeDto draft : List.of(viaPricingRequest, viaTicket)) { assertThat(draft.depositPercent()).isEqualByComparingTo("0.50"); assertThat(draft.items().stream().map(it -> line(it.qty(), it.netUnitPrice())).toList()) .isEqualTo(legacyLines); + assertThat(draft.reference()).isEqualTo(legacyAccepted.number()); } } diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeDealQuotationAutofillIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeDealQuotationAutofillIntegrationTest.java index f9bc3dcb8..d7494fa77 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeDealQuotationAutofillIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeDealQuotationAutofillIntegrationTest.java @@ -11,14 +11,14 @@ import th.co.glr.hr.support.AbstractPostgresIntegrationTest; /** - * Real-DB proof for {@link DepositNoticeRepository#findDealQuotationItemsForDeposit(long)} (the - * 2026-09-29 fix) — the V2 deal-quotation item source {@code DepositNoticeService - * #buildItemsFromRequest} falls back to once both the legacy {@code ticket_item.approved_price} - * path and the {@code CustomerQuotationRepository}-scoped {@code pickQuotation} path yield - * nothing. Mockito cannot exercise this: the method's whole job is the SQL — the status/origin - * filter and the {@code ORDER BY} preference — so this needs the real query against real - * Postgres, not a mocked repository. See that method's own Javadoc for the exact selection rule - * this test pins. + * Real-DB proof for the V2 deal-quotation item source of a ticket-level deposit notice (the + * 2026-09-29 fix): {@link DepositNoticeRepository#findGoverningQuotation} picks the quotation and + * {@link DepositNoticeRepository#findDealQuotationItems} reads its lines, the pair {@code + * DepositNoticeService#buildItemsFromRequest} uses once the legacy {@code + * ticket_item.approved_price} path yields nothing. Mockito cannot exercise this: the job is the + * SQL (the status/origin filter, the {@code ORDER BY} preference, the column mapping), so this + * needs the real queries against real Postgres. The mixed legacy/v2 ranking is pinned in {@code + * DepositNoticeRepositoryDepositPercentIntegrationTest}. */ class DepositNoticeDealQuotationAutofillIntegrationTest extends AbstractPostgresIntegrationTest { @@ -44,7 +44,7 @@ void pricingRequestIssuedQuotation_returnsItemsInSeqOrderWithDiscountLabels() { insertQuotationItem(quotationId, 1, "กระเบื้อง A", new BigDecimal("10"), "แผ่น", new BigDecimal("4365.29"), new BigDecimal("873.06"), new BigDecimal("3492.23")); - List items = repo.findDealQuotationItemsForDeposit(ticketId); + List items = itemsForDeposit(ticketId); assertThat(items).hasSize(2); assertThat(items.get(0).seq()).isEqualTo(1); @@ -72,7 +72,7 @@ void dealDirectApprovedQuotation_isReturned_theCoreRegressionThisFixCloses() { insertQuotationItem(quotationId, 1, "DEAL_DIRECT item", new BigDecimal("2"), "แผ่น", new BigDecimal("1000.00"), BigDecimal.ZERO, new BigDecimal("1000.00")); - List items = repo.findDealQuotationItemsForDeposit(ticketId); + List items = itemsForDeposit(ticketId); assertThat(items).hasSize(1); assertThat(items.get(0).description()).isEqualTo("DEAL_DIRECT item"); @@ -92,7 +92,7 @@ void acceptedPricingRequestQuotation_winsOverAnIssuedOne_onTheSameTicket() { insertQuotationItem(acceptedQuotationId, 1, "Accepted item (must win)", BigDecimal.ONE, "แผ่น", new BigDecimal("222.00"), BigDecimal.ZERO, new BigDecimal("222.00")); - List items = repo.findDealQuotationItemsForDeposit(ticketId); + List items = itemsForDeposit(ticketId); assertThat(items).hasSize(1); assertThat(items.get(0).description()).isEqualTo("Accepted item (must win)"); @@ -124,13 +124,20 @@ void onlyNonLiveStatusQuotations_returnsEmptyList() { insertQuotationItem(pendingApprovalId, 1, "Pending-approval item", BigDecimal.ONE, "แผ่น", BigDecimal.TEN, BigDecimal.ZERO, BigDecimal.TEN); - List items = repo.findDealQuotationItemsForDeposit(ticketId); + List items = itemsForDeposit(ticketId); assertThat(items).isEmpty(); } // ── helpers ────────────────────────────────────────────────────────────────────────────── + /** The new-engine branch of {@code DepositNoticeService#buildItemsFromRequest}. */ + private List itemsForDeposit(long ticketId) { + return repo.findGoverningQuotation(ticketId) + .map(governing -> repo.findDealQuotationItems(governing.quotationId())) + .orElse(List.of()); + } + private long insertEmployee(String code) { return jdbc.queryForObject( "INSERT INTO hr.employee (employee_code, first_name_th, last_name_th) " @@ -171,7 +178,7 @@ private long insertQuotation(long ticketId, String origin, String docStatus) { private void insertQuotationItem(long quotationId, int seq, String description, BigDecimal qty, String rawUnit, BigDecimal unitPrice, BigDecimal salesDiscount, BigDecimal finalUnitPrice) { - // amount is legacy NOT NULL (V49) — irrelevant to findDealQuotationItemsForDeposit, which + // amount is legacy NOT NULL (V49) — irrelevant to findDealQuotationItems, which // reads unit_price/final_unit_price instead, so a harmless qty*unitPrice placeholder. BigDecimal amount = qty.multiply(unitPrice); jdbc.update(""" diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java index 10e44b251..fd0b774f1 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeRepositoryDepositPercentIntegrationTest.java @@ -5,7 +5,6 @@ import java.math.BigDecimal; import java.time.LocalDate; import java.util.List; -import java.util.Optional; import java.util.UUID; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -35,13 +34,11 @@ import th.co.glr.hr.dealquotation.WastageCalculator; /** - * Real-Postgres coverage for {@link DepositNoticeRepository#findDealQuotationDepositPercent} — - * owner ruling (2026-09-29): the deposit-notice PERCENT is LOCKED to the source quotation's own - * {@code sales.quotation.deposit_percent}, never editable at the deposit-notice stage. A Mockito - * unit test on {@code DepositNoticeServiceTest} already proves the SERVICE calls this repository - * method and trusts its result over a caller-supplied value — it cannot prove the method's own - * SQL actually reads the right row out of {@code sales.quotation}. This class drives that SQL - * directly. + * Real-Postgres coverage for {@link DepositNoticeRepository#findGoverningQuotation}, the one pick + * of the quotation a ticket-level deposit notice bills: its lines and its percent (owner rulings + * 2026-09-29 and 2026-10-23). {@code DepositNoticeServiceTest} proves the SERVICE takes both from + * the row this returns; it cannot prove the SQL picks the right row out of {@code sales.quotation}. + * This class drives that SQL directly. * *

Fixture note: rows are written straight over JDBC rather than through {@code * CustomerQuotationService}/{@code DealQuotationService} — neither ever writes {@code @@ -82,28 +79,24 @@ void setUp() { } // ── Legacy chain (origin IS NULL, pricing_request_id IS NOT NULL): ACCEPTED > ISSUED ────── + // The legacy PDF prints 50% whatever the row stores (ticket.QuotationRenderer), so every live + // legacy row bills 0.50; the fixtures store other percents to prove the column is never read. @Test - void legacyChain_prefersAcceptedQuotationsDepositPercentOverAnIssuedOne() { + void legacyChain_anAcceptedQuotationBeatsANewerIssuedOne_andBillsItsPdfsFifty() { long ticketId = newTicketWithPricingRequestDraft(); + long accepted = insertLegacyQuotation(ticketId, "ACCEPTED", (short) 30); insertLegacyQuotation(ticketId, "ISSUED", (short) 20); - insertLegacyQuotation(ticketId, "ACCEPTED", (short) 30); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.30"); + assertGoverning(ticketId, accepted, true, "0.50"); } @Test - void legacyChain_fallsBackToIssuedWhenNoneAccepted() { + void legacyChain_fallsBackToIssuedWhenNoneAccepted_andBillsItsPdfsFifty() { long ticketId = newTicketWithPricingRequestDraft(); - insertLegacyQuotation(ticketId, "ISSUED", (short) 20); + long issued = insertLegacyQuotation(ticketId, "ISSUED", (short) 20); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.20"); + assertGoverning(ticketId, issued, true, "0.50"); } @Test @@ -112,73 +105,69 @@ void legacyChain_ignoresNonLiveStatusesLikeDraftOrRejected() { insertLegacyQuotation(ticketId, "DRAFT", (short) 40); insertLegacyQuotation(ticketId, "REJECTED", (short) 50); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isEmpty(); + assertThat(depositNotices.findGoverningQuotation(ticketId)).isEmpty(); } - /** The legacy PDF prints a null percent as 50% ({@code ticket.QuotationRenderer}). */ @Test void legacyChain_nullDepositPercentOnTheLiveQuotationYieldsTheFiftyItsPdfPrints() { long ticketId = newTicketWithPricingRequestDraft(); - insertLegacyQuotation(ticketId, "ACCEPTED", null); + long accepted = insertLegacyQuotation(ticketId, "ACCEPTED", null); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.50"); + assertGoverning(ticketId, accepted, true, "0.50"); } - /** A live legacy row wins outright, so a sibling pricing request's v2 row (whose null reads as - * 30) is never consulted. */ + // ── Mixed ticket: the latest quotation the customer ANSWERED governs, whichever engine ───── + // (owner ruling 2026-10-23). Answered = ACCEPTED, or a DEAL_DIRECT quotation's terminal + // APPROVED. An unanswered ISSUED row governs only when nothing was answered. + + /** The legacy row wins because the customer answered it, not because it is legacy: the + * sibling v2 row is newer. */ @Test - void mixedTicket_liveLegacyNullPercent_winsOverASiblingV2NullPercent() { + void mixedTicket_anAnsweredLegacyQuotationBeatsANewerUnansweredV2One() { long ticketId = newTicketWithPricingRequestDraft(); - insertLegacyQuotation(ticketId, "ACCEPTED", null); + long legacyAccepted = insertLegacyQuotation(ticketId, "ACCEPTED", null); insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ISSUED", null); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.50"); + assertGoverning(ticketId, legacyAccepted, true, "0.50"); } - // ── V2 deal-quotation chain (origin IN ('PRICING_REQUEST','DEAL_DIRECT')) ───────────────── - - /** The new-engine PDF prints a null percent as 30% ({@code DealQuotationRenderAdapter}). */ @Test - void pricingRequestOrigin_nullDepositPercentOnTheLiveQuotationYieldsTheThirtyItsPdfPrints() { - long ticketId = newBareTicket(); - insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ACCEPTED", null); + void mixedTicket_withNothingAnswered_theNewestIssuedQuotationGoverns() { + long ticketId = newTicketWithPricingRequestDraft(); + insertLegacyQuotation(ticketId, "ISSUED", null); + long v2Issued = insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ISSUED", (short) 40); + + assertGoverning(ticketId, v2Issued, false, "0.40"); + } - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); + @Test + void mixedTicket_aDealDirectApprovedQuotationIsAnAnswer_andBeatsALegacyIssuedOne() { + long ticketId = newTicketWithPricingRequestDraft(); + insertLegacyQuotation(ticketId, "ISSUED", null); + long dealDirect = insertOriginTaggedQuotation(ticketId, "DEAL_DIRECT", "APPROVED", (short) 40); - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.30"); + assertGoverning(ticketId, dealDirect, false, "0.40"); } + // ── V2 deal-quotation chain (origin IN ('PRICING_REQUEST','DEAL_DIRECT')) ───────────────── + + /** The new-engine PDF prints a null percent as 30% ({@code DealQuotationRenderAdapter}). */ @Test - void dealDirectOrigin_readsApprovedQuotationsDepositPercent() { + void pricingRequestOrigin_nullDepositPercentOnTheLiveQuotationYieldsTheThirtyItsPdfPrints() { long ticketId = newBareTicket(); - insertOriginTaggedQuotation(ticketId, "DEAL_DIRECT", "APPROVED", (short) 50); + long accepted = insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ACCEPTED", null); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.50"); + assertGoverning(ticketId, accepted, false, "0.30"); } @Test - void pricingRequestOrigin_prefersAcceptedOverIssuedOverApproved() { + void pricingRequestOrigin_anAcceptedQuotationBeatsNewerIssuedAndApprovedOnes() { long ticketId = newBareTicket(); - insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "APPROVED", (short) 10); + long accepted = insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ACCEPTED", (short) 30); insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ISSUED", (short) 20); - insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "ACCEPTED", (short) 30); - - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); + insertOriginTaggedQuotation(ticketId, "PRICING_REQUEST", "APPROVED", (short) 10); - assertThat(result).isPresent(); - assertThat(result.get()).isEqualByComparingTo("0.30"); + assertGoverning(ticketId, accepted, false, "0.30"); } @Test @@ -187,16 +176,22 @@ void dealDirectOrigin_ignoresNonLiveStatusesLikeDraftOrCancelled() { insertOriginTaggedQuotation(ticketId, "DEAL_DIRECT", "DRAFT", (short) 60); insertOriginTaggedQuotation(ticketId, "DEAL_DIRECT", "CANCELLED", (short) 70); - Optional result = depositNotices.findDealQuotationDepositPercent(ticketId); - - assertThat(result).isEmpty(); + assertThat(depositNotices.findGoverningQuotation(ticketId)).isEmpty(); } @Test void noQuotationAtAllYieldsEmpty() { long ticketId = newBareTicket(); - assertThat(depositNotices.findDealQuotationDepositPercent(ticketId)).isEmpty(); + assertThat(depositNotices.findGoverningQuotation(ticketId)).isEmpty(); + } + + private void assertGoverning(long ticketId, long quotationId, boolean legacy, String depositPercent) { + DepositNoticeRepository.GoverningQuotation governing = + depositNotices.findGoverningQuotation(ticketId).orElseThrow(); + assertThat(governing.quotationId()).isEqualTo(quotationId); + assertThat(governing.legacy()).isEqualTo(legacy); + assertThat(governing.depositPercent()).isEqualByComparingTo(depositPercent); } // ───────────────────────────────────────────────────────────────────────────────────── @@ -241,16 +236,17 @@ null, new BigDecimal("10"), new BigDecimal("0.36"), WastageCalculator.QUANTITY_M /** Legacy quotation row: {@code origin IS NULL}, {@code pricing_request_id IS NOT NULL} — * exactly the scope {@code CustomerQuotationRepository#findByTicket} uses. */ - private void insertLegacyQuotation(long ticketId, String docStatus, Short depositPercent) { + private long insertLegacyQuotation(long ticketId, String docStatus, Short depositPercent) { long pricingRequestId = pricingRequests.findByTicket(ticketId).get(0).id(); int version = ++quotationVersionSeq; - jdbc.update(""" + return jdbc.queryForObject(""" INSERT INTO sales.quotation (ticket_id, number, issued_by, doc_status, quotation_version, pricing_request_id, origin, deposit_percent) VALUES (:ticketId, :number, :issuedBy, :docStatus, :version, :pricingRequestId, NULL, :depositPercent) + RETURNING quotation_id """, new MapSqlParameterSource() .addValue("ticketId", ticketId) @@ -262,18 +258,19 @@ private void insertLegacyQuotation(long ticketId, String docStatus, Short deposi .addValue("docStatus", docStatus) .addValue("version", version) .addValue("pricingRequestId", pricingRequestId) - .addValue("depositPercent", depositPercent)); + .addValue("depositPercent", depositPercent), Long.class); } /** V2 deal-quotation row: {@code origin IN ('PRICING_REQUEST','DEAL_DIRECT')} — no {@code * pricing_request_id} needed (nullable; the v2 engine's own scope is {@code origin} alone). */ - private void insertOriginTaggedQuotation(long ticketId, String origin, String docStatus, Short depositPercent) { + private long insertOriginTaggedQuotation(long ticketId, String origin, String docStatus, Short depositPercent) { int version = ++quotationVersionSeq; - jdbc.update(""" + return jdbc.queryForObject(""" INSERT INTO sales.quotation (ticket_id, number, issued_by, doc_status, quotation_version, origin, deposit_percent) VALUES (:ticketId, :number, :issuedBy, :docStatus, :version, :origin, :depositPercent) + RETURNING quotation_id """, new MapSqlParameterSource() .addValue("ticketId", ticketId) @@ -282,7 +279,7 @@ private void insertOriginTaggedQuotation(long ticketId, String origin, String do .addValue("docStatus", docStatus) .addValue("version", version) .addValue("origin", origin) - .addValue("depositPercent", depositPercent)); + .addValue("depositPercent", depositPercent), Long.class); } private TicketItemRequest ticketItem() { diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeServiceTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeServiceTest.java index 05e2b444d..439f57eea 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeServiceTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeServiceTest.java @@ -260,8 +260,8 @@ void remainingInvoiceXlsx_nullLineSubtotalFallsBackToFinalUnitPriceTimesQtyHalfU void remainingInvoiceOptions_fallsBackToIssuedDepositNoticeItemsWhenNoAcceptedQuotation() { stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); // An ISSUED (not ACCEPTED) quotation must be ignored by the remaining-invoice path — - // unlike buildItemsFromRequest's pickQuotation (deposit notice creation), which DOES fall - // back to ISSUED. See resolveRemainingInvoice's own Javadoc for why these differ. + // unlike DepositNoticeRepository#findGoverningQuotation (deposit notice creation), which + // DOES fall back to ISSUED. See resolveRemainingInvoice's own Javadoc for why these differ. // // Finding 1 (vacuous test): this test used to only assert itemCount/depositAmount, which // stayed green even under the mutation "let ISSUED quotations qualify too" — that mutation @@ -842,7 +842,6 @@ void createDraft_autofillsHeaderFromCustomerMasterAndTicketProjectName() { stubTicketWithCustomer(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED", 42L, "Project Zeta"); when(customerRepo.findById(42L)).thenReturn(Optional.of( new CustomerDto(42L, "ACME", "0100000000000", "123 Road", "Showroom", "02-000-0000"))); - when(quotationRepo.findByTicket(10L)).thenReturn(List.of()); when(docs.createDraft(eq(10L), any(), any())).thenReturn(201L); stubDraft(201L, 10L); @@ -862,7 +861,6 @@ void createDraft_nullCustomerIdLeavesHeaderFieldsBlankWithoutThrowing() { // Base fixture's ticket carries customerId = null — must not NPE, and the customer // repository must never even be consulted. stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); - when(quotationRepo.findByTicket(10L)).thenReturn(List.of()); when(docs.createDraft(eq(10L), any(), any())).thenReturn(202L); stubDraft(202L, 10L); @@ -876,12 +874,16 @@ void createDraft_nullCustomerIdLeavesHeaderFieldsBlankWithoutThrowing() { verify(customerRepo, never()).findById(anyLong()); } + /** With no approved_price on any ticket item, the lines come from the governing quotation + * (DepositNoticeRepository#findGoverningQuotation), here a legacy one. */ @Test - void createDraft_fallsBackToAcceptedQuotationWhenNoTicketItemHasApprovedPrice() { + void createDraft_takesTheLinesOfTheGoverningLegacyQuotationWhenNoTicketItemHasApprovedPrice() { stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); // items: List.of() — no approved_price CustomerQuotationItemDto item = quotationItem("กระเบื้อง A", new BigDecimal("10"), "PER_SQM", new BigDecimal("100"), BigDecimal.ZERO, new BigDecimal("100")); - when(quotationRepo.findByTicket(10L)).thenReturn(List.of(quotation(1L, 10L, "ACCEPTED", 1, List.of(item)))); + when(docs.findGoverningQuotation(10L)).thenReturn(Optional.of( + new DepositNoticeRepository.GoverningQuotation(1L, true, "QT-2026-1", new BigDecimal("0.50")))); + when(quotationRepo.findById(1L)).thenReturn(Optional.of(quotation(1L, 10L, "ACCEPTED", 1, List.of(item)))); when(docs.createDraft(eq(10L), any(), any())).thenReturn(203L); stubDraft(203L, 10L); @@ -898,41 +900,16 @@ void createDraft_fallsBackToAcceptedQuotationWhenNoTicketItemHasApprovedPrice() assertThat(items.get(0).unitPrice()).isEqualTo(new BigDecimal("100")); } - @Test - void createDraft_prefersAcceptedQuotationOverAnIssuedOne() { - stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); - CustomerQuotationItemDto issuedItem = quotationItem("Issued Item", BigDecimal.ONE, "PER_PIECE", - BigDecimal.TEN, BigDecimal.ZERO, BigDecimal.TEN); - CustomerQuotationItemDto acceptedItem = quotationItem("Accepted Item", BigDecimal.ONE, "PER_PIECE", - BigDecimal.TEN, BigDecimal.ZERO, BigDecimal.TEN); - // findByTicket's own contract: ascending by quotation_id ALONE (see its own Javadoc — - // quotation_revision_no is scoped to a single pricing request, not comparable across - // the multiple pricing requests one ticket can have) — the accepted revision here has - // the higher quotation_id, same as a real revise-after-accept flow. - when(quotationRepo.findByTicket(10L)).thenReturn(List.of( - quotation(1L, 10L, "ISSUED", 1, List.of(issuedItem)), - quotation(2L, 10L, "ACCEPTED", 2, List.of(acceptedItem)))); - when(docs.createDraft(eq(10L), any(), any())).thenReturn(204L); - stubDraft(204L, 10L); - - service.createDraft(10L, - new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), owner); - - @SuppressWarnings("unchecked") - ArgumentCaptor> itemsCaptor = ArgumentCaptor.forClass(List.class); - verify(docs).createDraft(eq(10L), any(), itemsCaptor.capture()); - assertThat(itemsCaptor.getValue()).hasSize(1); - assertThat(itemsCaptor.getValue().get(0).description()).isEqualTo("Accepted Item"); - } - @Test void createDraft_legacyApprovedPriceItemsWinOverQuotationChainWhenBothExist() { TicketItemDto approvedItem = new TicketItemDto(1L, 10L, "SCG", "Model", "White", "Matte", "60x60", "Factory", new BigDecimal("10"), null, null, null, null, null, new BigDecimal("100"), "THB", 0, null, null, null, "PIECE", null, null); stubTicketWithItems(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED", List.of(approvedItem)); - // Even though an ACCEPTED quotation exists too, the legacy approved_price path must win - // and the quotation chain must never even be queried (buildLegacyItems short-circuits). + // Even though a governing quotation exists too, the legacy approved_price path must win + // and that quotation's lines must never even be read (buildLegacyItems short-circuits). + when(docs.findGoverningQuotation(10L)).thenReturn(Optional.of( + new DepositNoticeRepository.GoverningQuotation(1L, true, "QT-2026-1", new BigDecimal("0.50")))); when(docs.createDraft(eq(10L), any(), any())).thenReturn(205L); stubDraft(205L, 10L); @@ -944,20 +921,21 @@ void createDraft_legacyApprovedPriceItemsWinOverQuotationChainWhenBothExist() { verify(docs).createDraft(eq(10L), any(), itemsCaptor.capture()); assertThat(itemsCaptor.getValue()).hasSize(1); assertThat(itemsCaptor.getValue().get(0).unitPrice()).isEqualTo(new BigDecimal("100")); - verify(quotationRepo, never()).findByTicket(10L); + verify(quotationRepo, never()).findById(anyLong()); + verify(docs, never()).findDealQuotationItems(anyLong()); } // ── Deposit percent LOCKED to the quotation (owner ruling, 2026-09-29) ──── // See DepositNoticeService.createDraft/update and DepositNoticeRepository - // #findDealQuotationDepositPercent's own comments for what changed and why: the deposit + // #findGoverningQuotation's own comments for what changed and why: the deposit // PERCENT is no longer sales-entered at the deposit-notice stage at all — it is read straight // off the source quotation's deposit_percent and is not editable afterward. @Test void createDraft_sourcesDepositPercentFromQuotation_ignoringCallerSuppliedValue() { stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); - when(quotationRepo.findByTicket(10L)).thenReturn(List.of()); - when(docs.findDealQuotationDepositPercent(10L)).thenReturn(Optional.of(new BigDecimal("0.30"))); + when(docs.findGoverningQuotation(10L)).thenReturn(Optional.of( + new DepositNoticeRepository.GoverningQuotation(5L, false, "QT-5", new BigDecimal("0.30")))); when(docs.createDraft(eq(10L), any(), any())).thenReturn(301L); stubDraft(301L, 10L); @@ -975,8 +953,7 @@ void createDraft_sourcesDepositPercentFromQuotation_ignoringCallerSuppliedValue( @Test void createDraft_fallsBackTo050WhenTicketHasNoQuotationDepositPercent() { stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); - when(quotationRepo.findByTicket(10L)).thenReturn(List.of()); - when(docs.findDealQuotationDepositPercent(10L)).thenReturn(Optional.empty()); + when(docs.findGoverningQuotation(10L)).thenReturn(Optional.empty()); when(docs.createDraft(eq(10L), any(), any())).thenReturn(302L); stubDraft(302L, 10L); @@ -989,35 +966,35 @@ void createDraft_fallsBackTo050WhenTicketHasNoQuotationDepositPercent() { } /** - * Proves the final fallback's wiring (2026-09-29 fix): when the legacy ticket-item path AND - * the CustomerQuotation {@code pickQuotation} path both yield nothing (no approved_price on - * any ticket item, and {@code quotationRepo.findByTicket} returns an empty list — no {@code - * origin IS NULL} quotation at all), {@code buildItemsFromRequest} must return EXACTLY what - * {@code docs.findDealQuotationItemsForDeposit(ticketId)} returns, not an empty list (the old - * {@code return List.of();} this branch replaced). The sentinel item below ("V2 deal quotation - * item", unit price 999) is deliberately unlike anything else in this file so a test bug that - * accidentally sourced items from elsewhere would show up as the wrong description/price - * rather than merely "some non-empty list". + * Lines, percent and reference all come from the ONE governing quotation (owner ruling + * 2026-10-23), here a new-engine one: its lines via {@code docs.findDealQuotationItems}, its + * percent, and its number as the reference. The sentinel item ("V2 deal quotation item", unit + * price 999) and the 0.40 are deliberately unlike anything else in this file, so a draft that + * took its lines or percent from elsewhere shows up as the wrong value, not merely "some value". */ @Test - void createDraft_fallsBackToDealQuotationRepositoryWhenLegacyAndPickQuotationBothYieldNothing() { + void createDraft_takesLinesPercentAndReferenceFromTheGoverningNewEngineQuotation() { stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); // items: List.of() — no approved_price - when(quotationRepo.findByTicket(10L)).thenReturn(List.of()); // pickQuotation -> null + when(docs.findGoverningQuotation(10L)).thenReturn(Optional.of( + new DepositNoticeRepository.GoverningQuotation(7L, false, "QT-7", new BigDecimal("0.40")))); DepositNoticeItemRequest sentinel = new DepositNoticeItemRequest( 1, "V2 deal quotation item", new BigDecimal("3"), "แผ่น", new BigDecimal("999"), null, new BigDecimal("999")); - when(docs.findDealQuotationItemsForDeposit(10L)).thenReturn(List.of(sentinel)); + when(docs.findDealQuotationItems(7L)).thenReturn(List.of(sentinel)); when(docs.createDraft(eq(10L), any(), any())).thenReturn(206L); stubDraft(206L, 10L); service.createDraft(10L, new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), owner); + ArgumentCaptor captor = ArgumentCaptor.forClass(DepositNoticeDraftRequest.class); @SuppressWarnings("unchecked") ArgumentCaptor> itemsCaptor = ArgumentCaptor.forClass(List.class); - verify(docs).createDraft(eq(10L), any(), itemsCaptor.capture()); + verify(docs).createDraft(eq(10L), captor.capture(), itemsCaptor.capture()); assertThat(itemsCaptor.getValue()).containsExactly(sentinel); - verify(docs).findDealQuotationItemsForDeposit(10L); + assertThat(captor.getValue().depositPercent()).isEqualByComparingTo("0.40"); + assertThat(captor.getValue().reference()).isEqualTo("QT-7"); + verify(quotationRepo, never()).findById(anyLong()); } @Test @@ -1133,9 +1110,9 @@ private void stubTicketWithItems(long ticketId, String status, String paymentSta .thenReturn(Optional.of(new TicketDto(summary, items, List.of(), null, List.of()))); } - // Minimal CustomerQuotationDto fixture — only the fields buildItemsFromRequest's pickQuotation - // and itemsFromQuotation actually read (ticketId, docStatus, quotationRevisionNo, items) carry - // meaningful values; everything else is a harmless placeholder. + // Minimal CustomerQuotationDto fixture — only the fields the service actually reads (ticketId, + // docStatus, quotationRevisionNo, items) carry meaningful values; everything else is a + // harmless placeholder. private CustomerQuotationDto quotation(long id, long ticketId, String docStatus, int revisionNo, List items) { return new CustomerQuotationDto( diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index cc9553d43..134548f0e 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -1,5 +1,7 @@ +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; import { describe, it, expect } from 'vitest'; -import { api } from './mockApi.js'; +import { api, MOCK_DEFAULT_DEPOSIT_PERCENT, MOCK_LEGACY_DEPOSIT_PERCENT } from './mockApi.js'; // Branch fix (deposit-notice autofill): drives the mock's own pricing-request chain end to end // — Step 1 create through Step 5 customer-accept, then confirmOrder — against the REAL mock @@ -184,10 +186,13 @@ describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notic expect(detail.items.every((it) => it.approvedPrice == null)).toBe(true); expect(detail.summary.status).toBe('quotation_issued'); + // A typed percent is ignored: the legacy quotation's PDF prints 50%, so the draft bills 50%. const { depositNotice } = await api.depositNotices.createDraft(ticketId, { notes: [], - depositPercent: 0.5, + depositPercent: 0.9, }); + expect(depositNotice.depositPercent).toBe(0.5); + expect(depositNotice.reference).toBe(quotation.number); // Items: sourced from the ACCEPTED quotation, not the (all-null) ticket_item rows. expect(depositNotice.items).toHaveLength(1); @@ -209,9 +214,11 @@ describe('mockApi.depositNotices.createDraft autofill (branch fix: deposit-notic // delegates to — this endpoint itself never passes a customer/project explicitly. const prId = (await api.pricingRequests.listForTicket(quotation.ticketId)).items .find((pr) => pr.status === 'QUOTATION_ACCEPTED').id; + // A typed percent is ignored here too: the legacy branch bills its PDF's 50%. const { depositNotice } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, { - depositPercent: 0.5, + depositPercent: 0.9, }); + expect(depositNotice.depositPercent).toBe(0.5); expect(depositNotice.customerTaxId).toBe(customer.taxId); expect(depositNotice.customerAddress).toBe(`${customer.address} ${customer.branch}`); @@ -241,15 +248,17 @@ describe('mockApi.pricingRequests.createDepositNoticeFromQuotation — new-engin expect(depositNotice.items[0].discountLabel).toBe('ส่วนลด 12 ต่อหน่วย'); }); - // 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 () => { + // Covers the ticket-level route's governing quotation (DepositNoticeRepository.findGoverningQuotation): + // with no items and no legacy quotation, it bills the new-engine quotation's lines, percent and number. + it('ticket-level createDraft with no items drafts the same lines, percent and reference as the pricing-request route', async () => { const { ticketId, prId, accepted } = await driveTicketToNewEngineAcceptedQuotation(); // The PDF prints a NULL percent as DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT (30). expect(accepted.depositPercent).toBeNull(); const { depositNotice: fromTicket } = await api.depositNotices.createDraft(ticketId, {}); const { depositNotice: fromPr } = await api.pricingRequests.createDepositNoticeFromQuotation(prId, {}); expect(fromPr.depositPercent).toBe(0.3); + expect(fromTicket.depositPercent).toBe(0.3); + expect(fromTicket.reference).toBe(fromPr.reference); expect(fromTicket.items.length).toBeGreaterThan(0); const pick = (items) => items.map(({ qty, netUnitPrice }) => ({ qty, netUnitPrice })); expect(pick(fromTicket.items)).toEqual(pick(fromPr.items)); @@ -389,3 +398,32 @@ describe('mockApi.depositNotices.issue account notification (GLA-32)', () => { expect(own.message).toContain('ไม่ระบุลูกค้า'); }); }); + +// The mock's blank-percent defaults restate two Java constants; read them out of the source so the +// copies cannot drift (the dealTrackingMeta.test.js approach). Throws rather than skips when the +// files move, so the guard cannot quietly disable itself. +function readBackendSource(relativePath) { + for (let dir = process.cwd(), depth = 0; depth < 8; depth += 1, dir = dirname(dir)) { + const candidate = resolve(dir, relativePath); + if (existsSync(candidate)) return readFileSync(candidate, 'utf8'); + } + throw new Error(`could not find ${relativePath} above ${process.cwd()}`); +} + +function javaIntConstant(relativePath, name) { + const match = readBackendSource(relativePath).match(new RegExp(`static final int ${name} = (\\d+);`)); + if (!match) throw new Error(`could not find int ${name} in ${relativePath}`); + return Number(match[1]); +} + +describe('mock deposit-percent defaults match the Java PDFs', () => { + it('a blank new-engine percent is DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT', () => { + expect(MOCK_DEFAULT_DEPOSIT_PERCENT).toBe(javaIntConstant( + 'backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationRenderAdapter.java', 'DEFAULT_DEPOSIT_PERCENT')); + }); + + it('a legacy quotation is QuotationRenderer.LEGACY_DEPOSIT_PERCENT', () => { + expect(MOCK_LEGACY_DEPOSIT_PERCENT).toBe(javaIntConstant( + 'backend/src/main/java/th/co/glr/hr/ticket/QuotationRenderer.java', 'LEGACY_DEPOSIT_PERCENT')); + }); +}); From c0d840c6c689079c2999560580b5ea44896782ed Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:38:26 +0700 Subject: [PATCH 08/32] fix(sales): the deposit notice follows the latest quotation the customer answered, lines and percent from one row Owner rulings 2026-10-23 (supersede "a live legacy quotation always wins" for the ranking). On a ticket with a legacy ISSUED quotation and a new-engine quotation the customer ACCEPTED at 20%, the ticket route billed 0.50 with the legacy lines; it now bills 0.20 with the accepted quotation's lines and number, the same draft the pricing-request route makes. - DepositNoticeRepository#findGoverningQuotation: one query over sales.quotation for both engines. Answered first (ACCEPTED, or DEAL_DIRECT APPROVED), then ISSUED, then APPROVED, newest quotation_id within a rank. A legacy row bills QuotationRenderer.LEGACY_DEPOSIT_PERCENT (its PDF never reads the column); a new-engine NULL bills DEFAULT_DEPOSIT_PERCENT. - DepositNoticeService: the ticket route takes lines, percent and reference from that one row. Deleted pickQuotation, findDealQuotationDepositPercent, findDealQuotationItemsForDeposit and firstOrNull. - OrderConfirmationService: the legacy branch passes LEGACY_DEPOSIT_PERCENT, so the pricing-request route never consults the ticket-wide pick. - QuotationRenderer: names LEGACY_DEPOSIT_PERCENT = 50 and uses it. - mockApi: the ticket route mirrors the same pick and defaults; the legacy pricing-request route bills a fixed 50%; both ignore a typed percent. - TicketRepository#findPricingChainFallbackItems: Javadoc only; it is a display pick and no longer claims lockstep with the deposit pick. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- .../dealquotation/DealQuotationService.java | 2 +- .../hr/deposit/DepositNoticeRepository.java | 184 +++++------------- .../glr/hr/deposit/DepositNoticeService.java | 74 +++---- .../OrderConfirmationService.java | 32 ++- .../co/glr/hr/ticket/QuotationRenderer.java | 8 +- .../th/co/glr/hr/ticket/TicketRepository.java | 14 +- .../TicketRepositoryIntegrationTest.java | 7 +- frontend/src/api/mockApi.js | 87 +++++---- frontend/src/data/demoSales.js | 2 +- .../src/features/tickets/DealDepositPanel.jsx | 2 +- 10 files changed, 150 insertions(+), 262 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java index c7d8e470e..14e0f4871 100644 --- a/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java +++ b/backend/src/main/java/th/co/glr/hr/dealquotation/DealQuotationService.java @@ -2193,7 +2193,7 @@ private PromoteToDealResultDto promotionResult(long ticketId, long quotationId) * qty_sqm, unit_basis, raw_unit. NO pricing field is copied (proposed/approved/raw price stay * null) — those columns carry the legacy ticket-native approval history, and a direct * quotation's typed price was never "approved" on this ticket in that sense; money documents - * read the quotation itself ({@code DepositNoticeRepository#findDealQuotationItemsForDeposit}). + * read the quotation itself ({@code DepositNoticeRepository#findDealQuotationItems}). * *

Only goods lines: an {@code ADJUSTMENT} row (a negative discount/surcharge line, quantity −1) * is money, not something to deliver, and a non-positive quantity would violate 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 59dfd3255..b5895898d 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 @@ -19,6 +19,7 @@ import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; import th.co.glr.hr.dealquotation.DealQuotationRenderAdapter; +import th.co.glr.hr.ticket.QuotationRenderer; @Repository public class DepositNoticeRepository { @@ -90,103 +91,57 @@ public List findByTicket(long ticketId) { ); } + /** The quotation a ticket-level deposit notice bills: its lines, its percent (a fraction, + * 30 -> 0.30) and its number as the reference. See {@link #findGoverningQuotation}. */ + public record GoverningQuotation(long quotationId, boolean legacy, String number, BigDecimal depositPercent) {} + /** - * Owner ruling (2026-09-29): the deposit PERCENTAGE on a deposit notice is LOCKED to the - * source quotation's own {@code sales.quotation.deposit_percent} — the value sales set when - * creating the quotation — and is never entered or edited at the deposit-notice stage. - * {@code deposit_percent} is a nullable SMALLINT whole-number percent (30/50/100); this - * returns it as a fraction (50 -> 0.50) via {@code BigDecimal.valueOf(pct, 2)}, which is - * exact for any integer/100 (denominator 100 = 2^2*5^2 always terminates). + * The ONE pick of the quotation a ticket-level deposit notice bills, across both engines, so + * its lines and its percent can never come from different rows (owner rulings 2026-09-29 and + * 2026-10-23). The PRICING-REQUEST route ({@code OrderConfirmationService + * #createDepositNoticeFromQuotation}) already knows its quotation and does not call this. + * + *

Live rows: a legacy pricing-chain quotation ({@code origin IS NULL}, {@code + * pricing_request_id IS NOT NULL}, the scope {@code CustomerQuotationRepository#findByTicket} + * uses) that is ACCEPTED or ISSUED, or a new-engine one ({@code origin IN + * ('PRICING_REQUEST','DEAL_DIRECT')}) that is ACCEPTED, ISSUED or APPROVED. Engines are + * exclusive per pricing request, not per ticket, so one ticket can hold both. + * + *

Ranking: the latest quotation the customer ANSWERED wins: ACCEPTED on either engine, or + * APPROVED on a DEAL_DIRECT quotation (its terminal state; it is never ACCEPTED). Only when + * nothing was answered does the newest ISSUED row win, then APPROVED (never reached in + * practice: a PRICING_REQUEST approval lands on ISSUED, {@code DealQuotationRepository + * #approve}). "Latest" is {@code quotation_id DESC}: one sequence across every engine and + * pricing request, set at creation. Not {@code accepted_at}/{@code approved_at}: those time + * different events (a customer answer against an internal approval), and either can be NULL + * on older or imported rows. * - *

Pick order deliberately mirrors {@link DepositNoticeService}'s own quotation selection - * for deposit-notice ITEMS, so the percent this returns always comes from the same row the - * items would be sourced from: - *

    - *
  • legacy customer-quotation chain ({@code origin IS NULL}, {@code pricing_request_id - * IS NOT NULL} — the exact scope {@code CustomerQuotationRepository#findByTicket} uses): - * ACCEPTED preferred, else ISSUED — the same rule {@code - * DepositNoticeService#pickQuotation} applies for items. Read as raw SQL rather than via - * {@code CustomerQuotationRepository}'s own DTO because {@code CustomerQuotationDto} - * does not carry {@code deposit_percent} at all (it predates that column and nothing - * else on that DTO's surface needed it) — adding a field there would widen every one of - * that repository's other read paths for a value only this one caller needs.
    • - *
  • v2 deal-quotation chain ({@code origin IN ('PRICING_REQUEST','DEAL_DIRECT')}): the - * single live quotation, preferring ACCEPTED, then ISSUED, then APPROVED (DEAL_DIRECT's - * own only live terminal state), newest by id.
    • - *
- * 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. A null {@code deposit_percent} reads as the percent that - * engine's PDF printed (owner ruling 2026-10-22): 50 on a legacy row ({@code - * ticket.QuotationRenderer#buildLegacyModel}), {@link - * DealQuotationRenderAdapter#DEFAULT_DEPOSIT_PERCENT} (30) on a v2 row. So a live legacy row - * always wins, and the v2 branch is reached only when the ticket has no live legacy quotation. - * Returns empty only when there is no live (ACCEPTED/ISSUED/APPROVED) quotation for this - * ticket — the caller ({@link DepositNoticeService#createDraft}) falls back to 0.50 then, - * this document's long-standing default. + *

Percent: what that quotation's PDF printed. A legacy row is always {@link + * QuotationRenderer#LEGACY_DEPOSIT_PERCENT}: the legacy PDF never reads the stored column, so + * this does not either. A new-engine row is its own {@code deposit_percent}, a NULL one {@link + * DealQuotationRenderAdapter#DEFAULT_DEPOSIT_PERCENT}. Empty when the ticket has no live + * quotation; {@link DepositNoticeService#createDraft} then bills 0.50. */ - public Optional findDealQuotationDepositPercent(long ticketId) { - // 50 = the legacy PDF's own default: ticket.QuotationRenderer#buildLegacyModel prints - // `quotation.depositPercent() != null ? quotation.depositPercent() : 50`. - Integer legacyPct = firstOrNull(jdbc.query(""" - SELECT COALESCE(deposit_percent, 50) AS deposit_percent FROM sales.quotation - WHERE ticket_id = :ticketId AND pricing_request_id IS NOT NULL AND origin IS NULL - AND doc_status IN ('ACCEPTED','ISSUED') - ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 ELSE 1 END, quotation_id DESC - LIMIT 1 - """, Map.of("ticketId", ticketId), - (rs, i) -> (Integer) rs.getObject("deposit_percent") - )); - if (legacyPct != null) { - return Optional.of(BigDecimal.valueOf(legacyPct, 2)); - } - - Integer v2Pct = firstOrNull(jdbc.query(""" - SELECT COALESCE(deposit_percent, :pdfDefault) AS deposit_percent FROM sales.quotation - WHERE ticket_id = :ticketId AND origin IN ('PRICING_REQUEST','DEAL_DIRECT') - AND doc_status IN ('ACCEPTED','ISSUED','APPROVED') - ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 WHEN 'ISSUED' THEN 1 ELSE 2 END, - quotation_id DESC - LIMIT 1 - """, Map.of("ticketId", ticketId, "pdfDefault", DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT), - (rs, i) -> (Integer) rs.getObject("deposit_percent") - )); - return v2Pct != null ? Optional.of(BigDecimal.valueOf(v2Pct, 2)) : Optional.empty(); - } - - /** The quotation a ticket-level deposit notice bills; {@code depositPercent} is a fraction (0.30). */ - public record GoverningQuotation(long quotationId, boolean legacy, String number, BigDecimal depositPercent) {} - - /** RED-commit compile scaffold: today's two-step pick (legacy first), unchanged behaviour. */ public Optional findGoverningQuotation(long ticketId) { - List legacy = jdbc.query(""" - SELECT quotation_id, number, COALESCE(deposit_percent, 50) AS pct FROM sales.quotation - WHERE ticket_id = :ticketId AND pricing_request_id IS NOT NULL AND origin IS NULL - AND doc_status IN ('ACCEPTED','ISSUED') - ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 ELSE 1 END, quotation_id DESC - LIMIT 1 - """, Map.of("ticketId", ticketId), (rs, i) -> new GoverningQuotation( - rs.getLong("quotation_id"), true, rs.getString("number"), BigDecimal.valueOf(rs.getInt("pct"), 2))); - if (!legacy.isEmpty()) return Optional.of(legacy.get(0)); return jdbc.query(""" - SELECT quotation_id, number, COALESCE(deposit_percent, :pdfDefault) AS pct FROM sales.quotation - WHERE ticket_id = :ticketId AND origin IN ('PRICING_REQUEST','DEAL_DIRECT') - AND doc_status IN ('ACCEPTED','ISSUED','APPROVED') - ORDER BY CASE doc_status WHEN 'ACCEPTED' THEN 0 WHEN 'ISSUED' THEN 1 ELSE 2 END, quotation_id DESC + SELECT quotation_id, origin IS NULL AS legacy, number, deposit_percent + FROM sales.quotation + WHERE ticket_id = :ticketId + AND ((origin IS NULL AND pricing_request_id IS NOT NULL AND doc_status IN ('ACCEPTED','ISSUED')) + OR (origin IN ('PRICING_REQUEST','DEAL_DIRECT') AND doc_status IN ('ACCEPTED','ISSUED','APPROVED'))) + ORDER BY CASE WHEN doc_status = 'ACCEPTED' OR (origin = 'DEAL_DIRECT' AND doc_status = 'APPROVED') THEN 0 + WHEN doc_status = 'ISSUED' THEN 1 + ELSE 2 + END, quotation_id DESC LIMIT 1 - """, Map.of("ticketId", ticketId, "pdfDefault", DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT), - (rs, i) -> new GoverningQuotation(rs.getLong("quotation_id"), false, rs.getString("number"), - BigDecimal.valueOf(rs.getInt("pct"), 2))).stream().findFirst(); - } - - /** First mapped row or {@code null} when the query matched nothing. Both queries above now - * COALESCE the percent, so a mapped value is never null; this stays index-based rather than - * {@code stream().findFirst()} so a future null column cannot turn into an NPE. */ - private static T firstOrNull(List rows) { - return rows.isEmpty() ? null : rows.get(0); + """, Map.of("ticketId", ticketId), (rs, i) -> { + boolean legacy = rs.getBoolean("legacy"); + Integer stored = (Integer) rs.getObject("deposit_percent"); + int percent = legacy ? QuotationRenderer.LEGACY_DEPOSIT_PERCENT + : stored != null ? stored : DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT; + return new GoverningQuotation(rs.getLong("quotation_id"), legacy, rs.getString("number"), + BigDecimal.valueOf(percent, 2)); + }).stream().findFirst(); } @Transactional @@ -400,53 +355,6 @@ private void insertItems(long docId, List items) { } } - /** - * Deal-quotation (V2) item source for deposit-notice autofill (2026-09-29 fix). The legacy - * autofill knew only two sources: {@code sales.ticket_item.approved_price} (never written by - * the pricing-request chain) and {@code CustomerQuotationRepository#findByTicket} (scoped to - * {@code origin IS NULL}, which excludes BOTH {@code DEAL_DIRECT} and {@code PRICING_REQUEST} - * rows). Every deal whose quotation was issued through the V2 deal-quotation flow therefore got - * an EMPTY deposit-notice item list — the reported "รายการ not autofilled" bug. - * - *

Selection covers BOTH V2 origins, whose live-terminal doc_status differs — the two - * never share a {@code ticket_id} (see {@code DealQuotationRepository#nextRevisionNo}), so the - * union below is unambiguous per ticket: - *

    - *
  • {@code PRICING_REQUEST} runs the customer-response lifecycle DRAFT → PENDING_APPROVAL → - * ISSUED → ACCEPTED, so it mirrors {@link DepositNoticeService#pickQuotation} exactly: - * prefer ACCEPTED (the customer-accepted document a deposit is raised against), else the - * latest ISSUED.
    • - *
  • {@code DEAL_DIRECT} has no customer-response lifecycle in the model today — it ends at - * APPROVED (terminal-until-revised; see {@code QuotationStatus.APPROVED}), which is the - * only "live" state it ever reaches. Without APPROVED here, every direct-deal deposit - * would still autofill empty.
    • - *
- * SUPERSEDED / CANCELLED / REJECTED / EXPIRED / DRAFT / PENDING_APPROVAL / REVISION_REQUESTED - * rows are never chosen. Preference ACCEPTED > ISSUED > APPROVED, then newest by id. - * 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. - * The lines themselves come from {@link #findDealQuotationItems}. - */ - public List findDealQuotationItemsForDeposit(long ticketId) { - List ids = jdbc.query(""" - SELECT quotation_id FROM sales.quotation - WHERE ticket_id = :ticketId - AND origin IN ('PRICING_REQUEST', 'DEAL_DIRECT') - AND doc_status IN ('ACCEPTED', 'ISSUED', 'APPROVED') - ORDER BY CASE doc_status - WHEN 'ACCEPTED' THEN 0 - WHEN 'ISSUED' THEN 1 - ELSE 2 - END, quotation_id DESC - LIMIT 1 - """, Map.of("ticketId", ticketId), (rs, i) -> rs.getLong("quotation_id")); - if (ids.isEmpty()) { - return List.of(); - } - return findDealQuotationItems(ids.get(0)); - } - /** One deal quotation's {@code sales.quotation_item} lines, by {@code seq}, as deposit-notice * 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 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 da2d1c40a..a31398ddc 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 @@ -96,8 +96,8 @@ public DepositNoticeDto createDraft(long ticketId, DepositNoticeDraftRequest req } /** {@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. */ + * {@code sourceDepositPercent} (the whole-number percent that quotation's PDF printed) is the + * draft's percent, and the ticket's governing quotation is not consulted. */ @Transactional public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDraftRequest req, Integer sourceDepositPercent, UserPrincipal actor) { @@ -106,9 +106,11 @@ public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDra TicketSummaryDto s = requireApprovedTicket(ticketId, actor); requireActiveLifecycle(s); - // Auto-populate items from approved ticket items (legacy) or, failing that, the - // ticket's own customer-quotation chain (new pricing-request flow) if not provided. - List items = buildItemsFromRequest(req, ticketId); + // Ticket route: the governing quotation (DepositNoticeRepository#findGoverningQuotation) + // supplies the lines, the percent and the reference, so they always come from one row. + DepositNoticeRepository.GoverningQuotation governing = sourceDepositPercent == null + ? docs.findGoverningQuotation(ticketId).orElse(null) : null; + List items = buildItemsFromRequest(req, ticketId, governing); List notes = req.notes() != null ? req.notes() : docs.findNoteTemplates().stream() .filter(DocumentNoteTemplateDto::defaultSelected) @@ -137,16 +139,16 @@ public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDra // through the DTO the frontend/OrderConfirmationService already send. Falls back to the // 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. + // chain) — see DepositNoticeRepository#findGoverningQuotation's own Javadoc for the pick. BigDecimal effectiveDepositPercent = sourceDepositPercent != null ? BigDecimal.valueOf(sourceDepositPercent, 2) - : docs.findDealQuotationDepositPercent(ticketId).orElse(new BigDecimal("0.50")); + : governing != null ? governing.depositPercent() : new BigDecimal("0.50"); + String reference = req.reference() == null && governing != null ? governing.number() : req.reference(); var effective = new DepositNoticeDraftRequest( req.customerName() != null ? req.customerName() : s.customerName(), header.taxId(), header.address(), - header.projectName(), req.reference(), + header.projectName(), reference, effectiveDepositPercent, notes, items ); @@ -839,7 +841,7 @@ private DepositRefBundle resolveDepositReferenceOptions(long ticketId, DepositNo /** Latest ACCEPTED revision only — see {@link #resolveRemainingInvoice}'s own Javadoc for why * this deliberately does NOT fall back to the latest ISSUED-but-not-yet-accepted one the way - * {@link #pickQuotation} does for the deposit notice. */ + * {@link DepositNoticeRepository#findGoverningQuotation} does for the deposit notice. */ private CustomerQuotationDto pickAcceptedQuotation(List candidates) { CustomerQuotationDto latestAccepted = null; for (CustomerQuotationDto q : candidates) { @@ -1210,7 +1212,8 @@ CustomerHeaderInfo resolveCustomerHeader(TicketSummaryDto s, return new CustomerHeaderInfo(customerTaxId, customerAddress, addressOnly, branch, projectName); } - private List buildItemsFromRequest(DepositNoticeDraftRequest req, long ticketId) { + private List buildItemsFromRequest(DepositNoticeDraftRequest req, long ticketId, + DepositNoticeRepository.GoverningQuotation governing) { if (req.items() != null && !req.items().isEmpty()) return req.items(); // Legacy path: sales.ticket_item.approved_price, written only by the @Deprecated @@ -1219,21 +1222,13 @@ private List buildItemsFromRequest(DepositNoticeDraftR List legacyItems = buildLegacyItems(ticketId); if (!legacyItems.isEmpty()) return legacyItems; - // New-chain fallback (this branch's fix): every deal created through the pricing-request - // chain has approved_price = NULL on every ticket_item (see this class's own diagnosis in - // the branch handoff) — buildLegacyItems above always returns empty for such a deal, which - // is exactly the bug. Source items from the ticket's own customer quotation instead. - CustomerQuotationDto chosen = pickQuotation(quotations.findByTicket(ticketId)); - if (chosen != null) { - return itemsFromQuotation(chosen.items()); - } - - // V2 deal-quotation chain (2026-09-29 fix): the pickQuotation/findByTicket source above is - // scoped to origin IS NULL, so it never returns a DEAL_DIRECT or PRICING_REQUEST quotation - // — every deal issued through the deal-quotation flow fell through to an empty item list. - // docs.findDealQuotationItemsForDeposit reads the ticket's live deal quotation (ACCEPTED, - // else ISSUED) directly; see that method's Javadoc for the selection and column choices. - return docs.findDealQuotationItemsForDeposit(ticketId); + // Every deal created through the pricing-request chain has approved_price = NULL on every + // ticket_item, so its lines come from the governing quotation, whichever engine it is. + if (governing == null) return List.of(); + if (!governing.legacy()) return docs.findDealQuotationItems(governing.quotationId()); + return quotations.findById(governing.quotationId()) + .map(q -> itemsFromQuotation(q.items())) + .orElse(List.of()); } private List buildLegacyItems(long ticketId) { @@ -1257,31 +1252,6 @@ private List buildLegacyItems(long ticketId) { .orElse(List.of()); } - /** - * Picks the quotation to source deposit-notice items from when no explicit items were given - * and no legacy {@code approved_price} exists: the LATEST ACCEPTED revision, or — if the - * customer has not yet accepted any revision — the latest ISSUED one. {@code - * CustomerQuotationRepository#findByTicket} returns rows ordered ASCENDING by {@code - * quotation_id} ALONE — deliberately NOT by {@code quotation_revision_no} (see that method's - * own Javadoc: the revision counter is scoped to a single pricing request and is not - * comparable across the multiple pricing requests one ticket can have) — so a single forward - * scan that keeps overwriting "latest seen" for each status is equivalent to sorting - * descending by {@code quotation_id} and taking the first match, without a second - * list/comparator. - */ - private CustomerQuotationDto pickQuotation(List candidates) { - CustomerQuotationDto latestAccepted = null; - CustomerQuotationDto latestIssued = null; - for (CustomerQuotationDto q : candidates) { - if (QuotationStatus.ACCEPTED.equals(q.docStatus())) { - latestAccepted = q; - } else if (QuotationStatus.ISSUED.equals(q.docStatus())) { - latestIssued = q; - } - } - 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) { @@ -1290,7 +1260,7 @@ public List itemsFromDealQuotation(long 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) + * used by both {@link #buildItemsFromRequest} (a legacy governing quotation's lines, above) * and {@code OrderConfirmationService.createDepositNoticeFromQuotation} (the explicit * quotation-driven entry point), so the two paths can never drift apart — extracted here * (rather than duplicated, as it was before this branch) because both already depend on this 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 6273e8ea8..393278548 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 @@ -27,6 +27,7 @@ import th.co.glr.hr.pricingrequest.PricingRequestRepository.OrderConfirmationState; import th.co.glr.hr.pricingrequest.PricingRequestStatus; import th.co.glr.hr.pricingrequest.UnitBasis; +import th.co.glr.hr.ticket.QuotationRenderer; import th.co.glr.hr.ticket.QuotationStatus; import th.co.glr.hr.ticket.TicketDto; import th.co.glr.hr.ticket.TicketEventKind; @@ -421,16 +422,15 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * the ticket's own customer/project, same as the ticket-level route) — see {@code * DepositNoticeService.createDraft}'s own comments for what changed and why. * - *

Deposit percent (owner ruling, 2026-09-29): {@code + *

Deposit percent (owner rulings 2026-09-29, 2026-10-23): {@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} (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. + * DepositNoticeService#createDraftFromQuotation} IGNORES it unconditionally: the percent is the + * one the chosen quotation's PDF printed, passed as {@code sourceDepositPercent}. For the + * legacy branch that is always {@link QuotationRenderer#LEGACY_DEPOSIT_PERCENT}, because the + * legacy PDF never reads the stored column. 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: when the pricing request has no ACCEPTED legacy * quotation, this falls back to its ACCEPTED new-engine {@code PRICING_REQUEST}-origin @@ -440,13 +440,11 @@ private String mapUnitBasisToTicketItem(String unitBasis) { * 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 - * 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 bills the PDF's default 30% - * instead of the 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. + * {@code deposit_percent}, a null one as the PDF's default 30%, goes to {@link + * DepositNoticeService#createDraftFromQuotation}. Neither branch consults the + * ticket's governing quotation ({@code DepositNoticeRepository#findGoverningQuotation}), + * which only the ticket-level route uses. The customer must have accepted: an ISSUED-only + * quotation answers 409. */ @Transactional public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, @@ -466,7 +464,7 @@ public DepositNoticeDto createDepositNoticeFromQuotation(long pricingRequestId, if (accepted != null) { reference = accepted.number(); items = DepositNoticeService.itemsFromQuotation(accepted.items()); - sourceDepositPercent = null; + sourceDepositPercent = QuotationRenderer.LEGACY_DEPOSIT_PERCENT; } else { th.co.glr.hr.dealquotation.DealQuotationDtos.DealQuotationDto newEngine = dealQuotations == null ? null diff --git a/backend/src/main/java/th/co/glr/hr/ticket/QuotationRenderer.java b/backend/src/main/java/th/co/glr/hr/ticket/QuotationRenderer.java index 416d4b542..4fd34ff40 100644 --- a/backend/src/main/java/th/co/glr/hr/ticket/QuotationRenderer.java +++ b/backend/src/main/java/th/co/glr/hr/ticket/QuotationRenderer.java @@ -73,6 +73,12 @@ public class QuotationRenderer { private static final Logger log = LoggerFactory.getLogger(QuotationRenderer.class); + /** The deposit percent the legacy PDF prints when its {@code QuotationDto} carries none. A + * pricing-chain legacy quotation never carries one ({@code CustomerQuotationService + * #loadRenderContext}), so its PDF always prints this, and its deposit notice bills it (owner + * ruling 2026-10-23). */ + public static final int LEGACY_DEPOSIT_PERCENT = 50; + // Real company XLS template — sheet "Update" private static final String TEMPLATE = "templates/quotation_template.xls"; @@ -322,7 +328,7 @@ private QuotationRenderModel buildLegacyModel(TicketDto ticket, QuotationDto quo List items = priceItems.stream().map(this::toLegacyRenderItem).toList(); LocalDate offerDate = quotation.offerDate() != null ? quotation.offerDate() : issueDate; - int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : 50; + int depositPct = quotation.depositPercent() != null ? quotation.depositPercent() : LEGACY_DEPOSIT_PERCENT; int deliveryDays = quotation.deliveryLeadDays() != null ? quotation.deliveryLeadDays() : 90; List remarkLines = List.of( "1.จำนวนที่เสนอข้างต้นเป็นจำนวนที่ได้รับมาเมื่อวันที่ " + shortThaiDate(offerDate), diff --git a/backend/src/main/java/th/co/glr/hr/ticket/TicketRepository.java b/backend/src/main/java/th/co/glr/hr/ticket/TicketRepository.java index df2a0447c..7b26c408f 100644 --- a/backend/src/main/java/th/co/glr/hr/ticket/TicketRepository.java +++ b/backend/src/main/java/th/co/glr/hr/ticket/TicketRepository.java @@ -888,12 +888,14 @@ private List findItemsByTicketId(long ticketId) { * own Javadoc on why this must not reach {@link #findById}/mutation call sites) falls back to * the deal's live quotation's line items, rendered READ-ONLY. * - *

"Live" is picked with the EXACT SAME rule as {@code DepositNoticeRepository - * #findDealQuotationItemsForDeposit} (ACCEPTED beats ISSUED beats APPROVED, across both - * PRICING_REQUEST and DEAL_DIRECT origins, most recent id wins a tie) -- duplicated here - * rather than called cross-package because deposit-notice and ticket-detail are separate - * bounded contexts, but it is the SAME business rule and must be changed in lockstep if that - * one ever changes. + *

"Live" is a display pick over new-engine rows only: ACCEPTED beats ISSUED beats APPROVED, + * across both PRICING_REQUEST and DEAL_DIRECT origins, most recent id wins a tie. It is NOT the + * deposit notice's pick ({@code DepositNoticeRepository#findGoverningQuotation}), which also + * reads legacy rows and ranks a DEAL_DIRECT APPROVED row as answered, level with ACCEPTED. The + * two agree on a ticket that holds one live new-engine quotation, which {@code + * DealQuotationRepository#supersedeOtherLiveOnTicket} keeps true since 2026-10-17; they can + * differ on a ticket with a live legacy quotation, or on older data that still holds both a + * DEAL_DIRECT APPROVED and a PRICING_REQUEST ISSUED row. * *

Only brand/model/color/texture/size/qty/qtySqm/unitBasis/rawUnit are mapped -- pricing * fields (proposedPrice/approvedPrice/calcedCost/manualPrice/...) are deliberately left at diff --git a/backend/src/test/java/th/co/glr/hr/ticket/TicketRepositoryIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/ticket/TicketRepositoryIntegrationTest.java index 9f72413b1..95ca5edb2 100644 --- a/backend/src/test/java/th/co/glr/hr/ticket/TicketRepositoryIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/ticket/TicketRepositoryIntegrationTest.java @@ -694,8 +694,8 @@ void addEventWithDocument_persistsTheLinkAndRejectsAHalfSetOne() { } // ── B5 fix: findPricingChainFallbackItems — the read-only display fallback for a deal - // whose sales.ticket_item is still empty (pre-order-confirmation), mirroring - // DepositNoticeRepository#findDealQuotationItemsForDeposit's live-quotation pick. + // whose sales.ticket_item is still empty (pre-order-confirmation), picking the deal's + // live new-engine quotation. // Real Postgres only: the priority CASE/ORDER BY and the origin/doc_status filters // cannot be proven against a mocked repository. ───────────────────────────────────── @@ -732,8 +732,7 @@ void findPricingChainFallbackItems_prefersAcceptedOverIssued_acrossOrigins() { insertQuotationWithItem(ticketId, "PRICING_REQUEST", "ISSUED", "Stale", "Issued", null, null, null, new BigDecimal("1")); // ...and a newer ACCEPTED DEAL_DIRECT-origin one. ACCEPTED must win regardless of - // which origin table it came from or insertion order (mirrors - // findDealQuotationItemsForDeposit's own CASE doc_status priority). + // which origin it came from or insertion order (the method's own CASE doc_status priority). insertQuotationWithItem(ticketId, "DEAL_DIRECT", "ACCEPTED", "Live", "Accepted", null, null, null, new BigDecimal("2")); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index c65a6acb6..fe3ba4c7d 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1455,22 +1455,27 @@ function mockDepositNoticeItemsFromDealQuotation(row) { }); } -// 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 -// contract as CustomerQuotationRepository.findByTicket — then a single forward scan overwriting -// "latest seen" per status is equivalent to sorting descending and taking the first match. -function mockPickTicketQuotationForDepositNotice(ticketId) { - const candidates = mockCustomerQuotations - .filter((q) => q.ticketId === Number(ticketId) && q.pricingRequestId != null) - .sort((a, b) => (a.quotationRevisionNo - b.quotationRevisionNo) || (a.id - b.id)); - let latestAccepted = null; - let latestIssued = null; - for (const q of candidates) { - if (q.docStatus === 'ACCEPTED') latestAccepted = q; - else if (q.docStatus === 'ISSUED') latestIssued = q; +// Mirrors DepositNoticeRepository#findGoverningQuotation: the quotation a ticket-level deposit +// notice bills (lines, percent, reference), across both engines. The latest one the customer +// answered (ACCEPTED, or a DEAL_DIRECT one APPROVED) wins, else the latest ISSUED one. +// ponytail: "latest" compares ids across mockCustomerQuotations and mockDealQuotations, two +// separate sequences where Java has one sales.quotation_id; good enough for single-engine mock deals. +function mockGoverningQuotationForDepositNotice(ticketId) { + const candidates = []; + for (const q of mockCustomerQuotations) { + if (q.ticketId !== ticketId || q.pricingRequestId == null) continue; + const rank = { ACCEPTED: 0, ISSUED: 1 }[q.docStatus]; + if (rank != null) candidates.push({ rank, quotation: q, legacy: true }); + } + for (const q of mockDealQuotations) { + if (q.ticketId !== ticketId) continue; + const answered = q.docStatus === 'ACCEPTED' + || ((q.origin || 'DEAL_DIRECT') === 'DEAL_DIRECT' && q.docStatus === 'APPROVED'); + const rank = answered ? 0 : { ISSUED: 1, APPROVED: 2 }[q.docStatus]; + if (rank != null) candidates.push({ rank, quotation: q, legacy: false }); } - return latestAccepted ?? latestIssued ?? null; + candidates.sort((a, b) => (a.rank - b.rank) || (b.quotation.id - a.quotation.id)); + return candidates[0] ?? null; } // B5 fix ("รายการสินค้า shows 0 รายการ while a deal is still in the pricing/quotation phase"): @@ -1498,7 +1503,7 @@ function mockPickLiveQuotationForItemFallback(ticketId) { } if (candidates.length === 0) return null; // ACCEPTED beats ISSUED beats APPROVED (rank ascending); most recent id wins a tie -- same - // priority findDealQuotationItemsForDeposit's CASE doc_status / ORDER BY quotation_id DESC + // priority findPricingChainFallbackItems's CASE doc_status / ORDER BY quotation_id DESC // encodes server-side. candidates.sort((a, b) => (a.rank - b.rank) || (b.id - a.id)); return candidates[0]; @@ -7155,7 +7160,10 @@ function resolveDealQuotationV3Header(payload, current = null) { // Mirrors DealQuotationRenderAdapter.DEFAULT_DEPOSIT_PERCENT: the percent a quotation's PDF // prints, and its deposit notice bills, when its depositPercent is null. -const MOCK_DEFAULT_DEPOSIT_PERCENT = 30; +export const MOCK_DEFAULT_DEPOSIT_PERCENT = 30; +// Mirrors QuotationRenderer.LEGACY_DEPOSIT_PERCENT: a legacy quotation's PDF always prints 50%, +// so its deposit notice bills 50% whatever percent was typed. +export const MOCK_LEGACY_DEPOSIT_PERCENT = 50; /** * Item 4 (V181, "ไม่รับมัดจำ", owner ruling 2026-09-16) — mirrors @@ -13285,23 +13293,21 @@ export const api = { const desc = [it.brand, it.model, it.color, it.texture, it.size].filter(Boolean).join(' '); return { seq: idx + 1, description: desc, qty: Number(it.qty), unit: 'แผ่น', unitPrice: Number(it.approvedPrice), discountLabel: null, netUnitPrice: Number(it.approvedPrice) }; }); - // 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). + // Every deal created through the pricing-request chain has approved_price = NULL on every + // ticket_item, so legacyItems above is empty for it: the governing quotation supplies the + // lines, and always the percent and reference (DepositNoticeService.createDraftFromQuotation). + const governing = mockGoverningQuotationForDepositNotice(Number(ticketId)); let items = payload.items?.length ? payload.items : legacyItems; - if (!items.length) { - const quotation = mockPickTicketQuotationForDepositNotice(ticketId); - if (quotation) items = mockDepositNoticeItemsFromQuotation(quotation.items); + if (!items.length && governing) { + items = governing.legacy + ? mockDepositNoticeItemsFromQuotation(governing.quotation.items) + : mockDepositNoticeItemsFromDealQuotation(governing.quotation); } - if (!items.length) { - // 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); - } + // A typed percent is ignored, as in Java; 0.5 only when the ticket has no live quotation. + let depositPercent = 0.5; + if (governing) { + depositPercent = (governing.legacy ? MOCK_LEGACY_DEPOSIT_PERCENT + : governing.quotation.depositPercent ?? MOCK_DEFAULT_DEPOSIT_PERCENT) / 100; } // Branch fix: header autofill (mirrors DepositNoticeService.createDraft) — a caller- @@ -13317,8 +13323,8 @@ export const api = { version: nextVer, docNumber: null, issueDate: null, status: 'DRAFT', customerName: payload.customerName ?? ticket.customerName ?? '', customerTaxId: customerTaxId ?? '', customerAddress: customerAddress ?? '', - projectName: projectName ?? '', reference: payload.reference ?? '', - depositPercent: payload.depositPercent ?? 0.5, vatPercent: 0.07, + projectName: projectName ?? '', reference: payload.reference ?? governing?.quotation.number ?? '', + depositPercent, vatPercent: 0.07, notes, items, issuedByName: null, preparerName: 'จินตนา หาญมนตรี', hasPdf: false, hasXlsx: false, createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), @@ -16191,7 +16197,7 @@ export const api = { return delay({ result: { ticket: buildTicketDetail(ticket), pricingRequest: buildPricingRequestSummary(pr) } }); }, - async createDepositNoticeFromQuotation(id, payload = {}) { + async createDepositNoticeFromQuotation(id, payload = {}) { // eslint-disable-line no-unused-vars -- hrApi arity; Java ignores the typed percent const user = hasRole('sales'); const pr = findPricingRequestRaw(id); const ticket = db.tickets.find((t) => t.id === pr.ticketId); @@ -16235,12 +16241,11 @@ export const api = { customerName: ticket.customerName ?? '', customerTaxId: customerTaxId ?? '', customerAddress: customerAddress ?? '', projectName: projectName ?? '', reference, - // Mirrors OrderConfirmationService.createDepositNoticeFromQuotation: the new-engine branch - // bills the chosen quotation's own whole-number percent, a null one as the PDF's default; - // the legacy branch keeps the old stand-in. - depositPercent: newEngine - ? (newEngine.depositPercent ?? MOCK_DEFAULT_DEPOSIT_PERCENT) / 100 - : payload.depositPercent ?? 0.5, + // Mirrors OrderConfirmationService.createDepositNoticeFromQuotation: the percent the chosen + // quotation's PDF printed; a typed payload.depositPercent is ignored. + depositPercent: (newEngine + ? newEngine.depositPercent ?? MOCK_DEFAULT_DEPOSIT_PERCENT + : MOCK_LEGACY_DEPOSIT_PERCENT) / 100, vatPercent: 0.07, notes, items, issuedByName: null, preparerName: 'จินตนา หาญมนตรี', hasPdf: false, hasXlsx: false, diff --git a/frontend/src/data/demoSales.js b/frontend/src/data/demoSales.js index 5da2c0e58..cf50b4ed2 100644 --- a/frontend/src/data/demoSales.js +++ b/frontend/src/data/demoSales.js @@ -1218,7 +1218,7 @@ export function buildDemoSalesSeed() { // FactoryPurchaseOrder CANCELLED; DepositNotice DRAFT (pre-drafted off the accepted // quotation while the deposit is awaited — D4 fix: was built from quotation19's // REVISION_REQUESTED doc, which neither createDepositNoticeFromQuotation (mockApi.js's - // hard 409) nor mockPickTicketQuotationForDepositNotice's ACCEPTED->ISSUED fallback would + // hard 409) nor mockGoverningQuotationForDepositNotice's answered->ISSUED fallback would // ever ACTUALLY pick, so that row could not exist in the real app). const pr20 = makePr({ ticketId: 18, recipientType: 'BUYER', recipientLabel: 'ผู้รับเหมา Fashion Island', diff --git a/frontend/src/features/tickets/DealDepositPanel.jsx b/frontend/src/features/tickets/DealDepositPanel.jsx index 1e0a78313..178855392 100644 --- a/frontend/src/features/tickets/DealDepositPanel.jsx +++ b/frontend/src/features/tickets/DealDepositPanel.jsx @@ -319,7 +319,7 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ {/* Owner ruling (2026-09-29): the deposit % is LOCKED to the accepted quotation's own deposit_percent — createDraft ignores any client-supplied value and always re-sources it from the quotation (see DepositNoticeService.createDraft / - DepositNoticeRepository#findDealQuotationDepositPercent). The % input that used + DepositNoticeRepository#findGoverningQuotation). The % input that used to sit here is gone — there is nothing left for it to control. */}