From 117bc031cd3a59fece2959ffa1b6fa4a346230f0 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:00:52 +0700 Subject: [PATCH 01/18] test(sales): pin that sales may change the deposit % on a draft notice and the amount follows (RED) Owner ruling 2026-10-23 reverses the 2026-09-29 lock. Red until DepositNoticeService.update stops overriding the percent, refuses 0, and the page offers an editable field on a draft. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- ...epositNoticeIssueGuardIntegrationTest.java | 41 +++++++++++++++ .../hr/deposit/DepositNoticeServiceTest.java | 40 ++++++++++++--- .../src/api/mockApi.depositNotices.test.js | 33 ++++++++++++ .../deposits/DepositNoticePage.test.jsx | 50 +++++++++++++------ 4 files changed, 143 insertions(+), 21 deletions(-) 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..5f8c592ef 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 @@ -232,6 +232,47 @@ void issue_secondDraftOnSameTicketSucceedsAndSupersedesTheFirst() { assertThat(secondReread.docNumber()).isEqualTo(secondDocNumber.get()); } + // 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 updated = depositNoticeService.update(draftId, new DepositNoticeDraftRequest( + "ACME", "0100000000000", "Bangkok", "Showroom", "REF-2", + new BigDecimal("0.40"), List.of(), secondDraftItems()), salesActor); + + assertThat(updated.depositPercent()).isEqualByComparingTo("0.40"); + assertThat(updated.subtotal()).isEqualByComparingTo("100.00"); + assertThat(updated.depositAmount()).isEqualByComparingTo("40.00"); + assertThat(updated.vatAmount()).isEqualByComparingTo("2.80"); + assertThat(updated.totalPayable()).isEqualByComparingTo("42.80"); + assertThat(depositNoticeService.preview(draftId, salesActor)) + .contains("ขอรับเงินมัดจำ (40%)") + .contains("40.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)"); + }); + + 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()); + } + // ───────────────────────────────────────────────────────────────────────────────────── // Fixture helpers — mirrors InventoryDeliveryFulfilmentIntegrationTest's approach for // driving a deal to an issued deposit notice through the real Steps 1-6 services. 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..ac035bfef 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 @@ -934,7 +934,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 +1004,52 @@ 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. + 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()); + assertThat(captor.getValue().depositPercent()).isEqualByComparingTo("0.50"); + } + + @Test + void update_refusesAZeroDepositPercent_andWritesNothing() { + stubDraft(99L, 10L); + 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"); + }); + verify(docs, never()).update(anyLong(), any()); + } + private static void assertForbidden(org.assertj.core.api.ThrowableAssert.ThrowingCallable action) { assertThatThrownBy(action) .isInstanceOfSatisfying(ApiException.class, e -> diff --git a/frontend/src/api/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index f54b3ad77..52e283fc9 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -425,6 +425,39 @@ 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 is 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); + + await expect(api.depositNotices.update(draft.id, { depositPercent: 0 })) + .rejects.toMatchObject({ status: 400, message: 'เปอร์เซ็นต์มัดจำต้องมากกว่า 0' }); + const { depositNotice: reread } = await api.depositNotices.get(draft.id); + expect(reread.depositPercent).toBe(0.4); + }); + + it('on an ISSUED notice is refused with 409 and changes nothing', async () => { + const { ticketId } = await driveTicketToAcceptedQuotation(); + const { depositNotice: draft } = await api.depositNotices.createDraft(ticketId, { notes: [] }); + await api.tickets.confirmCustomer(ticketId); + const { depositNotice: issued } = await api.depositNotices.issue(draft.id); + + 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); + }); +}); + // 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 diff --git a/frontend/src/features/deposits/DepositNoticePage.test.jsx b/frontend/src/features/deposits/DepositNoticePage.test.jsx index b885e40dc..de8cff311 100644 --- a/frontend/src/features/deposits/DepositNoticePage.test.jsx +++ b/frontend/src/features/deposits/DepositNoticePage.test.jsx @@ -214,8 +214,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 draft's starting percent comes from the ticket's + // own quotation on the backend. Sales changes it on the draft afterwards. expect(api.depositNotices.createDraft).toHaveBeenCalledWith('701', { notes: ['หมายเหตุมาตรฐาน'], }); @@ -241,8 +241,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 +296,45 @@ 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. 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. + 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: '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 }), + )); + }); + + 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 From 9b415d57c1a78e6d618bbdb2fca31ba88da4d931 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:08:36 +0700 Subject: [PATCH 02/18] feat(sales): sales may change the deposit % on a draft notice; the amount follows Owner ruling 2026-10-23 reverses the 2026-09-29 lock. DepositNoticeService.update now persists the request's percent (null keeps the draft's own, 0 or less is refused with 400); requireDraft still 409s an issued notice. The repository already recomputes the amount from the percent on update. DepositNoticePage shows the % as a whole-number field on a draft, recalculates the amount as sales types, and is read-only once issued. The mock refuses 0 the same way. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- .../glr/hr/deposit/DepositNoticeService.java | 17 +++++---- frontend/src/api/mockApi.js | 1 + .../features/deposits/DepositNoticePage.jsx | 35 ++++++++----------- .../src/features/tickets/DealDepositPanel.jsx | 4 +++ 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeService.java index b042d76a1..feeffc978 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 @@ -149,7 +149,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"); @@ -177,15 +178,17 @@ public static ApiException zeroDepositRefusal(String quotationNumber) { public DepositNoticeDto update(long docId, DepositNoticeDraftRequest req, UserPrincipal actor) { 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"); + } var effective = new DepositNoticeDraftRequest( req.customerName(), req.customerTaxId(), req.customerAddress(), req.projectName(), req.reference(), - doc.depositPercent(), + depositPercent, req.notes(), req.items() ); docs.update(docId, effective); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 26632a528..e71fb545d 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -13394,6 +13394,7 @@ export const api = { const doc = mockDepositNotices.find((d) => d.id === Number(docId)); if (!doc) fail('ไม่พบใบแจ้งรับมัดจำนี้', 404); if (doc.status !== 'DRAFT') fail('ใบแจ้งรับมัดจำนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)', 409); + if (payload.depositPercent != null && !(Number(payload.depositPercent) > 0)) fail('เปอร์เซ็นต์มัดจำต้องมากกว่า 0', 400); Object.assign(doc, { customerName: payload.customerName ?? doc.customerName, customerTaxId: payload.customerTaxId ?? doc.customerTaxId, diff --git a/frontend/src/features/deposits/DepositNoticePage.jsx b/frontend/src/features/deposits/DepositNoticePage.jsx index a8d374b24..afd84dd04 100644 --- a/frontend/src/features/deposits/DepositNoticePage.jsx +++ b/frontend/src/features/deposits/DepositNoticePage.jsx @@ -274,10 +274,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 takes the new draft's + // percent from the ticket's own quotation (see DepositNoticeService.createDraft), falling + // back to 0.50 only when there is no quotation at all. Sales changes it on the draft. if (acceptedPricingRequestId != null) { return api.pricingRequests.createDepositNoticeFromQuotation(acceptedPricingRequestId, {}); } @@ -377,7 +376,7 @@ 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'); @@ -469,7 +468,7 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa 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); + const deposit = 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); @@ -822,21 +821,15 @@ 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. */} + {[ { label: 'รวมเป็นเงิน', value: subtotal }, { label: `ขอรับเงินมัดจำ (${Math.round(Number(form.depositPercent) * 100)}%)`, value: deposit }, diff --git a/frontend/src/features/tickets/DealDepositPanel.jsx b/frontend/src/features/tickets/DealDepositPanel.jsx index 5356d5bae..d842ad002 100644 --- a/frontend/src/features/tickets/DealDepositPanel.jsx +++ b/frontend/src/features/tickets/DealDepositPanel.jsx @@ -317,6 +317,10 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ ) : canCreateNotice ? (

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

