diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeController.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeController.java index 34756b3b1..10819741e 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeController.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeController.java @@ -98,9 +98,11 @@ ResponseEntity preview(@PathVariable long docId, HttpSession session) { // Issue (assign doc number + transition ticket) @PostMapping("/deposit-notices/{docId}/issue") - Map issue(@PathVariable long docId, HttpSession session) { + Map issue(@PathVariable long docId, + @RequestBody(required = false) DepositNoticeIssueRequest body, + HttpSession session) { UserPrincipal user = sessions.requireUser(session); - return Map.of("depositNotice", service.issue(docId, user)); + return Map.of("depositNotice", service.issue(docId, body == null ? null : body.expectedUpdatedAt(), user)); } // File download diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeDraftRequest.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeDraftRequest.java index 336c68813..dd0561ece 100644 --- a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeDraftRequest.java +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeDraftRequest.java @@ -5,6 +5,7 @@ import jakarta.validation.constraints.DecimalMin; import jakarta.validation.constraints.Size; import java.math.BigDecimal; +import java.time.OffsetDateTime; import java.util.List; public record DepositNoticeDraftRequest( @@ -13,7 +14,17 @@ public record DepositNoticeDraftRequest( @Size(max = 255) String customerAddress, @Size(max = 255) String projectName, @Size(max = 255) String reference, - @DecimalMin("0") @DecimalMax("1") BigDecimal depositPercent, + @DecimalMin(value = "0", inclusive = false, message = "เปอร์เซ็นต์มัดจำต้องมากกว่า 0") + @DecimalMax(value = "1", message = "เปอร์เซ็นต์มัดจำต้องไม่เกิน 100") BigDecimal depositPercent, List notes, - @Valid List items -) {} + @Valid List items, + // The draft's updatedAt as this client last read it. null skips the stale-save check (older clients). + OffsetDateTime expectedUpdatedAt +) { + /** Without a stamp: creation and the server-side callers. */ + public DepositNoticeDraftRequest(String customerName, String customerTaxId, String customerAddress, + String projectName, String reference, BigDecimal depositPercent, + List notes, List items) { + this(customerName, customerTaxId, customerAddress, projectName, reference, depositPercent, notes, items, null); + } +} diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeIssueRequest.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeIssueRequest.java new file mode 100644 index 000000000..681ef084a --- /dev/null +++ b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeIssueRequest.java @@ -0,0 +1,6 @@ +package th.co.glr.hr.deposit; + +import java.time.OffsetDateTime; + +/** Optional body of the issue call: the draft's updatedAt as the caller last read it (null skips the check). */ +public record DepositNoticeIssueRequest(OffsetDateTime expectedUpdatedAt) {} 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 271ca9750..634932892 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 @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import org.springframework.http.HttpStatus; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; import org.springframework.jdbc.support.GeneratedKeyHolder; @@ -23,6 +24,9 @@ @Repository public class DepositNoticeRepository { + static final String NOT_DRAFT = "ใบแจ้งรับมัดจำนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)"; + static final String EDITED_ELSEWHERE = "ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง"; + private final NamedParameterJdbcTemplate jdbc; public DepositNoticeRepository(NamedParameterJdbcTemplate jdbc) { @@ -199,14 +203,22 @@ public long createDraft(long ticketId, DepositNoticeDraftRequest req, List storedItems) { + BigDecimal depositPct = req.depositPercent(); BigDecimal vatPct = new BigDecimal("0.07"); List items = req.items() != null ? req.items() : List.of(); - BigDecimal subtotal = items.stream() - .map(it -> it.netUnitPrice().multiply(it.qty())) + // No items in the request (null or empty) keeps the stored lines (they are only replaced + // below when items are sent), so the subtotal must come from those same stored lines. + BigDecimal subtotal = (items.isEmpty() + ? storedItems.stream().map(it -> it.netUnitPrice().multiply(it.qty())) + : items.stream().map(it -> it.netUnitPrice().multiply(it.qty()))) .reduce(BigDecimal.ZERO, BigDecimal::add) .setScale(2, RoundingMode.HALF_UP); BigDecimal deposit = subtotal.multiply(depositPct).setScale(2, RoundingMode.HALF_UP); @@ -215,7 +227,7 @@ public void update(long docId, DepositNoticeDraftRequest req) { String[] notesArr = req.notes() != null ? req.notes().toArray(String[]::new) : new String[0]; - jdbc.update(""" + int rows = jdbc.update(""" UPDATE sales.deposit_notice SET customer_name = :customerName, customer_tax_id = :customerTaxId, @@ -230,9 +242,11 @@ public void update(long docId, DepositNoticeDraftRequest req) { notes = :notes, updated_at = now() WHERE deposit_notice_id = :id AND status = 'DRAFT' + AND (CAST(:expectedUpdatedAt AS timestamptz) IS NULL OR updated_at = :expectedUpdatedAt) """, new MapSqlParameterSource() .addValue("id", docId) + .addValue("expectedUpdatedAt", req.expectedUpdatedAt()) .addValue("customerName", req.customerName()) .addValue("customerTaxId", req.customerTaxId()) .addValue("customerAddress", req.customerAddress()) @@ -245,6 +259,12 @@ public void update(long docId, DepositNoticeDraftRequest req) { .addValue("totalPayable", total) .addValue("notes", notesArr) ); + if (rows == 0) { + boolean stillDraft = req.expectedUpdatedAt() != null && !jdbc.queryForList( + "SELECT 1 FROM sales.deposit_notice WHERE deposit_notice_id = :id AND status = 'DRAFT'", + Map.of("id", docId), Integer.class).isEmpty(); + throw new th.co.glr.hr.common.ApiException(HttpStatus.CONFLICT, stillDraft ? EDITED_ELSEWHERE : NOT_DRAFT); + } if (!items.isEmpty()) { jdbc.update("DELETE FROM sales.deposit_notice_item WHERE deposit_notice_id = :id", Map.of("id", docId)); insertItems(docId, items); @@ -270,8 +290,12 @@ public void update(long docId, DepositNoticeDraftRequest req) { * * @return the new doc number, or {@link Optional#empty()} if the row was not DRAFT. */ - @Transactional public Optional issue(long docId, long actorId, String actorName) { + return issue(docId, actorId, actorName, null); + } + + @Transactional + public Optional issue(long docId, long actorId, String actorName, java.time.OffsetDateTime expectedUpdatedAt) { int thaiYear = Year.now().getValue() + 543; String docNumber = nextDocNumber("DEPOSIT_NOTICE", thaiYear); @@ -285,21 +309,23 @@ public Optional issue(long docId, long actorId, String actorName) { updated_at = now() WHERE deposit_notice_id = :id AND status = 'DRAFT' + AND (CAST(:expectedUpdatedAt AS timestamptz) IS NULL OR updated_at = :expectedUpdatedAt) """, - Map.of("num", docNumber, "actorId", actorId, "actorName", actorName, "id", docId)); + new MapSqlParameterSource() + .addValue("num", docNumber).addValue("actorId", actorId).addValue("actorName", actorName) + .addValue("id", docId).addValue("expectedUpdatedAt", expectedUpdatedAt)); if (rows == 0) { + // Still a draft with a stamp given => it was saved after the caller read it. + boolean editedSince = expectedUpdatedAt != null && !jdbc.queryForList( + "SELECT 1 FROM sales.deposit_notice WHERE deposit_notice_id = :id AND status = 'DRAFT'", + Map.of("id", docId), Integer.class).isEmpty(); + if (editedSince) { + throw new th.co.glr.hr.common.ApiException(HttpStatus.CONFLICT, EDITED_ELSEWHERE); + } return Optional.empty(); } - // Supersede all older versions for same ticket - jdbc.update(""" - UPDATE sales.deposit_notice SET status = 'SUPERSEDED', updated_at = now() - WHERE ticket_id = (SELECT ticket_id FROM sales.deposit_notice WHERE deposit_notice_id = :id) - AND deposit_notice_id <> :id - AND status = 'ISSUED' - """, Map.of("id", docId)); - return Optional.of(docNumber); } @@ -365,14 +391,23 @@ private void insertItems(long docId, List items) { * items. Reads the RESOLVED display columns the quotation itself prints * ({@code qty}/{@code raw_unit}/{@code unit_price}/{@code final_unit_price}), NOT the legacy * {@code requested_*}/{@code approved_unit_price} columns the V2 chain leaves null. The - * discount-label wording matches {@link DepositNoticeService#itemsFromQuotation}. */ + * discount-label wording matches {@link DepositNoticeService#itemsFromQuotation}. An ADJUSTMENT + * row is folded into the goods lines, see {@link #foldAdjustments}. */ public List findDealQuotationItems(long quotationId) { - return jdbc.query(""" - SELECT seq, description, qty, raw_unit, unit_price, sales_discount, final_unit_price + List goods = new java.util.ArrayList<>(); + BigDecimal[] adjustment = {BigDecimal.ZERO}; + jdbc.query(""" + SELECT seq, description, qty, raw_unit, unit_price, sales_discount, final_unit_price, line_type FROM sales.quotation_item WHERE quotation_id = :quotationId ORDER BY seq - """, Map.of("quotationId", quotationId), (rs, i) -> { + """, Map.of("quotationId", quotationId), rs -> { + if ("ADJUSTMENT".equals(rs.getString("line_type"))) { + BigDecimal net = rs.getBigDecimal("final_unit_price"); + BigDecimal qty = rs.getBigDecimal("qty"); + if (net != null && qty != null) adjustment[0] = adjustment[0].add(net.multiply(qty)); + return; + } String description = rs.getString("description"); if (description == null || description.isBlank()) { description = "รายการสินค้า"; @@ -385,10 +420,49 @@ public List findDealQuotationItems(long quotationId) { String discountLabel = discount != null && discount.signum() > 0 ? "ส่วนลด " + discount.stripTrailingZeros().toPlainString() + " ต่อหน่วย" : null; - return new DepositNoticeItemRequest( + goods.add(new DepositNoticeItemRequest( rs.getInt("seq"), description, rs.getBigDecimal("qty"), unit, - rs.getBigDecimal("unit_price"), discountLabel, rs.getBigDecimal("final_unit_price")); + rs.getBigDecimal("unit_price"), discountLabel, rs.getBigDecimal("final_unit_price"))); }); + return foldAdjustments(goods, adjustment[0]); + } + + /** + * Owner rulings 2026-10-23: a DEAL_DIRECT ส่วนลดพิเศษ row (ADJUSTMENT, qty -1) cannot be a + * deposit line (the editor's PUT requires qty >= 0), so its amount is spread over the goods + * lines in proportion to their amount. "2 decimals for all": each share is HALF_UP to 2 dp and + * each line's new net price is 2 dp, with no remainder pushed onto a last line. The notice + * totals are the sum of its own stored 2 dp lines ({@code amount = net x qty}), so they can + * differ from the quotation's total by under qty x 0.005 baht per line; that is the ruled + * outcome, not a bug. A line keeps the quotation's own discount wording, and gains the folded + * discount after it only when the fold lowered its price. A free line stays as it is. + */ + static List foldAdjustments(List goods, + BigDecimal adjustment) { + BigDecimal goodsTotal = goods.stream().map(it -> it.netUnitPrice().multiply(it.qty())) + .reduce(BigDecimal.ZERO, BigDecimal::add); + if (adjustment.signum() == 0 || goodsTotal.signum() == 0) { + return goods; + } + List folded = new java.util.ArrayList<>(); + for (DepositNoticeItemRequest it : goods) { + BigDecimal amount = it.netUnitPrice().multiply(it.qty()); + if (amount.signum() == 0) { + folded.add(it); + continue; + } + BigDecimal share = adjustment.multiply(amount).divide(goodsTotal, 2, RoundingMode.HALF_UP); + BigDecimal net = amount.add(share).divide(it.qty(), 2, RoundingMode.HALF_UP); + BigDecimal foldedPerUnit = it.netUnitPrice().subtract(net); + String label = it.discountLabel(); + if (foldedPerUnit.signum() > 0) { + String extra = "ส่วนลดพิเศษ " + foldedPerUnit.stripTrailingZeros().toPlainString() + " ต่อหน่วย"; + label = label == null || label.isBlank() ? extra : label + " + " + extra; + } + folded.add(new DepositNoticeItemRequest(it.seq(), it.description(), it.qty(), it.unit(), it.unitPrice(), + label, net)); + } + return folded; } private List findItems(long docId) { 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 b042d76a1..84afd8e65 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 @@ -103,6 +103,12 @@ public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDra Integer sourceDepositPercent, UserPrincipal actor) { requireRole(actor, SALES_ROLES); requireTicketOwner(ticketId, actor); + // Owner ruling 2026-10-23: one deposit notice per deal. A new draft would re-seed the % from + // the quotation and, once issued, silently supersede the customer's document. + docs.findByTicket(ticketId).stream().filter(d -> "ISSUED".equals(d.status())).findFirst().ifPresent(d -> { + throw new ApiException(HttpStatus.CONFLICT, + "ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ " + d.docNumber() + " แล้ว จึงสร้างฉบับร่างใหม่ไม่ได้"); + }); TicketSummaryDto s = requireApprovedTicket(ticketId, actor); requireActiveLifecycle(s); // Both routes: a deal off the deposit path (a ไม่รับมัดจำ quotation, a waiver, a credit @@ -149,7 +155,8 @@ public DepositNoticeDto createDraftFromQuotation(long ticketId, DepositNoticeDra // req.depositPercent() is never read (DepositNoticeDraftRequest carries it for update's // round trip). 0.50 only when the ticket has no live quotation at all (e.g. a manually // approved legacy ticket with no quotation chain); see - // DepositNoticeRepository#findGoverningQuotation for the pick. + // DepositNoticeRepository#findGoverningQuotation for the pick. Sales may change the percent + // on the DRAFT afterwards (update, ruling 2026-10-23). BigDecimal effectiveDepositPercent = sourceDepositPercent != null ? BigDecimal.valueOf(sourceDepositPercent, 2) : governing != null ? governing.depositPercent() : new BigDecimal("0.50"); @@ -175,20 +182,29 @@ public static ApiException zeroDepositRefusal(String quotationNumber) { @Transactional public DepositNoticeDto update(long docId, DepositNoticeDraftRequest req, UserPrincipal actor) { + requireRole(actor, SALES_ROLES); DepositNoticeDto doc = requireDraft(docId); requireTicketOwner(doc.ticketId(), actor); - // Same owner ruling as createDraft above: the deposit percent is locked to the quotation - // at draft-creation time and must NEVER change on update. The frontend no longer offers a - // control for it, but a client (or a stale/forged request) sending a different value must - // not be trusted either — override whatever req carries with the draft's own already- - // stored depositPercent() before persisting. + // Owner ruling 2026-10-23: sales may change the deposit percent on a DRAFT (requireDraft + // above 409s an issued notice). docs.update recomputes the amount from it. A request + // without a percent keeps the draft's own. + BigDecimal depositPercent = req.depositPercent() != null ? req.depositPercent() : doc.depositPercent(); + if (depositPercent.signum() <= 0) { + throw new ApiException(HttpStatus.BAD_REQUEST, "เปอร์เซ็นต์มัดจำต้องมากกว่า 0"); + } + // Whole percents only: the document prints the percent rounded (40.5% shows as "41%"). A + // percent stored before that rule (0.3330) still saves while it is sent back unchanged; + // issue() refuses it until sales picks a whole one. + if (depositPercent.compareTo(doc.depositPercent()) != 0 && !isWholePercent(depositPercent)) { + throw new ApiException(HttpStatus.BAD_REQUEST, "เปอร์เซ็นต์มัดจำต้องเป็นจำนวนเต็ม"); + } var effective = new DepositNoticeDraftRequest( req.customerName(), req.customerTaxId(), req.customerAddress(), req.projectName(), req.reference(), - doc.depositPercent(), - req.notes(), req.items() + depositPercent, + req.notes(), req.items(), req.expectedUpdatedAt() ); - docs.update(docId, effective); + docs.update(docId, effective, doc.items()); return docs.findById(docId).orElseThrow(); } @@ -203,11 +219,25 @@ public String preview(long docId, UserPrincipal actor) { } // Issue: assign doc number, freeze document, transition ticket to document_issued + // Without a stamp. @Transactional here too: a self-call from this overload would bypass the proxy. @Transactional public DepositNoticeDto issue(long docId, UserPrincipal actor) { + return issue(docId, null, actor); + } + + @Transactional + public DepositNoticeDto issue(long docId, java.time.OffsetDateTime expectedUpdatedAt, UserPrincipal actor) { DepositNoticeDto doc = requireDraft(docId); requireRole(actor, SALES_ROLES); requireTicketOwner(doc.ticketId(), actor); + // The stored percent is what the document prints: outside a whole 1-100 (a 0% ticket-route + // draft, a 0.3330 typed before whole percents) it takes no number until fixed on the draft. + BigDecimal pct = doc.depositPercent(); + if (pct.signum() <= 0 || pct.compareTo(BigDecimal.ONE) > 0 || !isWholePercent(pct)) { + throw new ApiException(HttpStatus.CONFLICT, "ใบแจ้งรับมัดจำนี้มีเปอร์เซ็นต์มัดจำ " + + pct.movePointRight(2).stripTrailingZeros().toPlainString() + + "% จึงออกเอกสารไม่ได้ (ต้องเป็นจำนวนเต็ม 1–100)"); + } // Issuing the deposit-notice DOCUMENT is the payment-track step: it requires a // customer-confirmed quotation and advances paymentStatus, leaving the main @@ -244,11 +274,21 @@ public DepositNoticeDto issue(long docId, UserPrincipal actor) { if (doc.depositAmount() == null || doc.depositAmount().signum() <= 0) { throw new ApiException(HttpStatus.CONFLICT, "ใบแจ้งรับมัดจำนี้มียอดมัดจำ 0 บาท จึงออกเอกสารไม่ได้"); } + // The guard below is check-then-act: lock the deal row first so two drafts issued at once + // serialize, and the second one sees the first as ISSUED. + tickets.lockTicketForUpdate(doc.ticketId()); + // Owner ruling 2026-10-23: one deposit notice per deal. A second draft (written before the + // first was issued) must not silently supersede the customer's document. + docs.findByTicket(doc.ticketId()).stream() + .filter(d -> d.id() != docId && "ISSUED".equals(d.status())).findFirst().ifPresent(d -> { + throw new ApiException(HttpStatus.CONFLICT, + "ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ " + d.docNumber() + " แล้ว จึงออกฉบับร่างนี้ไม่ได้"); + }); // Issuing advances the payment track — a paused/terminal deal must not move // (Phase 1 lifecycle gate; mirrors TicketService.requireActive). requireActiveLifecycle(s); - String docNumber = docs.issue(docId, actor.id(), actor.name()) + String docNumber = docs.issue(docId, actor.id(), actor.name(), expectedUpdatedAt) .orElseThrow(() -> new ApiException(HttpStatus.CONFLICT, "ใบแจ้งรับมัดจำนี้ถูกออกไปแล้วโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง")); @@ -1120,7 +1160,7 @@ private DepositNoticeDto requireDraft(long docId) { DepositNoticeDto doc = docs.findById(docId) .orElseThrow(() -> new ApiException(HttpStatus.NOT_FOUND, "ไม่พบใบแจ้งรับมัดจำนี้")); if (!"DRAFT".equals(doc.status())) { - throw new ApiException(HttpStatus.CONFLICT, "ใบแจ้งรับมัดจำนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)"); + throw new ApiException(HttpStatus.CONFLICT, DepositNoticeRepository.NOT_DRAFT); } return doc; } @@ -1171,6 +1211,11 @@ private void requireRole(UserPrincipal actor, java.util.Set allowed) { } } + /** 0.40 is a whole 40%; 0.405 and 0.3330 are not. */ + private static boolean isWholePercent(BigDecimal fraction) { + return fraction.movePointRight(2).stripTrailingZeros().scale() <= 0; + } + private String blankToNull(String value) { return value == null || value.isBlank() ? null : value; } diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java index d615fff84..9a5c6e4a6 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java @@ -34,6 +34,8 @@ import th.co.glr.hr.dealquotation.DealQuotationRequests.UpsertDealQuotationRequest; import th.co.glr.hr.deposit.DepositNoticeDraftRequest; import th.co.glr.hr.deposit.DepositNoticeDto; +import th.co.glr.hr.deposit.DepositNoticeItemDto; +import th.co.glr.hr.deposit.DepositNoticeItemRequest; import th.co.glr.hr.deposit.DepositNoticeRenderer; import th.co.glr.hr.deposit.DepositNoticeRepository; import th.co.glr.hr.deposit.DepositNoticeService; @@ -252,7 +254,30 @@ void quotationFirstGhost_stillConfirms_landsAtOrderReceived_withQuotationLines_a DepositNoticeDto draft = depositNoticeService.createDraft(ghost, new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor); assertThat(draft.ticketId()).isEqualTo(ghost); - assertThat(draft.items()).isNotEmpty(); + // Owner ruling 2026-10-23: the ส่วนลดพิเศษ ADJUSTMENT row (qty -1, -100.00) is folded into the + // goods lines by amount: TILE 10 x 100 and PLAIN 2 x 500 take 50.00 each. No negative qty, the + // descriptions are the goods lines' own, and the subtotal is the quotation's 1,900. + assertThat(draft.items()) + .extracting(i -> i.qty().stripTrailingZeros().toPlainString(), i -> i.unitPrice().toPlainString(), + i -> i.netUnitPrice().toPlainString(), DepositNoticeItemDto::discountLabel) + .containsExactly( + org.assertj.core.groups.Tuple.tuple("10", "100.00", "95.00", "ส่วนลดพิเศษ 5 ต่อหน่วย"), + org.assertj.core.groups.Tuple.tuple("2", "500.00", "475.00", "ส่วนลดพิเศษ 25 ต่อหน่วย")); + assertThat(draft.items()).extracting(DepositNoticeItemDto::description).doesNotContain("ส่วนลดพิเศษ") + .last().isEqualTo("ค่าขนส่ง"); + assertThat(draft.subtotal()).isEqualByComparingTo("1900.00"); + + // The page sends every line back on save; @PositiveOrZero qty used to refuse the whole PUT. + DepositNoticeDraftRequest pagePut = new DepositNoticeDraftRequest( + draft.customerName(), draft.customerTaxId(), draft.customerAddress(), draft.projectName(), + draft.reference(), draft.depositPercent(), draft.notes(), + draft.items().stream().map(i -> new DepositNoticeItemRequest(i.seq(), i.description(), i.qty(), + i.unit(), i.unitPrice(), i.discountLabel(), i.netUnitPrice())).toList(), + draft.updatedAt()); + try (var factory = jakarta.validation.Validation.buildDefaultValidatorFactory()) { + assertThat(factory.getValidator().validate(pagePut)).isEmpty(); + } + assertThat(depositNoticeService.update(draft.id(), pagePut, salesActor).subtotal()).isEqualByComparingTo("1900.00"); } /** The same deposit-notice draft is REFUSED before confirmation — the negative half of "the money 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 504680c7c..d406b1301 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 @@ -580,7 +580,7 @@ void issue_zeroAmountDraft_409_staysDraft_andConsumesNoNumber() { assertThatThrownBy(() -> depositNoticeService.issue(draft.id(), salesActor)) .isInstanceOf(ApiException.class) .hasFieldOrPropertyWithValue("status", HttpStatus.CONFLICT) - .hasMessageContaining("0 บาท"); + .hasMessageContaining("0%"); Map row = jdbc.queryForMap( "SELECT status, doc_number FROM sales.deposit_notice WHERE deposit_notice_id = :id", Map.of("id", draft.id())); diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeAccountNotificationIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeAccountNotificationIntegrationTest.java index 307817868..fa0d21a79 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeAccountNotificationIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeAccountNotificationIntegrationTest.java @@ -276,42 +276,6 @@ void issue_secondCallOnAnAlreadyIssuedDocIsRefused_andRaisesNoSecondAccountNotif assertThat(mailer.sent()).isEmpty(); } - // ───────────────────────────────────────────────────────────────────────────────────── - // Revision re-issue (the DEPOSIT_NOTICE_ISSUED self-loop) notifies account again - // ───────────────────────────────────────────────────────────────────────────────────── - - @Test - void issue_revisionReissue_notifiesAccountAgainMarkedAsARevision() { - DraftFixture fixture = buildDraftDepositNotice(); - depositNoticeService.issue(fixture.docId(), salesActor); - assertThat(countAccountNotificationRows()).isEqualTo(1); - mailer.sent.clear(); - - // Repository-direct on purpose: DepositNoticeService.createDraft only ever sources the - // FIRST draft from the ticket's approved items/quotation, so building a literal SECOND - // draft (the revision) this way is the shortest path to the state under test here, and - // it is only issue() itself (called through the real service below) that this test cares - // about exercising faithfully. - long secondDocId = new DepositNoticeRepository(jdbc).createDraft(fixture.ticketId(), - new DepositNoticeDraftRequest("ACME Revision", "0100000000000", "Bangkok", "Showroom", "REF-REV", - new BigDecimal("0.50"), List.of(), null), - List.of(new DepositNoticeItemRequest( - 1, "Revision item", new BigDecimal("1"), "แผ่น", - new BigDecimal("100.00"), null, new BigDecimal("100.00")))); - - DepositNoticeDto revisionIssued = depositNoticeService.issue(secondDocId, salesActor); - - assertThat(countAccountNotificationRows()).isEqualTo(2); - String secondMessage = (String) jdbc.queryForMap(""" - SELECT message FROM hr.notification - WHERE employee_id = :accountId AND type = 'DEPOSIT_NOTICE_ISSUED' - ORDER BY notification_id DESC LIMIT 1 - """, Map.of("accountId", accountEmployeeId)).get("message"); - assertThat(secondMessage).contains("ฉบับแก้ไข"); - assertThat(secondMessage).contains(revisionIssued.docNumber()); - assertThat(mailer.recipients()).contains("account@glr.co.th"); - } - // ───────────────────────────────────────────────────────────────────────────────────── // Rollback: the critical AfterCommit trap this branch exists to avoid regressing // ───────────────────────────────────────────────────────────────────────────────────── diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeControllerTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeControllerTest.java index e71523e65..182d821bf 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeControllerTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeControllerTest.java @@ -1,19 +1,27 @@ package th.co.glr.hr.deposit; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.math.BigDecimal; +import java.time.Instant; import java.time.LocalDate; import java.util.List; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.springframework.http.HttpHeaders; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpSession; @@ -124,6 +132,42 @@ void createDraftRejectsInvalidDepositPercent() throws Exception { .andExpect(status().isBadRequest()); } + @Test + void updateRejectsADepositPercentOutsideZeroTo100WithAThaiMessage() throws Exception { + for (String[] c : new String[][] { + {"1.5", "depositPercent เปอร์เซ็นต์มัดจำต้องไม่เกิน 100"}, + {"-0.05", "depositPercent เปอร์เซ็นต์มัดจำต้องมากกว่า 0"}, + {"0", "depositPercent เปอร์เซ็นต์มัดจำต้องมากกว่า 0"}}) { + mvc.perform(put("/api/deposit-notices/99") + .session(session()) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"depositPercent\": " + c[0] + "}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.message").value(c[1])); + } + verify(service, never()).update(anyLong(), any(), any()); + } + + // The record has a second, 8-argument constructor: the JSON must still bind through the + // canonical one, so the page's stamp reaches the service at full precision. + @Test + void updatePassesTheExpectedUpdatedAtStampThrough() throws Exception { + when(service.update(eq(99L), any(), any(UserPrincipal.class))).thenReturn(document()); + + mvc.perform(put("/api/deposit-notices/99") + .session(session()) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"depositPercent": 0.4, "expectedUpdatedAt": "2026-10-23T08:57:19.732123Z"} + """)) + .andExpect(status().isOk()); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DepositNoticeDraftRequest.class); + verify(service).update(eq(99L), captor.capture(), any(UserPrincipal.class)); + assertThat(captor.getValue().expectedUpdatedAt().toInstant()) + .isEqualTo(Instant.parse("2026-10-23T08:57:19.732123Z")); + } + @Test void createDraftRejectsNegativeItemUnitPrice() throws Exception { String body = """ diff --git a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeIssueGuardIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeIssueGuardIntegrationTest.java index 42aaadd72..42d433225 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeIssueGuardIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/DepositNoticeIssueGuardIntegrationTest.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.math.BigDecimal; import java.time.LocalDate; +import java.time.OffsetDateTime; import java.util.List; import java.util.Map; import java.util.Optional; @@ -209,27 +210,178 @@ void issue_repositoryLevelCallOnAlreadyIssuedRowRefusesWithoutRemintingDocNumber assertThat(reread.status()).isEqualTo("ISSUED"); } + // The deal page's confirm dialog showed amounts read at T0; the draft was saved since. Issuing + // with the old stamp is refused (nothing numbered); with the current one it issues. Repository + // level: the service's one-per-deal guard would answer first on this fixture's ticket. @Test - void issue_secondDraftOnSameTicketSucceedsAndSupersedesTheFirst() { - // POSITIVE control: the guard must not block a genuine DRAFT -> ISSUED transition, and the - // pre-existing "supersede older ISSUED versions" behaviour in the same method must still - // work correctly with the new predicate in place. + void issue_withAStaleUpdatedAt_isRefused409_andNothingIsIssued() { + IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); + long draftId = depositNoticeRepository.createDraft(fixture.ticketId(), secondDraftHeader(), secondDraftItems()); + OffsetDateTime loaded = depositNoticeRepository.findById(draftId).orElseThrow().updatedAt(); + DepositNoticeDto saved = depositNoticeService.update(draftId, stampedRequest("0.40", secondDraftItems(), loaded), salesActor); + + assertThatThrownBy(() -> depositNoticeRepository.issue(draftId, salesRepId, "x", loaded)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo("ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง"); + }); + DepositNoticeDto reread = depositNoticeRepository.findById(draftId).orElseThrow(); + assertThat(reread.status()).isEqualTo("DRAFT"); + assertThat(reread.docNumber()).isNull(); + + assertThat(depositNoticeRepository.issue(draftId, salesRepId, "x", saved.updatedAt())).isPresent(); + } + + @Test + void issue_secondDraftOnSameTicket_refusedUntilBillable_thenStillRefusedWhileAnotherNoticeIsIssued() { IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); + // A stored percent outside whole 1-100 (a 0% ticket-route draft on a ไม่รับมัดจำ quotation, + // a pre-09-29 free-typed 0.3330, an out-of-range 1.5) or a 0-baht amount takes no number. long secondDocId = depositNoticeRepository.createDraft( - fixture.ticketId(), secondDraftHeader(), secondDraftItems()); - Optional secondDocNumber = depositNoticeRepository.issue(secondDocId, salesRepId, "Sales"); + fixture.ticketId(), draftHeader(BigDecimal.ZERO), secondDraftItems()); + assertIssueRefusedAndNothingChanged(fixture, secondDocId, + "ใบแจ้งรับมัดจำนี้มีเปอร์เซ็นต์มัดจำ 0% จึงออกเอกสารไม่ได้ (ต้องเป็นจำนวนเต็ม 1–100)"); + assertIssueRefusedAndNothingChanged(fixture, + depositNoticeRepository.createDraft(fixture.ticketId(), draftHeader(new BigDecimal("0.3330")), secondDraftItems()), + "ใบแจ้งรับมัดจำนี้มีเปอร์เซ็นต์มัดจำ 33.3% จึงออกเอกสารไม่ได้ (ต้องเป็นจำนวนเต็ม 1–100)"); + assertIssueRefusedAndNothingChanged(fixture, + depositNoticeRepository.createDraft(fixture.ticketId(), draftHeader(new BigDecimal("1.5")), secondDraftItems()), + "ใบแจ้งรับมัดจำนี้มีเปอร์เซ็นต์มัดจำ 150% จึงออกเอกสารไม่ได้ (ต้องเป็นจำนวนเต็ม 1–100)"); + assertIssueRefusedAndNothingChanged(fixture, + depositNoticeRepository.createDraft(fixture.ticketId(), secondDraftHeader(), List.of(new DepositNoticeItemRequest( + 1, "Free item", new BigDecimal("2"), "แผ่น", BigDecimal.ZERO, null, BigDecimal.ZERO))), + "ใบแจ้งรับมัดจำนี้มียอดมัดจำ 0 บาท จึงออกเอกสารไม่ได้"); + + // Owner ruling 2026-10-23: one deposit notice per deal. Even once the percent is billable, + // a SECOND draft cannot take a number while another notice is ISSUED (it used to silently + // supersede the customer's document): 409, the draft keeps no number, the first stays ISSUED. + depositNoticeService.update(secondDocId, secondDraftHeader(), salesActor); + assertIssueRefusedAndNothingChanged(fixture, secondDocId, + "ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ " + fixture.docNumber() + " แล้ว จึงออกฉบับร่างนี้ไม่ได้"); + } + + // Owner ruling 2026-10-23: one deposit notice per deal. While one is ISSUED, neither create route + // drafts another (re-creating re-seeded the % from the quotation and a later issue silently + // superseded the customer's document). Re-versioning an issued notice is a separate change. + @Test + void createDraft_isRefusedOnBothRoutesWhileAnIssuedNoticeExists_noNewRow() { + IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); + String message = "ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ " + fixture.docNumber() + " แล้ว จึงสร้างฉบับร่างใหม่ไม่ได้"; + + assertThatThrownBy(() -> depositNoticeService.createDraft(fixture.ticketId(), + new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), salesActor)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo(message); + }); + assertThatThrownBy(() -> orderConfirmation.createDepositNoticeFromQuotation(fixture.pricingRequestId(), + new CreateDepositNoticeFromQuotationRequest(null), salesActor)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo(message); + }); + + assertThat(depositNoticeRepository.findByTicket(fixture.ticketId())) + .extracting(DepositNoticeDto::id).containsExactly(fixture.docId()); + } - assertThat(secondDocNumber).isPresent(); - assertThat(secondDocNumber.get()).isNotEqualTo(fixture.docNumber()); + // Owner ruling 2026-10-23: sales may change the deposit % on a DRAFT; the amount follows. + @Test + void update_onADraft_changesThePercentAndTheAmountFollows() { + IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); + long draftId = depositNoticeRepository.createDraft( + fixture.ticketId(), secondDraftHeader(), secondDraftItems()); // 0.50 of 100.00 - DepositNoticeDto firstReread = depositNoticeRepository.findById(fixture.docId()).orElseThrow(); - assertThat(firstReread.status()).isEqualTo("SUPERSEDED"); - assertThat(firstReread.docNumber()).isEqualTo(fixture.docNumber()); + DepositNoticeDto updated = depositNoticeService.update(draftId, new DepositNoticeDraftRequest( + "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal("0.40"), List.of(), secondDraftItems()), salesActor); + + // secondDraftItems: net x qty sums to 75.02 (unit x qty 90.00, stored amounts 75.03, nets alone 43.34). + assertThat(updated.depositPercent()).isEqualByComparingTo("0.40"); + assertThat(updated.subtotal()).isEqualByComparingTo("75.02"); + assertThat(updated.depositAmount()).isEqualByComparingTo("30.01"); + assertThat(updated.vatAmount()).isEqualByComparingTo("2.10"); + assertThat(updated.totalPayable()).isEqualByComparingTo("32.11"); + assertThat(depositNoticeService.preview(draftId, salesActor)) + .contains("ขอรับเงินมัดจำ (40%)") + .contains("30.01 บาท"); + + // A request without items (null, or an empty list) keeps the stored lines; the amount is + // recomputed from them at the new percent. + DepositNoticeDto nullItems = depositNoticeService.update(draftId, new DepositNoticeDraftRequest( + "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal("0.30"), List.of(), null), salesActor); + assertThat(nullItems.items()).isEqualTo(updated.items()); + assertThat(nullItems.subtotal()).isEqualByComparingTo("75.02"); + assertThat(nullItems.depositAmount()).isEqualByComparingTo("22.51"); - DepositNoticeDto secondReread = depositNoticeRepository.findById(secondDocId).orElseThrow(); - assertThat(secondReread.status()).isEqualTo("ISSUED"); - assertThat(secondReread.docNumber()).isEqualTo(secondDocNumber.get()); + DepositNoticeDto emptyItems = depositNoticeService.update(draftId, new DepositNoticeDraftRequest( + "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal("0.20"), List.of(), List.of()), salesActor); + assertThat(emptyItems.items()).isEqualTo(updated.items()); + assertThat(emptyItems.subtotal()).isEqualByComparingTo("75.02"); + assertThat(emptyItems.depositAmount()).isEqualByComparingTo("15.00"); + } + + // Two tabs on one draft: both loaded the same updatedAt. A saves; B's save with that now-stale + // stamp is refused and changes nothing; reloading (the fresh stamp) saves. + @Test + void update_withAStaleUpdatedAt_isRefused409_andTheFirstSaveStands() { + IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); + long draftId = depositNoticeRepository.createDraft(fixture.ticketId(), secondDraftHeader(), secondDraftItems()); + OffsetDateTime loaded = depositNoticeRepository.findById(draftId).orElseThrow().updatedAt(); + + DepositNoticeDto savedByA = depositNoticeService.update(draftId, stampedRequest("0.40", secondDraftItems(), loaded), salesActor); + + List tabBItems = List.of(new DepositNoticeItemRequest( + 1, "Tab B item", new BigDecimal("1"), "แผ่น", new BigDecimal("999.00"), null, new BigDecimal("999.00"))); + assertThatThrownBy(() -> depositNoticeService.update(draftId, stampedRequest("0.30", tabBItems, loaded), salesActor)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo("ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง"); + }); + DepositNoticeDto reread = depositNoticeRepository.findById(draftId).orElseThrow(); + assertThat(reread.depositPercent()).isEqualByComparingTo("0.40"); + assertThat(reread.depositAmount()).isEqualByComparingTo(savedByA.depositAmount()); + assertThat(reread.items()).isEqualTo(savedByA.items()); + assertThat(reread.updatedAt()).isEqualTo(savedByA.updatedAt()); + + DepositNoticeDto reloaded = depositNoticeService.update( + draftId, stampedRequest("0.30", tabBItems, savedByA.updatedAt()), salesActor); + assertThat(reloaded.depositPercent()).isEqualByComparingTo("0.30"); + assertThat(reloaded.subtotal()).isEqualByComparingTo("999.00"); + } + + @Test + void update_onAnIssuedNotice_refusesThePercentEditWith409_rowUnchanged() { + IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); + DepositNoticeDto before = depositNoticeRepository.findById(fixture.docId()).orElseThrow(); + + assertThatThrownBy(() -> depositNoticeService.update(fixture.docId(), new DepositNoticeDraftRequest( + "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal("0.40"), List.of(), secondDraftItems()), salesActor)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo("ใบแจ้งรับมัดจำนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)"); + }); + + // Repository level: a save that passed requireDraft before issue() committed reaches the + // UPDATE with the row already ISSUED. It must refuse before touching the lines, which the + // PDF/XLSX print. + assertThatThrownBy(() -> depositNoticeRepository.update(fixture.docId(), new DepositNoticeDraftRequest( + "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal("0.40"), List.of(), secondDraftItems()), before.items())) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo("ใบแจ้งรับมัดจำนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)"); + }); + + DepositNoticeDto reread = depositNoticeRepository.findById(fixture.docId()).orElseThrow(); + assertThat(reread.status()).isEqualTo("ISSUED"); + assertThat(reread.depositPercent()).isEqualByComparingTo(before.depositPercent()); + assertThat(reread.depositAmount()).isEqualByComparingTo(before.depositAmount()); + assertThat(reread.items()).isEqualTo(before.items()); } // ───────────────────────────────────────────────────────────────────────────────────── @@ -240,15 +392,43 @@ void issue_secondDraftOnSameTicketSucceedsAndSupersedesTheFirst() { private record IssuedNoticeFixture(long ticketId, long pricingRequestId, long docId, String docNumber) {} private DepositNoticeDraftRequest secondDraftHeader() { + return draftHeader(new BigDecimal("0.50")); + } + + private DepositNoticeDraftRequest draftHeader(BigDecimal depositPercent) { return new DepositNoticeDraftRequest( "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", - new BigDecimal("0.50"), List.of(), null); + depositPercent, List.of(), null); } + private DepositNoticeDraftRequest stampedRequest(String depositPercent, List items, + OffsetDateTime expectedUpdatedAt) { + return new DepositNoticeDraftRequest("ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal(depositPercent), List.of(), items, expectedUpdatedAt); + } + + // Fractional qty, net below unit: net x qty (49.995 + 25.025 = 75.02), unit x qty (90.00), the + // per-line rounded amounts (50.00 + 25.03 = 75.03) and the nets alone (43.34) all differ. private List secondDraftItems() { - return List.of(new DepositNoticeItemRequest( - 1, "Second draft item", new BigDecimal("1"), "แผ่น", - new BigDecimal("100.00"), null, new BigDecimal("100.00"))); + return List.of( + new DepositNoticeItemRequest(1, "Second draft item A", new BigDecimal("1.5"), "แผ่น", + new BigDecimal("40.00"), "ส่วนลด 6.67 ต่อหน่วย", new BigDecimal("33.33")), + new DepositNoticeItemRequest(2, "Second draft item B", new BigDecimal("2.5"), "แผ่น", + new BigDecimal("12.00"), "ส่วนลด 1.99 ต่อหน่วย", new BigDecimal("10.01"))); + } + + private void assertIssueRefusedAndNothingChanged(IssuedNoticeFixture fixture, long draftId, String message) { + assertThatThrownBy(() -> depositNoticeService.issue(draftId, salesActor)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).isEqualTo(message); + }); + DepositNoticeDto draft = depositNoticeRepository.findById(draftId).orElseThrow(); + assertThat(draft.status()).isEqualTo("DRAFT"); + assertThat(draft.docNumber()).isNull(); + DepositNoticeDto first = depositNoticeRepository.findById(fixture.docId()).orElseThrow(); + assertThat(first.status()).isEqualTo("ISSUED"); + assertThat(first.docNumber()).isEqualTo(fixture.docNumber()); } private IssuedNoticeFixture buildTicketWithIssuedDepositNotice() { 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 0472cc0bd..5358e59c0 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 @@ -9,12 +9,14 @@ import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.math.BigDecimal; import java.time.Instant; import java.time.LocalDate; +import java.time.OffsetDateTime; import java.util.List; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -61,8 +63,8 @@ class DepositNoticeServiceTest { private final UserPrincipal owner = new UserPrincipal( 1L, "sales@glr.co.th", "Sales", "sales", 1L, true, LocalDate.of(2026, 1, 1), false, 1L, false); - // sales_manager: read-only oversight of deposit notices — never owns a ticket - // (no create access), so every owner-gated write is denied by ownership alone. + // sales_manager: read-only oversight of deposit notices. It can create (and so own) a deal + // since deal entry was widened, so every deposit-notice write is role-gated to sales. private final UserPrincipal salesManagerActor = new UserPrincipal( 8L, "sales_manager@glr.co.th", "Sales Manager", "sales_manager", 8L, true, LocalDate.of(2026, 1, 1), false, null, false); @@ -73,7 +75,7 @@ class DepositNoticeServiceTest { void issue_advancesPaymentTrackAndKeepsTicketStatus() { stubDraft(99L, 10L); stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); - when(docs.issue(99L, 1L, "Sales")).thenReturn(Optional.of("GLRD69001")); + when(docs.issue(eq(99L), eq(1L), eq("Sales"), any())).thenReturn(Optional.of("GLRD69001")); service.issue(99L, owner); @@ -88,6 +90,24 @@ void issue_advancesPaymentTrackAndKeepsTicketStatus() { eq(TicketEventKind.DOCUMENT_ISSUED), anyString(), anyString(), anyString()); } + // One notice per deal: the "another notice is ISSUED" guard is check-then-act, so the ticket + // row is locked first — two drafts issued at once serialize and the second sees the first. + // (A real two-thread race is not reproducible in this suite; this pins the order.) + @Test + void issue_locksTheTicketRowBeforeReadingTheOneIssuedGuard_andPassesTheStampOn() { + stubDraft(99L, 10L); + stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "CUSTOMER_CONFIRMED"); + OffsetDateTime stamp = OffsetDateTime.parse("2026-10-23T08:57:19.732123Z"); + when(docs.issue(eq(99L), eq(1L), eq("Sales"), eq(stamp))).thenReturn(Optional.of("GLRD69001")); + + service.issue(99L, stamp, owner); + + var order = org.mockito.Mockito.inOrder(ticketRepo, docs); + order.verify(ticketRepo).lockTicketForUpdate(10L); + order.verify(docs).findByTicket(10L); + order.verify(docs).issue(99L, 1L, "Sales", stamp); + } + @Test void issue_requiresQuotationIssuedStatus() { stubDraft(99L, 10L); @@ -120,7 +140,7 @@ void issue_requiresCustomerConfirmedOrDepositNoticeIssuedPayment() { void issue_fromDepositNoticeIssued_isNowAllowedAsARevision() { stubDraft(99L, 10L); stubTicket(10L, TicketStatus.QUOTATION_ISSUED, "DEPOSIT_NOTICE_ISSUED"); - when(docs.issue(99L, 1L, "Sales")).thenReturn(Optional.of("GLRD69002")); + when(docs.issue(eq(99L), eq(1L), eq("Sales"), any())).thenReturn(Optional.of("GLRD69002")); service.issue(99L, owner); // must not throw @@ -808,14 +828,18 @@ void createDraft_rejectsSalesManagerRole() { @Test void update_rejectsSalesManagerRole() { - // update() has NO role gate — only requireTicketOwner. sales_manager can - // never own a ticket, so the ownership check alone denies it. + // Role-gated (SALES_ROLES) like createDraft and issue: a sales_manager that OWNS the deal + // (stubTicket's createdById is 1) passes the owner check, so only the role gate refuses it. + UserPrincipal salesManagerOwner = new UserPrincipal( + 1L, "sales_manager@glr.co.th", "Sales Manager", "sales_manager", 1L, true, + LocalDate.of(2026, 1, 1), false, null, false); stubDraft(99L, 10L); stubTicket(10L, TicketStatus.APPROVED, null); assertForbidden(() -> service.update(99L, new DepositNoticeDraftRequest(null, null, null, null, null, null, null, null), - salesManagerActor)); + salesManagerOwner)); + verify(docs, never()).update(anyLong(), any(), any()); } @Test @@ -934,7 +958,8 @@ void createDraft_legacyApprovedPriceItemsWinOverQuotationChainWhenBothExist() { // 2026-10-23) ──── // The governing quotation's percent (DepositNoticeRepository#findGoverningQuotation: legacy // 50, new-engine its own, blank 30); a caller-supplied percent is never read on create, and - // 0.50 applies only when the ticket has no live quotation. + // 0.50 applies only when the ticket has no live quotation. update() then lets sales change it + // on a DRAFT; an ISSUED notice refuses any edit (requireDraft, 409). @Test void createDraft_sourcesDepositPercentFromQuotation_ignoringCallerSuppliedValue() { @@ -1003,25 +1028,101 @@ void createDraft_takesLinesPercentAndReferenceFromTheGoverningNewEngineQuotation } @Test - void update_preservesTheDraftsOwnStoredDepositPercent_ignoringCallerSuppliedValue() { + void update_onADraft_persistsTheCallersDepositPercent() { // stubDraft's fixture stores depositPercent = 0.50 on doc 99 / ticket 10. stubDraft(99L, 10L); stubTicket(10L, TicketStatus.APPROVED, null); - // A caller sends 0.99 — must never reach the repository update. service.update(99L, new DepositNoticeDraftRequest("ACME", null, null, null, "REF-2", - new BigDecimal("0.99"), null, null), + new BigDecimal("0.40"), null, null), owner); ArgumentCaptor captor = ArgumentCaptor.forClass(DepositNoticeDraftRequest.class); - verify(docs).update(eq(99L), captor.capture()); - assertThat(captor.getValue().depositPercent()).isEqualByComparingTo("0.50"); - // Every other field still passes through untouched — only depositPercent is overridden. + verify(docs).update(eq(99L), captor.capture(), any()); + assertThat(captor.getValue().depositPercent()).isEqualByComparingTo("0.40"); assertThat(captor.getValue().customerName()).isEqualTo("ACME"); assertThat(captor.getValue().reference()).isEqualTo("REF-2"); } + @Test + void update_withNoDepositPercent_keepsTheDraftsStoredPercent() { + stubDraft(99L, 10L); // stored 0.50 + stubTicket(10L, TicketStatus.APPROVED, null); + + service.update(99L, + new DepositNoticeDraftRequest("ACME", null, null, null, "REF-2", null, null, null), + owner); + + ArgumentCaptor captor = ArgumentCaptor.forClass(DepositNoticeDraftRequest.class); + verify(docs).update(eq(99L), captor.capture(), any()); + assertThat(captor.getValue().depositPercent()).isEqualByComparingTo("0.50"); + } + + @Test + void update_refusesZeroAndANewFractionalPercent_butAStoredFractionSentBackUnchangedSaves() { + // Stored 0.3330: free input from before the whole-percent rule. The page shows it as 33. + stubDraft(99L, 10L, new BigDecimal("0.3330")); + stubTicket(10L, TicketStatus.APPROVED, null); + + assertThatThrownBy(() -> service.update(99L, + new DepositNoticeDraftRequest("ACME", null, null, null, "REF-2", BigDecimal.ZERO, null, null), + owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(e.getMessage()).isEqualTo("เปอร์เซ็นต์มัดจำต้องมากกว่า 0"); + }); + // 40.5% would print as "41%" on the document while billing 40.5%. + assertThatThrownBy(() -> service.update(99L, + new DepositNoticeDraftRequest("ACME", null, null, null, "REF-2", new BigDecimal("0.405"), null, null), + owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(e.getMessage()).isEqualTo("เปอร์เซ็นต์มัดจำต้องเป็นจำนวนเต็ม"); + }); + verify(docs, never()).update(anyLong(), any(), any()); + + // The stored value sent back unchanged (0.333 == 0.3330) still saves the rest of the draft; + // a whole percent replaces it. + service.update(99L, + new DepositNoticeDraftRequest("ACME", null, null, null, "REF-2", new BigDecimal("0.333"), null, null), + owner); + service.update(99L, + new DepositNoticeDraftRequest("ACME", null, null, null, "REF-2", new BigDecimal("0.40"), null, null), + owner); + ArgumentCaptor captor = ArgumentCaptor.forClass(DepositNoticeDraftRequest.class); + verify(docs, times(2)).update(eq(99L), captor.capture(), any()); + assertThat(captor.getAllValues()) + .extracting(r -> r.depositPercent().stripTrailingZeros().toPlainString()) + .containsExactly("0.333", "0.4"); + } + + // DepositNoticeRepository#foldAdjustments (owner rulings 2026-10-23, "2 decimals for all"): the + // ส่วนลดพิเศษ amount is spread by line amount, each share HALF_UP to 2 dp, each new net price + // 2 dp. There is no remainder trick: the notice total is the sum of its own 2 dp lines (here + // 200.01, a cent over the quotation's 200.00). A line keeps the quotation's own discount wording + // and gains the folded discount after it; a line the fold did not touch keeps its label as is. + @Test + void foldAdjustments_keepsEachLinesOwnLabel_appendsTheFoldedDiscount_andTotalsFollowTheTwoDpLines() { + List folded = DepositNoticeRepository.foldAdjustments(List.of( + foldLine(1, "1", "100.00", "ส่วนลด 5 ต่อหน่วย"), foldLine(2, "1", "100.00", null), + foldLine(3, "1", "100.00", null), foldLine(4, "2", "0.00", "ส่วนลด 9 ต่อหน่วย")), + new BigDecimal("-100.00")); + + assertThat(folded).extracting(it -> it.netUnitPrice().toPlainString()) + .containsExactly("66.67", "66.67", "66.67", "0.00"); + assertThat(folded.stream().map(it -> it.netUnitPrice().multiply(it.qty())).reduce(BigDecimal.ZERO, BigDecimal::add)) + .isEqualByComparingTo("200.01"); + assertThat(folded).extracting(DepositNoticeItemRequest::discountLabel).containsExactly( + "ส่วนลด 5 ต่อหน่วย + ส่วนลดพิเศษ 33.33 ต่อหน่วย", "ส่วนลดพิเศษ 33.33 ต่อหน่วย", + "ส่วนลดพิเศษ 33.33 ต่อหน่วย", "ส่วนลด 9 ต่อหน่วย"); + } + + private static DepositNoticeItemRequest foldLine(int seq, String qty, String net, String label) { + return new DepositNoticeItemRequest(seq, "Line " + seq, new BigDecimal(qty), "แผ่น", + new BigDecimal(net), label, new BigDecimal(net)); + } + private static void assertForbidden(org.assertj.core.api.ThrowableAssert.ThrowingCallable action) { assertThatThrownBy(action) .isInstanceOfSatisfying(ApiException.class, e -> @@ -1031,10 +1132,14 @@ private static void assertForbidden(org.assertj.core.api.ThrowableAssert.Throwin // ── fixtures ───────────────────────────────────────────────────────────── private void stubDraft(long docId, long ticketId) { + stubDraft(docId, ticketId, new BigDecimal("0.50")); + } + + private void stubDraft(long docId, long ticketId, BigDecimal depositPercent) { DepositNoticeDto draft = new DepositNoticeDto( docId, ticketId, "DEPOSIT_NOTICE", 1, null, null, "DRAFT", "ACME", "0100000000000", "Bangkok", "Showroom", "REF-1", "THB", - new BigDecimal("0.50"), new BigDecimal("1000.00"), new BigDecimal("500.00"), + depositPercent, new BigDecimal("1000.00"), new BigDecimal("500.00"), new BigDecimal("0.07"), new BigDecimal("35.00"), new BigDecimal("535.00"), List.of(), false, false, "Sales", "Preparer", null, null, List.of()); when(docs.findById(docId)).thenReturn(Optional.of(draft)); diff --git a/backend/src/test/java/th/co/glr/hr/deposit/RemainingInvoiceServiceIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/RemainingInvoiceServiceIntegrationTest.java index 2c7338774..8abc6a411 100644 --- a/backend/src/test/java/th/co/glr/hr/deposit/RemainingInvoiceServiceIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/deposit/RemainingInvoiceServiceIntegrationTest.java @@ -811,11 +811,11 @@ null, new BigDecimal("10"), new BigDecimal("0.36"), WastageCalculator.QUANTITY_M /** A SECOND, wholly separate pricing-request chain on an ALREADY-EXISTING ticket — its own * catalog product, pricing request, factory quote, pricing decision and CustomerQuotation, - * driven to ACCEPTED + its own issued deposit notice, exactly like {@link #buildAcceptedDeal} + * driven to ACCEPTED (no second deposit notice: one ISSUED per deal), exactly like {@link #buildAcceptedDeal} * except it reuses the given ticket (and its first ticket item) instead of creating a new one. * Used by the O2 (one live remaining invoice per deal) tests to prove superseding/blocking * reaches a genuinely different chain, not merely a different id on the same fixture. */ - private DealFixture buildSecondAcceptedQuotationChain(long ticketId, UserPrincipal owner, String label) { + private void buildSecondAcceptedQuotationChain(long ticketId, UserPrincipal owner, String label) { long catalogProductId = insertCatalogProduct(FACTORY, "IT", "TEST-RI-" + label + "-" + UUID.randomUUID().toString().substring(0, 8), new BigDecimal("100.00"), "THB", "per_piece"); @@ -866,11 +866,14 @@ null, new BigDecimal("10"), new BigDecimal("0.36"), WastageCalculator.QUANTITY_M orderConfirmation.confirmOrder(pricingRequestId, new ConfirmOrderRequest(UUID.randomUUID().toString()), owner); - DepositNoticeDto depositDraft = orderConfirmation.createDepositNoticeFromQuotation(pricingRequestId, - new CreateDepositNoticeFromQuotationRequest(null), owner); - DepositNoticeDto depositIssued = depositNoticeService.issue(depositDraft.id(), owner); + // Chain B has no deposit notice of its own, so it qualifies for a remaining invoice only through + // the bypass branch: flip the deal to NOT_REQUIRED (chain A's drafts/invoices were made earlier). + jdbc.update("UPDATE sales.ticket SET deposit_policy = 'NOT_REQUIRED' WHERE ticket_id = :id", + Map.of("id", ticketId)); + // No deposit notice for chain B: a deal has at most one ISSUED deposit notice (owner ruling + // 2026-10-28), so chain A's stays the deal's only one; the remaining-invoice rules under test + // key off the accepted quotations, not a second notice. - return new DealFixture(ticketId, pricingRequestId, depositIssued.id()); } private void driveDraftPricingRequestToQuotationAccepted(long pricingRequestId, BigDecimal quantity, UserPrincipal owner) { diff --git a/backend/src/test/java/th/co/glr/hr/ticket/PaymentTrackIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/ticket/PaymentTrackIntegrationTest.java index f6301a785..61a005e91 100644 --- a/backend/src/test/java/th/co/glr/hr/ticket/PaymentTrackIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/ticket/PaymentTrackIntegrationTest.java @@ -272,7 +272,7 @@ void bypassPath_walksEndToEnd_nullThroughFullyPaid_neverVisitsDepositNoticeIssue } @Test - void depositNoticeRevision_secondIssueSucceeds_mintsNewDocSupersedesFirst_paymentStatusStaysDepositNoticeIssued() { + void depositNoticeSecondIssue_isRefused_firstStaysIssued_noNumberConsumed_paymentStatusUnchanged() { Deal deal = buildDealToQuotationAccepted("revision"); orderConfirmation.confirmOrder( deal.pricingRequestId(), new ConfirmOrderRequest(UUID.randomUUID().toString()), salesActor); @@ -281,18 +281,26 @@ void depositNoticeRevision_secondIssueSucceeds_mintsNewDocSupersedesFirst_paymen DepositNoticeDto firstIssued = depositNoticeService.issue(firstDraft.id(), salesActor); assertThat(paymentStatusOf(deal.ticketId())).isEqualTo(PaymentTrack.DEPOSIT_NOTICE_ISSUED); - // A revision: a NEW draft on the SAME ticket, issued while payment_status is still - // DEPOSIT_NOTICE_ISSUED — PaymentTrack's one legal self-loop (guard e). - long secondDraftId = depositNoticeService.createDraft(deal.ticketId(), bareDraftRequest(), salesActor).id(); - DepositNoticeDto secondIssued = depositNoticeService.issue(secondDraftId, salesActor); - - assertThat(secondIssued.docNumber()).isNotEqualTo(firstIssued.docNumber()); - assertThat(secondIssued.status()).isEqualTo("ISSUED"); - // PR #698's "AND status = 'DRAFT'" guard on DepositNoticeRepository.issue composes with - // this change: the second row really was DRAFT (createDraft never touches payment_status) - // when it was issued. + // Owner ruling 2026-10-23: one deposit notice per deal. The create routes refuse a draft + // while a notice is ISSUED; a second draft that exists anyway (written by the repository, + // as a concurrent or pre-ruling draft would be) is refused at issue(), so the + // DEPOSIT_NOTICE_ISSUED self-loop is not reachable through this service. + assertThatThrownBy(() -> depositNoticeService.createDraft(deal.ticketId(), bareDraftRequest(), salesActor)) + .isInstanceOfSatisfying(ApiException.class, + e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); + long secondDraftId = depositNoticeRepository.createDraft( + deal.ticketId(), bareDraftRequest(), bareDraftRequest().items()); + assertThatThrownBy(() -> depositNoticeService.issue(secondDraftId, salesActor)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).contains(firstIssued.docNumber()); + }); + + DepositNoticeDto second = depositNoticeRepository.findById(secondDraftId).orElseThrow(); + assertThat(second.status()).isEqualTo("DRAFT"); + assertThat(second.docNumber()).isNull(); DepositNoticeDto firstReread = depositNoticeRepository.findById(firstIssued.id()).orElseThrow(); - assertThat(firstReread.status()).isEqualTo("SUPERSEDED"); + assertThat(firstReread.status()).isEqualTo("ISSUED"); assertThat(firstReread.docNumber()).isEqualTo(firstIssued.docNumber()); assertThat(paymentStatusOf(deal.ticketId())).isEqualTo(PaymentTrack.DEPOSIT_NOTICE_ISSUED); } diff --git a/frontend/src/api/hrApi.js b/frontend/src/api/hrApi.js index 5382e61cd..88e500b14 100644 --- a/frontend/src/api/hrApi.js +++ b/frontend/src/api/hrApi.js @@ -480,7 +480,8 @@ export const api = { get: (id) => apiRequest(API_ROUTES.depositNotices.get(id)), update: (id, payload) => apiRequest(API_ROUTES.depositNotices.update(id), { method: 'PUT', body: payload }), preview: (id) => apiRequest(API_ROUTES.depositNotices.preview(id), { method: 'POST' }), - issue: (id) => apiRequest(API_ROUTES.depositNotices.issue(id), { method: 'POST' }), + // payload.expectedUpdatedAt (optional): the draft's updatedAt as the caller last read it; 409 if it moved on. + issue: (id, payload) => apiRequest(API_ROUTES.depositNotices.issue(id), { method: 'POST', body: payload }), downloadXlsx: async (id) => { const res = await fetch(API_ROUTES.depositNotices.file(id, 'xlsx'), { credentials: 'include' }); if (!res.ok) throw new Error('Download failed'); diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index f54b3ad77..cd731ff92 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -425,6 +425,119 @@ describe('mockApi.depositNotices.issue hasPdf/hasXlsx (issue #752: mock reproduc }); }); +// Owner ruling 2026-10-23 (mirrors DepositNoticeService.update): sales may change the deposit % +// on a DRAFT and the amount follows; 0 is refused; an ISSUED notice refuses every edit. +describe('mockApi.depositNotices.update deposit percent', () => { + it('on a DRAFT takes the new percent and recomputes the amount; 0, over 100% and fractional are refused', async () => { + const { ticketId } = await driveTicketToAcceptedQuotation(); + const { depositNotice: draft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + expect(draft.subtotal).toBeGreaterThan(0); + + // Items sent the way DepositNoticePage sends them on save. + const { depositNotice: updated } = await api.depositNotices.update(draft.id, { depositPercent: 0.4, items: draft.items }); + expect(updated.depositPercent).toBe(0.4); + expect(updated.depositAmount).toBe(Math.round(draft.subtotal * 0.4 * 100) / 100); + + // DepositNoticeDraftRequest's @DecimalMin/@DecimalMax run before the service (field-prefixed). + await expect(api.depositNotices.update(draft.id, { depositPercent: 0 })) + .rejects.toMatchObject({ status: 400, message: 'depositPercent เปอร์เซ็นต์มัดจำต้องมากกว่า 0' }); + await expect(api.depositNotices.update(draft.id, { depositPercent: 1.5 })) + .rejects.toMatchObject({ status: 400, message: 'depositPercent เปอร์เซ็นต์มัดจำต้องไม่เกิน 100' }); + await expect(api.depositNotices.update(draft.id, { depositPercent: 0.405 })) + .rejects.toMatchObject({ status: 400, message: 'เปอร์เซ็นต์มัดจำต้องเป็นจำนวนเต็ม' }); + const { depositNotice: afterRejects } = await api.depositNotices.get(draft.id); + expect(afterRejects.depositPercent).toBe(0.4); + expect(afterRejects.depositAmount).toBe(updated.depositAmount); + // 0.29 * 100 is 28.999999999999996 in floating point; it is still a whole 29%. + await api.depositNotices.update(draft.id, { depositPercent: 0.29, items: draft.items }); + const { depositNotice: reread } = await api.depositNotices.get(draft.id); + expect(reread.depositPercent).toBe(0.29); + }); + + // @Valid runs before DepositNoticeService.update looks anything up, so a bad % is a 400 even + // for a notice that does not exist or is already issued. + it('validates the % before the 404 and 409 lookups', async () => { + await api.auth.login({ role: 'sales' }); + await expect(api.depositNotices.update(999999, { depositPercent: 1.5 })) + .rejects.toMatchObject({ status: 400 }); + const { ticketId } = await driveTicketToAcceptedQuotation(); + const { depositNotice: draft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + await api.tickets.confirmCustomer(ticketId); + await api.depositNotices.issue(draft.id); + await expect(api.depositNotices.update(draft.id, { depositPercent: 0 })) + .rejects.toMatchObject({ status: 400 }); + }); + + // Mirrors the stale-save check (DepositNoticeRepository#update): a save carrying an older + // updatedAt than the stored one is refused; one carrying the current stamp, or none, saves. + it('refuses a save whose expectedUpdatedAt is stale', async () => { + const { ticketId } = await driveTicketToAcceptedQuotation(); + const { depositNotice: draft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + const { depositNotice: savedByA } = await api.depositNotices.update(draft.id, + { depositPercent: 0.4, items: draft.items, expectedUpdatedAt: draft.updatedAt }); + + await expect(api.depositNotices.update(draft.id, + { depositPercent: 0.3, items: draft.items, expectedUpdatedAt: draft.updatedAt })) + .rejects.toMatchObject({ status: 409, message: 'ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง' }); + expect((await api.depositNotices.get(draft.id)).depositNotice.depositPercent).toBe(0.4); + + await api.depositNotices.update(draft.id, { depositPercent: 0.3, expectedUpdatedAt: savedByA.updatedAt }); + expect((await api.depositNotices.get(draft.id)).depositNotice.depositPercent).toBe(0.3); + }); + + // Mirrors DepositNoticeService.update/issue: sales only (requireRole), then the deal owner. + it('refuses update and issue to a sales rep who does not own the deal, and to a non-sales role', async () => { + const { ticketId } = await driveTicketToAcceptedQuotation(); + const { depositNotice: draft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + await api.tickets.confirmCustomer(ticketId); + + await api.auth.login({ email: 'sales2@glr.co.th', password: 'demo1234' }); + await expect(api.depositNotices.update(draft.id, { customerName: 'x' })) + .rejects.toMatchObject({ status: 403, message: 'ไม่มีสิทธิ์เข้าถึงรายการนี้' }); + await expect(api.depositNotices.issue(draft.id)).rejects.toMatchObject({ status: 403 }); + await api.auth.login({ role: 'sales_manager' }); + await expect(api.depositNotices.update(draft.id, { customerName: 'x' })).rejects.toMatchObject({ status: 403 }); + await expect(api.depositNotices.issue(draft.id)).rejects.toMatchObject({ status: 403 }); + + await api.auth.login({ role: 'sales' }); + const { depositNotice: reread } = await api.depositNotices.get(draft.id); + expect(reread.status).toBe('DRAFT'); + expect(reread.customerName).toBe(draft.customerName); + }); + + it('on an ISSUED notice is refused with 409 and changes nothing; no second draft on either route', async () => { + const { ticketId } = await driveTicketToAcceptedQuotation(); + const { depositNotice: draft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + const { depositNotice: secondDraft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + await api.tickets.confirmCustomer(ticketId); + const { depositNotice: issued } = await api.depositNotices.issue(draft.id); + + // Owner ruling 2026-10-23: a second draft that predates the issue takes no number either. + await expect(api.depositNotices.issue(secondDraft.id)).rejects.toMatchObject({ + status: 409, + message: `ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ ${issued.docNumber} แล้ว จึงออกฉบับร่างนี้ไม่ได้`, + }); + const { depositNotice: secondReread } = await api.depositNotices.get(secondDraft.id); + expect(secondReread.status).toBe('DRAFT'); + expect(secondReread.docNumber).toBeNull(); + expect((await api.depositNotices.get(issued.id)).depositNotice.status).toBe('ISSUED'); + + await expect(api.depositNotices.update(issued.id, { depositPercent: 0.4 })) + .rejects.toMatchObject({ status: 409 }); + const { depositNotice: reread } = await api.depositNotices.get(issued.id); + expect(reread.depositPercent).toBe(issued.depositPercent); + expect(reread.depositAmount).toBe(issued.depositAmount); + + // Owner ruling 2026-10-23 (DepositNoticeService.createDraftFromQuotation): one notice per deal. + const message = `ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ ${issued.docNumber} แล้ว จึงสร้างฉบับร่างใหม่ไม่ได้`; + await expect(api.depositNotices.createDraft(ticketId, {})).rejects.toMatchObject({ status: 409, message }); + const prId = (await api.pricingRequests.listForTicket(ticketId)).items + .find((pr) => pr.status === 'QUOTATION_ACCEPTED').id; + await expect(api.pricingRequests.createDepositNoticeFromQuotation(prId, {})).rejects.toMatchObject({ status: 409, message }); + expect((await api.depositNotices.listByTicket(ticketId)).depositNotices).toHaveLength(2); // no third + }); +}); + // GLA-32 (2026-09-19): mirrors DepositNoticeService.issue's account-role notification (backend // backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java) — issuing a deposit // notice notifies the account role, in-app, so account picks up the deal here rather than @@ -507,9 +620,11 @@ describe('mock deposit-percent defaults match the Java PDFs', () => { describe('mockApi.depositNotices.issue refuses a 0-baht notice', () => { it('409s, and the draft stays DRAFT with no number', async () => { const { ticketId } = await driveTicketToNewEngineAcceptedQuotation({ depositPercent: 30 }); - // create ignores a typed percent now; the mock's update still honours one, which stands in for the Java test's jdbc insert. + // update refuses a 0% now, so a free line (net price 0) stands in for the Java test's jdbc insert. const { depositNotice: created } = await api.depositNotices.createDraft(ticketId, {}); - const { depositNotice: draft } = await api.depositNotices.update(created.id, { depositPercent: 0 }); + const { depositNotice: draft } = await api.depositNotices.update(created.id, { + items: [{ seq: 1, description: 'Free item', qty: 2, unit: 'แผ่น', unitPrice: 0, netUnitPrice: 0 }], + }); expect(draft.depositAmount).toBe(0); await expect(api.depositNotices.issue(draft.id)) .rejects.toMatchObject({ status: 409, message: expect.stringContaining('0 บาท') }); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 26632a528..3cc13d9d7 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1441,18 +1441,36 @@ function mockDepositNoticeItemsFromQuotation(items) { // Mirrors DepositNoticeRepository.findDealQuotationItems. Values are read off the stored row, // never recomputed (label excepted — see below). function mockDepositNoticeItemsFromDealQuotation(row) { - return row.items.map((item, idx) => { + // DepositNoticeRepository#foldAdjustments: an ADJUSTMENT row (qty -1) is no deposit line; its + // amount is spread over the goods lines by amount, each share and each new net price 2 dp (no + // remainder on a last line: the notice total is its own 2 dp lines). A line keeps the quotation's + // own discount wording and gains the folded discount after it. + // ponytail: the mock computes no % adjustment amount (MOCK_NOT_COMPUTED), so that row is only dropped. + const goods = row.items.filter((item) => item.lineType !== 'ADJUSTMENT'); + const adjustment = row.items.filter((item) => item.lineType === 'ADJUSTMENT') + .reduce((sum, item) => sum + (Number(item.netUnitPrice) || 0) * Number(item.quantity), 0); + const amountOf = (item) => item.netUnitPrice * item.quantity; + const total = goods.reduce((sum, item) => sum + amountOf(item), 0); + return goods.map((item, idx) => { + let netUnitPrice = item.netUnitPrice; + if (adjustment && total && amountOf(item)) { + const share = round2((adjustment * amountOf(item)) / total); + netUnitPrice = round2((amountOf(item) + share) / item.quantity); + } // Display label only: the mock stores no sales_discount, so this restates the column Java reads // (DealQuotationRepository#itemParams writes it as unitPrice − netUnitPrice). const salesDiscount = round2(item.unitPrice - item.netUnitPrice); + const folded = round2(item.netUnitPrice - netUnitPrice); + const own = salesDiscount > 0 ? `ส่วนลด ${salesDiscount} ต่อหน่วย` : null; + const extra = folded > 0 ? `ส่วนลดพิเศษ ${folded} ต่อหน่วย` : null; return { seq: item.seq ?? idx + 1, description: item.descriptionLine?.trim() ? item.descriptionLine : 'รายการสินค้า', qty: item.quantity, unit: item.unit?.trim() ? item.unit : 'หน่วย', unitPrice: item.unitPrice, - discountLabel: salesDiscount > 0 ? `ส่วนลด ${salesDiscount} ต่อหน่วย` : null, - netUnitPrice: item.netUnitPrice, + discountLabel: own && extra ? `${own} + ${extra}` : own ?? extra, + netUnitPrice, }; }); } @@ -2417,6 +2435,13 @@ function stripCeoPriceModeFields(decision, user) { }; } +// Mirrors DepositNoticeService.createDraftFromQuotation (both create routes): one deposit notice +// per deal, so no new draft while one is ISSUED (owner ruling 2026-10-23). +function requireNoIssuedDepositNotice(ticketId) { + const issued = mockDepositNotices.find((d) => d.ticketId === Number(ticketId) && d.status === 'ISSUED'); + if (issued) fail(`ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ ${issued.docNumber} แล้ว จึงสร้างฉบับร่างใหม่ไม่ได้`, 409); +} + function buildMockDoc(doc) { const items = doc.items ?? []; const depositPct = doc.depositPercent ?? 0.5; @@ -13316,6 +13341,7 @@ export const api = { const user = hasRole('sales'); const ticket = findTicketRaw(Number(ticketId)); if (ticket.createdById !== user.id) fail('ไม่มีสิทธิ์เข้าถึงรายการนี้', 403); + requireNoIssuedDepositNotice(ticket.id); // Phase 1 lifecycle gate (mirrors DepositNoticeService.createDraft). requireActive(ticket); if (!['approved', 'quotation_issued', 'document_issued'].includes(ticket.status)) fail('ดีลต้องได้รับการอนุมัติก่อน', 409); @@ -13391,9 +13417,25 @@ export const api = { async update(docId, payload) { requireSession(); + // DepositNoticeDraftRequest's @DecimalMin/@DecimalMax: @Valid runs before any lookup. + const sent = payload.depositPercent == null ? null : Number(payload.depositPercent) * 100; + if (sent != null && sent > 100) fail('depositPercent เปอร์เซ็นต์มัดจำต้องไม่เกิน 100', 400); + if (sent != null && !(sent > 0)) fail('depositPercent เปอร์เซ็นต์มัดจำต้องมากกว่า 0', 400); + // DepositNoticeService.update: sales only, a DRAFT, the deal owner, then the % rules. + const user = hasRole('sales'); const doc = mockDepositNotices.find((d) => d.id === Number(docId)); if (!doc) fail('ไม่พบใบแจ้งรับมัดจำนี้', 404); if (doc.status !== 'DRAFT') fail('ใบแจ้งรับมัดจำนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)', 409); + if (findTicketRaw(doc.ticketId).createdById !== user.id) fail('ไม่มีสิทธิ์เข้าถึงรายการนี้', 403); + const stored = Number(doc.depositPercent) * 100; + const pct = sent ?? stored; + if (!(pct > 0)) fail('เปอร์เซ็นต์มัดจำต้องมากกว่า 0', 400); + // A stored legacy fraction sent back unchanged still saves. + if (pct !== stored && Math.abs(pct - Math.round(pct)) > 1e-9) fail('เปอร์เซ็นต์มัดจำต้องเป็นจำนวนเต็ม', 400); + // DepositNoticeRepository#update: a save over someone else's newer one is refused. + if (payload.expectedUpdatedAt != null && payload.expectedUpdatedAt !== doc.updatedAt) { + fail('ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง', 409); + } Object.assign(doc, { customerName: payload.customerName ?? doc.customerName, customerTaxId: payload.customerTaxId ?? doc.customerTaxId, @@ -13403,7 +13445,8 @@ export const api = { depositPercent: payload.depositPercent ?? doc.depositPercent, notes: payload.notes ?? doc.notes, items: payload.items?.length ? payload.items : doc.items, - updatedAt: new Date().toISOString(), + // Strictly later than the stored stamp, so two saves in one millisecond still differ. + updatedAt: new Date(Math.max(Date.now(), (Date.parse(doc.updatedAt) || 0) + 1)).toISOString(), }); const updated = buildMockDoc(doc); Object.assign(doc, updated); @@ -13419,12 +13462,23 @@ export const api = { return mockPreviewHtml(buildMockDoc(doc)); }, - async issue(docId) { + async issue(docId, payload) { const user = requireSession(); const doc = mockDepositNotices.find((d) => d.id === Number(docId)); if (!doc) fail('ไม่พบใบแจ้งรับมัดจำนี้', 404); if (doc.status !== 'DRAFT') fail('ไม่ได้อยู่ในสถานะร่าง (DRAFT)', 409); const ticket = findTicketRaw(doc.ticketId); + // DepositNoticeService.issue: sales only, the deal owner, and a stored % the document can print. + if (user.role !== 'sales' || ticket.createdById !== user.id) fail('ไม่มีสิทธิ์เข้าถึงรายการนี้', 403); + const pct = Number(doc.depositPercent) * 100; + if (!(pct > 0 && pct <= 100) || Math.abs(pct - Math.round(pct)) > 1e-9) { + fail(`ใบแจ้งรับมัดจำนี้มีเปอร์เซ็นต์มัดจำ ${+pct.toFixed(4)}% จึงออกเอกสารไม่ได้ (ต้องเป็นจำนวนเต็ม 1–100)`, 409); + } + + // Mirrors DepositNoticeService.issue's one-notice-per-deal refusal (owner ruling 2026-10-23). + // Checked before the payment-state gate: the mock cannot re-issue from DEPOSIT_NOTICE_ISSUED. + const other = mockDepositNotices.find((d) => d.ticketId === doc.ticketId && d.id !== doc.id && d.status === 'ISSUED'); + if (other) fail(`ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ ${other.docNumber} แล้ว จึงออกฉบับร่างนี้ไม่ได้`, 409); // Mirrors DepositNoticeService.issue: the document IS the payment-track step. // Requires a customer-confirmed quotation; advances paymentStatus and leaves @@ -13436,9 +13490,10 @@ export const api = { if (!(Number(doc.depositAmount) > 0)) fail('ใบแจ้งรับมัดจำนี้มียอดมัดจำ 0 บาท จึงออกเอกสารไม่ได้', 409); // Phase 1 lifecycle gate (mirrors DepositNoticeService.requireActiveLifecycle). requireActive(ticket); - - // Supersede previous issued docs - mockDepositNotices.forEach((d) => { if (d.ticketId === doc.ticketId && d.id !== doc.id && d.status === 'ISSUED') d.status = 'SUPERSEDED'; }); + // DepositNoticeRepository#issue: the draft was saved after the caller read it. + if (payload?.expectedUpdatedAt != null && payload.expectedUpdatedAt !== doc.updatedAt) { + fail('ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง', 409); + } const thaiYear = new Date().getFullYear() + 543; doc.docNumber = `GLRD${String(thaiYear).slice(-2)}${String(mockDocNumberSeq++).padStart(3,'0')}`; @@ -16258,6 +16313,7 @@ export const api = { if (!accepted && !newEngine) fail('ยังไม่มีใบเสนอราคาที่ลูกค้ายอมรับสำหรับคำขอราคานี้', 409); // Mirrors the zero-deposit refusal: a 0% ("ไม่รับมัดจำ") quotation has nothing to bill. if (newEngine?.depositPercent === 0) fail(mockZeroDepositRefusal(newEngine.number), 409); + requireNoIssuedDepositNotice(ticket.id); const reference = (accepted ?? newEngine).number; const items = accepted ? mockDepositNoticeItemsFromQuotation(accepted.items) diff --git a/frontend/src/api/mockApi.promoteToDeal.test.js b/frontend/src/api/mockApi.promoteToDeal.test.js index c55b599f3..ff9bb889f 100644 --- a/frontend/src/api/mockApi.promoteToDeal.test.js +++ b/frontend/src/api/mockApi.promoteToDeal.test.js @@ -35,9 +35,9 @@ async function quotationOnlyTicket() { return ticket.summary.id; } -async function approvedQuotation(ticketId) { +async function approvedQuotation(ticketId, items = [TILE, PLAIN, ADJUSTMENT]) { await asRole('sales'); - const { quotation: draft } = await api.dealQuotations.create(ticketId, { recipientType: 'DESIGNER', items: [TILE, PLAIN, ADJUSTMENT] }); + const { quotation: draft } = await api.dealQuotations.create(ticketId, { recipientType: 'DESIGNER', items }); await api.dealQuotations.submit(draft.id); await asRole('sales_manager'); const { quotation } = await api.dealQuotations.approve(draft.id); @@ -126,6 +126,27 @@ describe('mock dealQuotations.promoteToDeal (GLA-136)', () => { expect(await listIds()).toContain(ghost); }); + // Owner ruling 2026-10-23 (DepositNoticeRepository#foldAdjustments): the deposit draft carries no + // qty -1 ส่วนลดพิเศษ row; a flat 95 is spread by line amount (TILE 8,500 takes 85, PLAIN 1,000 + // takes 10), so the subtotal is the quotation's 9,405. The mock computes no % adjustment amount + // (MOCK_NOT_COMPUTED), so a % row is only dropped. + it('the deposit draft folds a flat ส่วนลดพิเศษ into the goods lines and drops a % one', async () => { + const ghost = await quotationOnlyTicket(); + const flat = await approvedQuotation(ghost, [TILE, PLAIN, { lineType: 'ADJUSTMENT', description: 'ส่วนลดพิเศษ', adjustmentAmount: 95 }]); + await api.dealQuotations.promoteToDeal(flat.id); + const { depositNotice } = await api.depositNotices.createDraft(ghost, {}); + expect(depositNotice.items.map((i) => [i.qty, i.unitPrice, i.netUnitPrice, i.discountLabel])).toEqual([ + [10, 850, 841.5, 'ส่วนลดพิเศษ 8.5 ต่อหน่วย'], + [2, 500, 495, 'ส่วนลดพิเศษ 5 ต่อหน่วย'], + ]); + expect(depositNotice.subtotal).toBe(9405); + + const other = await quotationOnlyTicket(); + await api.dealQuotations.promoteToDeal((await approvedQuotation(other)).id); + const { depositNotice: pctDraft } = await api.depositNotices.createDraft(other, {}); + expect(pctDraft.items.map((i) => i.qty)).toEqual([10, 2]); + }); + it('a replay is idempotent — same deal, no second event', async () => { const ghost = await quotationOnlyTicket(); const approved = await approvedQuotation(ghost); diff --git a/frontend/src/features/deposits/DepositNoticePage.jsx b/frontend/src/features/deposits/DepositNoticePage.jsx index a8d374b24..9e5268354 100644 --- a/frontend/src/features/deposits/DepositNoticePage.jsx +++ b/frontend/src/features/deposits/DepositNoticePage.jsx @@ -14,6 +14,7 @@ import { StatusBadge } from '../../components/common/StatusBadge.jsx'; import { fieldErrorId } from '../../components/common/FormField.jsx'; import { depositNoticeStatusLabel, formatThaiDate } from '../../utils/format.js'; import { downloadBlob } from '../../utils/download.js'; +import { round2 } from '../quotations/quotationMeta.js'; function money(v) { if (v == null) return '—'; @@ -35,6 +36,43 @@ function formatVatPercent(rate) { return String(Math.round(Number(rate) * 100 * 100) / 100); } +// A line's net unit price: 0 is a real price (a 100%-discounted line), only a missing one falls +// back to the unit price. +function lineNet(item) { + return Number(item.netUnitPrice ?? item.unitPrice) || 0; +} + +// An untouched เพิ่มรายการ row (or one emptied again): no description and nothing to bill. Left +// out of the save (the backend refuses a line without a description) and of the issue checks. +const NO_ITEMS_MESSAGE = 'กรุณาเพิ่มรายการสินค้าอย่างน้อย 1 รายการ'; + +function isBlankRow(item) { + return !String(item.description ?? '').trim() && !(lineNet(item) * (Number(item.qty) || 0)); +} + +/** The confirm-before-issue amounts, shared by this editor and DealDepositPanel's ออกเอกสาร. */ +export function DepositIssueSummary({ customerName, depositPercent, deposit, vatPercent, vat, total }) { + return ( +
+

+ ตรวจสอบยอดเงินก่อนออกเอกสารให้ {customerName || 'ลูกค้า'} — หลังจากนี้จะไม่สามารถแก้ไขได้ +

+
+ ขอรับเงินมัดจำ ({Math.round(Number(depositPercent) * 100)}%) + {money(deposit)} บาท +
+
+ ภาษีมูลค่าเพิ่ม {formatVatPercent(vatPercent)}% + {money(vat)} บาท +
+
+ รวมเป็นเงินที่ต้องชำระ + {money(total)} บาท +
+
+ ); +} + // ── validation (UX-03) ────────────────────────────────────────────────────── // This gate fires ONLY from handleIssue() — never from handleSave() or the // preview path. This is a draft editor: users save partial drafts @@ -69,7 +107,7 @@ function makeLineItemSchema(rowNumber) { * handleIssue() can scroll/focus the first one without depending on zod's * internal issue ordering. */ -function validateIssueForm({ customerName, items, total }) { +function validateIssueForm({ customerName, items, total, depositPercent }) { const errors = {}; const order = []; @@ -79,11 +117,12 @@ function validateIssueForm({ customerName, items, total }) { order.push('customerName'); } - if (items.length === 0) { - errors.items = 'กรุณาเพิ่มรายการสินค้าอย่างน้อย 1 รายการ'; + if (items.every(isBlankRow)) { + errors.items = NO_ITEMS_MESSAGE; order.push('items'); } else { items.forEach((item, index) => { + if (isBlankRow(item)) return; const result = makeLineItemSchema(index + 1).safeParse(item); if (!result.success) { for (const issue of result.error.issues) { @@ -95,6 +134,14 @@ function validateIssueForm({ customerName, items, total }) { }); } + // The % sits after the items on screen. A whole 1–100 is what the backend issues; outside it the + // ฿0 rule below would blame the unit price (0%) or the backend would refuse behind the dialog (150%). + const percent = depositPercent === '' ? NaN : Math.round(Number(depositPercent) * 100); + if (!(percent >= 1 && percent <= 100)) { + errors.depositPercent = 'กรุณากรอก % มัดจำเป็นจำนวนเต็ม 1–100'; + order.push('depositPercent'); + } + // Rule 4: issuing a ฿0 deposit notice is meaningless and would still // advance the ticket's payment track. Only checked once the rules above // are otherwise satisfied — a ฿0 total caused by missing item data is @@ -229,7 +276,8 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa customerAddress: address, projectName: d.projectName ?? '', reference: d.reference ?? '', - depositPercent: d.depositPercent ?? 0.5, + // As stored (null -> blank, which saves as "keep"): a save of another field must not rewrite it. + depositPercent: d.depositPercent ?? '', notes: d.notes ?? [], items: (d.items ?? []).map((it) => ({ ...it })), }); @@ -240,6 +288,10 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa // background refetch of the same doc (e.g. a save doesn't bump // version/status, so no reseed fires and in-flight edits aren't clobbered). const docSeedKey = doc ? `${doc.id}:${doc.version}:${doc.status}` : null; + // The draft's updatedAt as last read from the server (the list, or this page's own save): + // sent as expectedUpdatedAt, so a save over someone else's newer one is refused (409). + const stampRef = useRef(null); + useEffect(() => { stampRef.current = doc?.updatedAt ?? null; }, [doc?.updatedAt]); useEffect(() => { if (doc) populateForm(doc, customers); // A reseed means a different document identity (or the same one flipping @@ -253,6 +305,22 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa return queryClient.invalidateQueries({ queryKey: queryKeys.depositNotices(ticketId) }); } + async function saveDraft() { + const res = await api.depositNotices.update(doc.id, buildPayload()); + stampRef.current = res?.depositNotice?.updatedAt ?? stampRef.current; + return res; + } + + // A 409 means the server's copy moved on (saved elsewhere, or issued): reload it, form included, + // and show the server's message. + async function onSaveError(err, fallback) { + showToast('error', err.message || fallback); + if (err.status !== 409) return; + const { data } = await depositNoticesQuery.refetch(); + const fresh = (data ?? []).find((d) => d.id === doc.id); + if (fresh) populateForm(fresh, customers); + } + const createDraftMutation = useMutation({ // Prefer the pricing-request-chain endpoint (createDepositNoticeFromQuotation) when this // deal has a pricing request whose customer quotation was accepted — that route sources @@ -274,10 +342,9 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa } catch { acceptedPricingRequestId = null; } - // depositPercent is deliberately NOT sent on either branch below (owner ruling, - // 2026-09-29): the backend always ignores a caller-supplied value now and re-sources it - // from the ticket's own quotation (see DepositNoticeService.createDraft), falling back to - // 0.50 only when there is no quotation to read it from at all. + // depositPercent is not sent on either branch below: the backend picks the new draft's + // percent (DepositNoticeService.createDraftFromQuotation; findGoverningQuotation, #1170). + // Sales changes it on the draft. if (acceptedPricingRequestId != null) { return api.pricingRequests.createDepositNoticeFromQuotation(acceptedPricingRequestId, {}); } @@ -294,34 +361,42 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa } const saveMutation = useMutation({ - mutationFn: () => api.depositNotices.update(doc.id, buildPayload()), + mutationFn: saveDraft, onSuccess: () => { showToast('success', 'บันทึกแล้ว'); invalidateDepositNotices(); }, - onError: (err) => showToast('error', err.message || 'บันทึกไม่สำเร็จ'), + onError: (err) => onSaveError(err, 'บันทึกไม่สำเร็จ'), }); + // Blank rows are not sent and an empty `items` makes the backend keep the stored lines, so a form + // whose rows were all cleared would "save" and bring the old lines back: refuse it instead. + function refusesClearedItems() { + if (!(doc.items?.length > 0 && form.items.every(isBlankRow))) return false; + setFieldErrors({ items: NO_ITEMS_MESSAGE }); + return true; + } + function handleSave() { - if (!doc) return; + if (!doc || refusesClearedItems()) return; saveMutation.mutate(); } const previewMutation = useMutation({ mutationFn: async () => { // Save first, then preview. - await api.depositNotices.update(doc.id, buildPayload()); + await saveDraft(); return api.depositNotices.preview(doc.id); }, onSuccess: (html) => { setPreview(html); invalidateDepositNotices(); }, - onError: (err) => showToast('error', err.message || 'Preview ไม่สำเร็จ'), + onError: (err) => onSaveError(err, 'Preview ไม่สำเร็จ'), }); function handlePreview() { - if (!doc) return; + if (!doc || refusesClearedItems()) return; previewMutation.mutate(); } @@ -331,7 +406,9 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa // sure?" for something that's then going to fail. `total` is read here // (never written) from the money-math block below; see the HARD // CONSTRAINT comment on that block. - const { errors, order } = validateIssueForm({ customerName: form.customerName, items: form.items, total }); + const { errors, order } = validateIssueForm({ + customerName: form.customerName, items: form.items, total, depositPercent: form.depositPercent, + }); if (order.length > 0) { setFieldErrors(errors); const node = fieldRefs.current[order[0]]; @@ -350,8 +427,8 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa const issueMutation = useMutation({ mutationFn: async () => { // Save first. - await api.depositNotices.update(doc.id, buildPayload()); - return api.depositNotices.issue(doc.id); + await saveDraft(); + return api.depositNotices.issue(doc.id, { expectedUpdatedAt: stampRef.current }); }, onSuccess: (res) => { showToast('success', `ออกเอกสาร ${res.depositNotice.docNumber} เรียบร้อย`); @@ -363,7 +440,10 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa queryClient.invalidateQueries({ queryKey: ['tickets', 'list'] }); queryClient.invalidateQueries({ queryKey: queryKeys.ticketDetail(ticketId) }); }, - onError: (err) => showToast('error', err.message || 'ออกเอกสารไม่สำเร็จ'), + onError: (err) => { + setConfirmIssue(false); + onSaveError(err, 'ออกเอกสารไม่สำเร็จ'); + }, }); function confirmIssueDocument() { @@ -377,10 +457,13 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa function setField(key, value) { setForm((f) => ({ ...f, [key]: value })); if (key === 'customerName') clearFieldError('customerName'); - // The deposit % toggle feeds directly into `deposit`/`total` below — + // The deposit % field feeds directly into `deposit`/`total` below — // clear a stale "total must be > 0" error so it can be re-checked fresh // on the next issue attempt. - if (key === 'depositPercent') clearFieldError('total'); + if (key === 'depositPercent') { + clearFieldError('total'); + clearFieldError('depositPercent'); + } } function selectCustomer(c) { @@ -450,31 +533,31 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa customerAddress: form.customerAddress, projectName: form.projectName, reference: form.reference, - depositPercent: Number(form.depositPercent), + // A cleared field keeps the stored % (null) rather than failing the whole save at 0. + depositPercent: form.depositPercent === '' ? null : Number(form.depositPercent), notes: form.notes, - items: form.items.map((it, idx) => ({ + expectedUpdatedAt: stampRef.current, + items: form.items.filter((it) => !isBlankRow(it)).map((it, idx) => ({ seq: idx + 1, description: it.description ?? '', qty: Number(it.qty) || 0, unit: it.unit ?? 'แผ่น', unitPrice: Number(it.unitPrice) || 0, discountLabel: it.discountLabel ?? null, - netUnitPrice: Number(it.netUnitPrice) || Number(it.unitPrice) || 0, + netUnitPrice: lineNet(it), })), }; } - // Recalculate summary from form items - const subtotal = form.items.reduce((sum, it) => { - const net = Number(it.netUnitPrice) || Number(it.unitPrice) || 0; - return sum + net * (Number(it.qty) || 0); - }, 0); - const deposit = subtotal * Number(form.depositPercent || 0.5); + // Recalculate summary from form items, rounding each step to the satang as + // DepositNoticeRepository does (HALF_UP), so the screen matches the stored document. + const subtotal = round2(form.items.reduce((sum, it) => sum + lineNet(it) * (Number(it.qty) || 0), 0)); + const deposit = round2(subtotal * (Number(form.depositPercent) || 0)); // Issue #756: the served document's own vatPercent always wins once one exists — only // before that (no `doc` yet) does this fall back to FALLBACK_VAT_PERCENT above. const vatPercent = Number(doc?.vatPercent ?? FALLBACK_VAT_PERCENT); - const vat = deposit * vatPercent; - const total = deposit + vat; + const vat = round2(deposit * vatPercent); + const total = round2(deposit + vat); if (loading) { return ( @@ -822,21 +905,24 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa aria-describedby={fieldErrors.total ? fieldErrorId('doc-summary-panel') : undefined} className="px-4.5 py-3.5" > - {/* - Owner rulings (2026-09-29, 2026-10-23): the deposit % is what the source - quotation's PDF printed (legacy 50%, new-engine its own, blank 30%; 50% with no - live quotation) and is never editable at the deposit-notice stage: createDraft - never reads a client-supplied value and update keeps the draft's stored one. The - 30/50/70/100% preset buttons this used to be are gone; this is now a plain - read-only label, shown regardless of isIssued since there is no draft-only - editable state left to distinguish. - */} -
- % มัดจำ - - {Math.round(Number(form.depositPercent) * 100)}% - -
+ {/* Owner ruling 2026-10-23: the % starts from the quotation; sales may change it on + a DRAFT (whole number on screen, a fraction in the payload). Read-only once issued. */} + + {fieldErrors.depositPercent ? ( + + ) : null} {[ { label: 'รวมเป็นเงิน', value: subtotal }, { label: `ขอรับเงินมัดจำ (${Math.round(Number(form.depositPercent) * 100)}%)`, value: deposit }, @@ -900,23 +986,8 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa open={confirmIssue} title="ยืนยันการออกเอกสาร" message={( -
-

- ตรวจสอบยอดเงินก่อนออกเอกสารให้ {form.customerName || 'ลูกค้า'} — หลังจากนี้จะไม่สามารถแก้ไขได้ -

-
- ขอรับเงินมัดจำ ({Math.round(Number(form.depositPercent) * 100)}%) - {money(deposit)} บาท -
-
- ภาษีมูลค่าเพิ่ม {formatVatPercent(vatPercent)}% - {money(vat)} บาท -
-
- รวมเป็นเงินที่ต้องชำระ - {money(total)} บาท -
-
+ )} confirmLabel="ออกเอกสาร" busy={saving} diff --git a/frontend/src/features/deposits/DepositNoticePage.test.jsx b/frontend/src/features/deposits/DepositNoticePage.test.jsx index b885e40dc..040901277 100644 --- a/frontend/src/features/deposits/DepositNoticePage.test.jsx +++ b/frontend/src/features/deposits/DepositNoticePage.test.jsx @@ -147,7 +147,9 @@ describe('DepositNoticePage', () => { // UX-03 rule 4: a ฿0 deposit notice is meaningless and would still advance // the ticket's payment track — issuing it must be blocked even though // rules 1-3 (customer name, >=1 item, description+qty) are all satisfied. - it('blocks issue when the computed total is zero', async () => { + // A % outside a whole 1–100 gets its own message on the % field (it used to blame the unit + // price, or pass the gate at 150% and fail behind the confirm dialog). + it('blocks issue when the computed total is zero, or the % is outside 1–100, before the dialog', async () => { api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [{ ...validDraftDoc, @@ -163,6 +165,20 @@ describe('DepositNoticePage', () => { expect(await screen.findByText(/ยอดเงินรวมต้องมากกว่า 0/)).not.toBeNull(); expect(screen.queryByRole('dialog')).toBeNull(); + + fireEvent.change(document.getElementById('doc-item-0-qty'), { target: { value: '5' } }); + fireEvent.change(screen.getByPlaceholderText('ราคา'), { target: { value: '100' } }); + const percentInput = screen.getByLabelText('% มัดจำ'); + for (const typed of ['0', '150']) { + fireEvent.change(percentInput, { target: { value: typed } }); + fireEvent.click(screen.getByRole('button', { name: /ออกเอกสาร/ })); + const error = await screen.findByText('กรุณากรอก % มัดจำเป็นจำนวนเต็ม 1–100'); + expect(percentInput.getAttribute('aria-invalid')).toBe('true'); + expect(percentInput.getAttribute('aria-describedby')).toBe(error.id); + expect(document.activeElement).toBe(percentInput); + expect(screen.queryByText(/ยอดเงินรวมต้องมากกว่า 0/)).toBeNull(); + expect(screen.queryByRole('dialog')).toBeNull(); + } expect(api.depositNotices.issue).not.toHaveBeenCalled(); }); @@ -188,10 +204,115 @@ describe('DepositNoticePage', () => { expect(screen.queryByRole('alert')).toBeNull(); }); + // What บันทึก sends, and the summary beside it, must match what the backend stores: + // - a stored whole 0.33 shows, labels and saves as 33%; + // - a 100%-discounted line (net 0, unit 100) adds 0 and is sent at net 0, never its unit price; + // - money rounds per step like the backend: 100.10 at 33% is 33.03 + VAT 2.31 = 35.34 (35.35 unrounded); + // - a blank เพิ่มรายการ row is left out (the backend refuses a line without a description); + // - the draft's updatedAt travels as expectedUpdatedAt (a stale save is refused server-side); + // - a cleared % sends null, so the save keeps the stored % instead of failing the whole PUT. + it('saves what the backend will store: rounded %, zero-net line, per-step rounding, no blank row, the stamp, null for a cleared %', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ + depositNotices: [{ + ...validDraftDoc, depositPercent: 0.33, updatedAt: '2026-10-23T08:57:19.732123Z', + items: [ + { seq: 1, description: 'กระเบื้อง A', qty: 1, unit: 'แผ่น', unitPrice: 100.1, netUnitPrice: 100.1 }, + { seq: 2, description: 'ของแถม', qty: 2, unit: 'แผ่น', unitPrice: 100, netUnitPrice: 0 }, + ], + }], + }); + renderDepositNoticePage(); + + await screen.findByText('รายการสินค้า (2 รายการ)'); + const percentInput = screen.getByLabelText('% มัดจำ'); + expect(percentInput.value).toBe('33'); + expect(screen.getByText('ขอรับเงินมัดจำ (33%)')).not.toBeNull(); + expect(screen.getByText('100.10 บาท')).not.toBeNull(); + expect(screen.getByText('33.03 บาท')).not.toBeNull(); + expect(screen.getByText('2.31 บาท')).not.toBeNull(); + expect(screen.getByText('35.34 บาท')).not.toBeNull(); + + fireEvent.click(screen.getByRole('button', { name: /เพิ่มรายการ/ })); + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + await waitFor(() => expect(api.depositNotices.update).toHaveBeenCalledTimes(1)); + const payload = api.depositNotices.update.mock.calls[0][1]; + expect(payload).toMatchObject({ depositPercent: 0.33, expectedUpdatedAt: '2026-10-23T08:57:19.732123Z' }); + expect(payload.items).toEqual([ + expect.objectContaining({ seq: 1, unitPrice: 100.1, netUnitPrice: 100.1 }), + expect.objectContaining({ seq: 2, qty: 2, unitPrice: 100, netUnitPrice: 0 }), + ]); + + fireEvent.change(percentInput, { target: { value: '' } }); + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + await waitFor(() => expect(api.depositNotices.update).toHaveBeenCalledTimes(2)); + expect(api.depositNotices.update.mock.calls[1][1].depositPercent).toBeNull(); + expect(screen.queryByRole('alert')).toBeNull(); + }); + + // The page must not rewrite what it did not edit: a stored 0.125 (shown as 13) survives a save of + // another field untouched. + it('keeps a stored non-whole % as-is when only another field is edited', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ + depositNotices: [{ ...validDraftDoc, depositPercent: 0.125, updatedAt: 'T1' }], + }); + renderDepositNoticePage(); + + fireEvent.change(await screen.findByLabelText(/ที่อยู่/), { target: { value: 'ที่อยู่ใหม่' } }); + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + await waitFor(() => expect(api.depositNotices.update).toHaveBeenCalledTimes(1)); + expect(api.depositNotices.update.mock.calls[0][1]).toMatchObject({ depositPercent: 0.125, customerAddress: 'ที่อยู่ใหม่' }); + }); + + // Blank rows are dropped from the payload and the backend keeps the stored lines when `items` is + // empty — so saving/previewing an all-blank form would "succeed" and bring the old lines back. + it('refuses to save or preview when every row was cleared, with the items error, and calls no API', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [validDraftDoc] }); + renderDepositNoticePage(); + + await screen.findByDisplayValue('บริษัท ทดสอบ จำกัด'); + fireEvent.change(document.getElementById('doc-item-0-description'), { target: { value: '' } }); + fireEvent.change(document.getElementById('doc-item-0-qty'), { target: { value: '0' } }); + + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + expect((await screen.findByText('กรุณาเพิ่มรายการสินค้าอย่างน้อย 1 รายการ')).getAttribute('role')).toBe('alert'); + expect(api.depositNotices.update).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByRole('button', { name: /Preview|ตัวอย่าง/ })); + expect(api.depositNotices.update).not.toHaveBeenCalled(); + expect(api.depositNotices.preview).not.toHaveBeenCalled(); + }); + + // Someone else saved the draft since this page loaded it: the backend answers 409. The page + // reloads the draft (form and stamp) and shows the server's message, so the next save is a + // deliberate one over the fresh copy. + it('on a 409 save, reloads the draft and shows the server message', async () => { + const showToast = vi.fn(); + api.depositNotices.listByTicket + .mockResolvedValueOnce({ depositNotices: [{ ...validDraftDoc, depositPercent: 0.3, updatedAt: 'T1' }] }) + .mockResolvedValue({ depositNotices: [{ ...validDraftDoc, depositPercent: 0.4, updatedAt: 'T2' }] }); + const stale = Object.assign(new Error('ใบแจ้งรับมัดจำนี้ถูกแก้ไขโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง'), { status: 409 }); + api.depositNotices.update.mockRejectedValueOnce(stale); + renderDepositNoticePage({ showToast }); + + await screen.findByDisplayValue('บริษัท ทดสอบ จำกัด'); + const percentInput = screen.getByLabelText('% มัดจำ'); + fireEvent.change(percentInput, { target: { value: '35' } }); + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + + await waitFor(() => expect(showToast).toHaveBeenCalledWith('error', stale.message)); + expect(api.depositNotices.update.mock.calls[0][1]).toMatchObject({ depositPercent: 0.35, expectedUpdatedAt: 'T1' }); + await waitFor(() => expect(percentInput.value).toBe('40')); + + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + await waitFor(() => expect(api.depositNotices.update).toHaveBeenCalledTimes(2)); + expect(api.depositNotices.update.mock.calls[1][1]).toMatchObject({ depositPercent: 0.4, expectedUpdatedAt: 'T2' }); + }); + // A fully valid document opens the confirm dialog, and confirming it calls // api.depositNotices.issue for that document. - it('opens the confirm dialog for a valid document and issues it on confirm', async () => { - api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [validDraftDoc] }); + it('opens the confirm dialog for a valid document and issues it on confirm, with the stamp its own save returned', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [{ ...validDraftDoc, updatedAt: 'T1' }] }); + api.depositNotices.update.mockResolvedValue({ depositNotice: { updatedAt: 'T2' } }); renderDepositNoticePage(); await screen.findByDisplayValue('บริษัท ทดสอบ จำกัด'); @@ -202,7 +323,7 @@ describe('DepositNoticePage', () => { fireEvent.click(within(dialog).getByRole('button', { name: 'ออกเอกสาร' })); - await waitFor(() => expect(api.depositNotices.issue).toHaveBeenCalledWith(9002)); + await waitFor(() => expect(api.depositNotices.issue).toHaveBeenCalledWith(9002, { expectedUpdatedAt: 'T2' })); }); it('renders the empty-state and lets the user create a draft', async () => { @@ -214,8 +335,8 @@ describe('DepositNoticePage', () => { fireEvent.click(createButton); await waitFor(() => expect(api.depositNotices.createDraft).toHaveBeenCalledTimes(1)); - // depositPercent is deliberately NOT sent (owner ruling, 2026-09-29): the backend always - // ignores a caller-supplied value now and re-sources it from the ticket's own quotation. + // depositPercent is not sent on create: the backend picks the draft's starting percent. + // Sales changes it on the draft afterwards. expect(api.depositNotices.createDraft).toHaveBeenCalledWith('701', { notes: ['หมายเหตุมาตรฐาน'], }); @@ -241,8 +362,7 @@ describe('DepositNoticePage', () => { fireEvent.click(createButton); await waitFor(() => expect(api.pricingRequests.createDepositNoticeFromQuotation).toHaveBeenCalledTimes(1)); - // depositPercent is deliberately NOT sent (owner ruling, 2026-09-29) — the backend always - // ignores a caller-supplied value and re-sources it from the accepted quotation itself. + // depositPercent is not sent on create: the backend takes it from the accepted quotation. expect(api.pricingRequests.createDepositNoticeFromQuotation).toHaveBeenCalledWith(502, {}); expect(api.depositNotices.createDraft).not.toHaveBeenCalled(); }); @@ -297,24 +417,51 @@ describe('DepositNoticePage', () => { expect(await screen.findByDisplayValue('บริษัท ทดสอบ จำกัด')).not.toBeNull(); }); - // Owner ruling (2026-09-29): the deposit % is LOCKED to the source quotation and is no longer - // user-settable at the deposit-notice stage — the 30/50/70/100% preset buttons are gone, and - // the % renders as a plain read-only label sourced from the loaded draft's own depositPercent. - it('renders the deposit percent as a read-only label, with no preset buttons to change it', async () => { + // Owner ruling 2026-10-23: on a DRAFT the deposit % is an editable whole-number field (a typed + // 39.6 rounds to 40). The amount recalculates as sales types, and บันทึก sends the fraction. + // subtotal 500: 30% -> 150.00, 40% -> 200.00 deposit, 14.00 VAT, 214.00 total; cleared -> 0. + it('lets sales change the deposit percent on a draft, recalculates the amount, and saves it', async () => { api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [{ ...validDraftDoc, depositPercent: 0.3 }], }); renderDepositNoticePage(); await screen.findByDisplayValue('บริษัท ทดสอบ จำกัด'); - - // No interactive control can change the percent anymore. - for (const label of ['30%', '50%', '70%', '100%']) { - expect(screen.queryByRole('button', { name: label })).toBeNull(); - } - // The value the draft was loaded with (30%, not the 0.5 default) still renders, as a label. - expect(screen.getByText('30%')).not.toBeNull(); + const percentInput = screen.getByLabelText('% มัดจำ'); + expect(percentInput.disabled).toBe(false); + expect(percentInput.value).toBe('30'); expect(screen.getByText('ขอรับเงินมัดจำ (30%)')).not.toBeNull(); + expect(screen.getByText('150.00 บาท')).not.toBeNull(); + + fireEvent.change(percentInput, { target: { value: '39.6' } }); + + expect(percentInput.value).toBe('40'); + expect(screen.getByText('ขอรับเงินมัดจำ (40%)')).not.toBeNull(); + expect(screen.getByText('200.00 บาท')).not.toBeNull(); + expect(screen.getByText('214.00 บาท')).not.toBeNull(); + expect(screen.queryByText('150.00 บาท')).toBeNull(); + + fireEvent.click(screen.getByRole('button', { name: 'บันทึก' })); + await waitFor(() => expect(api.depositNotices.update).toHaveBeenCalledWith( + 9002, expect.objectContaining({ depositPercent: 0.4 }), + )); + + // A cleared field bills nothing: 0% and ฿0 for deposit, VAT and total, never a silent 50%. + fireEvent.change(percentInput, { target: { value: '' } }); + expect(screen.getByText('ขอรับเงินมัดจำ (0%)')).not.toBeNull(); + expect(screen.getAllByText('0.00 บาท')).toHaveLength(3); + }); + + it('shows the deposit percent read-only once the notice is issued', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ + depositNotices: [{ ...validDraftDoc, status: 'ISSUED', docNumber: 'GLRD69001', depositPercent: 0.3 }], + }); + renderDepositNoticePage(); + + await screen.findByDisplayValue('บริษัท ทดสอบ จำกัด'); + const percentInput = screen.getByLabelText('% มัดจำ'); + expect(percentInput.disabled).toBe(true); + expect(percentInput.value).toBe('30'); }); // Issue #756: the VAT rate must come from the served document's vatPercent, not a diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx index b08b0f1ff..ed27a51ba 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx @@ -36,6 +36,7 @@ import { canCreateCommercialOnlyRevision, canCreateCustomerQuotation, canCreateDepositNoticeFromQuotation, + issuedDepositNotice, canDecideDiscountApproval, canManageCustomerQuotation, canRecordCustomerQuotationOutcome, @@ -1721,6 +1722,14 @@ export function PricingRequestDetailPage({ user, showToast }) { if (acceptConfirm) setAcceptToConfirm({ engine, quotation }); else recordAccepted(engine, quotation); } + // Step 6: one deposit notice per deal (owner ruling 2026-10-23). Read only when the owner could + // otherwise be offered "สร้างใบแจ้งยอดเงินรับมัดจำ", so the block can point at an ISSUED one instead. + const depositNoticesQuery = useQuery({ + queryKey: queryKeys.depositNotices(summary?.ticketId), + queryFn: () => api.depositNotices.listByTicket(summary.ticketId).then((r) => r.depositNotices ?? []), + enabled: summary != null && canCreateDepositNoticeFromQuotation(user, summary), + }); + const issuedNotice = issuedDepositNotice(depositNoticesQuery.data); // itemId -> the factoryId Import has picked (as a string, matching SearchableCombobox's own // `code` convention) for that line. B6 (GLA-135): this used to hold the free-typed factory // NAME; it holds an id now, resolved server-side to the canonical master name on save. Same @@ -4318,7 +4327,15 @@ export function PricingRequestDetailPage({ user, showToast }) { ) : null} - {canCreateDepositNoticeFromQuotation(user, summary, dealQuotationForPr) ? ( + {issuedNotice && canCreateDepositNoticeFromQuotation(user, summary) ? ( +

+ ดีลนี้ออกใบแจ้งยอดเงินรับมัดจำแล้ว —{' '} + + เปิดเลขที่ {issuedNotice.docNumber} + +

+ ) : null} + {canCreateDepositNoticeFromQuotation(user, summary, dealQuotationForPr, depositNoticesQuery.data) ? (

ยืนยันคำสั่งซื้อแล้ว — สร้างใบแจ้งยอดเงินรับมัดจำจากใบเสนอราคาที่ลูกค้ายอมรับ (แก้ไข/ออกเอกสารในหน้าใบแจ้งยอดเงินรับมัดจำ) diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx index 5dee09c5e..e5ccc9d3c 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx @@ -75,6 +75,10 @@ vi.mock('../../api/index.js', () => ({ confirmOrder: vi.fn(), createDepositNoticeFromQuotation: vi.fn(), }, + // Step 6: the deal's deposit notices, so the create action is not offered once one is ISSUED. + depositNotices: { + listByTicket: vi.fn(), + }, // CR-1 (GLA-167): lead-time change requests (R2 / R10). leadTimeChanges: { listForPricingRequest: vi.fn(), @@ -328,6 +332,7 @@ function setApiDefaults() { result: { ticket: { summary: { id: 701 } }, pricingRequest: { id: 501, orderConfirmedAt: '2026-07-21T00:00:00Z' } }, }); api.pricingRequests.createDepositNoticeFromQuotation.mockResolvedValue({ depositNotice: { id: 9901, status: 'DRAFT' } }); + api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [] }); api.catalog.prices.mockResolvedValue({ items: [] }); // B6 (GLA-135): a small real-looking factory roster — enough for the picker + duplicate-name // tests below without pulling in the full catalog fixture this file doesn't otherwise need. @@ -3523,6 +3528,23 @@ describe('PricingRequestDetailPage Step 5: Customer Decision and Commercial Revi }); describe('PricingRequestDetailPage Step 6: Deposit, Payment, and Order Confirmation', () => { + // Owner ruling 2026-10-23: one deposit notice per deal. Once one is ISSUED the create action is + // gone (the backend 409s it) and the block says which notice exists and where to open it. + it('replaces "สร้างใบแจ้งยอดเงินรับมัดจำ" with a pointer to the deal\'s ISSUED notice', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ + depositNotices: [{ id: 77, status: 'ISSUED', docNumber: 'GLRD69001', version: 1 }], + }); + const request = buildRequest({ summary: { status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' } }); + renderDetailPage({ user: salesOwner, request }); + await waitForLoaded(request); + + const link = await screen.findByRole('link', { name: /GLRD69001/ }); + expect(link.getAttribute('href')).toBe(`/tickets/${request.summary.ticketId}/deposit`); + expect(screen.getByText(/ออกใบแจ้งยอดเงินรับมัดจำแล้ว/)).not.toBeNull(); + expect(screen.queryByRole('button', { name: 'สร้างใบแจ้งยอดเงินรับมัดจำ' })).toBeNull(); + expect(api.depositNotices.listByTicket).toHaveBeenCalledWith(request.summary.ticketId); + }); + it('offers "ยืนยันคำสั่งซื้อ" to the owning sales rep once QUOTATION_ACCEPTED, before the bridge has run', async () => { const request = buildRequest({ summary: { status: 'QUOTATION_ACCEPTED', orderConfirmedAt: null } }); renderDetailPage({ user: salesOwner, request }); @@ -3568,6 +3590,23 @@ describe('PricingRequestDetailPage Step 6: Deposit, Payment, and Order Confirmat expect(screen.queryByRole('button', { name: 'สร้างใบแจ้งยอดเงินรับมัดจำ' })).toBeNull(); }); + // Owner ruling 2026-10-23: one deposit notice per deal. Once one is ISSUED the create action is + // gone (the backend 409s it) and the block says which notice exists and where to open it. + it('replaces "สร้างใบแจ้งยอดเงินรับมัดจำ" with a pointer to the deal\'s ISSUED notice', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ + depositNotices: [{ id: 77, status: 'ISSUED', docNumber: 'GLRD69001', version: 1 }], + }); + const request = buildRequest({ summary: { status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' } }); + renderDetailPage({ user: salesOwner, request }); + await waitForLoaded(request); + + const link = await screen.findByRole('link', { name: /GLRD69001/ }); + expect(link.getAttribute('href')).toBe(`/tickets/${request.summary.ticketId}/deposit`); + expect(screen.getByText(/ออกใบแจ้งยอดเงินรับมัดจำแล้ว/)).not.toBeNull(); + expect(screen.queryByRole('button', { name: 'สร้างใบแจ้งยอดเงินรับมัดจำ' })).toBeNull(); + expect(api.depositNotices.listByTicket).toHaveBeenCalledWith(request.summary.ticketId); + }); + it('never shows either Step 6 button to CEO or Import — read-only', async () => { const request = buildRequest({ summary: { status: 'QUOTATION_ACCEPTED', orderConfirmedAt: null } }); renderDetailPage({ user: ceoUser, request }); diff --git a/frontend/src/features/pricingRequests/pricingRequestMeta.js b/frontend/src/features/pricingRequests/pricingRequestMeta.js index c176b2bbc..3d5810f92 100644 --- a/frontend/src/features/pricingRequests/pricingRequestMeta.js +++ b/frontend/src/features/pricingRequests/pricingRequestMeta.js @@ -308,10 +308,18 @@ export function confirmOrderBlockedReason(pr) { * only, reachable once the bridge action above has run (the endpoint's own precondition — * ticket.status=quotation_issued — is only true after confirmOrder). `acceptedQuotation` is the * page's new-engine quotation, when it has one: a ไม่รับมัดจำ (0%) one 409s on the server (#1173), - * so it is not offered. The deal's deposit policy is not on the pricing-request payload. */ -export function canCreateDepositNoticeFromQuotation(user, pr, acceptedQuotation = null) { + * so it is not offered. The deal's deposit policy is not on the pricing-request payload. Never + * offered while the deal has an ISSUED deposit notice (one per deal, + * DepositNoticeService.createDraftFromQuotation). */ +export function canCreateDepositNoticeFromQuotation(user, pr, acceptedQuotation = null, depositNotices = []) { return canManageCustomerQuotation(user, pr) && pr?.status === 'QUOTATION_ACCEPTED' && Boolean(pr?.orderConfirmedAt) - && !(acceptedQuotation?.docStatus === 'ACCEPTED' && acceptedQuotation.depositPercent === 0); + && !(acceptedQuotation?.docStatus === 'ACCEPTED' && acceptedQuotation.depositPercent === 0) + && !issuedDepositNotice(depositNotices); +} + +/** The deal's ISSUED deposit notice (at most one is live), or null. */ +export function issuedDepositNotice(depositNotices = []) { + return (depositNotices ?? []).find((d) => d.status === 'ISSUED') ?? null; } /** Mirrors PricingRequestService.cancel: owner sales OR ceo, any cancellable status. */ diff --git a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js index 49116ef43..18aa32b8c 100644 --- a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js +++ b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js @@ -452,6 +452,15 @@ describe('canCreateDepositNoticeFromQuotation', () => { { docStatus: 'ACCEPTED', depositPercent: null })).toBe(true); expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, null)).toBe(true); // legacy chain }); + + // Owner ruling 2026-10-23 (DepositNoticeService.createDraftFromQuotation): one notice per deal. + it('rejects once the deal has an ISSUED deposit notice; a DRAFT or SUPERSEDED one does not block', () => { + const confirmed = pr({ status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' }); + expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, null, + [{ id: 1, status: 'SUPERSEDED' }, { id: 2, status: 'ISSUED', docNumber: 'GLRD69001' }])).toBe(false); + expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, null, + [{ id: 1, status: 'SUPERSEDED' }, { id: 3, status: 'DRAFT' }])).toBe(true); + }); }); describe('canCancelPricingRequest', () => { diff --git a/frontend/src/features/quotations/quotationMeta.js b/frontend/src/features/quotations/quotationMeta.js index 236e78a19..66e1fa7a5 100644 --- a/frontend/src/features/quotations/quotationMeta.js +++ b/frontend/src/features/quotations/quotationMeta.js @@ -882,7 +882,7 @@ export function usesDirectNetPriceSqm(item, documentLanguage = 'TH') { // still always the SERVER's (see itemInputFromRow's own comment on that), and nothing here feeds // money math. They exist solely so the input can render/accept "แผ่น/ตร.ม." while the value that // actually travels to the server, and back, is still ตร.ม./แผ่น. -function round2(n) { +export function round2(n) { return Math.round((Number(n) + Number.EPSILON) * 100) / 100; } diff --git a/frontend/src/features/tickets/DealDepositPanel.jsx b/frontend/src/features/tickets/DealDepositPanel.jsx index 5356d5bae..3e5984594 100644 --- a/frontend/src/features/tickets/DealDepositPanel.jsx +++ b/frontend/src/features/tickets/DealDepositPanel.jsx @@ -4,12 +4,15 @@ import { Link, useNavigate } from 'react-router-dom'; import { api, ROLE_PERMISSIONS } from '../../api/index.js'; import { queryKeys } from '../../api/queryKeys.js'; import { Button } from '../../components/common/Button.jsx'; +import { ConfirmDialog } from '../../components/common/ConfirmDialog.jsx'; import { Panel } from '../../components/common/Layout.jsx'; import { Modal } from '../../components/common/Modal.jsx'; import { StatusBadge } from '../../components/common/StatusBadge.jsx'; import { depositNoticeStatusLabel, depositPolicyLabel } from '../../utils/format.js'; import { downloadBlob } from '../../utils/download.js'; import { canCreateDepositNoticeFromQuotation } from '../pricingRequests/pricingRequestMeta.js'; +import { newestQuotationByRequest } from '../quotations/quotationMeta.js'; +import { DepositIssueSummary } from '../deposits/DepositNoticePage.jsx'; import { buttonVariants } from '../../components/common/Button.jsx'; import { cn } from '../../utils/cn.js'; @@ -86,7 +89,7 @@ function pickAcceptedPricingRequest(pricingRequests = []) { * its own "ยืนยันคำสั่งซื้อ..." box. Both panels can show that specific * create action at once until a notice exists for this deal. */ -export function DealDepositPanel({ user, ticketId, summary, availableActions = [], pricingRequests = [], showToast }) { +export function DealDepositPanel({ user, ticketId, summary, availableActions = [], pricingRequests = [], dealQuotations = [], showToast }) { const queryClient = useQueryClient(); const navigate = useNavigate(); const role = user?.role; @@ -100,6 +103,7 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ const [policyReason, setPolicyReason] = useState(''); const [downloadingFormat, setDownloadingFormat] = useState(null); const [previewHtml, setPreviewHtml] = useState(''); + const [confirmIssue, setConfirmIssue] = useState(false); const st = summary?.status; const ps = summary?.paymentStatus; @@ -157,13 +161,18 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ }); const issueMutation = useMutation({ - mutationFn: () => api.depositNotices.issue(doc.id), + // The dialog shows the amounts as listed; the stamp makes the server refuse if the draft moved on since. + mutationFn: () => api.depositNotices.issue(doc.id, { expectedUpdatedAt: doc.updatedAt ?? null }), onSuccess: (res) => { showToast?.('success', `ออกเอกสาร ${res.depositNotice.docNumber} แล้ว`); setPreviewHtml(''); + setConfirmIssue(false); invalidateAfterDepositChange(); }, - onError: (err) => showToast?.('error', err.message || 'ออกเอกสารไม่สำเร็จ'), + onError: (err) => { + setConfirmIssue(false); + showToast?.('error', err.message || 'ออกเอกสารไม่สำเร็จ'); + }, }); const previewMutation = useMutation({ @@ -207,7 +216,7 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ // Rule 4's cutoff), but the role/ownership check is kept here too, defense-in-depth, so this // control can never render editable off a stale/misconfigured availableActions response. const canSetPolicy = hasAction('WAIVE_DEPOSIT') && (role === 'sales_manager' || (isSales && isOwner)); - const canCreateNotice = pr != null && canCreateDepositNoticeFromQuotation(user, pr); + const canCreateNotice = pr != null && canCreateDepositNoticeFromQuotation(user, pr, newestQuotationByRequest(dealQuotations).get(pr.id) ?? null, depositNoticesQuery.data); const canManageThisNotice = isSales && isOwner; // Legacy dual-track creation path (ticket.confirmCustomer → ISSUE_DEPOSIT_NOTICE), // independent of the PricingRequest/CustomerQuotation chain above — mirrors the @@ -298,11 +307,14 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ ) : canManageThisNotice ? ( ) : null}

+ {doc.status === 'ISSUED' ? ( +

ดีลนี้ออกใบแจ้งยอดเงินรับมัดจำแล้ว — สร้างฉบับใหม่ไม่ได้

+ ) : null} {previewHtml ? (
@@ -317,6 +329,9 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ ) : canCreateNotice ? (

ลูกค้ายืนยันคำสั่งซื้อแล้ว — สร้างใบแจ้งยอดเงินรับมัดจำจากใบเสนอราคาที่ยอมรับได้เลย

+ {/* The backend picks a new notice's % and ignores a client value + (DepositNoticeService.createDraftFromQuotation; findGoverningQuotation, #1170), so no + % input here. Sales changes the % on the draft (DepositNoticePage, ruling 2026-10-23). */}
+ {doc ? ( + + )} + confirmLabel="ออกเอกสาร" + busy={issueMutation.isPending} + onCancel={() => setConfirmIssue(false)} + onConfirm={() => issueMutation.mutate()} + /> + ) : null} + {policyOpen ? ( ) : null} diff --git a/frontend/src/features/tickets/TicketDetailPage.test.jsx b/frontend/src/features/tickets/TicketDetailPage.test.jsx index aeddb49b3..ff8b39439 100644 --- a/frontend/src/features/tickets/TicketDetailPage.test.jsx +++ b/frontend/src/features/tickets/TicketDetailPage.test.jsx @@ -3162,6 +3162,72 @@ describe('TicketDetailPage', () => { expect(section.queryByRole('button', { name: 'ยืนยันรับมัดจำ' })).toBeNull(); }); + // Owner ruling 2026-10-23: ออกเอกสาร on the deal page goes through a confirm dialog with the + // stored percent and amounts (the notice is a numbered customer document from then on). + it('the owning rep issues a draft notice only through a confirm dialog showing its % and amounts', async () => { + api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [{ + id: 9002, ticketId: 701, version: 1, status: 'DRAFT', docNumber: null, customerName: 'บริษัท ทดสอบ จำกัด', + depositPercent: 0.4, depositAmount: 200, vatPercent: 0.07, vatAmount: 14, totalPayable: 214, items: [], + updatedAt: '2026-10-23T08:57:19.732123Z', + }] }); + api.depositNotices.issue.mockResolvedValue({ depositNotice: { id: 9002, status: 'ISSUED', docNumber: 'GLRD69001' } }); + + renderTicketDetailPage(salesOwnerUser); + const section = await depositSection(); + fireEvent.click(await section.findByRole('button', { name: 'ออกเอกสาร' })); + + const dialog = await screen.findByRole('dialog'); + expect(within(dialog).getByText('ขอรับเงินมัดจำ (40%)')).not.toBeNull(); + expect(within(dialog).getByText('200.00 บาท')).not.toBeNull(); + expect(within(dialog).getByText('ภาษีมูลค่าเพิ่ม 7%')).not.toBeNull(); + expect(within(dialog).getByText('14.00 บาท')).not.toBeNull(); + expect(within(dialog).getByText('214.00 บาท')).not.toBeNull(); + expect(api.depositNotices.issue).not.toHaveBeenCalled(); + + fireEvent.click(within(dialog).getByRole('button', { name: 'ออกเอกสาร' })); + // The amounts in the dialog are the draft as read at that stamp; an edit since is refused server-side. + await waitFor(() => expect(api.depositNotices.issue).toHaveBeenCalledWith(9002, { expectedUpdatedAt: '2026-10-23T08:57:19.732123Z' })); + }); + + // Owner ruling 2026-10-23: one deposit notice per deal. With one ISSUED the panel never offers + // to create another (the backend 409s it), and says so beside the issued number. + it('with an ISSUED notice, offers no create action and says the deal already has its notice', async () => { + api.pricingRequests.listForTicket.mockResolvedValue({ items: [ + { id: 501, ticketId: 701, ticketCreatedById: 1, status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' }, + ] }); + api.depositNotices.listByTicket.mockResolvedValue({ depositNotices: [{ + id: 9001, ticketId: 701, version: 1, status: 'ISSUED', docNumber: 'GLRD69001', depositPercent: 0.4, + depositAmount: 200, vatPercent: 0.07, vatAmount: 14, totalPayable: 214, items: [], + }] }); + + renderTicketDetailPage(salesOwnerUser); + const section = await depositSection(); + + expect(await section.findByText('GLRD69001')).not.toBeNull(); + expect(section.getByText('ดีลนี้ออกใบแจ้งยอดเงินรับมัดจำแล้ว — สร้างฉบับใหม่ไม่ได้')).not.toBeNull(); + expect(section.queryByRole('button', { name: 'สร้างใบแจ้งยอดเงินรับมัดจำ' })).toBeNull(); + }); + + // A ไม่รับมัดจำ (0%) accepted quotation 409s on the server (#1173): the panel passes the deal's + // accepted quotation (3rd arg) and the notices (4th) — it used to put the notices in the 3rd slot. + it('offers the create action for an accepted quotation, but not when that quotation is 0%', async () => { + api.pricingRequests.listForTicket.mockResolvedValue({ items: [ + { id: 501, ticketId: 701, ticketCreatedById: 1, status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' }, + ] }); + const quotation = (depositPercent) => ({ id: 31, origin: 'PRICING_REQUEST', pricingRequestId: 501, docStatus: 'ACCEPTED', depositPercent }); + api.dealQuotations.listForTicket.mockResolvedValue({ items: [quotation(0)] }); + const { unmount } = renderTicketDetailPage(salesOwnerUser); + const section = await depositSection(); + await waitFor(() => expect(api.dealQuotations.listForTicket).toHaveBeenCalled()); + expect(section.queryByRole('button', { name: 'สร้างใบแจ้งยอดเงินรับมัดจำ' })).toBeNull(); + unmount(); + + api.dealQuotations.listForTicket.mockResolvedValue({ items: [quotation(0.3)] }); + renderTicketDetailPage(salesOwnerUser); + const section2 = await depositSection(); + expect(await section2.findByRole('button', { name: 'สร้างใบแจ้งยอดเงินรับมัดจำ' })).not.toBeNull(); + }); + it('import (no business in the deposit section) never gets a "การเงิน" tab at all', async () => { renderTicketDetailPage({ id: 7, employeeId: 7, name: 'ฝ่ายนำเข้า', role: 'import' });