+ {/* A new notice takes its deposit % from the accepted quotation's own deposit_percent + — createDraft ignores any client-supplied value (see DepositNoticeService.createDraft / + DepositNoticeRepository#findGoverningQuotation), so no % input here. Sales + changes the % on the draft itself (DepositNoticePage, owner ruling 2026-10-23). */}
) : 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/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 78af451d6..18aa32b8c 100644 --- a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js +++ b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js @@ -456,9 +456,9 @@ describe('canCreateDepositNoticeFromQuotation', () => { // 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, + expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, null, [{ id: 1, status: 'SUPERSEDED' }, { id: 2, status: 'ISSUED', docNumber: 'GLRD69001' }])).toBe(false); - expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, + expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, null, [{ id: 1, status: 'SUPERSEDED' }, { id: 3, status: 'DRAFT' }])).toBe(true); }); }); 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 d842ad002..f7dae2815 100644 --- a/frontend/src/features/tickets/DealDepositPanel.jsx +++ b/frontend/src/features/tickets/DealDepositPanel.jsx @@ -4,12 +4,14 @@ 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 { DepositIssueSummary } from '../deposits/DepositNoticePage.jsx'; import { buttonVariants } from '../../components/common/Button.jsx'; import { cn } from '../../utils/cn.js'; @@ -100,6 +102,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; @@ -161,9 +164,13 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ 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 +214,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, 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 +305,14 @@ export function DealDepositPanel({ user, ticketId, summary, availableActions = [ ) : canManageThisNotice ? ( ) : null}

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

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

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

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

- {/* A new notice takes its deposit % from the accepted quotation's own deposit_percent - — createDraft ignores any client-supplied value (see DepositNoticeService.createDraft / - DepositNoticeRepository#findGoverningQuotation), so no % input here. Sales - changes the % on the draft itself (DepositNoticePage, owner ruling 2026-10-23). */} + {/* 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 ? ( Date: Fri, 23 Oct 2026 19:56:38 +0700 Subject: [PATCH 14/18] chore(sales): move the one-notice mock helper and two new tests away from #1170/#1173's hunks No behaviour change: requireNoIssuedDepositNotice now sits above buildMockDoc (it was adjacent to #1170's rewrite of mockPickTicketQuotationForDepositNotice), and the new predicate and step-6 tests open their describe blocks instead of sharing #1173's insertion points. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- frontend/src/api/mockApi.js | 7 +++++++ .../PricingRequestDetailPage.test.jsx | 17 +++++++++++++++++ .../pricingRequests/pricingRequestMeta.test.js | 8 ++++++++ 3 files changed, 32 insertions(+) diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 721a33d67..a0c5a2240 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -2440,6 +2440,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; diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx index e84473f1c..e5ccc9d3c 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx @@ -3528,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 }); diff --git a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js index 18aa32b8c..0af4167d7 100644 --- a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js +++ b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js @@ -428,6 +428,14 @@ describe('canConfirmOrder (roles)', () => { }); describe('canCreateDepositNoticeFromQuotation', () => { + // 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, + [{ id: 1, status: 'SUPERSEDED' }, { id: 2, status: 'ISSUED', docNumber: 'GLRD69001' }])).toBe(false); + expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, + [{ id: 1, status: 'SUPERSEDED' }, { id: 3, status: 'DRAFT' }])).toBe(true); + }); it('allows the owner sales rep only AFTER the bridge has run (orderConfirmedAt set)', () => { expect(canCreateDepositNoticeFromQuotation(salesOwner, pr({ status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' }))).toBe(true); From 9e0dfdf58c5357e15af07c5ba217eb927efab07f Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Fri, 23 Oct 2026 20:21:25 +0700 Subject: [PATCH 15/18] chore(sales): qualify ApiException in DepositNoticeRepository so its import does not collide with #1170's No behaviour change (DepositNoticeIssueGuardIntegrationTest 7, DepositNoticeServiceTest 51, DepositNoticeControllerTest 10 green with clean). Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01UXmTJDVga17Rn5az4YBjZ2 --- .../java/th/co/glr/hr/deposit/DepositNoticeRepository.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java index 159d5d4c9..0d9bca1ab 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 @@ -21,7 +21,6 @@ import org.springframework.transaction.annotation.Transactional; import th.co.glr.hr.dealquotation.DealQuotationRenderAdapter; import th.co.glr.hr.ticket.QuotationRenderer; -import th.co.glr.hr.common.ApiException; @Repository public class DepositNoticeRepository { @@ -264,7 +263,7 @@ public void update(long docId, DepositNoticeDraftRequest req, List Date: Wed, 28 Oct 2026 02:10:46 +0700 Subject: [PATCH 16/18] fix(sales): issuing is refused while another deposit notice on the deal is issued; 2-decimal lines; the quotation's own discount wording kept MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Owner rulings 2026-10-28 on the #1176 review: - Item 1(a): DepositNoticeService.issue refuses while another notice on the deal is ISSUED ("ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ … แล้ว จึงออกฉบับร่างนี้ไม่ได้"), so a second draft can no longer replace the document the customer already has. Mock mirrors it. - Item 2 ("use 2 decimals for all"): foldAdjustments rounds each share and each new net price to 2 decimals with no remainder pushed onto a last line; the notice total is the sum of its own 2-dp lines and may differ from the quotation by a few satang (documented as the ruled outcome). - A line keeps the quotation's own discount wording and gains "+ ส่วนลดพิเศษ … ต่อหน่วย" only when the fold lowered its price. - The rebase onto develop left a duplicate requireNoIssuedDepositNotice in mockApi.js; one removed. Tests: issue_secondDraftOnSameTicket_…StillRefusedWhileAnotherNoticeIsIssued (IT; red with the guard disabled, green with it), depositNoticeSecondIssue_isRefused_… (PaymentTrack IT), foldAdjustments_keepsEachLinesOwnLabel_…TwoDpLines (unit), mock deposit-notice tests. Green: DepositNoticeServiceTest 50, DepositNoticeIssueGuardIntegrationTest 7, PaymentTrackIntegrationTest 14 (local Postgres), mockApi.depositNotices + deposits 36. Co-Authored-By: Claude Opus 5.5 --- .../hr/deposit/DepositNoticeRepository.java | 35 +++++++++---------- .../glr/hr/deposit/DepositNoticeService.java | 7 ++++ ...epositNoticeIssueGuardIntegrationTest.java | 21 ++++------- .../hr/deposit/DepositNoticeServiceTest.java | 28 ++++++++------- .../ticket/PaymentTrackIntegrationTest.java | 28 ++++++++------- .../src/api/mockApi.depositNotices.test.js | 19 ++++++++-- frontend/src/api/mockApi.js | 31 ++++++++-------- 7 files changed, 90 insertions(+), 79 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java b/backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeRepository.java index 0d9bca1ab..cf79e4fb0 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 @@ -422,13 +422,14 @@ public List findDealQuotationItems(long quotationId) { } /** - * Owner ruling 2026-10-23: a DEAL_DIRECT ส่วนลดพิเศษ row (ADJUSTMENT, qty -1) cannot be a + * 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: each share HALF_UP to 2 dp, the remainder on the last - * line with an amount, so the shares sum to the adjustment exactly. Descriptions are kept; a - * changed line's label shows its new per-unit discount. - * ponytail: the new net is stored at 2 dp, so a line whose qty does not divide its new amount - * moves the subtotal by under qty x 0.005 baht; spread per unit if that ever matters. + * 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) { @@ -437,27 +438,23 @@ static List foldAdjustments(List folded = new java.util.ArrayList<>(); - BigDecimal left = adjustment; - for (int i = 0; i < goods.size(); i++) { - DepositNoticeItemRequest it = goods.get(i); + for (DepositNoticeItemRequest it : goods) { BigDecimal amount = it.netUnitPrice().multiply(it.qty()); if (amount.signum() == 0) { folded.add(it); continue; } - BigDecimal share = i == last ? left - : adjustment.multiply(amount).divide(goodsTotal, 2, RoundingMode.HALF_UP); - left = left.subtract(share); + BigDecimal share = adjustment.multiply(amount).divide(goodsTotal, 2, RoundingMode.HALF_UP); BigDecimal net = amount.add(share).divide(it.qty(), 2, RoundingMode.HALF_UP); - BigDecimal perUnit = it.unitPrice().subtract(net); + 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(), - perUnit.signum() > 0 ? "ส่วนลด " + perUnit.stripTrailingZeros().toPlainString() + " ต่อหน่วย" : null, - net)); + label, net)); } return folded; } 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 47b3237f4..5fe1b2df6 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 @@ -268,6 +268,13 @@ public DepositNoticeDto issue(long docId, UserPrincipal actor) { if (doc.depositAmount() == null || doc.depositAmount().signum() <= 0) { throw new ApiException(HttpStatus.CONFLICT, "ใบแจ้งรับมัดจำนี้มียอดมัดจำ 0 บาท จึงออกเอกสารไม่ได้"); } + // 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); 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 be44b820a..ef80caa1e 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 @@ -211,7 +211,7 @@ void issue_repositoryLevelCallOnAlreadyIssuedRowRefusesWithoutRemintingDocNumber } @Test - void issue_secondDraftOnSameTicket_refusedUntilItsPercentAndAmountAreBillable_thenSupersedesTheFirst() { + void issue_secondDraftOnSameTicket_refusedUntilBillable_thenStillRefusedWhileAnotherNoticeIsIssued() { IssuedNoticeFixture fixture = buildTicketWithIssuedDepositNotice(); // A stored percent outside whole 1-100 (a 0% ticket-route draft on a ไม่รับมัดจำ quotation, @@ -231,21 +231,12 @@ void issue_secondDraftOnSameTicket_refusedUntilItsPercentAndAmountAreBillable_th 1, "Free item", new BigDecimal("2"), "แผ่น", BigDecimal.ZERO, null, BigDecimal.ZERO))), "ใบแจ้งรับมัดจำนี้มียอดมัดจำ 0 บาท จึงออกเอกสารไม่ได้"); - // POSITIVE control: once the percent is billable, the DRAFT -> ISSUED transition and the - // "supersede older ISSUED versions" behaviour work with the repository predicate in place. + // 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); - Optional secondDocNumber = depositNoticeRepository.issue(secondDocId, salesRepId, "Sales"); - - assertThat(secondDocNumber).isPresent(); - assertThat(secondDocNumber.get()).isNotEqualTo(fixture.docNumber()); - - DepositNoticeDto firstReread = depositNoticeRepository.findById(fixture.docId()).orElseThrow(); - assertThat(firstReread.status()).isEqualTo("SUPERSEDED"); - assertThat(firstReread.docNumber()).isEqualTo(fixture.docNumber()); - - DepositNoticeDto secondReread = depositNoticeRepository.findById(secondDocId).orElseThrow(); - assertThat(secondReread.status()).isEqualTo("ISSUED"); - assertThat(secondReread.docNumber()).isEqualTo(secondDocNumber.get()); + assertIssueRefusedAndNothingChanged(fixture, secondDocId, + "ดีลนี้ออกใบแจ้งรับมัดจำเลขที่ " + fixture.docNumber() + " แล้ว จึงออกฉบับร่างนี้ไม่ได้"); } // Owner ruling 2026-10-23: one deposit notice per deal. While one is ISSUED, neither create route 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 7d4d828d2..80c49c1ce 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 @@ -1078,26 +1078,30 @@ void update_refusesZeroAndANewFractionalPercent_butAStoredFractionSentBackUnchan .containsExactly("0.333", "0.4"); } - // DepositNoticeRepository#foldAdjustments (owner ruling 2026-10-23): the ส่วนลดพิเศษ amount is - // spread by line amount, HALF_UP per share, the remainder on the last line that has an amount, - // so the folded lines still sum to the quotation's total. A free line keeps its 0. + // 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_putsTheRoundingRemainderOnTheLastLineWithAnAmount() { + void foldAdjustments_keepsEachLinesOwnLabel_appendsTheFoldedDiscount_andTotalsFollowTheTwoDpLines() { List folded = DepositNoticeRepository.foldAdjustments(List.of( - foldLine(1, "1", "100.00"), foldLine(2, "1", "100.00"), foldLine(3, "1", "100.00"), - foldLine(4, "2", "0.00")), new BigDecimal("-100.00")); + 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.66", "0.00"); + .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.00"); - assertThat(folded.get(2).discountLabel()).isEqualTo("ส่วนลด 33.34 ต่อหน่วย"); - assertThat(folded.get(3).discountLabel()).isNull(); + .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) { + private static DepositNoticeItemRequest foldLine(int seq, String qty, String net, String label) { return new DepositNoticeItemRequest(seq, "Line " + seq, new BigDecimal(qty), "แผ่น", - new BigDecimal(net), null, new BigDecimal(net)); + new BigDecimal(net), label, new BigDecimal(net)); } private static void assertForbidden(org.assertj.core.api.ThrowableAssert.ThrowingCallable action) { 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 f529ef509..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,24 +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). Since the owner - // ruling of 2026-10-23 the create routes refuse a draft while a notice is ISSUED, so the - // second draft is written by the repository, as the separate re-version change will. + // 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()); - 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. + 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/mockApi.depositNotices.test.js b/frontend/src/api/mockApi.depositNotices.test.js index 71e4c97d1..cd731ff92 100644 --- a/frontend/src/api/mockApi.depositNotices.test.js +++ b/frontend/src/api/mockApi.depositNotices.test.js @@ -508,9 +508,20 @@ describe('mockApi.depositNotices.update deposit percent', () => { 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); @@ -523,7 +534,7 @@ describe('mockApi.depositNotices.update deposit percent', () => { 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(1); + expect((await api.depositNotices.listByTicket(ticketId)).depositNotices).toHaveLength(2); // no third }); }); @@ -609,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 a0c5a2240..dfc62f1b0 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -1442,44 +1442,39 @@ function mockDepositNoticeItemsFromQuotation(items) { // never recomputed (label excepted — see below). function mockDepositNoticeItemsFromDealQuotation(row) { // DepositNoticeRepository#foldAdjustments: an ADJUSTMENT row (qty -1) is no deposit line; its - // amount is spread over the goods lines by amount, the remainder on the last line with one. + // 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); - const last = goods.findLastIndex((item) => amountOf(item) !== 0); - let left = adjustment; return goods.map((item, idx) => { let netUnitPrice = item.netUnitPrice; if (adjustment && total && amountOf(item)) { - const share = idx === last ? round2(left) : round2((adjustment * amountOf(item)) / total); - left -= share; + 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 - 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, + discountLabel: own && extra ? `${own} + ${extra}` : own ?? extra, netUnitPrice, }; }); } -// 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); -} - // Mirrors DepositNoticeService.zeroDepositRefusal: both deposit routes refuse a 0% quotation. const mockZeroDepositRefusal = (number) => `ใบเสนอราคา ${number} ระบุไม่มีมัดจำ (ชำระเต็มจำนวน) จึงออกใบแจ้งยอดเงินรับมัดจำไม่ได้`; @@ -13480,6 +13475,11 @@ export const api = { 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 // the main status at quotation_issued (no more document_issued flip). @@ -13491,9 +13491,6 @@ export const api = { // 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'; }); - const thaiYear = new Date().getFullYear() + 543; doc.docNumber = `GLRD${String(thaiYear).slice(-2)}${String(mockDocNumberSeq++).padStart(3,'0')}`; doc.issueDate = new Date().toISOString().slice(0, 10); From 9512462cdd8a678b633212eff0d35b08fb24a09e Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Wed, 28 Oct 2026 02:34:31 +0700 Subject: [PATCH 17/18] fix(deposit): panel passes quotation+notices to the create gate, issue locks the deal and takes an optional stamp, page refuses cleared rows and keeps stored % - DealDepositPanel: notices were landing in the accepted-quotation slot; pass the deal's quotation (0% hides create) and notices separately. - DepositNoticeService.issue: lock the ticket row before the one-ISSUED guard; optional expectedUpdatedAt (409 if the draft moved on). Dead SUPERSEDE update removed. - DepositNoticePage: save/preview refused when every row is cleared; a stored non-whole % is no longer rewritten; issue sends the stamp. Co-Authored-By: Claude Opus 5.5 --- .../hr/deposit/DepositNoticeController.java | 6 ++- .../hr/deposit/DepositNoticeIssueRequest.java | 6 +++ .../hr/deposit/DepositNoticeRepository.java | 26 ++++++----- .../glr/hr/deposit/DepositNoticeService.java | 11 ++++- ...epositNoticeIssueGuardIntegrationTest.java | 22 ++++++++++ .../hr/deposit/DepositNoticeServiceTest.java | 23 +++++++++- frontend/src/api/hrApi.js | 3 +- frontend/src/api/mockApi.js | 6 ++- .../features/deposits/DepositNoticePage.jsx | 23 +++++++--- .../deposits/DepositNoticePage.test.jsx | 44 ++++++++++++++++--- .../src/features/tickets/DealDepositPanel.jsx | 8 ++-- .../src/features/tickets/TicketDetailPage.jsx | 1 + .../tickets/TicketDetailPage.test.jsx | 24 +++++++++- 13 files changed, 170 insertions(+), 33 deletions(-) create mode 100644 backend/src/main/java/th/co/glr/hr/deposit/DepositNoticeIssueRequest.java 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/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 cf79e4fb0..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 @@ -290,8 +290,12 @@ public void update(long docId, DepositNoticeDraftRequest req, List 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); @@ -305,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); } 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 5fe1b2df6..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 @@ -219,8 +219,14 @@ 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); @@ -268,6 +274,9 @@ 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() @@ -279,7 +288,7 @@ public DepositNoticeDto issue(long docId, UserPrincipal actor) { // (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, "ใบแจ้งรับมัดจำนี้ถูกออกไปแล้วโดยผู้ใช้อื่น กรุณาโหลดข้อมูลใหม่แล้วลองอีกครั้ง")); 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 ef80caa1e..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 @@ -210,6 +210,28 @@ 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_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(); 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 80c49c1ce..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 @@ -16,6 +16,7 @@ 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; @@ -74,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); @@ -89,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); @@ -121,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 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.js b/frontend/src/api/mockApi.js index dfc62f1b0..3cc13d9d7 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -13462,7 +13462,7 @@ 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); @@ -13490,6 +13490,10 @@ export const api = { if (!(Number(doc.depositAmount) > 0)) fail('ใบแจ้งรับมัดจำนี้มียอดมัดจำ 0 บาท จึงออกเอกสารไม่ได้', 409); // Phase 1 lifecycle gate (mirrors DepositNoticeService.requireActiveLifecycle). requireActive(ticket); + // 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')}`; diff --git a/frontend/src/features/deposits/DepositNoticePage.jsx b/frontend/src/features/deposits/DepositNoticePage.jsx index 530728c2f..9e5268354 100644 --- a/frontend/src/features/deposits/DepositNoticePage.jsx +++ b/frontend/src/features/deposits/DepositNoticePage.jsx @@ -44,6 +44,8 @@ function lineNet(item) { // 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)); } @@ -116,7 +118,7 @@ function validateIssueForm({ customerName, items, total, depositPercent }) { } if (items.every(isBlankRow)) { - errors.items = 'กรุณาเพิ่มรายการสินค้าอย่างน้อย 1 รายการ'; + errors.items = NO_ITEMS_MESSAGE; order.push('items'); } else { items.forEach((item, index) => { @@ -274,9 +276,8 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa customerAddress: address, projectName: d.projectName ?? '', reference: d.reference ?? '', - // Rounded to the whole % the field shows, so the field, the label and the payload agree - // (a draft stored at 0.3330 shows 33 and saves 0.33). - depositPercent: Math.round(Number(d.depositPercent) * 100) / 100, + // 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 })), }); @@ -368,8 +369,16 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa 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(); } @@ -387,7 +396,7 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa }); function handlePreview() { - if (!doc) return; + if (!doc || refusesClearedItems()) return; previewMutation.mutate(); } @@ -419,7 +428,7 @@ export function DepositNoticePage({ ticketId, onBack, onNavigateTickets, showToa mutationFn: async () => { // Save first. await saveDraft(); - return api.depositNotices.issue(doc.id); + return api.depositNotices.issue(doc.id, { expectedUpdatedAt: stampRef.current }); }, onSuccess: (res) => { showToast('success', `ออกเอกสาร ${res.depositNotice.docNumber} เรียบร้อย`); diff --git a/frontend/src/features/deposits/DepositNoticePage.test.jsx b/frontend/src/features/deposits/DepositNoticePage.test.jsx index 64bb1cbd8..040901277 100644 --- a/frontend/src/features/deposits/DepositNoticePage.test.jsx +++ b/frontend/src/features/deposits/DepositNoticePage.test.jsx @@ -205,7 +205,7 @@ describe('DepositNoticePage', () => { }); // What บันทึก sends, and the summary beside it, must match what the backend stores: - // - a stored 0.333 (free input before whole percents) shows, labels and saves as 33%; + // - 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); @@ -214,7 +214,7 @@ describe('DepositNoticePage', () => { 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.333, updatedAt: '2026-10-23T08:57:19.732123Z', + ...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 }, @@ -249,6 +249,39 @@ describe('DepositNoticePage', () => { 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. @@ -277,8 +310,9 @@ describe('DepositNoticePage', () => { // 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('บริษัท ทดสอบ จำกัด'); @@ -289,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 () => { diff --git a/frontend/src/features/tickets/DealDepositPanel.jsx b/frontend/src/features/tickets/DealDepositPanel.jsx index f7dae2815..3e5984594 100644 --- a/frontend/src/features/tickets/DealDepositPanel.jsx +++ b/frontend/src/features/tickets/DealDepositPanel.jsx @@ -11,6 +11,7 @@ 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'; @@ -88,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; @@ -160,7 +161,8 @@ 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(''); @@ -214,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, depositNoticesQuery.data); + 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 diff --git a/frontend/src/features/tickets/TicketDetailPage.jsx b/frontend/src/features/tickets/TicketDetailPage.jsx index 4e1d847b0..00f6663ca 100644 --- a/frontend/src/features/tickets/TicketDetailPage.jsx +++ b/frontend/src/features/tickets/TicketDetailPage.jsx @@ -2598,6 +2598,7 @@ export function TicketDetailPage({ user, ticketId, onBack, showToast }) { summary={summary} availableActions={availableActions} pricingRequests={pricingRequests} + dealQuotations={dealQuotationRows} showToast={showToast} /> ) : null} diff --git a/frontend/src/features/tickets/TicketDetailPage.test.jsx b/frontend/src/features/tickets/TicketDetailPage.test.jsx index a73c6eab7..ff8b39439 100644 --- a/frontend/src/features/tickets/TicketDetailPage.test.jsx +++ b/frontend/src/features/tickets/TicketDetailPage.test.jsx @@ -3168,6 +3168,7 @@ describe('TicketDetailPage', () => { 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' } }); @@ -3184,7 +3185,8 @@ describe('TicketDetailPage', () => { expect(api.depositNotices.issue).not.toHaveBeenCalled(); fireEvent.click(within(dialog).getByRole('button', { name: 'ออกเอกสาร' })); - await waitFor(() => expect(api.depositNotices.issue).toHaveBeenCalledWith(9002)); + // 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 @@ -3206,6 +3208,26 @@ describe('TicketDetailPage', () => { 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' }); From a8512a61c96b0380548e78ef72e4bf8299777093 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Wed, 28 Oct 2026 04:07:46 +0700 Subject: [PATCH 18/18] test(deposit): revise tests pinned to the pre-ruling re-issue supersede and remainder rounding Co-Authored-By: Claude Opus 5.5 --- ...lQuotationConfirmOrderIntegrationTest.java | 4 +-- .../DealQuotationOutcomeIntegrationTest.java | 2 +- ...iceAccountNotificationIntegrationTest.java | 36 ------------------- ...emainingInvoiceServiceIntegrationTest.java | 19 +++++----- .../src/api/mockApi.promoteToDeal.test.js | 4 +-- .../pricingRequestMeta.test.js | 8 ----- 6 files changed, 14 insertions(+), 59 deletions(-) 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 782510778..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 @@ -261,8 +261,8 @@ void quotationFirstGhost_stillConfirms_landsAtOrderReceived_withQuotationLines_a .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 ต่อหน่วย")); + 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"); 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/RemainingInvoiceServiceIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/deposit/RemainingInvoiceServiceIntegrationTest.java index 08f02f0e1..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,15 +866,14 @@ null, new BigDecimal("10"), new BigDecimal("0.36"), WastageCalculator.QUANTITY_M orderConfirmation.confirmOrder(pricingRequestId, new ConfirmOrderRequest(UUID.randomUUID().toString()), owner); - // Chain A already holds an ISSUED deposit notice on this ticket, so both create routes refuse - // a second draft (owner ruling 2026-10-23). Chain B's notice is written by the repository with - // the reference and lines the pricing-request route would use, then issued for real. - long depositDraftId = new DepositNoticeRepository(jdbc).createDraft(ticketId, - new DepositNoticeDraftRequest(null, null, null, null, issuedB.number(), new BigDecimal("0.50"), List.of(), null), - DepositNoticeService.itemsFromQuotation(issuedB.items())); - DepositNoticeDto depositIssued = depositNoticeService.issue(depositDraftId, 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/frontend/src/api/mockApi.promoteToDeal.test.js b/frontend/src/api/mockApi.promoteToDeal.test.js index 72842f489..ff9bb889f 100644 --- a/frontend/src/api/mockApi.promoteToDeal.test.js +++ b/frontend/src/api/mockApi.promoteToDeal.test.js @@ -136,8 +136,8 @@ describe('mock dealQuotations.promoteToDeal (GLA-136)', () => { 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 ต่อหน่วย'], + [10, 850, 841.5, 'ส่วนลดพิเศษ 8.5 ต่อหน่วย'], + [2, 500, 495, 'ส่วนลดพิเศษ 5 ต่อหน่วย'], ]); expect(depositNotice.subtotal).toBe(9405); diff --git a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js index 0af4167d7..18aa32b8c 100644 --- a/frontend/src/features/pricingRequests/pricingRequestMeta.test.js +++ b/frontend/src/features/pricingRequests/pricingRequestMeta.test.js @@ -428,14 +428,6 @@ describe('canConfirmOrder (roles)', () => { }); describe('canCreateDepositNoticeFromQuotation', () => { - // 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, - [{ id: 1, status: 'SUPERSEDED' }, { id: 2, status: 'ISSUED', docNumber: 'GLRD69001' }])).toBe(false); - expect(canCreateDepositNoticeFromQuotation(salesOwner, confirmed, - [{ id: 1, status: 'SUPERSEDED' }, { id: 3, status: 'DRAFT' }])).toBe(true); - }); it('allows the owner sales rep only AFTER the bridge has run (orderConfirmedAt set)', () => { expect(canCreateDepositNoticeFromQuotation(salesOwner, pr({ status: 'QUOTATION_ACCEPTED', orderConfirmedAt: '2026-07-21T00:00:00Z' }))).toBe(true);