From a2e849f0ce1bbe9139627450e331ed7e0f61d11d Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Wed, 23 Sep 2026 07:00:12 +0700 Subject: [PATCH 001/204] feat(billing): wire can_issue_billing_note grant and billing-note API surface (GLA-129 step 4, part 1/3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surfaces the already-merged GLA-99 billing-note backend (V189) to the frontend: the can_issue_billing_note per-employee grant now flows through AuthResponse/App.jsx exactly like canCreateQuotation, and hrApi/mockApi gain a full billingNotes namespace mirroring BillingNoteService's lifecycle, numbering, and authz split. No screen calls it yet — /finance stays untouched and unreachable for grant holders on purpose, since AccountFinancePage.jsx has no internal tab scoping today; that lands atomically with the UI branches stacked on top of this one. Co-Authored-By: Claude Sonnet 5 --- .../java/th/co/glr/hr/auth/AuthResponse.java | 6 +- .../java/th/co/glr/hr/auth/AuthService.java | 9 +- .../th/co/glr/hr/auth/AuthServiceTest.java | 12 + docs/api/response-fields.json | 78 ++- frontend/src/App.jsx | 11 +- frontend/src/api/hrApi.js | 28 + .../src/api/mockApi.billingNoteAuthz.test.js | 320 +++++++++++ frontend/src/api/mockApi.js | 530 +++++++++++++++++- frontend/src/api/routes.js | 16 + frontend/src/api/serverContract.test.js | 53 +- frontend/src/app/permissions.js | 28 +- frontend/src/components/layout/AppShell.jsx | 4 + frontend/src/data/demoData.js | 7 +- frontend/src/data/demoSales.js | 8 +- 14 files changed, 1059 insertions(+), 51 deletions(-) create mode 100644 frontend/src/api/mockApi.billingNoteAuthz.test.js diff --git a/backend/src/main/java/th/co/glr/hr/auth/AuthResponse.java b/backend/src/main/java/th/co/glr/hr/auth/AuthResponse.java index 087dab5af..134cf9560 100644 --- a/backend/src/main/java/th/co/glr/hr/auth/AuthResponse.java +++ b/backend/src/main/java/th/co/glr/hr/auth/AuthResponse.java @@ -15,6 +15,10 @@ *

{@code canCreateQuotation} is the same shape for the per-employee "can create quotations" * capability (owner ruling, Ploy 2026-09-09): a hint only, re-checked live by * {@code th.co.glr.hr.dealquotation.DealQuotationService} on every request. + * + *

{@code canIssueBillingNote} is the same shape again for the per-employee "can issue billing + * notes" capability (GLA-99/GLA-129, Ploy 2026-09-19): a hint only, re-checked live by + * {@code th.co.glr.hr.billing.BillingNoteService.hasWriteGrant}. */ -public record AuthResponse(UserPrincipal user, boolean admin, boolean canCreateQuotation) { +public record AuthResponse(UserPrincipal user, boolean admin, boolean canCreateQuotation, boolean canIssueBillingNote) { } diff --git a/backend/src/main/java/th/co/glr/hr/auth/AuthService.java b/backend/src/main/java/th/co/glr/hr/auth/AuthService.java index 5fac72228..8b46d130c 100644 --- a/backend/src/main/java/th/co/glr/hr/auth/AuthService.java +++ b/backend/src/main/java/th/co/glr/hr/auth/AuthService.java @@ -58,7 +58,8 @@ public AuthResponse login(LoginRequest request, HttpServletRequest httpRequest) } session.setAttribute(SessionContext.SESSION_USER_KEY, principal); recordLogin(principal); - return new AuthResponse(principal, employees.isAdmin(principal.id()), employees.canCreateQuotation(principal.id())); + return new AuthResponse(principal, employees.isAdmin(principal.id()), employees.canCreateQuotation(principal.id()), + employees.canIssueBillingNote(principal.id())); } /** @@ -103,7 +104,8 @@ public AuthResponse me(HttpSession session) { if (value instanceof UserPrincipal user) { // Re-read per call rather than trusting the session, so granting or revoking admin // shows up on the next page load instead of at the holder's next login. - return new AuthResponse(user, employees.isAdmin(user.id()), employees.canCreateQuotation(user.id())); + return new AuthResponse(user, employees.isAdmin(user.id()), employees.canCreateQuotation(user.id()), + employees.canIssueBillingNote(user.id())); } throw new ApiException(HttpStatus.UNAUTHORIZED, "กรุณาเข้าสู่ระบบก่อนใช้งาน"); } @@ -132,7 +134,8 @@ public AuthResponse changePassword(ChangePasswordRequest request, HttpSession se // is that this employee set their own password at this moment. Until now the sole trace was // must_change_password flipping to false, which fires once per person and never again. recordAuthEvent(refreshed, "CHANGE_PASSWORD"); - return new AuthResponse(refreshed, employees.isAdmin(refreshed.id()), employees.canCreateQuotation(refreshed.id())); + return new AuthResponse(refreshed, employees.isAdmin(refreshed.id()), employees.canCreateQuotation(refreshed.id()), + employees.canIssueBillingNote(refreshed.id())); } public void logout(HttpSession session) { diff --git a/backend/src/test/java/th/co/glr/hr/auth/AuthServiceTest.java b/backend/src/test/java/th/co/glr/hr/auth/AuthServiceTest.java index 1ab3d95db..0b0829ffe 100644 --- a/backend/src/test/java/th/co/glr/hr/auth/AuthServiceTest.java +++ b/backend/src/test/java/th/co/glr/hr/auth/AuthServiceTest.java @@ -110,6 +110,18 @@ void authenticatesWithCorrectPasswordAndDerivesHrRole() { assertThat(response.user().mustChangePassword()).isFalse(); } + @Test + void loginSurfacesCanIssueBillingNoteGrantAsAUiHint() { + when(employees.findByEmail("hr@glr.co.th")) + .thenReturn(Optional.of(employee(17L, encoder.encode("Str0ngPass!"), false))); + when(employees.canIssueBillingNote(42L)).thenReturn(true); + + AuthResponse response = service.login( + new LoginRequest("hr@glr.co.th", "Str0ngPass!", null), new MockHttpServletRequest()); + + assertThat(response.canIssueBillingNote()).isTrue(); + } + @Test void surfacesMustChangePasswordFlagFromTemporaryHash() { when(employees.findByEmail("employee@glr.co.th")) diff --git a/docs/api/response-fields.json b/docs/api/response-fields.json index 3e74c775a..06be2f922 100644 --- a/docs/api/response-fields.json +++ b/docs/api/response-fields.json @@ -1,7 +1,7 @@ { "$comment" : "GENERATED by ResponseFieldContractTest — do not hand-edit. Regenerate: cd backend && ./mvnw -Dtest=ResponseFieldContractTest -Dresponse.fields.update=true test", "source" : "Schema.field pairs from springdoc /v3/api-docs response schemas, kept where the field name is read in frontend/src (excluding mockApi, hrApi, routes and tests)", - "count" : 1207, + "count" : 1283, "fields" : [ "ActivityLogEntryDto.durationMs", "ActivityLogEntryDto.employeeCode", @@ -91,6 +91,30 @@ "AuditEventDto.entityId", "AuditEventDto.subjectName", "AuthResponse.canCreateQuotation", + "AuthResponse.canIssueBillingNote", + "BillingNoteCandidateDto.docDate", + "BillingNoteCandidateDto.docNumber", + "BillingNoteCandidateDto.dueDate", + "BillingNoteCandidateDto.ticketId", + "BillingNoteCandidatesDto.customerId", + "BillingNoteCandidatesDto.customerName", + "BillingNoteDocumentDto.createdAt", + "BillingNoteDocumentDto.createdById", + "BillingNoteDocumentDto.createdByName", + "BillingNoteDocumentDto.customerAddress", + "BillingNoteDocumentDto.customerId", + "BillingNoteDocumentDto.customerName", + "BillingNoteDocumentDto.customerTaxId", + "BillingNoteDocumentDto.docNumber", + "BillingNoteDocumentDto.issuedAt", + "BillingNoteDocumentDto.issuedByName", + "BillingNoteDocumentDto.receivedAt", + "BillingNoteDocumentDto.totalAmount", + "BillingNoteDocumentDto.updatedAt", + "BillingNoteLineDto.docDate", + "BillingNoteLineDto.docNumber", + "BillingNoteLineDto.dueDate", + "BillingNoteLineDto.ticketId", "CatalogDto.sqmPerPiece", "CommissionMonthlySummaryDto.belowFloor", "CommissionMonthlySummaryDto.commissionableBase", @@ -157,6 +181,7 @@ "CustomerQuotationDto.outcomeNote", "CustomerQuotationDto.parentQuotationId", "CustomerQuotationDto.paymentTerms", + "CustomerQuotationDto.pricingRequestId", "CustomerQuotationDto.quotationRevisionNo", "CustomerQuotationDto.quotationVersion", "CustomerQuotationDto.recipientLabel", @@ -172,6 +197,7 @@ "CustomerQuotationItemDto.lineSubtotal", "CustomerQuotationItemDto.lineTotal", "CustomerQuotationItemDto.minimumSellingPricePerRequestedUnit", + "CustomerQuotationItemDto.pricingDecisionItemId", "CustomerQuotationItemDto.pricingRequestItemId", "CustomerQuotationItemDto.requestedQuantity", "CustomerQuotationItemDto.requestedUnitBasis", @@ -192,6 +218,7 @@ "DealQuotationDto.approvedAt", "DealQuotationDto.approvedByName", "DealQuotationDto.approvedByNameEn", + "DealQuotationDto.ceoPriceMode", "DealQuotationDto.contactEmail", "DealQuotationDto.contactId", "DealQuotationDto.contactName", @@ -215,16 +242,21 @@ "DealQuotationDto.documentLanguage", "DealQuotationDto.fullPaymentTerm", "DealQuotationDto.grandTotal", + "DealQuotationDto.itemsRemovedFromCeoCount", "DealQuotationDto.offerDate", "DealQuotationDto.omitContactHonorific", "DealQuotationDto.parentQuotationId", "DealQuotationDto.priceMode", + "DealQuotationDto.priceModeChangedFromCeo", + "DealQuotationDto.pricingRequestCode", + "DealQuotationDto.pricingRequestId", "DealQuotationDto.printedByDisplayId", "DealQuotationDto.printedByDisplayName", "DealQuotationDto.printedByDisplayNameEn", "DealQuotationDto.projectName", "DealQuotationDto.quotationDate", "DealQuotationDto.remainderMode", + "DealQuotationDto.removedCeoItems", "DealQuotationDto.revisionNo", "DealQuotationDto.salesRepDisplayId", "DealQuotationDto.salesRepDisplayName", @@ -250,6 +282,11 @@ "DealQuotationItemDto.areaSqm", "DealQuotationItemDto.calculationLine", "DealQuotationItemDto.catalogPriceId", + "DealQuotationItemDto.ceoDirectNetPrice", + "DealQuotationItemDto.ceoDiscountPct", + "DealQuotationItemDto.ceoListUnitPrice", + "DealQuotationItemDto.ceoNetUnitPrice", + "DealQuotationItemDto.ceoSpecialPriceSqm", "DealQuotationItemDto.descriptionLine", "DealQuotationItemDto.discountPct", "DealQuotationItemDto.itemNotes", @@ -263,6 +300,7 @@ "DealQuotationItemDto.piecesFinal", "DealQuotationItemDto.piecesInput", "DealQuotationItemDto.piecesPerBox", + "DealQuotationItemDto.priceChangedFromCeo", "DealQuotationItemDto.productCode", "DealQuotationItemDto.quantityMode", "DealQuotationItemDto.roundToFullBox", @@ -318,6 +356,7 @@ "DepositNoticeItemDto.discountLabel", "DepositNoticeItemDto.netUnitPrice", "DepositNoticeItemDto.unitPrice", + "DiscountApprovalDto.pricingRequestId", "DiscountApprovalDto.quotationItemId", "DiscountApprovalDto.rejectionReason", "DiscountApprovalDto.requestedAt", @@ -383,6 +422,7 @@ "FactoryQuoteDto.leadTimeText", "FactoryQuoteDto.negotiationNote", "FactoryQuoteDto.paymentTerms", + "FactoryQuoteDto.pricingRequestId", "FactoryQuoteDto.receivedAt", "FactoryQuoteDto.requestedAt", "FactoryQuoteDto.revisionNo", @@ -745,6 +785,7 @@ "PricingClearanceFeeDto.qtyMinSqm", "PricingCostingDto.costingCode", "PricingCostingDto.createdAt", + "PricingCostingDto.pricingRequestId", "PricingCostingDto.submittedAt", "PricingCostingDto.totalLandedCostThb", "PricingCostingDto.updatedAt", @@ -787,6 +828,7 @@ "PricingDecisionDto.fxSource", "PricingDecisionDto.priceMode", "PricingDecisionDto.pricingCostingId", + "PricingDecisionDto.pricingRequestId", "PricingDecisionDto.updatedAt", "PricingDecisionItemDto.approvedMarginPct", "PricingDecisionItemDto.approvedSellingPricePerRequestedUnit", @@ -812,11 +854,14 @@ "PricingDecisionItemDto.updatedAt", "PricingDecisionSalesItemDto.approvedSellingPricePerRequestedUnit", "PricingDecisionSalesItemDto.minimumSellingPricePerRequestedUnit", + "PricingDecisionSalesItemDto.pricingDecisionItemId", "PricingDecisionSalesItemDto.pricingRequestItemId", "PricingDecisionSalesItemDto.productDescription", "PricingDecisionSalesItemDto.requestedQuantity", "PricingDecisionSalesItemDto.requestedUnitBasis", "PricingDecisionSalesViewDto.approvedAt", + "PricingDecisionSalesViewDto.newFormPricing", + "PricingDecisionSalesViewDto.pricingRequestId", "PricingDutyRateDto.dutyPct", "PricingDutyRateDto.dutyRateId", "PricingDutyRateDto.productLabel", @@ -845,12 +890,14 @@ "PricingRequestAttachmentDto.fileName", "PricingRequestAttachmentDto.fileSize", "PricingRequestAttachmentDto.includeInFactoryEmail", + "PricingRequestAttachmentDto.pricingRequestId", "PricingRequestAttachmentDto.uploadedAt", "PricingRequestAttachmentDto.uploadedBy", "PricingRequestDetailResponse.pricingRequest", "PricingRequestEventDto.actorName", "PricingRequestEventDto.createdAt", "PricingRequestEventDto.eventKind", + "PricingRequestEventDto.pricingRequestId", "PricingRequestEventDto.ticketId", "PricingRequestItemDto.areaSqm", "PricingRequestItemDto.catalogBasePrice", @@ -866,6 +913,7 @@ "PricingRequestItemDto.originCountryOther", "PricingRequestItemDto.piecesInput", "PricingRequestItemDto.piecesPerBox", + "PricingRequestItemDto.pricingRequestId", "PricingRequestItemDto.productCode", "PricingRequestItemDto.productDescription", "PricingRequestItemDto.productId", @@ -962,6 +1010,29 @@ "QuotationDto.ticketId", "QuotationDto.totalAmount", "QuotationDto.validityDate", + "RemainingInvoiceDocumentDto.createdAt", + "RemainingInvoiceDocumentDto.createdById", + "RemainingInvoiceDocumentDto.createdByName", + "RemainingInvoiceDocumentDto.customerAddress", + "RemainingInvoiceDocumentDto.customerName", + "RemainingInvoiceDocumentDto.customerQuotationId", + "RemainingInvoiceDocumentDto.customerTaxId", + "RemainingInvoiceDocumentDto.depositDeduction", + "RemainingInvoiceDocumentDto.depositReference", + "RemainingInvoiceDocumentDto.docDate", + "RemainingInvoiceDocumentDto.docNumber", + "RemainingInvoiceDocumentDto.grandTotal", + "RemainingInvoiceDocumentDto.issuedAt", + "RemainingInvoiceDocumentDto.issuedByName", + "RemainingInvoiceDocumentDto.itemsTotal", + "RemainingInvoiceDocumentDto.netAmount", + "RemainingInvoiceDocumentDto.projectName", + "RemainingInvoiceDocumentDto.ticketId", + "RemainingInvoiceDocumentDto.updatedAt", + "RemainingInvoiceDocumentDto.vatAmount", + "RemainingInvoiceDocumentItemDto.discountLabel", + "RemainingInvoiceDocumentItemDto.netUnitPrice", + "RemainingInvoiceDocumentItemDto.unitPrice", "RemainingInvoiceOptionsDto.blockingReason", "RemainingInvoiceOptionsDto.defaultDepositReference", "RemainingInvoiceOptionsDto.defaultIssueDate", @@ -981,6 +1052,8 @@ "RemainingInvoiceOptionsDto.vatAmount", "RemittanceLedgerRowDto.payrollMonth", "RemittanceLedgerRowDto.payrollPeriodId", + "RemovedLinkedItemDto.pricingDecisionItemId", + "RemovedLinkedItemDto.sizeText", "ResetPasswordResponse.temporaryPassword", "RotateAgentTokenResponse.device_code", "SalaryHistoryDto.newSalary", @@ -996,6 +1069,9 @@ "SensitiveDto.socialSecurityHospital", "SensitiveDto.socialSecurityNo", "SensitiveDto.taxId", + "SpecialMoneyApprovalPreviewDto.eligibleAmount", + "SpecialMoneyApprovalPreviewDto.payrollMonth", + "SpecialMoneyApprovalPreviewDto.requestedAmount", "SpecialMoneyAttachmentDto.fileName", "SpecialMoneyAttachmentDto.sizeBytes", "SpecialMoneyAttachmentDto.uploadedAt", diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 643850ef8..ab205aa7f 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -89,9 +89,18 @@ const SafeFormSubmitterProbe = lazy(() => import('./dev/SafeFormSubmitterProbe.j // `canCreateQuotation` is the same shape, carried by the same record (AuthResponse.java's own // javadoc) -- Quotation v2's per-employee "can create quotations" capability (owner ruling, Ploy // 2026-09-09). Also a hint only: DealQuotationService re-reads the grant live on every request. +// +// `canIssueBillingNote` is the same shape again -- the per-employee "can issue billing notes" +// capability (GLA-99/GLA-129, owner ruling, Ploy 2026-09-19). Also a hint only: +// BillingNoteService.hasWriteGrant re-reads the grant live on every request. function userFromAuthResponse(response) { if (!response?.user) return null; - return { ...response.user, admin: Boolean(response.admin), canCreateQuotation: Boolean(response.canCreateQuotation) }; + return { + ...response.user, + admin: Boolean(response.admin), + canCreateQuotation: Boolean(response.canCreateQuotation), + canIssueBillingNote: Boolean(response.canIssueBillingNote), + }; } function TicketDetailRoute({ user, showToast }) { diff --git a/frontend/src/api/hrApi.js b/frontend/src/api/hrApi.js index 9cc36de48..4fe8ed490 100644 --- a/frontend/src/api/hrApi.js +++ b/frontend/src/api/hrApi.js @@ -515,6 +515,34 @@ export const api = { return res.blob(); }, }, + // Mirrors BillingNoteController — the STORED ใบวางบิล aggregate (V189, GLA-99 step 3 / GLA-129 + // step 4). Customer-scoped, DRAFT -> ISSUED -> {SUPERSEDED | CANCELLED | SETTLED}. `createDraft`'s + // `payload` is a BillingNoteDraftRequest-shaped body (type required on create; lines, when + // supplied, whole-value replace). Authorisation is enforced entirely in BillingNoteService — + // these methods carry no gate of their own. + billingNotes: { + candidates: (customerId) => apiRequest(API_ROUTES.billingNotes.candidates(customerId)), + listForCustomer: (customerId) => apiRequest(API_ROUTES.billingNotes.forCustomer(customerId)), + createDraft: (customerId, payload) => + apiRequest(API_ROUTES.billingNotes.forCustomer(customerId), { method: 'POST', body: payload }), + get: (id) => apiRequest(API_ROUTES.billingNotes.get(id)), + update: (id, payload) => apiRequest(API_ROUTES.billingNotes.get(id), { method: 'PUT', body: payload }), + issue: (id) => apiRequest(API_ROUTES.billingNotes.issue(id), { method: 'POST' }), + revise: (id) => apiRequest(API_ROUTES.billingNotes.revise(id), { method: 'POST' }), + cancel: (id, reason) => apiRequest(API_ROUTES.billingNotes.cancel(id), { method: 'POST', body: { reason } }), + markReceived: (id, payload) => apiRequest(API_ROUTES.billingNotes.markReceived(id), { method: 'POST', body: payload }), + markSettled: (id) => apiRequest(API_ROUTES.billingNotes.markSettled(id), { method: 'POST' }), + remove: (id) => apiRequest(API_ROUTES.billingNotes.get(id), { method: 'DELETE' }), + // Binary, so it goes through fetch directly — same shape as storedRemainingInvoices.download above. + download: async (id) => { + const res = await fetch(API_ROUTES.billingNotes.file(id), { credentials: 'include' }); + if (!res.ok) { + const err = await res.json().catch(() => ({})); + throw new Error(err.message || 'ดาวน์โหลดใบวางบิลไม่สำเร็จ'); + } + return res.blob(); + }, + }, // Mirrors ImportRequestController's SINGULAR (preview) routes. Import/CEO only, enforced in // ImportRequestService — these methods carry no gate of their own and must not be read as one. importRequests: { diff --git a/frontend/src/api/mockApi.billingNoteAuthz.test.js b/frontend/src/api/mockApi.billingNoteAuthz.test.js new file mode 100644 index 000000000..bfbc3e1a6 --- /dev/null +++ b/frontend/src/api/mockApi.billingNoteAuthz.test.js @@ -0,0 +1,320 @@ +import { describe, it, expect } from 'vitest'; +import { api } from './mockApi.js'; + +// GLA-129 branch 1 review round 1 (2026-09-23): "mock tests must pin the gates" — the same rule +// mockApi.remainingInvoiceAuthz.test.js/mockApi.gla118PaymentAuthz.test.js already state at length. +// Before this file, requireBillingNoteWriteGate/requireBillingNoteReadAccess (mirrors +// BillingNoteService#hasWriteGrant/#requireReadAccess) had ZERO mock-side pinning — a future edit +// could silently drift back to more-permissive-than-production, or drop the existence-oracle +// ordering fix (F2/S7), with nothing here to go red. Per CLAUDE.md, this is NOT authz evidence on +// its own — the mock's authorization is never authoritative, see +// BillingNoteServiceIntegrationTest (backend/src/test/java/th/co/glr/hr/billing/, including its +// own wrong-way-round suite) for the real evidence — it only proves the mock's plumbing matches +// the rules it claims to mirror. +// +// Customer 6 / ticket 19 (demoSales.js, SALES1-owned, ISSUED deposit notice 2026-08-11) is the +// one seed customer wired to a real candidate — see mockCustomers' own comment on why. SALES1 is +// sales@glr.co.th (role quick-login `sales` resolves to the FIRST active `sales`-role db.users +// row, which is this one); sales2@glr.co.th is a non-owning sales rep, same as the sibling +// remaining-invoice test's OTHER_SALES_REP. + +const GRANT_HOLDER = { email: 'employee@glr.co.th', password: 'demo1234' }; // role employee, canIssueBillingNote: true +const NON_OWNING_SALES = { email: 'sales2@glr.co.th', password: 'demo1234' }; +const NO_GRANT_ROLES = ['account', 'sales_manager', 'import']; + +async function loginRole(role) { + await api.auth.login({ role }); +} +async function loginGrantHolder() { + await api.auth.login(GRANT_HOLDER); +} + +// The seed carries exactly ONE real ISSUED source document under a customer row (ticket 19's +// deposit notice, via customer 6) — every earlier test in this file that needs a claimable +// candidate consumes it and never releases it, so a LATER test cannot reuse it (state persists +// across it()s in this module, same as the sibling remaining-invoice test file). Minting a fresh +// storedRemainingInvoices chain on ticket 19 (SALES1-owned, real approvedPrice items, status +// quotation_issued — createDraft's own precondition) gives each test its own independent +// candidate instead. A distinct `quotationId` tag per call keeps the chains from colliding with +// each other (storedRemainingInvoices' own one-live-per-chain rule). +async function mintFreshRemainingInvoiceCandidate(quotationId) { + await loginRole('sales'); // resolves to SALES1, ticket 19's owner + const { remainingInvoice: draft } = await api.storedRemainingInvoices.createDraft(19, { quotationId }); + const { remainingInvoice: issued } = await api.storedRemainingInvoices.issue(draft.id); + return issued; +} + +describe('mockApi.billingNotes — write gate (mirrors BillingNoteService#hasWriteGrant)', () => { + it('createDraft refuses every non-ceo, non-grant role with 403, and creates nothing', async () => { + for (const role of [...NO_GRANT_ROLES, 'sales']) { + await loginRole(role); + await expect(api.billingNotes.createDraft(6, { type: 'GOODS' })).rejects.toMatchObject({ status: 403 }); + } + await loginGrantHolder(); + const { billingNotes } = await api.billingNotes.listForCustomer(6); + expect(billingNotes).toHaveLength(0); + }); + + it('createDraft succeeds for ceo and for a plain employee holding the grant', async () => { + await loginRole('ceo'); + const { billingNote: byCeo } = await api.billingNotes.createDraft(6, { type: 'FREIGHT' }); + expect(byCeo.status).toBe('DRAFT'); + + await loginGrantHolder(); + const { billingNote: byGrant } = await api.billingNotes.createDraft(6, { type: 'FREIGHT' }); + expect(byGrant.status).toBe('DRAFT'); + expect(byGrant.createdByName).toBeTruthy(); + }); + + // F2/S7 (matching BillingNoteService's own fix): the write gate must run BEFORE the existence + // lookup, so an unauthorized caller cannot tell "no such note" (404) from "wrong state" (409) + // without ever passing the gate. Review round 1 caught this reversed for all seven methods. + it('update/issue/revise/cancel/markReceived/markSettled/remove refuse a non-grant role with 403 even against a BOGUS id (gate before lookup)', async () => { + const bogusId = 999999; + for (const role of NO_GRANT_ROLES) { + await loginRole(role); + await expect(api.billingNotes.update(bogusId, {})).rejects.toMatchObject({ status: 403 }); + await expect(api.billingNotes.issue(bogusId)).rejects.toMatchObject({ status: 403 }); + await expect(api.billingNotes.revise(bogusId)).rejects.toMatchObject({ status: 403 }); + await expect(api.billingNotes.cancel(bogusId, 'เพราะเหตุผลทดสอบ')).rejects.toMatchObject({ status: 403 }); + await expect(api.billingNotes.markReceived(bogusId, { receivedByName: 'x' })).rejects.toMatchObject({ status: 403 }); + await expect(api.billingNotes.markSettled(bogusId)).rejects.toMatchObject({ status: 403 }); + await expect(api.billingNotes.remove(bogusId)).rejects.toMatchObject({ status: 403 }); + } + }); + + it('update refuses a non-owning-grant role and leaves the draft unchanged', async () => { + await loginGrantHolder(); + const { billingNote: draft } = await api.billingNotes.createDraft(6, { type: 'GOODS' }); + + await loginRole('account'); + await expect(api.billingNotes.update(draft.id, { note: 'SHOULD-NEVER-STICK' })) + .rejects.toMatchObject({ status: 403 }); + + await loginGrantHolder(); + const { billingNote: reread } = await api.billingNotes.get(draft.id); + expect(reread.note).not.toBe('SHOULD-NEVER-STICK'); + }); +}); + +describe('mockApi.billingNotes — createDraft leaves no orphan on a line-validation failure', () => { + it('a fully-typed MANUAL line with a zero amount 409s and creates nothing', async () => { + await loginGrantHolder(); + const before = (await api.billingNotes.listForCustomer(6)).billingNotes.length; + + await expect(api.billingNotes.createDraft(6, { + type: 'FREIGHT', + lines: [{ sourceType: 'MANUAL', manualDocNumber: 'INV-1', manualAmount: 0 }], + })).rejects.toMatchObject({ status: 400 }); + + const after = (await api.billingNotes.listForCustomer(6)).billingNotes.length; + expect(after).toBe(before); // no orphan DRAFT with zero lines + }); + + // Distinct from the zero-amount case above: 0 is rejected whether or not the cent-rounding step + // runs at all, so it cannot catch a regression in that specific line. A sub-satang amount only + // 409s if it is rounded to the cent FIRST (mirrors setScale(2, HALF_UP).signum() <= 0) — + // reverting to a bare `manualAmount > 0` check would let this one through. + it('a sub-satang MANUAL amount (0.004) rounds to zero and is refused', async () => { + await loginGrantHolder(); + await expect(api.billingNotes.createDraft(6, { + type: 'FREIGHT', + lines: [{ sourceType: 'MANUAL', manualDocNumber: 'INV-2', manualAmount: 0.004 }], + })).rejects.toMatchObject({ status: 400 }); + }); +}); + +describe('mockApi.billingNotes — read gate (mirrors BillingNoteService#requireReadAccess)', () => { + it('account/sales_manager/ceo read a note unconditionally; import and a non-owning sales rep are refused', async () => { + await loginGrantHolder(); + const { billingNote: draft } = await api.billingNotes.createDraft(6, { type: 'GOODS' }); + + for (const role of ['account', 'sales_manager', 'ceo']) { + await loginRole(role); + await expect(api.billingNotes.get(draft.id)).resolves.toBeTruthy(); + } + await loginRole('import'); + await expect(api.billingNotes.get(draft.id)).rejects.toMatchObject({ status: 403 }); + await api.auth.login(NON_OWNING_SALES); + await expect(api.billingNotes.get(draft.id)).rejects.toMatchObject({ status: 403 }); + }); + + it('a sales rep reads once the note references a ticket they own, but a different sales rep still cannot', async () => { + await loginGrantHolder(); + const { candidates } = await api.billingNotes.candidates(6); + const depositCandidate = candidates.find((c) => c.sourceType === 'DEPOSIT_NOTICE'); + expect(depositCandidate).toBeTruthy(); // ticket 19's ISSUED deposit notice, SALES1-owned + + const { billingNote: draft } = await api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'DEPOSIT_NOTICE', sourceId: depositCandidate.sourceId }], + }); + + await loginRole('sales'); // resolves to SALES1, the owner of ticket 19 + await expect(api.billingNotes.get(draft.id)).resolves.toBeTruthy(); + + await api.auth.login(NON_OWNING_SALES); + await expect(api.billingNotes.get(draft.id)).rejects.toMatchObject({ status: 403 }); + }); +}); + +describe('mockApi.billingNotes — double-billing claim tracking', () => { + it('a second note cannot claim a source already claimed by a live (DRAFT or ISSUED) note', async () => { + const ri = await mintFreshRemainingInvoiceCandidate(301); + await loginGrantHolder(); + const { candidates } = await api.billingNotes.candidates(6); + const riCandidate = candidates.find((c) => c.sourceType === 'REMAINING_INVOICE' && c.sourceId === ri.id); + expect(riCandidate).toBeTruthy(); + + const { billingNote: first } = await api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }], + }); + expect(first.lines).toHaveLength(1); + + await expect(api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }], + })).rejects.toMatchObject({ status: 409 }); + + // Same source no longer shows as an open candidate — it shows as alreadyBilled instead. + const { candidates: after, alreadyBilled } = await api.billingNotes.candidates(6); + expect(after.some((c) => c.sourceId === riCandidate.sourceId)).toBe(false); + expect(alreadyBilled.some((c) => c.sourceId === riCandidate.sourceId && c.blockedByNoteId === first.id)).toBe(true); + }); + + it('revise() releases the predecessor line immediately, and remove()ing the abandoned correction restores the claim', async () => { + const ri = await mintFreshRemainingInvoiceCandidate(302); + await loginGrantHolder(); + const { candidates } = await api.billingNotes.candidates(6); + const riCandidate = candidates.find((c) => c.sourceType === 'REMAINING_INVOICE' && c.sourceId === ri.id); + expect(riCandidate).toBeTruthy(); + + const { billingNote: draft } = await api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }], + }); + const { billingNote: issued } = await api.billingNotes.issue(draft.id); + expect(issued.status).toBe('ISSUED'); + + // Claimed by the ISSUED note — a brand-new note cannot also claim it. + await expect(api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }], + })).rejects.toMatchObject({ status: 409 }); + + // revise() hands the claim to the correction draft, not to the open pool — mirrors + // releaseLines(id) immediately followed by replaceLines(newId, copied) inside the SAME + // transaction, so the source is never actually free mid-revise. It shows as alreadyBilled, + // now blocked by the CORRECTION rather than by the original issued note. + const { billingNote: correction } = await api.billingNotes.revise(issued.id); + expect(correction.status).toBe('DRAFT'); + expect(correction.lines).toHaveLength(1); + const { alreadyBilled: duringCorrection } = await api.billingNotes.candidates(6); + expect(duringCorrection.some((c) => c.sourceId === riCandidate.sourceId && c.blockedByNoteId === correction.id)).toBe(true); + + // Dropping the line from the correction (update() whole-value-replaces lines) frees the + // source — the predecessor's own claim was already released at revise() time, so nothing + // else still holds it. This is the scenario release existed for: "revise note N, then update + // the correction to drop a line — the dropped source must become billable again immediately." + await api.billingNotes.update(correction.id, { lines: [] }); + const { candidates: afterDrop } = await api.billingNotes.candidates(6); + expect(afterDrop.some((c) => c.sourceId === riCandidate.sourceId)).toBe(true); + + // Re-claim it on the correction, then abandon the correction entirely — restores the + // predecessor's own claim (mirrors restoreLinesToIssued). + await api.billingNotes.update(correction.id, { lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }] }); + await api.billingNotes.remove(correction.id); + const { candidates: afterAbandon, alreadyBilled: blockedAfterAbandon } = await api.billingNotes.candidates(6); + expect(afterAbandon.some((c) => c.sourceId === riCandidate.sourceId)).toBe(false); + expect(blockedAfterAbandon.some((c) => c.sourceId === riCandidate.sourceId && c.blockedByNoteId === issued.id)).toBe(true); + }); + + // Review round 2 (2026-09-23): remove() used to restore the predecessor's claim + // UNCONDITIONALLY, which let it double-claim a source a THIRD note had grabbed in the meantime + // — a sequential path a UI user can walk (no concurrency needed), and the exact + // more-permissive-than-production direction CLAUDE.md warns about. Mirrors + // restoreLinesToIssued's own DataIntegrityViolationException -> 409 mapping. + it('remove() on an abandoned correction refuses to restore a claim a THIRD note has since taken, and changes nothing', async () => { + const ri = await mintFreshRemainingInvoiceCandidate(303); + await loginGrantHolder(); + const { candidates } = await api.billingNotes.candidates(6); + const riCandidate = candidates.find((c) => c.sourceType === 'REMAINING_INVOICE' && c.sourceId === ri.id); + + const { billingNote: draft } = await api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }], + }); + const { billingNote: issued } = await api.billingNotes.issue(draft.id); + const { billingNote: correction } = await api.billingNotes.revise(issued.id); + // Drop the line from the correction — frees the source (finding #6's own fix, tested above). + await api.billingNotes.update(correction.id, { lines: [] }); + // A third, unrelated note claims the now-free source. + const { billingNote: interloper } = await api.billingNotes.createDraft(6, { + type: 'GOODS', + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: riCandidate.sourceId }], + }); + + // Abandoning the correction must now refuse — restoring issued's claim would double-claim + // the source the interloper already holds. + await expect(api.billingNotes.remove(correction.id)).rejects.toMatchObject({ status: 409 }); + + // Nothing changed: the correction draft still exists, the interloper still holds the claim. + const { billingNote: correctionReread } = await api.billingNotes.get(correction.id); + expect(correctionReread.status).toBe('DRAFT'); + const { alreadyBilled } = await api.billingNotes.candidates(6); + expect(alreadyBilled.some((c) => c.sourceId === riCandidate.sourceId && c.blockedByNoteId === interloper.id)).toBe(true); + }); +}); + +describe('mockApi.billingNotes — field handling on issue/markReceived/revise', () => { + it('issue() defaults billDate to today when the draft never set one', async () => { + await loginGrantHolder(); + const { billingNote: draft } = await api.billingNotes.createDraft(6, { type: 'FREIGHT', + lines: [{ sourceType: 'MANUAL', manualDocNumber: 'INV-3', manualAmount: 500 }] }); + expect(draft.billDate).toBeNull(); + const { billingNote: issued } = await api.billingNotes.issue(draft.id); + expect(issued.billDate).toBe(new Date().toISOString().slice(0, 10)); + }); + + it('markReceived clears paymentAppointmentDate on a second call that omits it, rather than keeping the first value', async () => { + await loginGrantHolder(); + const { billingNote: draft } = await api.billingNotes.createDraft(6, { type: 'FREIGHT', + lines: [{ sourceType: 'MANUAL', manualDocNumber: 'INV-4', manualAmount: 500 }] }); + const { billingNote: issued } = await api.billingNotes.issue(draft.id); + + const { billingNote: firstReceipt } = await api.billingNotes.markReceived(issued.id, + { receivedByName: 'คุณทดสอบ', paymentAppointmentDate: '2026-10-15' }); + expect(firstReceipt.paymentAppointmentDate).toBe('2026-10-15'); + + const { billingNote: secondReceipt } = await api.billingNotes.markReceived(issued.id, { receivedByName: 'คุณทดสอบสอง' }); + expect(secondReceipt.paymentAppointmentDate).toBeNull(); + }); + + it('revise() does not carry receivedByName/receivedAt/paymentAppointmentDate onto the correction draft', async () => { + await loginGrantHolder(); + const { billingNote: draft } = await api.billingNotes.createDraft(6, { type: 'FREIGHT', + lines: [{ sourceType: 'MANUAL', manualDocNumber: 'INV-5', manualAmount: 500 }] }); + const { billingNote: issued } = await api.billingNotes.issue(draft.id); + await api.billingNotes.markReceived(issued.id, { receivedByName: 'คุณทดสอบ', paymentAppointmentDate: '2026-11-01' }); + + const { billingNote: correction } = await api.billingNotes.revise(issued.id); + expect(correction.receivedByName).toBeNull(); + expect(correction.receivedAt).toBeNull(); + expect(correction.paymentAppointmentDate).toBeNull(); + }); +}); + +describe('mockApi.billingNotes — response shape', () => { + it('never exposes the mock-only `released` bookkeeping field on a line (not on the real BillingNoteLineDto)', async () => { + await loginGrantHolder(); + const { billingNote: draft } = await api.billingNotes.createDraft(6, { type: 'FREIGHT', + lines: [{ sourceType: 'MANUAL', manualDocNumber: 'INV-6', manualAmount: 500 }] }); + expect(draft.lines[0]).not.toHaveProperty('released'); + + const { billingNotes: listed } = await api.billingNotes.listForCustomer(6); + for (const note of listed) { + for (const line of note.lines) expect(line).not.toHaveProperty('released'); + } + }); +}); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 26321356c..34043bc95 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -518,6 +518,15 @@ const mockCustomers = [ // inverse of the usual fixture lie (a mock more permissive than production): a fixture too // EMPTY to reach the code path at all is just as useless for verification. { id: 5, name: 'บริษัท แฟชั่นไอส์แลนด์ จำกัด', taxId: '0105545678901', address: '587, 589 ถนนรามอินทรา แขวงคันนายาว กรุงเทพฯ 10230', branch: 'สำนักงานใหญ่', phone: '02-947-5000' }, + // Same reasoning as customer 5's own comment, one level down (GLA-129, review round 1, + // 2026-09-23): billing-note candidates() joins an ISSUED remaining invoice/deposit notice back + // to its ticket's customerId, and every one of demoSales.js's money-cycle deals (tickets 19-25, + // "the ฝ่ายบัญชี money-cycle deals") predates that field and carries none. Without at least one + // linked customer, mockApi.billingNotes.candidates() would return an empty picker for every + // seeded customer, in mock mode ONLY — the exact "fixture too EMPTY to reach the code path" + // trap customer 5's comment already names. Ticket 19 (ISSUED deposit notice, 2026-08-11) is + // wired to this row via demoSales.js's makeMoneyDeal `customerId` param. + { id: 6, name: 'บริษัท สยามพารากอน ดีเวลลอปเมนท์ จำกัด', taxId: '0105512345678', address: '991 ถนนพระราม 1 แขวงปทุมวัน กรุงเทพฯ 10330', branch: 'สำนักงานใหญ่', phone: '02-610-8000' }, ]; let mockCustomerSeq = mockCustomers.length + 1; @@ -551,6 +560,12 @@ const mockContacts = [ // so the picker on the seeded deal has something to CHANGE to and not just one forced answer. { id: 6, customerId: 5, firstName: 'ณัฐพงศ์', lastName: 'ศรีวิไล', position: 'ฝ่ายจัดซื้อ', email: 'nattapong@fashionisland.co.th', phone: '086-222-3333' }, { id: 7, customerId: 5, firstName: 'พิมพ์ใจ', lastName: 'บุญมาก', position: 'ผู้จัดการโครงการ', email: 'pimjai@fashionisland.co.th', phone: '086-444-5555' }, + // Customer 6 (its own comment above) had no contact row — harmless before ticket 19 carried a + // customerId at all (resolveDealQuotationContact skips the ownership check when + // ticket.customerId is null), but wiring ticket 19 to a real customer activates that check for + // any future quotation work on it. One row keeps that path from dead-ending (review round 2, + // 2026-09-23). + { id: 8, customerId: 6, firstName: 'วรวุฒิ', lastName: 'พารากร', position: 'ฝ่ายจัดซื้อ', email: 'worawut@siamparagon-dev.co.th', phone: '02-610-8001' }, ]; let mockContactSeq = mockContacts.length + 1; @@ -2236,14 +2251,20 @@ function publicUser(user) { }; } -// Mirrors AuthResponse.java's shape: `{ user, admin, canCreateQuotation }` -- `admin` and -// `canCreateQuotation` are siblings of `user`, not fields ON it (App.jsx's userFromAuthResponse -// flattens them back onto one object). auth.login/me/changePassword all resolve this so -// `canCreateQuotation` actually reaches the frontend under mock mode (#H4) -- previously these -// three returned `{ user: publicUser(user) }` alone, so `response.canCreateQuotation` was always -// undefined regardless of the seed's own `canCreateQuotation` field on `db.users`. +// Mirrors AuthResponse.java's shape: `{ user, admin, canCreateQuotation, canIssueBillingNote }` -- +// `admin`, `canCreateQuotation` and `canIssueBillingNote` are siblings of `user`, not fields ON it +// (App.jsx's userFromAuthResponse flattens them back onto one object). auth.login/me/changePassword +// all resolve this so `canCreateQuotation` actually reaches the frontend under mock mode (#H4) -- +// previously these three returned `{ user: publicUser(user) }` alone, so `response.canCreateQuotation` +// was always undefined regardless of the seed's own `canCreateQuotation` field on `db.users`. +// `canIssueBillingNote` (GLA-129) follows the same discipline, sourced from the seed's own field. function authResponse(user) { - return { user: publicUser(user), admin: Boolean(user?.admin), canCreateQuotation: Boolean(user?.canCreateQuotation) }; + return { + user: publicUser(user), + admin: Boolean(user?.admin), + canCreateQuotation: Boolean(user?.canCreateQuotation), + canIssueBillingNote: Boolean(user?.canIssueBillingNote), + }; } function requireSession() { @@ -2383,6 +2404,193 @@ function mockRemainingInvoiceSnapshot(ticket) { return { items, itemsTotal, depositDeduction, netAmount, vatAmount, grandTotal }; } +// ── The STORED ใบวางบิล aggregate (V189, GLA-99 step 3 / GLA-129 step 4) ───────────────────────── +// A CUSTOMER-level cover sheet (not ticket-level, unlike remaining invoices above) rolling several +// already-ISSUED remaining invoices / deposit notices — possibly across several of the customer's +// deals — into one billed total for one credit cycle. Mirrors +// th.co.glr.hr.billing.BillingNoteService's lifecycle/numbering shape (DRAFT -> ISSUED -> +// {SUPERSEDED (revise) | CANCELLED | SETTLED}, minted on the SAME shared AR_GLR sequence remaining +// invoices use — reuses mockArGlrSeqByYear above) and its exact authz split (write: +// requireBillingNoteWriteGate below; read: requireBillingNoteReadAccess). Per CLAUDE.md's +// mock-contract rule, this does NOT reproduce BillingNoteService's real settlement-reconcile +// machinery (BillingNoteRepository#reconcileSettlementForCustomer's automatic ISSUED -> SETTLED on +// every read) — markSettled() below is the only path to SETTLED in mock mode, always explicit, +// which is honestly narrower than production rather than silently wrong. Nor does it reproduce the +// remaining-invoice outstanding-amount approximation's own caveats beyond mirroring its exact +// signed-sum SQL (mockBillingNoteSignedReceiptSum, shared with the deposit-notice half) — the +// documented double-count-across-a-ticket's-full-history gap on the real side is not reproduced +// here at all, since mock mode never accumulates that history the same way. +let mockBillingNoteSeq = 1; +const mockBillingNotes = []; // BillingNoteDocumentDto-shaped rows, each with its own `lines`. +const MOCK_BILLING_NOTE_MAX_LINES = 15; // mirrors BillingNoteRenderer.MAX_LINE_ROWS + +// Mirrors BillingNoteService#hasWriteGrant: ceo, or any employee holding the live +// can_issue_billing_note grant (App.jsx's userFromAuthResponse flattens it onto `user`, same as +// canCreateQuotation). Unlike requireRemainingInvoiceWriteGate above, this is NOT ticket-scoped — +// a billing note is a customer-level document with no single owning deal. +function requireBillingNoteWriteGate() { + const user = requireSession(); + if (user.role !== 'ceo' && !user.canIssueBillingNote) { + fail('ไม่มีสิทธิ์ออกหรือแก้ไขใบวางบิล', 403); + } + return user; +} + +const MOCK_BILLING_NOTE_READ_ROLES = ['account', 'sales_manager', 'ceo']; + +// Mirrors BillingNoteService#requireReadAccess: the write set, plus account/sales_manager +// unconditionally, plus a sales rep who owns at least one deal referenced by the note's own lines. +function requireBillingNoteReadAccess(note) { + const user = requireSession(); + const hasWriteGrant = user.role === 'ceo' || Boolean(user.canIssueBillingNote); + if (MOCK_BILLING_NOTE_READ_ROLES.includes(user.role) || hasWriteGrant) return user; + if (user.role === 'sales') { + const ticketIds = [...new Set(note.lines.map((l) => l.ticketId).filter((id) => id != null))]; + const owns = ticketIds.some((id) => findTicketRaw(id).createdById === user.id); + if (owns) return user; + } + fail('ไม่มีสิทธิ์เข้าถึงใบวางบิลนี้', 403); + return user; +} + +function findStoredBillingNoteRaw(id) { + const row = mockBillingNotes.find((n) => n.id === Number(id)); + if (!row) fail('ไม่พบใบวางบิลนี้', 404); + return row; +} + +// BillingNoteLineDto (11 components — id, billingNoteId, seq, sourceType, sourceId, ticketId, +// docNumber, docDate, dueDate, amount, note) carries no `released` field — it is this mock's own +// internal bookkeeping for mockBillingNoteLiveClaims, with no counterpart on the real DTO (the +// real invariant lives in sales.billing_note_line.note_status, a column the API never serializes +// either). Every response below goes through this rather than a bare structuredClone, so a caller +// never sees a field the real API would never send — review round 2 (2026-09-23) caught the leak. +function toBillingNoteResponse(note) { + const clone = structuredClone(note); + clone.lines = clone.lines.map(({ released, ...line }) => line); + return clone; +} + +// Signed payment-receipt sum, mirroring both OUTSTANDING_EXPRs' shared +// `SUM(CASE WHEN kind = 'ADJUSTMENT' THEN -amount ELSE amount END)` — an ADJUSTMENT receipt +// INCREASES outstanding (it is stored as a reduction elsewhere, so negating it here restores the +// balance owed), every other kind reduces it at face value. +function mockBillingNoteSignedReceiptSum(receipts) { + return receipts.reduce((sum, r) => sum + (r.kind === 'ADJUSTMENT' ? -Number(r.amount || 0) : Number(r.amount || 0)), 0); +} + +// Every ISSUED remaining invoice / deposit notice across this customer's deals, with its +// VAT-inclusive outstanding amount — mirrors BillingNoteRepository#candidatesForCustomer's own +// CANDIDATES_SQL (see this section's own header comment for the simplifications this mock keeps). +function mockBillingNoteCandidateRows(customerId) { + const rows = []; + for (const ri of mockRemainingInvoices) { + if (ri.status !== 'ISSUED') continue; + const ticket = findTicketRaw(ri.ticketId); + if (ticket.customerId !== Number(customerId)) continue; + // REMAINING_INVOICE_OUTSTANDING_EXPR: ticket-scoped, kind IN ('BALANCE','ADJUSTMENT') only. + const paid = mockBillingNoteSignedReceiptSum(db.paymentReceipts + .filter((r) => r.ticketId === ri.ticketId && (r.kind === 'BALANCE' || r.kind === 'ADJUSTMENT'))); + rows.push({ + sourceType: 'REMAINING_INVOICE', sourceId: ri.id, ticketId: ri.ticketId, + docNumber: ri.docNumber, docDate: ri.docDate, dueDate: ticket.dueDate ?? null, + outstandingAmount: Math.round((ri.grandTotal - paid) * 100) / 100, + }); + } + for (const dn of mockDepositNotices) { + if (dn.status !== 'ISSUED') continue; + const ticket = findTicketRaw(dn.ticketId); + if (ticket.customerId !== Number(customerId)) continue; + // DEPOSIT_NOTICE_OUTSTANDING_EXPR: EVERY receipt linked by deposit_notice_id, no kind filter + // at all (unlike the remaining-invoice half above) — a DEPOSIT-kind receipt from + // confirmDepositPaid counts here just as a BALANCE one would. + const paid = mockBillingNoteSignedReceiptSum(db.paymentReceipts.filter((r) => r.depositNoticeId === dn.id)); + rows.push({ + sourceType: 'DEPOSIT_NOTICE', sourceId: dn.id, ticketId: dn.ticketId, + docNumber: dn.docNumber, docDate: dn.issueDate, dueDate: ticket.dueDate ?? null, + outstandingAmount: Math.round((dn.totalPayable - paid) * 100) / 100, + }); + } + // ORDER BY doc_date NULLS LAST, doc_number — a plain localeCompare on a possibly-null docDate + // would sort nulls FIRST, the opposite of the real query. + return rows.sort((a, b) => { + if (a.docDate == null && b.docDate != null) return 1; + if (a.docDate != null && b.docDate == null) return -1; + return (a.docDate ?? '').localeCompare(b.docDate ?? '') || (a.docNumber ?? '').localeCompare(b.docNumber ?? ''); + }); +} + +// Mirrors BillingNoteRepository#findLiveClaimsForCustomer / #findLiveClaim: which (sourceType, +// sourceId) pairs are already claimed by a DRAFT or ISSUED note's line, keyed "TYPE:id" — global, +// not customer-scoped, same as the real unique index (a document belongs to exactly one customer +// anyway, so scoping would change nothing). +function mockBillingNoteLiveClaims() { + const claims = new Map(); + for (const note of mockBillingNotes) { + if (note.status !== 'DRAFT' && note.status !== 'ISSUED') continue; + for (const line of note.lines) { + if (line.sourceId == null || line.released) continue; + claims.set(`${line.sourceType}:${line.sourceId}`, { billingNoteId: note.id, docNumber: note.docNumber }); + } + } + return claims; +} + +// Mirrors BillingNoteService#resolveLine — validates one requested line against the customer's +// live candidate pool (ERP-sourced) or the request's own manual* fields, refusing a source that is +// fully paid, unknown, or already claimed by a DIFFERENT live note. +function mockResolveBillingNoteLine(sel, candidatesByKey, claims, excludeNoteId) { + const type = (sel?.sourceType ?? '').trim().toUpperCase(); + if (!['REMAINING_INVOICE', 'DEPOSIT_NOTICE', 'MANUAL'].includes(type)) { + fail(`ไม่รองรับประเภทรายการ '${sel?.sourceType ?? ''}'`, 400); + } + if (type === 'MANUAL') { + if (sel.sourceId != null) fail('รายการที่พิมพ์เองต้องไม่ระบุเอกสารอ้างอิง', 400); + if (!sel.manualDocNumber || !String(sel.manualDocNumber).trim()) fail('รายการที่พิมพ์เองต้องระบุเลขที่เอกสาร', 400); + // Mirrors manualAmount.setScale(2, HALF_UP).signum() <= 0 in SHAPE, not exact arithmetic — + // round to the cent BEFORE the zero/negative check, so a sub-satang amount like 0.004 is + // rejected rather than stored as-is. NOT a faithful mirror at an exact .xx5 half-satang + // boundary: Java's BigDecimal.setScale(2, HALF_UP) rounds 100.005 to 100.01 exactly, while + // this float multiply/round (100.005 * 100 = 100.00499999999999...) rounds down to 100.00. + // Only the boundary value differs; the reject-if-non-positive semantics this fix exists for + // are unaffected. + const roundedAmount = Math.round(Number(sel.manualAmount) * 100) / 100; + if (!(roundedAmount > 0)) fail('รายการที่พิมพ์เองต้องระบุจำนวนเงินมากกว่า 0', 400); + return { + sourceType: 'MANUAL', sourceId: null, ticketId: null, + docNumber: sel.manualDocNumber, docDate: sel.manualDocDate ?? null, dueDate: sel.manualDueDate ?? null, + amount: roundedAmount, note: sel.manualNote ?? null, + }; + } + if (sel.sourceId == null) fail('ต้องระบุเอกสารอ้างอิงสำหรับรายการประเภทนี้', 400); + const candidate = candidatesByKey.get(`${type}:${sel.sourceId}`); + if (!candidate) fail('ไม่พบเอกสารนี้สำหรับลูกค้ารายนี้ หรือเอกสารยังไม่ได้ออกเลข', 400); + if (!(candidate.outstandingAmount > 0)) fail(`เอกสาร ${candidate.docNumber} ชำระครบแล้ว ไม่สามารถวางบิลได้`, 409); + const claim = claims.get(`${type}:${sel.sourceId}`); + if (claim && claim.billingNoteId !== excludeNoteId) { + fail(`เอกสาร ${candidate.docNumber} อยู่ในใบวางบิลอื่นอยู่แล้ว${claim.docNumber ? ` (เลขที่ ${claim.docNumber})` : ' (ร่าง)'}`, 409); + } + return { + sourceType: type, sourceId: candidate.sourceId, ticketId: candidate.ticketId, + docNumber: candidate.docNumber, docDate: candidate.docDate, + dueDate: sel.manualDueDate ?? candidate.dueDate, amount: candidate.outstandingAmount, note: sel.manualNote ?? null, + }; +} + +// Whole-value replace of a note's lines, mirroring BillingNoteService#writeLines — validates the +// row-count cap, resolves every selection, then overwrites `lines` atomically (no partial write on +// a validation failure, since every fail() throws before this function returns). +function mockWriteBillingNoteLines(note, selections) { + if (selections.length > MOCK_BILLING_NOTE_MAX_LINES) { + fail(`ใบวางบิลมีรายการ ${selections.length} รายการ เกินความจุของแบบฟอร์ม (สูงสุด ${MOCK_BILLING_NOTE_MAX_LINES} แถว) กรุณาลดจำนวนรายการหรือออกเอกสารหลายฉบับ`, 409); + } + const candidatesByKey = new Map(mockBillingNoteCandidateRows(note.customerId).map((r) => [`${r.sourceType}:${r.sourceId}`, r])); + const claims = mockBillingNoteLiveClaims(); + const resolved = selections.map((sel) => mockResolveBillingNoteLine(sel, candidatesByKey, claims, note.id)); + note.lines = resolved.map((l, i) => ({ id: `${note.id}-${i + 1}`, billingNoteId: note.id, seq: i + 1, ...l, released: false })); + note.totalAmount = Math.round(note.lines.reduce((sum, l) => sum + Number(l.amount || 0), 0) * 100) / 100; +} + // Mirrors th.co.glr.hr.importrequest.ImportRequestStep — same codes, same order, same Thai labels. const MOCK_IMPORT_REQUEST_STEPS = ['CONTACTED', 'ORDERED', 'PICKED_UP', 'IN_TRANSIT', 'AWAITING_CUSTOMS', 'RECEIVED']; const MOCK_IMPORT_REQUEST_STEP_LABELS_TH = { @@ -3689,6 +3897,24 @@ function buildMockRemainingInvoiceXlsxFromRow(row) { return mockDocPlaceholderBlob(lines); } +// ── Billing note XLSX (demo placeholder) — real file from BillingNoteRenderer.java ──────────── +// Same discipline as buildMockRemainingInvoiceXlsxFromRow above: renders from THIS row's own +// stored lines, never recomputed from live ticket/customer data. +function buildMockBillingNoteXlsxFromRow(row) { + const lines = [ + `ใบวางบิล เลขที่ ${row.docNumber ?? '(ร่าง)'}`, + `วันที่: ${row.billDate ? mockThaiDate(new Date(row.billDate)) : ''}`, + `ลูกค้า: ${row.customerName ?? ''}`, + ...(row.customerTaxId ? [`เลขประจำตัวผู้เสียภาษี: ${row.customerTaxId}`] : []), + '', + ...row.lines.map((l, i) => + `${i + 1}. ${l.docNumber ?? ''} ${l.docDate ? mockThaiDate(new Date(l.docDate)) : ''} — ${l.amount ?? 0}`), + '', + `รวมเป็นเงิน: ${row.totalAmount ?? 0}`, + ]; + return mockDocPlaceholderBlob(lines); +} + function pushEvent(ticket, actor, kind, fromStatus, toStatus, message, itemSnapshot = null) { const nextId = Math.max(...db.tickets.flatMap((t) => t.events.map((e) => e.id)), 0) + 1; ticket.events.push({ id: nextId, ticketId: ticket.id, actorId: actor.id, actorName: actor.name, kind, fromStatus, toStatus, message, createdAt: new Date().toISOString(), itemSnapshot }); @@ -12016,6 +12242,296 @@ export const api = { }, }, + // Mirrors BillingNoteController — the STORED ใบวางบิล aggregate (V189, GLA-99 step 3 / GLA-129 + // step 4). See this file's own "STORED ใบวางบิล aggregate" header comment (near + // mockBillingNotes) for what is and is not mirrored faithfully. + billingNotes: { + async candidates(customerId) { + const user = requireSession(); + const hasWriteGrant = user.role === 'ceo' || Boolean(user.canIssueBillingNote); + if (!MOCK_BILLING_NOTE_READ_ROLES.includes(user.role) && !hasWriteGrant) { + fail('ไม่มีสิทธิ์ดูรายการที่วางบิลได้ของลูกค้ารายนี้', 403); + } + const customer = mockCustomers.find((c) => c.id === Number(customerId)); + if (!customer) fail('ไม่พบลูกค้ารายนี้', 404); + const rows = mockBillingNoteCandidateRows(customerId); + const claims = mockBillingNoteLiveClaims(); + const candidates = []; + const alreadyBilled = []; + for (const row of rows) { + if (!(row.outstandingAmount > 0)) continue; // fully paid — excluded from both lists + const claim = claims.get(`${row.sourceType}:${row.sourceId}`); + if (claim) { + alreadyBilled.push({ ...row, blockedByNoteId: claim.billingNoteId, blockedByNoteNumber: claim.docNumber }); + } else { + candidates.push({ ...row, blockedByNoteId: null, blockedByNoteNumber: null }); + } + } + return delay({ customerId: Number(customerId), customerName: customer.name, candidates, alreadyBilled }); + }, + + async listForCustomer(customerId) { + const user = requireSession(); + const hasWriteGrant = user.role === 'ceo' || Boolean(user.canIssueBillingNote); + const fullAccess = MOCK_BILLING_NOTE_READ_ROLES.includes(user.role) || hasWriteGrant; + if (!fullAccess && user.role !== 'sales') { + fail('ไม่มีสิทธิ์เข้าถึงใบวางบิลของลูกค้ารายนี้', 403); + } + if (!mockCustomers.some((c) => c.id === Number(customerId))) fail('ไม่พบลูกค้ารายนี้', 404); + const all = mockBillingNotes.filter((n) => n.customerId === Number(customerId)); + const visible = fullAccess ? all : all.filter((n) => { + const ticketIds = [...new Set(n.lines.map((l) => l.ticketId).filter((id) => id != null))]; + return ticketIds.some((id) => findTicketRaw(id).createdById === user.id); + }); + // Mirrors findByCustomer's own ORDER BY COALESCE(base_number, ''), type, version — groups a + // revision chain together (same base_number) and puts a not-yet-issued draft (base_number + // null -> '') first within it, not newest-created-first. + return delay({ + billingNotes: visible.sort((a, b) => + (a.baseNumber ?? '').localeCompare(b.baseNumber ?? '') + || a.type.localeCompare(b.type) + || a.version - b.version).map(toBillingNoteResponse), + }); + }, + + // B1 (owner ruling 2026-09-20): a brand-new draft starts its OWN chain — no predecessor/ + // customer+type conflict check at all, several may legitimately be in progress at once. + async createDraft(customerId, payload = {}) { + const user = requireBillingNoteWriteGate(); + const customer = mockCustomers.find((c) => c.id === Number(customerId)); + if (!customer) fail('ไม่พบลูกค้ารายนี้', 404); + const type = (payload.type ?? '').trim().toUpperCase(); + if (!['GOODS', 'FREIGHT'].includes(type)) fail('ต้องระบุประเภทใบวางบิลเป็น GOODS หรือ FREIGHT', 400); + const now = new Date().toISOString(); + const id = mockBillingNoteSeq++; + const note = { + id, customerId: Number(customerId), type, baseNumber: null, version: 1, docNumber: null, + status: 'DRAFT', supersededById: null, revisionOfId: null, + cancelReason: null, cancelledById: null, cancelledByName: null, cancelledAt: null, + billDate: payload.billDate ?? null, paymentDueNote: payload.paymentDueNote ?? null, + paymentAppointmentDate: null, receivedByName: null, receivedAt: null, note: payload.note ?? null, + customerName: customer.name, customerTaxId: customer.taxId, customerBranch: customer.branch, + customerAddress: customer.address, + totalAmount: 0, + createdById: user.id, createdByName: user.name, createdAt: now, updatedAt: now, + issuedById: null, issuedByName: null, issuedAt: null, + settledById: null, settledByName: null, settledAt: null, + lines: [], + }; + // Resolve/validate lines BEFORE pushing the note — mirrors @Transactional: a line-validation + // failure (cap, unknown source, already-claimed, fully-paid, bad MANUAL fields) must leave no + // trace, not an orphan DRAFT with zero lines. mockWriteBillingNoteLines only reads OTHER + // notes' claims via mockBillingNotes, so calling it before this note is in that array is safe + // — excludeNoteId is a plain id comparison, not a presence check. + mockWriteBillingNoteLines(note, payload.lines ?? []); + mockBillingNotes.push(note); + return delay({ billingNote: toBillingNoteResponse(note) }); + }, + + async get(id) { + const row = findStoredBillingNoteRaw(id); + requireBillingNoteReadAccess(row); + return delay({ billingNote: toBillingNoteResponse(row) }); + }, + + // PATCH: an absent field is left alone; `lines`, when supplied, whole-value replaces (matching + // RemainingInvoiceDraftRequest's own replace-not-append discipline) — mirrors + // BillingNoteService#updateDraft. + // F2/S7 (matching BillingNoteService's own fix, applied verbatim here): the write gate runs + // BEFORE the existence lookup in every write method below — an unauthorized caller must not be + // able to distinguish "no such note" (404) from "exists but wrong state" (409) without ever + // passing the write gate. Review round 1 (GLA-129 branch 1) caught this ordering reversed. + async update(id, payload = {}) { + requireBillingNoteWriteGate(); + const row = findStoredBillingNoteRaw(id); + if (row.status !== 'DRAFT') fail('ใบวางบิลนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)', 409); + if (payload.billDate != null) row.billDate = payload.billDate; + if (payload.paymentDueNote != null) row.paymentDueNote = payload.paymentDueNote; + if (payload.note != null) row.note = payload.note; + const customer = mockCustomers.find((c) => c.id === row.customerId); + if (customer) { + row.customerName = customer.name; row.customerTaxId = customer.taxId; + row.customerBranch = customer.branch; row.customerAddress = customer.address; + } + if (payload.lines != null) mockWriteBillingNoteLines(row, payload.lines); + row.updatedAt = new Date().toISOString(); + return delay({ billingNote: toBillingNoteResponse(row) }); + }, + + // DRAFT -> ISSUED: mints/carries base_number the same way RemainingInvoiceService#issue does, + // via ITS OWN revisionOfId pointer (B1) — never by scanning for "the" ISSUED note of this + // customer+type, since several may legitimately coexist across credit cycles. + async issue(id) { + const user = requireBillingNoteWriteGate(); + const row = findStoredBillingNoteRaw(id); + if (row.status !== 'DRAFT') fail('ใบวางบิลนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)', 409); + if (!row.lines.length) fail('ใบวางบิลนี้ยังไม่มีรายการ', 409); + + let predecessor = null; + if (row.revisionOfId != null) { + predecessor = mockBillingNotes.find((n) => n.id === row.revisionOfId) ?? null; + if (!predecessor || predecessor.status !== 'ISSUED') { + fail('ใบวางบิลต้นฉบับไม่ได้อยู่ในสถานะออกแล้วอีกต่อไป ไม่สามารถออกฉบับแก้ไขนี้ได้', 409); + } + } + let baseNumber; + let version; + if (predecessor) { + baseNumber = predecessor.baseNumber; + version = predecessor.version + 1; + predecessor.status = 'SUPERSEDED'; + predecessor.supersededById = row.id; + } else { + const thaiYear = new Date().getFullYear() + 543; + mockArGlrSeqByYear[thaiYear] = (mockArGlrSeqByYear[thaiYear] ?? 0) + 1; + baseNumber = `GLR${String(thaiYear).slice(-2)}${String(mockArGlrSeqByYear[thaiYear]).padStart(5, '0')}`; + version = 1; + } + const today = new Date().toISOString().slice(0, 10); + row.billDate = row.billDate ?? today; // mirrors `bill_date = COALESCE(bill_date, :issueDate)` + row.baseNumber = baseNumber; + row.version = version; + row.docNumber = `${baseNumber}-${version}`; + row.status = 'ISSUED'; + row.issuedById = user.id; + row.issuedByName = user.name; + row.issuedAt = new Date().toISOString(); + row.updatedAt = row.issuedAt; + const ticketIds = [...new Set(row.lines.map((l) => l.ticketId).filter((tid) => tid != null))]; + for (const ticketId of ticketIds) { + const ticket = findTicketRaw(ticketId); + pushEvent(ticket, user, 'DOCUMENT_ISSUED', ticket.status, ticket.status, `ใบวางบิล ${row.docNumber} ออกแล้ว`); + } + return delay({ billingNote: toBillingNoteResponse(row) }); + }, + + // Prepares a correction: a new DRAFT tied to its predecessor via revisionOfId (B1), copying the + // ISSUED note's own body/lines VERBATIM — mirrors BillingNoteService#revise. The predecessor + // stays ISSUED until the replacement is actually issued (issue() above does the supersede), but + // its LINES release their live claim immediately (mirrors stored.releaseLines) so a corrected + // draft that drops a line frees it for another note right away, rather than waiting for issue(). + async revise(id) { + const user = requireBillingNoteWriteGate(); + const issued = findStoredBillingNoteRaw(id); + // Mirrors requireIssued: revise() reuses the SAME generic "must be ISSUED" gate cancel/ + // markReceived/markSettled use, not a bespoke revise-only message. + if (issued.status !== 'ISSUED') fail('ดำเนินการได้เฉพาะใบวางบิลที่ออกแล้ว (ISSUED)', 409); + const hasLiveDraft = mockBillingNotes.some((n) => n.status === 'DRAFT' && n.revisionOfId === issued.id); + if (hasLiveDraft) fail('มีร่างฉบับแก้ไขของใบวางบิลนี้อยู่แล้ว', 409); + + const now = new Date().toISOString(); + const newId = mockBillingNoteSeq++; + const clone = structuredClone(issued); + const note = { + ...clone, id: newId, baseNumber: null, version: 1, docNumber: null, status: 'DRAFT', + supersededById: null, revisionOfId: issued.id, + cancelReason: null, cancelledById: null, cancelledByName: null, cancelledAt: null, + // Neither the predecessor's receivedByName/receivedAt NOR its paymentAppointmentDate carry + // to the correction draft — mirrors insertDraft, which never passes any of the three. + receivedByName: null, receivedAt: null, paymentAppointmentDate: null, + createdById: user.id, createdByName: user.name, createdAt: now, updatedAt: now, + issuedById: null, issuedByName: null, issuedAt: null, + settledById: null, settledByName: null, settledAt: null, + lines: clone.lines.map((l, i) => ({ ...l, id: `${newId}-${i + 1}`, billingNoteId: newId, released: false })), + }; + mockBillingNotes.push(note); + // Release the predecessor's OWN lines (the live object, not the clone) so their sources + // become billable again immediately — mirrors stored.releaseLines(id) in BillingNoteService + // #revise, run before the new draft's lines are written on the real side too. + issued.lines.forEach((l) => { l.released = true; }); + return delay({ billingNote: toBillingNoteResponse(note) }); + }, + + async cancel(id, reason) { + const user = requireBillingNoteWriteGate(); + const row = findStoredBillingNoteRaw(id); + if (row.status !== 'ISSUED') fail('ดำเนินการได้เฉพาะใบวางบิลที่ออกแล้ว (ISSUED)', 409); + if (!reason || !String(reason).trim()) fail('ต้องระบุเหตุผลการยกเลิก', 400); + row.status = 'CANCELLED'; + row.cancelReason = reason; + row.cancelledById = user.id; + row.cancelledByName = user.name; + row.cancelledAt = new Date().toISOString(); + row.updatedAt = row.cancelledAt; + return delay({ billingNote: toBillingNoteResponse(row) }); + }, + + async markReceived(id, payload = {}) { + requireBillingNoteWriteGate(); + const row = findStoredBillingNoteRaw(id); + if (row.status !== 'ISSUED') fail('ดำเนินการได้เฉพาะใบวางบิลที่ออกแล้ว (ISSUED)', 409); + if (!payload.receivedByName || !String(payload.receivedByName).trim()) fail('ต้องระบุชื่อผู้รับวางบิล', 400); + row.receivedByName = payload.receivedByName; + row.receivedAt = new Date().toISOString(); + // Unconditional, matching the real UPDATE — omitting paymentAppointmentDate on a SECOND + // markReceived call clears it, it does not keep the first call's value. + row.paymentAppointmentDate = payload.paymentAppointmentDate ?? null; + row.updatedAt = row.receivedAt; + return delay({ billingNote: toBillingNoteResponse(row) }); + }, + + // C1 (owner ruling 2026-09-20): the only caller-triggered path to SETTLED in mock mode — this + // file does not reproduce BillingNoteRepository#reconcileSettlementForCustomer's automatic + // recompute-on-read (see this section's own header comment), so unlike production, a + // non-all-MANUAL note here only ever settles via this same explicit action. + async markSettled(id) { + const user = requireBillingNoteWriteGate(); + const row = findStoredBillingNoteRaw(id); + if (row.status !== 'ISSUED') fail('ดำเนินการได้เฉพาะใบวางบิลที่ออกแล้ว (ISSUED)', 409); + row.status = 'SETTLED'; + row.settledById = user.id; + row.settledByName = user.name; + row.settledAt = new Date().toISOString(); + row.updatedAt = row.settledAt; + return delay({ billingNote: toBillingNoteResponse(row) }); + }, + + async remove(id) { + requireBillingNoteWriteGate(); + const row = findStoredBillingNoteRaw(id); + if (row.status !== 'DRAFT') fail('ใบวางบิลนี้ไม่ได้อยู่ในสถานะร่าง (DRAFT)', 409); + // Look up the predecessor (if any) BEFORE removing — mirrors deleteDraft's own ordering. + const predecessor = row.revisionOfId != null + ? mockBillingNotes.find((n) => n.id === row.revisionOfId) ?? null : null; + // Mirrors restoreLinesToIssued's own guard (`AND bn.status = 'ISSUED'`, a no-op restore if + // the predecessor is no longer ISSUED) PLUS the ux_billing_note_line_source_live race it can + // trip: if any of the predecessor's own sources was claimed by a DIFFERENT live note in the + // window since revise() released it (e.g. C creates a fresh draft on a source this draft's + // own update() dropped), restoring would double-claim that source across two notes at once — + // review round 2 (2026-09-23) caught this permitted where production 409s. Checked and + // refused BEFORE the splice below, mirroring the real @Transactional rollback: the whole + // delete fails atomically, the draft is not removed either, not silently half-applied. + const predecessorIsLive = predecessor?.status === 'ISSUED'; + if (predecessorIsLive) { + const claims = mockBillingNoteLiveClaims(); + const stolen = predecessor.lines.some((l) => { + if (l.sourceId == null) return false; + const claim = claims.get(`${l.sourceType}:${l.sourceId}`); + return claim && claim.billingNoteId !== row.id && claim.billingNoteId !== predecessor.id; + }); + if (stolen) { + fail('ไม่สามารถคืนสถานะรายการของใบวางบิลต้นฉบับได้ เนื่องจากเอกสารอ้างอิงถูกใช้ในใบวางบิลอื่นไปแล้วระหว่างการลบ', 409); + } + } + mockBillingNotes.splice(mockBillingNotes.indexOf(row), 1); + // Abandoning a correction-in-progress restores the predecessor's own lines to their live + // ISSUED claim — mirrors restoreLinesToIssued, the counterpart of revise()'s own release + // above. Skipped (silently, matching the real no-op) when the predecessor is no longer + // ISSUED — its lines are already excluded from mockBillingNoteLiveClaims by status alone. + if (predecessorIsLive) predecessor.lines.forEach((l) => { l.released = false; }); + return delay({ deleted: true }); + }, + + // Binary, so it goes through fetch directly — same shape as storedRemainingInvoices.download + // above. Renders from THIS row's own stored snapshot, never recomputed from live data. + async download(id) { + const row = findStoredBillingNoteRaw(id); + requireBillingNoteReadAccess(row); + const blob = await tryBackendBlob(`/api/billing-notes/${id}/file`); + return blob ?? buildMockBillingNoteXlsxFromRow(row); + }, + }, + // Mirrors PriceImportController + PriceImportService (catalog/importer/). priceImport: { async factories() { diff --git a/frontend/src/api/routes.js b/frontend/src/api/routes.js index 826fc2c4c..d316fb77f 100644 --- a/frontend/src/api/routes.js +++ b/frontend/src/api/routes.js @@ -238,6 +238,22 @@ export const API_ROUTES = { revise: (id) => `/api/remaining-invoices/${id}/revise`, file: (id) => `/api/remaining-invoices/${id}/file`, }, + // Mirrors BillingNoteController — the STORED ใบวางบิล aggregate (V189, GLA-99 step 3 / GLA-129 + // step 4). Customer-scoped (many deals bill together), DRAFT -> ISSUED -> {SUPERSEDED (revised) | + // CANCELLED | SETTLED}, minted on the same shared AR_GLR sequence remaining invoices use. + // Authorisation is entirely BillingNoteService's own grant (can_issue_billing_note OR ceo, plus a + // wider read set) — these methods carry no gate of their own. + billingNotes: { + candidates: (customerId) => `/api/customers/${customerId}/billing-note-candidates`, + forCustomer: (customerId) => `/api/customers/${customerId}/billing-notes`, + get: (id) => `/api/billing-notes/${id}`, + issue: (id) => `/api/billing-notes/${id}/issue`, + revise: (id) => `/api/billing-notes/${id}/revise`, + cancel: (id) => `/api/billing-notes/${id}/cancel`, + markReceived: (id) => `/api/billing-notes/${id}/mark-received`, + markSettled: (id) => `/api/billing-notes/${id}/mark-settled`, + file: (id) => `/api/billing-notes/${id}/file`, + }, catalog: { search: (q) => `/api/catalog${q ? `?q=${encodeURIComponent(q)}` : ''}`, prices: (q, factoryId, limit) => { diff --git a/frontend/src/api/serverContract.test.js b/frontend/src/api/serverContract.test.js index bfc032c52..a8e217752 100644 --- a/frontend/src/api/serverContract.test.js +++ b/frontend/src/api/serverContract.test.js @@ -301,40 +301,6 @@ const SERVER_ONLY = { + 'the per-quotation approve/reject flow the task required (see the comment above). Capability ' + 'built backend-first; wire it up if a cross-deal CEO queue view is ever requested.', - // GLA-99 step 3 (ใบวางบิล / billing note, V189): the STORED backend + renderer landed on this - // branch; the /finance ใบวางบิล tab that calls these eleven routes is step 4, a separate branch - // not yet merged. BillingNoteService/BillingNoteController/BillingNoteRenderer are already - // covered by BillingNoteServiceIntegrationTest (real Postgres, including the wrong-way-round - // authz suite) and BillingNoteRendererTest. Delete this whole block once step 4's screen calls - // these through hrApi.js. - 'GET /api/customers/{}/billing-note-candidates': - 'BillingNoteService.candidates — the customer-level outstanding-documents picker step 4\'s ' - + 'ใบวางบิล tab will call before creating a draft. Backend-first; no screen yet.', - 'GET /api/customers/{}/billing-notes': - 'BillingNoteService.list — per-customer billing note history. Backend-first; no screen yet.', - 'POST /api/customers/{}/billing-notes': - 'BillingNoteService.createDraft. Backend-first; no screen yet.', - 'GET /api/billing-notes/{}': - 'BillingNoteService.get. Backend-first; no screen yet.', - 'PUT /api/billing-notes/{}': - 'BillingNoteService.updateDraft. Backend-first; no screen yet.', - 'POST /api/billing-notes/{}/issue': - 'BillingNoteService.issue — mints the AR_GLR number. Backend-first; no screen yet.', - 'POST /api/billing-notes/{}/revise': - 'BillingNoteService.revise — prepares a correction DRAFT. Backend-first; no screen yet.', - 'POST /api/billing-notes/{}/cancel': - 'BillingNoteService.cancel — voids an ISSUED note and releases its lines for re-billing. ' - + 'Backend-first; no screen yet.', - 'POST /api/billing-notes/{}/mark-received': - 'BillingNoteService.markReceived — records ผู้รับวางบิล/วันนัดชำระเงิน after the customer ' - + 'signs on paper. Backend-first; no screen yet.', - 'POST /api/billing-notes/{}/mark-settled': - 'BillingNoteService.markSettled — owner ruling C1: the only caller-triggered settlement path, ' - + 'for an all-MANUAL note (e.g. ค่าขนส่ง) that can never auto-settle. Backend-first; no screen yet.', - 'DELETE /api/billing-notes/{}': - 'BillingNoteService.deleteDraft. Backend-first; no screen yet.', - 'GET /api/billing-notes/{}/file': - 'BillingNoteService.file — downloads the rendered .xls. Backend-first; no screen yet.', }; // ───────────────────────────────────────────────────────────────────────────── @@ -572,6 +538,25 @@ describe('controller surface / hrApi.js contract', () => { * stale-entry test below deletes the entry for you. */ const UNREACHABLE_FROM_UI = new Set([ + // GLA-99 step 3 (ใบวางบิล / billing note, V189): the STORED backend + renderer + hrApi/mockApi + // wiring landed on this branch (GLA-129 step 4 part 1); the /finance ใบวางบิล tab and the + // redesigned deal-page การเงิน tab that actually CALL these twelve methods are step 4 parts + // 2/3, separate branches not yet merged. BillingNoteService/BillingNoteController/ + // BillingNoteRenderer are already covered by BillingNoteServiceIntegrationTest (real Postgres, + // including the wrong-way-round authz suite) and BillingNoteRendererTest. Remove this block once + // those screens call these through hrApi.js. + 'DELETE /api/billing-notes/{}', + 'GET /api/billing-notes/{}', + 'GET /api/billing-notes/{}/file', + 'GET /api/customers/{}/billing-note-candidates', + 'GET /api/customers/{}/billing-notes', + 'POST /api/billing-notes/{}/cancel', + 'POST /api/billing-notes/{}/issue', + 'POST /api/billing-notes/{}/mark-received', + 'POST /api/billing-notes/{}/mark-settled', + 'POST /api/billing-notes/{}/revise', + 'POST /api/customers/{}/billing-notes', + 'PUT /api/billing-notes/{}', 'DELETE /api/factory-quote-attachments/{}', 'GET /api/catalog', // GET/PUT /api/catalog/thickness-defaults joined this list on 2026-09-17: the CEO settings diff --git a/frontend/src/app/permissions.js b/frontend/src/app/permissions.js index fabf13bac..71ac22097 100644 --- a/frontend/src/app/permissions.js +++ b/frontend/src/app/permissions.js @@ -94,7 +94,15 @@ const PATH_GUARDS = [ // Account's money-lifecycle worklist (งานการเงิน) — mirrors ROLE_PERMISSIONS // .canConfirmPayments exactly (account/ceo), same audience as the ticket // confirmDepositPaid/confirmFinalPayment/confirmCloseReady actions this - // page's rows drive. + // page's rows drive. Deliberately NOT OR'd with isBillingNoteReleaseUser + // (GLA-129) yet — AccountFinancePage.jsx has no internal tab scoping today, + // so opening this guard for a can_issue_billing_note grant holder before + // that scoping exists would hand a plain `employee` grant holder the WHOLE + // finance worklist (payment-confirmation queue, close-ready actions, etc.), + // not just ใบวางบิล — caught in review (GLA-129 branch 1, 2026-09-23). The + // step-4 UI branch adds this OR at the SAME time it adds the page's own + // tab-level gate, so the two land atomically and there is no window where + // the page is reachable but unscoped. { test: (p) => p === '/finance', can: (u) => hasPermission(u.role, 'canConfirmPayments') }, // Split (issue #390): mirrors PayrollController exactly -- every GET plus the non-persisting // POST /preview and /preview/export/{kind} are hasAnyRole('HR','CEO'); every write stays @@ -242,6 +250,24 @@ function isQuotationPath(path) { return pathname === '/quotations' || pathname.startsWith('/quotations/'); } +// Billing-note release predicate (GLA-99/GLA-129, owner ruling, Ploy 2026-09-19): identifies a +// holder of the per-employee can_issue_billing_note grant (BillingNoteService.hasWriteGrant), +// which — like canCreateQuotation above — can land on a role none of the other guards would admit +// (e.g. a plain `employee`). Grant-driven, not role-driven, same shape as isQuotationReleaseUser — +// and deliberately NOT account/ceo here: `account` is NOT exempt from the self-service lockdown +// (only hr/ceo are, SELF_SERVICE_EXEMPT_ROLES above), so a caller wiring this predicate into a +// lockdown exemption must never let it un-hide a path for a plain account role while locked. +// +// NOT YET WIRED into PATH_GUARDS, the lockdown bypass below, or AppShell's nav — deliberately, per +// review (GLA-129 branch 1, 2026-09-23): AccountFinancePage.jsx has no internal tab-level scoping +// today, so opening /finance's route guard for a grant holder before that scoping exists would +// expose the WHOLE finance worklist, not just ใบวางบิล. The step-4 UI branch wires this predicate +// into PATH_GUARDS' /finance entry, the lockdown exemption, and AppShell's nav item in the SAME +// change that adds AccountFinancePage.jsx's own tab gate, so the three always land together. +export function isBillingNoteReleaseUser(user) { + return Boolean(user?.canIssueBillingNote); +} + export function canAccessPath(path, user) { if (!user) return false; // Checked BEFORE the guards below, not after: the lockdown has to deny paths diff --git a/frontend/src/components/layout/AppShell.jsx b/frontend/src/components/layout/AppShell.jsx index 69e4a3284..a3490f3fc 100644 --- a/frontend/src/components/layout/AppShell.jsx +++ b/frontend/src/components/layout/AppShell.jsx @@ -221,6 +221,10 @@ export function AppShell({ user, employee, onLogout, pendingRequestCount }) { { path: '/commissions', label: 'ค่าคอมมิชชัน', helper: 'Commissions', icon: 'badgeDollar', group: 'sales', show: hasPermission(user.role, 'canViewCommissions') && SALES_ENABLED && user.role !== 'account' }, // Account's money-lifecycle worklist (Account role-scoped views): deposit // -> final payment -> close-ready -> record-invoice/commission, one page. + // Deliberately NOT OR'd with the per-employee canIssueBillingNote grant (GLA-129) yet — see + // permissions.js's isBillingNoteReleaseUser for why: the grant is wired into this nav item, + // the PATH_GUARDS entry, and AccountFinancePage.jsx's own tab scoping together, on the branch + // that adds the scoping. { path: '/finance', label: 'งานการเงิน', helper: 'Finance worklist', icon: 'badgeDollar', group: 'finance', show: hasPermission(user.role, 'canConfirmPayments') && SALES_ENABLED }, { path: '/hr', label: 'ภาพรวม HR', helper: 'HR overview', icon: 'home', group: 'hr', show: hasPermission(user.role, 'canViewEmployees') }, { path: '/employees', label: 'พนักงานทั้งหมด', helper: 'Employees', icon: 'users', group: 'hr', show: hasPermission(user.role, 'canViewEmployees') }, diff --git a/frontend/src/data/demoData.js b/frontend/src/data/demoData.js index 6612d6098..6066b702d 100644 --- a/frontend/src/data/demoData.js +++ b/frontend/src/data/demoData.js @@ -228,7 +228,12 @@ export function createDemoDatabase() { // 2026-09-09 -- ภิญญดา, employee 144, QC&ISO / role `qc`, which this demo seed has no persona // for) on a non-sales `employee` role, mirroring AuthResponse.canCreateQuotation / // hr.employee.can_create_quotation (V166). See quotationMeta.js's hasDealQuotationGrant. - { id: 4, email: 'employee@glr.co.th', password: 'demo1234', name: employees[8].nameTh, role: 'employee', employeeId: employees[8].id, active: true, createdAt: iso(2025, 2, 11), canCreateQuotation: true }, + // canIssueBillingNote: true exercises the billing-note per-employee grant (GLA-99/GLA-129, + // owner ruling 2026-09-19) on the same non-sales `employee` role -- on prod this is the same + // real person (ภิญญดา holds both grants), so reusing this persona rather than inventing a + // second one keeps the demo faithful. Mirrors AuthResponse.canIssueBillingNote / + // hr.employee.can_issue_billing_note (V189). See permissions.js's isBillingNoteReleaseUser. + { id: 4, email: 'employee@glr.co.th', password: 'demo1234', name: employees[8].nameTh, role: 'employee', employeeId: employees[8].id, active: true, createdAt: iso(2025, 2, 11), canCreateQuotation: true, canIssueBillingNote: true }, // WHL division manager — lets the seeded stage-1 OT approval (OT#1, whose // employee reports to this ผู้จัดการฝ่าย) be demoed. role is 'employee' on // purpose: a division manager's OT-review authority is derived from the org diff --git a/frontend/src/data/demoSales.js b/frontend/src/data/demoSales.js index ad44e5ba9..c2270d22c 100644 --- a/frontend/src/data/demoSales.js +++ b/frontend/src/data/demoSales.js @@ -569,7 +569,7 @@ export function buildDemoSalesSeed() { * ฝ่ายบัญชี banks the money and confirms the close. */ function makeMoneyDeal({ - id, title, customerName, owner = SALES1, priority = 'NORMAL', + id, title, customerName, customerId = null, owner = SALES1, priority = 'NORMAL', paymentStatus, fulfillmentStatus = null, salesStage, depositPolicy = 'REQUIRED', depositPolicyReason = null, entryChannel = 'BUYER_DIRECT', @@ -611,7 +611,7 @@ export function buildDemoSalesSeed() { title, status: 'quotation_issued', priority, createdById: owner.id, createdByName: owner.name, assignedToId: IMPORT1.id, assignedToName: IMPORT1.name, - customerName, note: null, + customerName, customerId, note: null, createdAt, updatedAt, closedAt: null, salesStage, lostReason: null, lostAt: null, stageUpdatedAt: timeline[timeline.length - 1][1], @@ -671,6 +671,10 @@ export function buildDemoSalesSeed() { makeMoneyDeal({ id: 19, title: 'สยามพารากอน ดีเวลลอปเมนท์', customerName: 'บริษัท สยามพารากอน ดีเวลลอปเมนท์ จำกัด', + // customerId 6 (mockApi.js's own mockCustomers) — the ONE ISSUED-deposit-notice money-cycle + // deal linked to a real customer row, so mockApi.billingNotes.candidates() has at least one + // reachable REMAINING_INVOICE/DEPOSIT_NOTICE candidate in mock mode (GLA-129 review round 1). + customerId: 6, priority: 'HIGH', paymentStatus: 'DEPOSIT_NOTICE_ISSUED', fulfillmentStatus: null, salesStage: 'ORDER_RECEIVED', From f773c3c3e1284bf9730df8a8b03e3066b418faf9 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Wed, 23 Sep 2026 08:35:46 +0700 Subject: [PATCH 002/204] =?UTF-8?q?feat(tickets):=20redesign=20the=20?= =?UTF-8?q?=E0=B8=81=E0=B8=B2=E0=B8=A3=E0=B9=80=E0=B8=87=E0=B8=B4=E0=B8=99?= =?UTF-8?q?=20tab=20=E2=80=94=20status=20strip,=20document=20pipeline,=20r?= =?UTF-8?q?emaining-invoice=20card=20(GLA-129=20step=204,=20part=202/3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a status strip and a 4-step read-only document pipeline (ใบแจ้งมัดจำ → ใบแจ้งหนี้ส่วนที่ เหลือ → ใบวางบิล → รับชำระครบ) above the deal page's existing money-tab content, plus a compact remaining-invoice entry point reusing the unmodified RemainingInvoiceDialog. DealDepositPanel stays untouched; DealMoneyTimeline stays untouched but moves to the end of the tab per the approved design's content-priority ordering. The ใบวางบิล step resolves status via a client-side join against the customer-scoped billing-note list (no new backend endpoint — that API landed in part 1). Its "ไปที่งานการเงิน" link is deliberately scoped to account/ceo only for now, not the eventual canIssueBillingNote grant audience: /finance's own PATH_GUARDS doesn't admit a grant-only holder until part 3 wires isBillingNoteReleaseUser in, and a grant has no role restriction, so widening this early could show a real, reachable role (e.g. sales_manager) a link that refuses them today. Reviewed twice by an independent pass before this commit — round 1 found a HIGH bug (the billing note's SETTLED status was unmatched, and BillingNoteRepository's own settlement-reconcile-on-read meant opening the money tab on a just-paid deal would self-trigger the regression) plus two MEDIUM bugs (a status-strip/pipeline contradiction on deposit-bypass-policy deals; a remaining-invoice button offered to a wider audience than its own comment claimed). Round 2 confirmed all fixes and found no new HIGH/MEDIUM issues. Co-Authored-By: Claude Sonnet 5 --- .design/gla-129-money-tab/DESIGN_BRIEF.md | 46 +++ .../INFORMATION_ARCHITECTURE.md | 56 ++++ docs/api/response-fields.json | 6 +- docs/api/ui-reachable.json | 3 +- frontend/src/api/queryKeys.js | 4 + frontend/src/api/serverContract.test.js | 13 +- .../features/tickets/DealDocumentPipeline.jsx | 163 ++++++++++ .../tickets/DealDocumentPipeline.test.jsx | 304 ++++++++++++++++++ .../features/tickets/DealMoneyStatusStrip.jsx | 60 ++++ .../tickets/DealMoneyStatusStrip.test.jsx | 74 +++++ .../tickets/DealRemainingInvoiceCard.jsx | 60 ++++ .../tickets/DealRemainingInvoiceCard.test.jsx | 77 +++++ .../src/features/tickets/TicketDetailPage.jsx | 65 +++- .../tickets/TicketDetailPage.test.jsx | 44 +++ .../src/features/tickets/dealMoneyStatus.js | 25 ++ 15 files changed, 981 insertions(+), 19 deletions(-) create mode 100644 .design/gla-129-money-tab/DESIGN_BRIEF.md create mode 100644 .design/gla-129-money-tab/INFORMATION_ARCHITECTURE.md create mode 100644 frontend/src/features/tickets/DealDocumentPipeline.jsx create mode 100644 frontend/src/features/tickets/DealDocumentPipeline.test.jsx create mode 100644 frontend/src/features/tickets/DealMoneyStatusStrip.jsx create mode 100644 frontend/src/features/tickets/DealMoneyStatusStrip.test.jsx create mode 100644 frontend/src/features/tickets/DealRemainingInvoiceCard.jsx create mode 100644 frontend/src/features/tickets/DealRemainingInvoiceCard.test.jsx create mode 100644 frontend/src/features/tickets/dealMoneyStatus.js diff --git a/.design/gla-129-money-tab/DESIGN_BRIEF.md b/.design/gla-129-money-tab/DESIGN_BRIEF.md new file mode 100644 index 000000000..463265f41 --- /dev/null +++ b/.design/gla-129-money-tab/DESIGN_BRIEF.md @@ -0,0 +1,46 @@ +# Design Brief: การเงิน tab redesign (GLA-129 step 4, part 2/3) + +## What + +Redesign the deal-page's การเงิน (money) tab on `TicketDetailPage.jsx` (`/tickets/:id?tab=money`) to add +a status strip and a document pipeline above the existing content, plus a remaining-invoice summary +card — without touching `DealMoneyTimeline` (the payment receipt log, explicitly frozen by owner +ruling) or rewriting `DealDepositPanel`. + +## Who + +- **Sales rep (owner of the deal):** primary user of the per-document action cards — this is where + they actually do work (set deposit policy, issue a remaining invoice). +- **Account / CEO:** oversight — the status strip and pipeline give an at-a-glance read without + opening every document. +- **Sales manager:** same visibility as sales, backup actor on some steps (per `DealDepositPanel`'s + existing `StepRoleTag` pattern). +- Import/HR/employee never reach this tab at all (`ticketDetailTabs.js`'s own gate) — not a target + audience for this redesign. + +## Why + +GLA-99's backend (deposit policy, stored remaining invoice, billing note) shipped over three merged +PRs with no UI to show it holistically — a sales rep today has no single place to see "where is this +deal's money at" beyond the raw receipt log. This redesign gives that at-a-glance view without +changing any of the underlying document workflows, which already exist and work +(`DealDepositPanel`, `RemainingInvoiceDialog`). + +## Approved design (Ploy, 2026-09-19, `billing-note-gla99-design-rulings` memory) + +Additive only, top to bottom in the money tab: +1. Status strip — วันวางบิล, ครบกำหนด, ติดตามครั้งถัดไป, deposit requested/paid. +2. Document pipeline — ใบแจ้งมัดจำ → ใบแจ้งหนี้ส่วนที่เหลือ → ใบวางบิล → รับชำระครบ, next step + highlighted, role-aware actions/links. +3. `DealDepositPanel` — unchanged. +4. Remaining-invoice summary card — new entry point, opens the existing `RemainingInvoiceDialog`. +5. `DealMoneyTimeline` — unchanged, kept exactly as-is. + +## Out of scope + +- Any change to `DealMoneyTimeline`. +- The ใบวางบิล step's link target (`/finance`) actually working — that's branch 3. This branch + renders the link inert-but-present for a role that would have access once branch 3 lands, and + plain status text (no link) for a role that never will. +- VAT ledger storage change — status strip/pipeline show VAT-inclusive figures as a derived display + only (GLA-107), the underlying ledger stays pre-VAT. diff --git a/.design/gla-129-money-tab/INFORMATION_ARCHITECTURE.md b/.design/gla-129-money-tab/INFORMATION_ARCHITECTURE.md new file mode 100644 index 000000000..84020a916 --- /dev/null +++ b/.design/gla-129-money-tab/INFORMATION_ARCHITECTURE.md @@ -0,0 +1,56 @@ +# Information Architecture: การเงิน tab redesign (GLA-129 step 4, part 2/3) + +## Site Map / Navigation Model / URL Strategy + +Unchanged. Same route (`/tickets/:id?tab=money`), same tab, no new sub-routes or query params. This +is a content-structure restructuring within an existing tab, not a new page. + +## Content Hierarchy + +### การเงิน tab (`money` TabPanel, `TicketDetailPage.jsx`) + +1. **Status strip** (new, `DealMoneyStatusStrip`) — the fastest possible read of "is this deal's + billing on track," first because it answers the question a returning user asks before anything + else: has this moved since I last looked. +2. **Document pipeline** (new, `DealDocumentPipeline`) — the four-stage sequence with the next + action highlighted. Comes second because it's the map that tells the user *which* of the cards + below to look at. +3. **`DealDepositPanel`** (existing, unchanged) — the first two pipeline stages' actual controls + (deposit policy, deposit notice, deposit payment). Directly below the pipeline that points at it. +4. **Remaining-invoice summary card** (new) — the third stage's control. Same reasoning: sits where + the pipeline points, right after the deposit panel since it's the next stage in sequence. +5. **`DealMoneyTimeline`** (existing, unchanged) — the payment receipt log. Last, because it's a + historical record a user consults for detail/verification, not the primary "what do I do next" + surface the four items above serve. (Note: this reorders `DealMoneyTimeline` from its CURRENT + position — today it renders first, above `DealDepositPanel`. Moving it to last is part of this + redesign's own reordering, not an oversight; "keep การชำระเงิน timeline as-is" in the owner + ruling means keep the *component* untouched, not keep its position.) + +## Naming Conventions + +| Concept | Label in UI | Notes | +|---|---|---| +| Deposit notice | ใบแจ้งมัดจำ | Existing term, `DealDepositPanel`'s own step 2 label is "ใบแจ้งยอดมัดจำ" — pipeline uses the shorter form per the GLA-129 ticket text; both refer to the same document. | +| Remaining invoice | ใบแจ้งหนี้ส่วนที่เหลือ | Existing term, `RemainingInvoiceDialog`'s own header. | +| Billing note | ใบวางบิล | Existing term, GLA-99/GLA-129's own name throughout. | +| Final settlement | รับชำระครบ | New pipeline-only label — no existing document/component uses this exact string; it's the pipeline's terminal state, not a document. | +| Payment history panel | การชำระเงิน | Existing `DealMoneyTimeline` panel title, unchanged. | +| Billing date | วันวางบิล | New, status strip only. | +| Due date | ครบกำหนด | New, status strip only. | +| Next follow-up | ติดตามครั้งถัดไป | New, status strip only. | + +## Component Reuse Map + +| Component | Used on | Behavior differences | +|---|---|---| +| `StatChip` shape (from `DealStateHeader.jsx`) | `DealMoneyStatusStrip` (new) | Pattern copied, not imported — `StatChip` is module-private to `DealStateHeader.jsx`. Same `label`/`value`/`tone` shape, same `

` grid convention. | +| Step-card shape (from `DealDepositPanel.jsx`: `StepNumber`, `StepRoleTag`, bordered card) | `DealDocumentPipeline` (new) | Pattern copied, not imported — both are module-private. `DealDocumentPipeline` has exactly 4 fixed steps (no accordion/collapse, unlike `DealStageStepper`). | +| `RemainingInvoiceDialog` | Remaining-invoice summary card (new) | Reused via the SAME external-boolean-state pattern already used elsewhere in `TicketDetailPage.jsx` (`remainingInvoiceDialogOpen`) — no changes to the dialog itself, just a second entry point into it. | +| `sections.payment` / `sections.depositNotice` (`salesViewScope.js`) | `DealMoneyStatusStrip`, `DealDocumentPipeline` | Reused as the existing gating vocabulary — no new section flag added. The ใบวางบิล step's link-vs-text-only behavior is a per-user check (`account`/`ceo`/`user.canIssueBillingNote`), not a new section flag, since it's about ONE step's affordance, not the whole tab's visibility. | +| `DealDepositPanel`, `DealMoneyTimeline` | Money tab | Unchanged components, only their position/surrounding content changes. | + +## Content Growth Plan + +Unchanged growth patterns: the pipeline's per-step status recomputes from live data on every render +(no accumulation of its own); `DealMoneyTimeline` remains the unbounded append-only log, already +handling growth via its existing chronological list. diff --git a/docs/api/response-fields.json b/docs/api/response-fields.json index 06be2f922..29cb0ff0c 100644 --- a/docs/api/response-fields.json +++ b/docs/api/response-fields.json @@ -1,7 +1,7 @@ { "$comment" : "GENERATED by ResponseFieldContractTest — do not hand-edit. Regenerate: cd backend && ./mvnw -Dtest=ResponseFieldContractTest -Dresponse.fields.update=true test", "source" : "Schema.field pairs from springdoc /v3/api-docs response schemas, kept where the field name is read in frontend/src (excluding mockApi, hrApi, routes and tests)", - "count" : 1283, + "count" : 1287, "fields" : [ "ActivityLogEntryDto.durationMs", "ActivityLogEntryDto.employeeCode", @@ -95,6 +95,8 @@ "BillingNoteCandidateDto.docDate", "BillingNoteCandidateDto.docNumber", "BillingNoteCandidateDto.dueDate", + "BillingNoteCandidateDto.sourceId", + "BillingNoteCandidateDto.sourceType", "BillingNoteCandidateDto.ticketId", "BillingNoteCandidatesDto.customerId", "BillingNoteCandidatesDto.customerName", @@ -114,6 +116,8 @@ "BillingNoteLineDto.docDate", "BillingNoteLineDto.docNumber", "BillingNoteLineDto.dueDate", + "BillingNoteLineDto.sourceId", + "BillingNoteLineDto.sourceType", "BillingNoteLineDto.ticketId", "CatalogDto.sqmPerPiece", "CommissionMonthlySummaryDto.belowFloor", diff --git a/docs/api/ui-reachable.json b/docs/api/ui-reachable.json index 421c1d107..876ab6058 100644 --- a/docs/api/ui-reachable.json +++ b/docs/api/ui-reachable.json @@ -1,7 +1,7 @@ { "$comment": "GENERATED by src/api/serverContract.test.js — do not hand-edit. Regenerate: cd frontend && UI_REACHABLE_UPDATE=1 npx vitest run src/api/serverContract.test.js", "source": "endpoints reachable from a component call site (api..( outside src/api/ and outside tests), resolved through hrApi by driving every method", - "count": 297, + "count": 298, "operations": [ "DELETE /api/attachments/{id}", "DELETE /api/catalog/prices/{priceId}", @@ -34,6 +34,7 @@ "GET /api/customer-quotations/{id}/file", "GET /api/customer-quotations/{quotationId}/discount-approvals", "GET /api/customers", + "GET /api/customers/{customerId}/billing-notes", "GET /api/customers/{customerId}/contacts", "GET /api/customers/{customerId}/projects", "GET /api/dashboard/summary", diff --git a/frontend/src/api/queryKeys.js b/frontend/src/api/queryKeys.js index 5b3b7a45d..af81be6d3 100644 --- a/frontend/src/api/queryKeys.js +++ b/frontend/src/api/queryKeys.js @@ -145,6 +145,10 @@ export const queryKeys = { // for one deal. Separate from remainingInvoiceOptions above, which stays the stateless prefill // source a new draft snapshots from. storedRemainingInvoices: (ticketId) => ['remainingInvoice', 'stored', ticketId], + // The STORED billing note aggregate (V189, GLA-99 step 3), customer-scoped — GLA-129's money-tab + // document pipeline joins this against a deal's own remaining invoice to resolve the ใบวางบิล + // step's status. + billingNotesForCustomer: (customerId) => ['billingNotes', 'customer', customerId ?? ''], customersSearch: (q) => ['customers', 'search', q ?? ''], // One customer MASTER row by id (quotation editor, owner 2026-09-11). There is no GET // /api/customers/{id}, so this is resolved through the name search and matched on id — see diff --git a/frontend/src/api/serverContract.test.js b/frontend/src/api/serverContract.test.js index a8e217752..79aa0d436 100644 --- a/frontend/src/api/serverContract.test.js +++ b/frontend/src/api/serverContract.test.js @@ -539,17 +539,20 @@ describe('controller surface / hrApi.js contract', () => { */ const UNREACHABLE_FROM_UI = new Set([ // GLA-99 step 3 (ใบวางบิล / billing note, V189): the STORED backend + renderer + hrApi/mockApi - // wiring landed on this branch (GLA-129 step 4 part 1); the /finance ใบวางบิล tab and the - // redesigned deal-page การเงิน tab that actually CALL these twelve methods are step 4 parts - // 2/3, separate branches not yet merged. BillingNoteService/BillingNoteController/ + // wiring landed on GLA-129 step 4 part 1; the /finance ใบวางบิล tab that calls the rest of these + // eleven methods is step 4 part 3, a separate branch not yet merged. `GET + // /api/customers/{}/billing-notes` (listForCustomer) left this list on part 2 — the redesigned + // deal-page การเงิน tab's DealDocumentPipeline now reads it to resolve the ใบวางบิล step's status + // (a client-side join against the deal's own remaining invoice, no new endpoint). Cancel/issue/ + // mark-received/mark-settled/revise and the create/update/delete/candidates/file routes are all + // still write-side or picker-only, with no screen yet. BillingNoteService/BillingNoteController/ // BillingNoteRenderer are already covered by BillingNoteServiceIntegrationTest (real Postgres, // including the wrong-way-round authz suite) and BillingNoteRendererTest. Remove this block once - // those screens call these through hrApi.js. + // those screens call the rest through hrApi.js. 'DELETE /api/billing-notes/{}', 'GET /api/billing-notes/{}', 'GET /api/billing-notes/{}/file', 'GET /api/customers/{}/billing-note-candidates', - 'GET /api/customers/{}/billing-notes', 'POST /api/billing-notes/{}/cancel', 'POST /api/billing-notes/{}/issue', 'POST /api/billing-notes/{}/mark-received', diff --git a/frontend/src/features/tickets/DealDocumentPipeline.jsx b/frontend/src/features/tickets/DealDocumentPipeline.jsx new file mode 100644 index 000000000..2c40e2ea6 --- /dev/null +++ b/frontend/src/features/tickets/DealDocumentPipeline.jsx @@ -0,0 +1,163 @@ +import { useQuery } from '@tanstack/react-query'; +import { Link } from 'react-router-dom'; +import { api } from '../../api/index.js'; +import { queryKeys } from '../../api/queryKeys.js'; +import { hasPermission } from '../../app/permissions.js'; +import { StatusBadge } from '../../components/common/StatusBadge.jsx'; +import { depositStatus } from './dealMoneyStatus.js'; + +// Ranks a billing note's "live" statuses for the step 3 join below — see that call site's own +// comment. Module-level (not rebuilt every render, unlike its two sibling constants right there). +const CLAIMING_NOTE_RANK = { SETTLED: 3, ISSUED: 2, DRAFT: 1 }; + +// Step-card shape copied from DealDepositPanel's own StepNumber/card wrapper (module-private +// there too) — see this feature's INFORMATION_ARCHITECTURE.md Component Reuse Map. +function StepNumber({ no }) { + return ( + + {no} + + ); +} + +function PipelineStep({ no, title, status, tone, detail, current, action }) { + return ( +
+
+
+ + {title} + {current ? ( + ขั้นตอนถัดไป + ) : null} +
+ {status} +
+ {detail ?

{detail}

: null} + {action} +
+ ); +} + +/** + * GLA-129: a read-only map of the four money documents in sequence — ใบแจ้งมัดจำ → ใบแจ้งหนี้ + * ส่วนที่เหลือ → ใบวางบิล → รับชำระครบ — with the next actionable step highlighted. This does not + * itself carry any document's controls (those stay on `DealDepositPanel` and the remaining-invoice + * card below it in the money tab); it only tells the viewer where to look next. + * + * The ใบวางบิล step needs a customer-scoped billing-note read + * (`api.billingNotes.listForCustomer`) to know whether this deal's remaining invoice has been + * billed — joined client-side against the deal's own ISSUED remaining invoice, matched by + * `sourceType === 'REMAINING_INVOICE' && sourceId`. No new backend endpoint: both reads already + * exist (GLA-129 step 4 part 1). + */ +export function DealDocumentPipeline({ ticketId, summary, user }) { + const remainingInvoicesQuery = useQuery({ + queryKey: queryKeys.storedRemainingInvoices(ticketId), + queryFn: () => api.storedRemainingInvoices.listForTicket(ticketId).then((r) => r.remainingInvoices ?? []), + enabled: !!ticketId, + }); + const remainingInvoices = remainingInvoicesQuery.data ?? []; + const issuedRemainingInvoice = remainingInvoices.find((ri) => ri.status === 'ISSUED') ?? null; + + // account/sales_manager/ceo read unconditionally; a `sales` caller is filtered server-side to + // notes referencing a deal they own (never 403s outright) — see BillingNoteService#list. Safe to + // call for every role that can render this pipeline at all (import/hr/employee never reach the + // money tab — ticketDetailTabs.js's own gate). + // Gated on `issuedRemainingInvoice` too (review round 2, 2026-09-23), not just customerId — this + // read's own findByCustomer runs BillingNoteRepository's settlement reconcile as its first + // statement, so calling it before step 2 has even produced a document to bill would trigger a + // customer-wide settlement recompute on every money-tab open for zero possible effect on this + // render (step 3 reads "รอใบแจ้งหนี้ส่วนที่เหลือ" whenever `issuedRemainingInvoice` is null, + // regardless of what this query would return). + const billingNotesQuery = useQuery({ + queryKey: queryKeys.billingNotesForCustomer(summary.customerId), + queryFn: () => api.billingNotes.listForCustomer(summary.customerId).then((r) => r.billingNotes ?? []), + enabled: !!summary.customerId && !!issuedRemainingInvoice, + }); + const billingNotes = billingNotesQuery.data ?? []; + + // Review round 1 (2026-09-23) caught two real gaps here: + // 1. SETTLED was missing from the matched statuses — a billing note's own lifecycle is DRAFT -> + // ISSUED -> {SUPERSEDED | CANCELLED | SETTLED}, and BillingNoteRepository#findByCustomer runs + // the settlement reconcile BEFORE every read (including this one), so a deal that just + // finished paying would self-trigger its own note into SETTLED on the very read this pipeline + // performs — silently regressing step 3 back to "ยังไม่ได้วางบิล" at the exact moment billing + // completed. + // 2. A plain .find() picked whichever status happened to be first in findByCustomer's own ORDER + // BY COALESCE(base_number, ''), type, version — which puts a not-yet-numbered correction DRAFT + // BEFORE its still-live ISSUED/SETTLED predecessor. Ranked explicitly instead (CLAIMING_NOTE_RANK + // above), so a live correction-in-progress never hides a billed deal's real status. + const claimingNote = issuedRemainingInvoice + ? billingNotes + .filter((note) => CLAIMING_NOTE_RANK[note.status] + && note.lines?.some((line) => line.sourceType === 'REMAINING_INVOICE' && line.sourceId === issuedRemainingInvoice.id)) + .sort((a, b) => CLAIMING_NOTE_RANK[b.status] - CLAIMING_NOTE_RANK[a.status])[0] ?? null + : null; + + // Step 1: ใบแจ้งมัดจำ + const { bypassed: depositBypassedByPolicy, paid: depositPaid, noticeIssued: depositNoticeIssued } = depositStatus(summary); + let step1 = { status: 'ยังไม่ออกใบแจ้ง', tone: 'neutral' }; + if (depositBypassedByPolicy) step1 = { status: 'ไม่ต้องมัดจำ', tone: 'neutral', done: true }; + else if (depositPaid) step1 = { status: 'ชำระแล้ว', tone: 'success', done: true }; + else if (depositNoticeIssued) step1 = { status: 'รอชำระ', tone: 'warning' }; + const step1Done = step1.done ?? false; + + // Step 2: ใบแจ้งหนี้ส่วนที่เหลือ + const draftRemainingInvoice = remainingInvoices.find((ri) => ri.status === 'DRAFT') ?? null; + let step2 = { status: 'รอขั้นตอนก่อนหน้า', tone: 'neutral' }; + if (remainingInvoicesQuery.isLoading) step2 = { status: 'กำลังโหลด…', tone: 'neutral' }; + else if (issuedRemainingInvoice) step2 = { status: `ออกแล้ว (${issuedRemainingInvoice.docNumber})`, tone: 'success', done: true }; + else if (draftRemainingInvoice) step2 = { status: 'อยู่ระหว่างจัดทำ (ร่าง)', tone: 'warning' }; + else if (step1Done) step2 = { status: 'ยังไม่ออก', tone: 'warning' }; + const step2Done = step2.done ?? false; + + // Step 3: ใบวางบิล — see this component's own doc comment above for the client-side join. + // Review round 2 (2026-09-23): deliberately NOT OR'd with canIssueBillingNote yet, even though + // that grant is the eventual audience per the design brief. permissions.js's own + // isBillingNoteReleaseUser is NOT wired into /finance's PATH_GUARDS until part 3 merges — and + // unlike the grant-only holders in today's seed (all role `employee`, which never reaches this + // tab), a grant CAN be issued to a role that DOES reach the money tab (sales_manager, sales — the + // grant has no role restriction, hr.employee.can_issue_billing_note is a plain boolean). Widening + // this before part 3 lands would show a real, reachable user a link to a route that refuses them + // today. Widen this in the SAME change that wires isBillingNoteReleaseUser into PATH_GUARDS. + const canSeeFinance = hasPermission(user?.role, 'canConfirmPayments'); + let step3 = { status: 'รอใบแจ้งหนี้ส่วนที่เหลือ', tone: 'neutral' }; + if (issuedRemainingInvoice && billingNotesQuery.isLoading) step3 = { status: 'กำลังโหลด…', tone: 'neutral' }; + else if (claimingNote?.status === 'SETTLED') step3 = { status: `ชำระแล้ว (${claimingNote.docNumber})`, tone: 'success', done: true }; + else if (claimingNote?.status === 'ISSUED') step3 = { status: `วางบิลแล้ว (${claimingNote.docNumber})`, tone: 'success', done: true }; + else if (claimingNote?.status === 'DRAFT') step3 = { status: 'อยู่ระหว่างจัดทำ (ร่าง)', tone: 'warning' }; + else if (issuedRemainingInvoice) step3 = { status: 'ยังไม่ได้วางบิล', tone: 'warning' }; + const step3Done = step3.done ?? false; + const step3Action = canSeeFinance && issuedRemainingInvoice + ? ไปที่งานการเงิน + : null; + + // Step 4: รับชำระครบ + const step4Done = summary.paymentStatus === 'FULLY_PAID'; + const step4 = step4Done + ? { status: 'ชำระครบแล้ว', tone: 'success' } + : { status: 'รอชำระให้ครบ', tone: 'neutral' }; + + const doneFlags = [step1Done, step2Done, step3Done, step4Done]; + // step4Done is derived purely from `summary` (no query), so it reads its final value on the + // FIRST render while the two queries above are still loading — without this guard, doneFlags + // would briefly read e.g. [true, false, false, true] before data arrives (step 2/3's own + // "กำลังโหลด…" branches carry no `done` flag), landing the marker on a step for one paint that + // is not actually where the next action is. Suppressed entirely while either query is loading, + // rather than showing a marker that would immediately jump once data settles. + const stepsLoading = remainingInvoicesQuery.isLoading || (!!summary.customerId && billingNotesQuery.isLoading); + const currentIndex = stepsLoading ? -1 : doneFlags.indexOf(false); // first not-yet-done step, or -1 if all done + + return ( +
+ + + + +
+ ); +} diff --git a/frontend/src/features/tickets/DealDocumentPipeline.test.jsx b/frontend/src/features/tickets/DealDocumentPipeline.test.jsx new file mode 100644 index 000000000..7ef12908f --- /dev/null +++ b/frontend/src/features/tickets/DealDocumentPipeline.test.jsx @@ -0,0 +1,304 @@ +import React from 'react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { MemoryRouter } from 'react-router-dom'; +import { act, render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { DealDocumentPipeline } from './DealDocumentPipeline.jsx'; +import { api } from '../../api/index.js'; + +globalThis.React = React; + +vi.mock('../../api/index.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + api: { + storedRemainingInvoices: { listForTicket: vi.fn() }, + billingNotes: { listForCustomer: vi.fn() }, + }, + }; +}); + +function baseSummary(overrides = {}) { + return { + customerId: 6, + depositPolicy: 'REQUIRED', + paymentStatus: null, + ...overrides, + }; +} + +function riRow(overrides = {}) { + return { id: 900, status: 'ISSUED', docNumber: 'GLR6900001-1', ...overrides }; +} + +function noteRow(overrides = {}) { + return { + id: 1, status: 'DRAFT', docNumber: null, + lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: 900 }], + ...overrides, + }; +} + +// The pipeline always renders a for the ใบวางบิล step's action slot when eligible, so +// every render needs router context — wrapping unconditionally here rather than per-test avoids +// the crash-and-leak failure mode a bare outside a Router produces (an uncaught exception +// that can pollute a LATER test's DOM if cleanup doesn't run cleanly). +function renderPipeline({ + summary = baseSummary(), + user = { role: 'sales' }, + remainingInvoices = [], + billingNotes = [], +} = {}) { + api.storedRemainingInvoices.listForTicket.mockResolvedValue({ remainingInvoices }); + api.billingNotes.listForCustomer.mockResolvedValue({ billingNotes }); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + + + , + ); +} + +describe('DealDocumentPipeline — step 1 ใบแจ้งมัดจำ', () => { + it('reads "ไม่ต้องมัดจำ" when the deposit policy bypasses the notice', async () => { + renderPipeline({ summary: baseSummary({ depositPolicy: 'WAIVED' }) }); + expect(await screen.findByText('ไม่ต้องมัดจำ')).toBeTruthy(); + }); + + it('reads "ยังไม่ออกใบแจ้ง" before any deposit notice exists', async () => { + renderPipeline(); + expect(await screen.findByText('ยังไม่ออกใบแจ้ง')).toBeTruthy(); + }); + + it('reads "รอชำระ" once the deposit notice is issued but unpaid', async () => { + renderPipeline({ summary: baseSummary({ paymentStatus: 'DEPOSIT_NOTICE_ISSUED' }) }); + expect(await screen.findByText('รอชำระ')).toBeTruthy(); + }); + + it('reads "ชำระแล้ว" once the deposit is paid', async () => { + renderPipeline({ summary: baseSummary({ paymentStatus: 'DEPOSIT_PAID' }) }); + expect(await screen.findByText('ชำระแล้ว')).toBeTruthy(); + }); +}); + +describe('DealDocumentPipeline — step 2 ใบแจ้งหนี้ส่วนที่เหลือ (blocked until step 1 is done)', () => { + it('reads "รอขั้นตอนก่อนหน้า" while the deposit itself is still outstanding', async () => { + renderPipeline({ summary: baseSummary({ paymentStatus: 'DEPOSIT_NOTICE_ISSUED' }) }); + expect(await screen.findByText('รอขั้นตอนก่อนหน้า')).toBeTruthy(); + }); + + it('reads "ยังไม่ออก" once step 1 is done but no remaining invoice exists yet', async () => { + renderPipeline({ summary: baseSummary({ paymentStatus: 'DEPOSIT_PAID' }) }); + expect(await screen.findByText('ยังไม่ออก')).toBeTruthy(); + }); + + it('shows the DRAFT remaining invoice as "อยู่ระหว่างจัดทำ (ร่าง)"', async () => { + renderPipeline({ + summary: baseSummary({ paymentStatus: 'DEPOSIT_PAID' }), + remainingInvoices: [riRow({ status: 'DRAFT', docNumber: null })], + }); + expect(await screen.findByText('อยู่ระหว่างจัดทำ (ร่าง)')).toBeTruthy(); + }); + + it('shows the ISSUED remaining invoice by its own docNumber', async () => { + renderPipeline({ + summary: baseSummary({ paymentStatus: 'DEPOSIT_PAID' }), + remainingInvoices: [riRow()], + }); + expect(await screen.findByText('ออกแล้ว (GLR6900001-1)')).toBeTruthy(); + }); +}); + +describe('DealDocumentPipeline — step 3 ใบวางบิล (client-side join against the customer-scoped list)', () => { + it('reads "รอใบแจ้งหนี้ส่วนที่เหลือ" when no remaining invoice has been issued', async () => { + renderPipeline(); + expect(await screen.findByText('รอใบแจ้งหนี้ส่วนที่เหลือ')).toBeTruthy(); + }); + + it('reads "ยังไม่ได้วางบิล" once the remaining invoice is issued but no note claims it', async () => { + renderPipeline({ remainingInvoices: [riRow()] }); + expect(await screen.findByText('ยังไม่ได้วางบิล')).toBeTruthy(); + }); + + it('matches a claiming note by sourceType+sourceId, not just by existing at all', async () => { + // A note that references a DIFFERENT source must not be read as claiming this deal's RI. + renderPipeline({ + remainingInvoices: [riRow()], + billingNotes: [noteRow({ lines: [{ sourceType: 'REMAINING_INVOICE', sourceId: 999 }] })], + }); + expect(await screen.findByText('ยังไม่ได้วางบิล')).toBeTruthy(); + }); + + it('reads "อยู่ระหว่างจัดทำ (ร่าง)" when the claiming note is a DRAFT', async () => { + renderPipeline({ remainingInvoices: [riRow()], billingNotes: [noteRow({ status: 'DRAFT' })] }); + expect(await screen.findByText('อยู่ระหว่างจัดทำ (ร่าง)')).toBeTruthy(); + }); + + it('reads "วางบิลแล้ว ()" when the claiming note is ISSUED', async () => { + renderPipeline({ + remainingInvoices: [riRow()], + billingNotes: [noteRow({ status: 'ISSUED', docNumber: 'GLR6900002-1' })], + }); + expect(await screen.findByText('วางบิลแล้ว (GLR6900002-1)')).toBeTruthy(); + }); + + // Role-aware link — the point of this whole step's "role-aware actions" requirement. + it('shows the ไปที่งานการเงิน link for account once a remaining invoice is issued, never for a plain sales rep', async () => { + const { unmount } = renderPipeline({ remainingInvoices: [riRow()], user: { role: 'sales' } }); + await screen.findByText('ยังไม่ได้วางบิล'); + expect(screen.queryByText('ไปที่งานการเงิน')).toBeNull(); + unmount(); + + renderPipeline({ remainingInvoices: [riRow()], user: { role: 'account' } }); + expect(await screen.findByText('ไปที่งานการเงิน')).toBeTruthy(); + }); + + // Review round 2 (2026-09-23): deliberately NOT widened to canIssueBillingNote yet — see + // DealDocumentPipeline.jsx's own comment on canSeeFinance. A can_issue_billing_note grant has no + // role restriction (hr.employee.can_issue_billing_note is a plain boolean), so a REAL, reachable + // role (sales_manager, unlike the seed's only current grant holder, employee — which never + // reaches this tab at all) could hold it. Showing the link to that role before part 3 wires + // isBillingNoteReleaseUser into /finance's own PATH_GUARDS would be a link a real user could + // click and get refused — this pins that it does NOT happen. + it('does not show the link for a can_issue_billing_note grant holder outside canConfirmPayments (part 3 not merged yet)', async () => { + renderPipeline({ + remainingInvoices: [riRow()], + user: { role: 'sales_manager', canIssueBillingNote: true }, + }); + await screen.findByText('ยังไม่ได้วางบิล'); + expect(screen.queryByText('ไปที่งานการเงิน')).toBeNull(); + }); + + it('never shows the link before a remaining invoice is issued, even for account', async () => { + renderPipeline({ user: { role: 'account' } }); + await screen.findByText('รอใบแจ้งหนี้ส่วนที่เหลือ'); + expect(screen.queryByText('ไปที่งานการเงิน')).toBeNull(); + }); + + // Review round 1 (2026-09-23): a source's sourceId is not unique ACROSS types — REMAINING_INVOICE + // and DEPOSIT_NOTICE are independent sequences, so id 900 can legitimately belong to each. Without + // the sourceType half of the match, this would false-positive as "billed". + it('does not match a line whose sourceId coincides but whose sourceType differs', async () => { + renderPipeline({ + remainingInvoices: [riRow()], + billingNotes: [noteRow({ lines: [{ sourceType: 'DEPOSIT_NOTICE', sourceId: 900 }] })], + }); + expect(await screen.findByText('ยังไม่ได้วางบิล')).toBeTruthy(); + }); + + // Review round 1: the billing note lifecycle has a FOURTH terminal status this step's match used + // to miss — DRAFT -> ISSUED -> {SUPERSEDED | CANCELLED | SETTLED}. BillingNoteRepository + // .findByCustomer runs the settlement reconcile before every read (including this one), so a + // deal reaching full payment can self-trigger its own note into SETTLED on the very read this + // component performs — regressing step 3 back to "not billed" at the exact moment billing + // completed, unless SETTLED gets its own branch. + it('reads "ชำระแล้ว ()" when the claiming note has settled', async () => { + renderPipeline({ + remainingInvoices: [riRow()], + billingNotes: [noteRow({ status: 'SETTLED', docNumber: 'GLR6900005-1' })], + }); + expect(await screen.findByText('ชำระแล้ว (GLR6900005-1)')).toBeTruthy(); + }); + + // Review round 1: revise() leaves the predecessor ISSUED/SETTLED (only its per-line note_status, + // never serialized, flips) while the new correction DRAFT copies the same sourceId — and + // findByCustomer's own ORDER BY COALESCE(base_number, ''), type, version puts the not-yet-numbered + // DRAFT BEFORE its predecessor. A plain array .find() would therefore read a billed, paid deal as + // "still a draft" while a correction is merely open. The live/settled note must win regardless of + // array order. + it('prefers an ISSUED claiming note over a DRAFT one for the same source, regardless of array order', async () => { + renderPipeline({ + remainingInvoices: [riRow()], + billingNotes: [ + noteRow({ id: 2, status: 'DRAFT', docNumber: null }), // the correction draft, listed FIRST + noteRow({ id: 1, status: 'ISSUED', docNumber: 'GLR6900006-1' }), // the still-live predecessor + ], + }); + expect(await screen.findByText('วางบิลแล้ว (GLR6900006-1)')).toBeTruthy(); + expect(screen.queryByText('อยู่ระหว่างจัดทำ (ร่าง)')).toBeNull(); + }); +}); + +describe('DealDocumentPipeline — step 4 รับชำระครบ', () => { + it('reads "รอชำระให้ครบ" before FULLY_PAID', async () => { + renderPipeline({ summary: baseSummary({ paymentStatus: 'AWAITING_FINAL_PAYMENT' }) }); + expect(await screen.findByText('รอชำระให้ครบ')).toBeTruthy(); + }); + + it('reads "ชำระครบแล้ว" once paymentStatus is FULLY_PAID', async () => { + renderPipeline({ summary: baseSummary({ paymentStatus: 'FULLY_PAID' }) }); + expect(await screen.findByText('ชำระครบแล้ว')).toBeTruthy(); + }); +}); + +describe('DealDocumentPipeline — next-step highlighting', () => { + it('marks step 1 as the current step on a brand-new deal', async () => { + renderPipeline(); + await screen.findByText('ขั้นตอนถัดไป'); + expect(screen.getAllByText('ขั้นตอนถัดไป')).toHaveLength(1); + }); + + it('advances the marker to step 4 once everything else is done', async () => { + renderPipeline({ + summary: baseSummary({ paymentStatus: 'AWAITING_FINAL_PAYMENT' }), + remainingInvoices: [riRow()], + billingNotes: [noteRow({ status: 'ISSUED', docNumber: 'GLR6900003-1' })], + }); + await screen.findByText('วางบิลแล้ว (GLR6900003-1)'); + const marker = screen.getByText('ขั้นตอนถัดไป'); + const card = marker.closest('div.rounded-md'); + expect(card?.textContent).toContain('รับชำระครบ'); + }); + + it('shows no current-step marker once every step is done', async () => { + renderPipeline({ + summary: baseSummary({ paymentStatus: 'FULLY_PAID' }), + remainingInvoices: [riRow()], + billingNotes: [noteRow({ status: 'ISSUED', docNumber: 'GLR6900004-1' })], + }); + // Wait for step 3's own final text (not just step 4's, which is derived purely from `summary` + // and reads its final value before either query resolves) so this assertion runs against + // settled data, not the loading-state render `stepsLoading` deliberately suppresses the marker + // for. + await screen.findByText('วางบิลแล้ว (GLR6900004-1)'); + expect(screen.queryByText('ขั้นตอนถัดไป')).toBeNull(); + }); + + // Review round 1 (2026-09-23): step4Done is derived purely from `summary`, with no query + // dependency, so it reads its FINAL value on the very first render while both queries are still + // in flight. Without the stepsLoading guard, doneFlags would briefly read + // [true, false, false, true] (step 2's own loading-state branch carries no `done` flag), landing + // the marker on step 2 for one paint before the data-driven tests elsewhere in this file ever get + // a chance to observe it — those tests all `await` a query-derived text FIRST, which is exactly + // why they cannot catch this on their own. This test controls the query's resolution explicitly + // to inspect the in-flight render directly. + it('shows no premature marker while queries are still loading, even on a FULLY_PAID deal', async () => { + // billingNotesQuery is gated on `issuedRemainingInvoice` (review round 2), so with + // remainingInvoices resolving to [] it never even enables — no need to mock/resolve it here. + let resolveRemainingInvoices; + api.storedRemainingInvoices.listForTicket.mockReturnValue( + new Promise((resolve) => { resolveRemainingInvoices = resolve; }), + ); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + + + , + ); + // step 4's final text is already on screen (it needs no query), proving this assertion runs + // during the exact window the guard protects, not before the component has rendered at all. + await screen.findByText('ชำระครบแล้ว'); + expect(screen.queryByText('ขั้นตอนถัดไป')).toBeNull(); + // Resolve and let the pending update flush inside act() before the test ends, so the state + // update this triggers cannot land after teardown and warn. + await act(async () => { + resolveRemainingInvoices({ remainingInvoices: [] }); + await Promise.resolve(); + }); + }); +}); diff --git a/frontend/src/features/tickets/DealMoneyStatusStrip.jsx b/frontend/src/features/tickets/DealMoneyStatusStrip.jsx new file mode 100644 index 000000000..911fd1884 --- /dev/null +++ b/frontend/src/features/tickets/DealMoneyStatusStrip.jsx @@ -0,0 +1,60 @@ +import { StatusBadge } from '../../components/common/StatusBadge.jsx'; +import { formatThaiDate } from '../../utils/format.js'; +import { depositStatus } from './dealMoneyStatus.js'; + +// Review round 1 (2026-09-23): this used to check paymentStatus alone, which is wrong for any +// bypass-policy deal (CREDIT_CUSTOMER/WAIVED/NOT_REQUIRED) — TicketService#recordPayment skips +// DEPOSIT_PAID entirely for those, advancing straight to AWAITING_FINAL_PAYMENT, so a customer who +// simply paid their invoice showed as "มัดจำ: ชำระแล้ว" though no deposit was ever requested. The +// bypass check must run FIRST, same priority DealDepositPanel's own skipsNotice/alreadyPaid chain +// uses — see dealMoneyStatus.js, now shared with DealDocumentPipeline's identical step 1 so the +// two cannot diverge again. +function depositChip(summary) { + const { bypassed, paid, noticeIssued } = depositStatus(summary); + if (bypassed) return { label: 'มัดจำ', value: 'ไม่ต้องมัดจำ', tone: 'neutral' }; + if (paid) return { label: 'มัดจำ', value: 'ชำระแล้ว', tone: 'success' }; + if (noticeIssued) return { label: 'มัดจำ', value: 'รอชำระ', tone: 'warning' }; + return { label: 'มัดจำ', value: 'ยังไม่แจ้ง', tone: 'neutral' }; +} + +// Same compact chip shape as DealStateHeader.jsx's own (module-private) StatChip — copied, not +// imported, since that component isn't exported; see this feature's own +// .design/gla-129-money-tab/INFORMATION_ARCHITECTURE.md Component Reuse Map. +function StatChip({ label, value, tone }) { + return ( +
+
{label}
+
+ {tone ? ( + {value} + ) : ( + {value} + )} +
+
+ ); +} + +/** + * GLA-129: an at-a-glance strip atop the money tab's document pipeline — วันวางบิล/ครบกำหนด/ + * ติดตามครั้งถัดไป mirror the "วันสำคัญ" ContextSection already on the deal tab (same `summary` + * fields, same `formatThaiDate`), repeated here so a user reading money-tab status doesn't have + * to switch tabs to see the dates it depends on. มัดจำ is new to this strip specifically — the + * document pipeline below covers the SAME lifecycle step in more detail; this is the one-glance + * version. + */ +export function DealMoneyStatusStrip({ summary }) { + const chips = [ + { label: 'วันวางบิล', value: formatThaiDate(summary.billingDate) }, + { label: 'ครบกำหนดชำระ', value: formatThaiDate(summary.dueDate), tone: summary.overdue ? 'danger' : undefined }, + { label: 'ติดตามครั้งถัดไป', value: formatThaiDate(summary.nextFollowUpAt) }, + depositChip(summary), + ]; + return ( +
+ {chips.map((chip) => ( + + ))} +
+ ); +} diff --git a/frontend/src/features/tickets/DealMoneyStatusStrip.test.jsx b/frontend/src/features/tickets/DealMoneyStatusStrip.test.jsx new file mode 100644 index 000000000..03283dd7e --- /dev/null +++ b/frontend/src/features/tickets/DealMoneyStatusStrip.test.jsx @@ -0,0 +1,74 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { DealMoneyStatusStrip } from './DealMoneyStatusStrip.jsx'; + +globalThis.React = React; + +function baseSummary(overrides = {}) { + return { + billingDate: null, + dueDate: null, + nextFollowUpAt: null, + overdue: false, + paymentStatus: null, + depositPolicy: 'REQUIRED', + ...overrides, + }; +} + +describe('DealMoneyStatusStrip', () => { + it('renders "-" for every unset date field', () => { + render(); + // formatThaiDate(null) === '-', and three date chips are unset here. + expect(screen.getAllByText('-')).toHaveLength(3); + }); + + it('shows มัดจำ as "ยังไม่แจ้ง" when no deposit notice has been issued', () => { + render(); + expect(screen.getByText('ยังไม่แจ้ง')).toBeTruthy(); + }); + + it('shows มัดจำ as "รอชำระ" once the deposit notice is issued but unpaid', () => { + render(); + expect(screen.getByText('รอชำระ')).toBeTruthy(); + }); + + // Wrong-way-round-adjacent: all three "already paid" statuses must read the same, not just + // the literal DEPOSIT_PAID one — a regression that narrowed this to one status would silently + // show "รอชำระ" for a deal already past AWAITING_FINAL_PAYMENT/FULLY_PAID. + it.each(['DEPOSIT_PAID', 'AWAITING_FINAL_PAYMENT', 'FULLY_PAID'])( + 'shows มัดจำ as "ชำระแล้ว" for paymentStatus %s', + (paymentStatus) => { + render(); + expect(screen.getByText('ชำระแล้ว')).toBeTruthy(); + }, + ); + + // Review round 1 (2026-09-23): the bug this pins — a bypass-policy deal (TicketService + // #recordPayment skips DEPOSIT_PAID entirely for these, advancing straight to + // AWAITING_FINAL_PAYMENT) used to read มัดจำ from paymentStatus alone, so a credit customer who + // simply paid their invoice showed "มัดจำ: ชำระแล้ว" though no deposit was ever requested or + // paid. The bypass check must outrank the paid-statuses check. + it.each(['NOT_REQUIRED', 'WAIVED', 'CREDIT_CUSTOMER'])( + 'shows มัดจำ as "ไม่ต้องมัดจำ" for a %s deal even once paymentStatus reaches AWAITING_FINAL_PAYMENT', + (depositPolicy) => { + render(); + expect(screen.getByText('ไม่ต้องมัดจำ')).toBeTruthy(); + expect(screen.queryByText('ชำระแล้ว')).toBeNull(); + }, + ); + + it('formats set dates in Thai Buddhist-era form', () => { + render(); + expect(screen.getByText('11 ส.ค. 2569')).toBeTruthy(); + }); + + it('renders ครบกำหนดชำระ as a danger-tone badge when overdue, plain text otherwise', () => { + const { container: onTime } = render(); + expect(onTime.querySelector('.status-danger')).toBeNull(); + + const { container: overdue } = render(); + expect(overdue.querySelector('.status-danger')?.textContent).toBe('11 ส.ค. 2569'); + }); +}); diff --git a/frontend/src/features/tickets/DealRemainingInvoiceCard.jsx b/frontend/src/features/tickets/DealRemainingInvoiceCard.jsx new file mode 100644 index 000000000..2feb68bb7 --- /dev/null +++ b/frontend/src/features/tickets/DealRemainingInvoiceCard.jsx @@ -0,0 +1,60 @@ +import { useQuery } from '@tanstack/react-query'; +import { api } from '../../api/index.js'; +import { queryKeys } from '../../api/queryKeys.js'; +import { Button } from '../../components/common/Button.jsx'; +import { StatusBadge } from '../../components/common/StatusBadge.jsx'; +import { formatThaiDate } from '../../utils/format.js'; + +const STATUS_LABEL = { DRAFT: 'ร่าง', ISSUED: 'ออกแล้ว' }; +const STATUS_TONE = { DRAFT: 'warning', ISSUED: 'success' }; + +/** + * GLA-129: a compact entry point into `RemainingInvoiceDialog` from the money tab — the dialog + * itself already carries the full generate → edit → issue → version-history → download lifecycle + * (GLA-99 step 2), so this card is new UI around an old, unmodified implementation, not a second + * one. `onManage` opens the SAME dialog instance/state TicketDetailPage's own sticky-bar button + * already controls (`remainingInvoiceDialogOpen`) — deliberately not a second independent + * `useState` here, which would risk two `RemainingInvoiceDialog` instances mounted at once if a + * viewer opened it from both entry points. + * + * `canManage` (review round 1, 2026-09-23): the status/docNumber summary is shown to the FULL + * sections.payment audience this card is mounted for (sales/sales_manager/ceo/account) — reading + * it leaks nothing the pipeline above doesn't already state. The "จัดการ..." BUTTON is narrower — + * the caller must pass exactly `can.downloadRemainingInvoice` + * (`isRemainingInvoiceReady(summary) && isSales`), the SAME gate the pre-existing sticky-bar entry + * point uses, so this card cannot offer a control account/ceo/non-sales never had, and cannot offer + * it on a deal that isn't ready yet — which would otherwise contradict the pipeline's own "รอ + * ขั้นตอนก่อนหน้า" reading for the exact same deal a few lines above. + */ +export function DealRemainingInvoiceCard({ ticketId, canManage, onManage }) { + const listQuery = useQuery({ + queryKey: queryKeys.storedRemainingInvoices(ticketId), + queryFn: () => api.storedRemainingInvoices.listForTicket(ticketId).then((r) => r.remainingInvoices ?? []), + enabled: !!ticketId, + }); + const rows = listQuery.data ?? []; + const live = rows.find((r) => r.status === 'ISSUED') ?? rows.find((r) => r.status === 'DRAFT') ?? null; + + return ( +
+
+ ใบแจ้งหนี้ส่วนที่เหลือ + {live ? {STATUS_LABEL[live.status]} : null} +
+ {listQuery.isLoading ? ( +

กำลังโหลด…

+ ) : live ? ( +

+ {live.docNumber ?? '(ร่าง)'} · {formatThaiDate(live.docDate)} +

+ ) : ( +

ยังไม่มีใบแจ้งหนี้ส่วนที่เหลือ

+ )} + {canManage ? ( + + ) : null} +
+ ); +} diff --git a/frontend/src/features/tickets/DealRemainingInvoiceCard.test.jsx b/frontend/src/features/tickets/DealRemainingInvoiceCard.test.jsx new file mode 100644 index 000000000..b1ca7238a --- /dev/null +++ b/frontend/src/features/tickets/DealRemainingInvoiceCard.test.jsx @@ -0,0 +1,77 @@ +import React from 'react'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { DealRemainingInvoiceCard } from './DealRemainingInvoiceCard.jsx'; +import { api } from '../../api/index.js'; + +globalThis.React = React; + +vi.mock('../../api/index.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + api: { storedRemainingInvoices: { listForTicket: vi.fn() } }, + }; +}); + +function renderCard({ remainingInvoices = [], canManage = true, onManage = vi.fn() } = {}) { + api.storedRemainingInvoices.listForTicket.mockResolvedValue({ remainingInvoices }); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + render( + + + , + ); + return { onManage }; +} + +describe('DealRemainingInvoiceCard', () => { + it('shows the empty state when no remaining invoice exists yet', async () => { + renderCard(); + expect(await screen.findByText('ยังไม่มีใบแจ้งหนี้ส่วนที่เหลือ')).toBeTruthy(); + }); + + it('prefers the ISSUED row over a DRAFT one when both exist', async () => { + renderCard({ + remainingInvoices: [ + { id: 1, status: 'DRAFT', docNumber: null, docDate: null }, + { id: 2, status: 'ISSUED', docNumber: 'GLR6900001-1', docDate: '2026-09-01' }, + ], + }); + expect(await screen.findByText('ออกแล้ว')).toBeTruthy(); + expect(screen.getByText((text) => text.includes('GLR6900001-1'))).toBeTruthy(); + }); + + it('falls back to the DRAFT row when no ISSUED row exists', async () => { + renderCard({ remainingInvoices: [{ id: 1, status: 'DRAFT', docNumber: null, docDate: '2026-09-01' }] }); + expect(await screen.findByText('ร่าง')).toBeTruthy(); + }); + + // The single most important wiring fact about this component: it must NOT own its own dialog + // state (which would risk two RemainingInvoiceDialog instances mounted from two entry points on + // the same page) — it only ever calls the caller's onManage. + it('calls onManage when the button is clicked', async () => { + const { onManage } = renderCard(); + await screen.findByText('ยังไม่มีใบแจ้งหนี้ส่วนที่เหลือ'); + fireEvent.click(screen.getByRole('button', { name: 'จัดการใบแจ้งหนี้ส่วนที่เหลือ' })); + expect(onManage).toHaveBeenCalledTimes(1); + }); + + // Review round 1 (2026-09-23): this card's status summary is shown to a wider audience + // (sections.payment: sales/sales_manager/ceo/account) than who may actually manage the + // document (canManage — the caller passes the SAME can.downloadRemainingInvoice gate the + // pre-existing sticky-bar entry point uses). The button must disappear, not just disable, + // when the caller says this viewer/deal-state combination isn't a manage case — otherwise a + // non-sales viewer or a not-yet-ready deal gets a control that contradicts what the pipeline + // right above says ("รอขั้นตอนก่อนหน้า"). + it('hides the button entirely when canManage is false, regardless of document state', async () => { + const { onManage } = renderCard({ + canManage: false, + remainingInvoices: [{ id: 1, status: 'ISSUED', docNumber: 'GLR6900001-1', docDate: '2026-09-01' }], + }); + await screen.findByText('ออกแล้ว'); + expect(screen.queryByRole('button', { name: 'จัดการใบแจ้งหนี้ส่วนที่เหลือ' })).toBeNull(); + expect(onManage).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/features/tickets/TicketDetailPage.jsx b/frontend/src/features/tickets/TicketDetailPage.jsx index 667dd5ae6..b0ea2d0a3 100644 --- a/frontend/src/features/tickets/TicketDetailPage.jsx +++ b/frontend/src/features/tickets/TicketDetailPage.jsx @@ -30,12 +30,15 @@ import { hasActivitySince, isReadyToAdvance, lastStageChangeAt, STAGE_ADVANCE_GA import { ContextSection, FieldRow } from './DealMetaFields.jsx'; import { DealAttachmentsPanel } from './DealAttachmentsPanel.jsx'; import { DealDepositPanel } from './DealDepositPanel.jsx'; +import { DealDocumentPipeline } from './DealDocumentPipeline.jsx'; import { DealDocumentRegister } from './DealDocumentRegister.jsx'; import { RemainingInvoiceDialog } from './RemainingInvoiceDialog.jsx'; import { DealFulfilmentPanel } from './DealFulfilmentPanel.jsx'; import { DealHistoryPanel } from './DealHistoryPanel.jsx'; import { DealLegacyQuotations } from './DealLegacyQuotations.jsx'; +import { DealMoneyStatusStrip } from './DealMoneyStatusStrip.jsx'; import { DealMoneyTimeline } from './DealMoneyTimeline.jsx'; +import { DealRemainingInvoiceCard } from './DealRemainingInvoiceCard.jsx'; import { DealQuotationPanel } from './DealQuotationPanel.jsx'; // Quotation v2 — direct deal quotation (QUOTATION-V2-PLAN.md). Sibling to DealQuotationPanel // above (the PCR-chain's own panel, untouched) -- renders `origin = 'DEAL_DIRECT'` rows, which @@ -2217,18 +2220,18 @@ export function TicketDetailPage({ user, ticketId, onBack, showToast }) { + {/* GLA-129 step 4 part 2: status strip + document pipeline — an at-a-glance read of + ใบแจ้งมัดจำ → ใบแจ้งหนี้ส่วนที่เหลือ → ใบวางบิล → รับชำระครบ, additive above the + existing deposit/remaining-invoice controls and the (unmodified) payment timeline. + Same sections.payment gate DealMoneyTimeline already used, since this is the same + audience reading the same underlying data. */} {sections.payment ? ( - + +
+ + +
+ ) : null} {/* "มัดจำ" (Phase 3 Slice S3 — see @@ -2252,6 +2255,23 @@ export function TicketDetailPage({ user, ticketId, onBack, showToast }) { ) : null} + {/* GLA-129 step 4 part 2: entry point into the existing RemainingInvoiceDialog (GLA-99 + step 2, unmodified) — reuses the SAME dialog-open state the deal-stage sticky bar's + own "ดาวน์โหลดใบแจ้งหนี้ส่วนที่เหลือ" button controls, see + DealRemainingInvoiceCard's own doc comment for why. The card's status summary is + visible to the full sections.payment audience; canManage passes the EXACT SAME gate + (can.downloadRemainingInvoice) the existing sticky-bar button uses, so this card + never offers a control account/ceo/non-sales never had, or on a deal not yet ready. */} + {sections.payment ? ( +
+ setRemainingInvoiceDialogOpen(true)} + /> +
+ ) : null} + {/* Slice C2b: relocated here from the dissolved การดำเนินการอื่น ๆ grab-bag — a close-lifecycle action, not an items action, so it belongs with การเงิน rather than สินค้าและราคา. Gate unchanged @@ -2263,7 +2283,7 @@ export function TicketDetailPage({ user, ticketId, onBack, showToast }) { ticketDetailTabs.js's own comment on this tab's gate), so the action can never end up stranded behind a hidden `money` tab. */} {can.revokeCloseConfirm && ( - +
)} + + {/* GLA-129 step 4 part 2: moved to the END of the tab (was first) — owner ruling "keep + การชำระเงิน timeline as-is" means the COMPONENT stays untouched, not its position; see + .design/gla-129-money-tab/INFORMATION_ARCHITECTURE.md's Content Hierarchy for the + reasoning (the four items above are "what do I do next", this is the historical + record a user checks for detail/verification, not the primary surface). */} + {sections.payment ? ( +
+ +
+ ) : null} diff --git a/frontend/src/features/tickets/TicketDetailPage.test.jsx b/frontend/src/features/tickets/TicketDetailPage.test.jsx index 48742957c..ecf46e6a8 100644 --- a/frontend/src/features/tickets/TicketDetailPage.test.jsx +++ b/frontend/src/features/tickets/TicketDetailPage.test.jsx @@ -121,6 +121,13 @@ vi.mock('../../api/index.js', async (importOriginal) => { deleteDraft: vi.fn(), download: vi.fn(), }, + // The STORED billing note aggregate (V189, GLA-129 step 4 part 2) — DealDocumentPipeline's + // own query is gated on `summary.customerId && issuedRemainingInvoice`, both absent from + // every fixture below, so this is never actually called by the existing tests; mocked so a + // future test that DOES set both doesn't throw on an undefined namespace. + billingNotes: { + listForCustomer: vi.fn().mockResolvedValue({ billingNotes: [] }), + }, // The items table converts a foreign-currency factory price to baht. Mocked because an // unmocked namespace makes every fxRates.list() call throw (api.fxRates is undefined), // which would silently leave the baht line absent rather than exercising it. @@ -3346,6 +3353,43 @@ describe('TicketDetailPage', () => { }); }); + // GLA-129 step 4 part 2 (review round 2, 2026-09-23): a wiring-level test for the money tab's + // new content — nothing previously asserted the "สถานะการเงิน" Panel or DealRemainingInvoiceCard + // actually render inside TicketDetailPage, so dropping or renaming the `canManage` prop at this + // call site would silently fail closed (the button disappears) with nothing here going red. + describe('money tab — GLA-129 status strip / pipeline / remaining-invoice card', () => { + it('renders the status strip and pipeline, and shows the remaining-invoice manage button for a ready deal', async () => { + api.tickets.get.mockResolvedValue({ + ticket: buildTicket({ + summary: { status: 'quotation_issued', fulfillmentStatus: 'GOODS_RECEIVED', createdById: 1 }, + }), + }); + + renderTicketDetailPage(salesOwnerUser); + await openTab(/^การเงิน/); + + expect(await screen.findByText('สถานะการเงิน')).toBeTruthy(); + expect(await screen.findByRole('button', { name: 'จัดการใบแจ้งหนี้ส่วนที่เหลือ' })).toBeTruthy(); + }); + + // Wrong-way-round: the same button must NOT appear once the deal is no longer ready + // (isRemainingInvoiceReady false) — canManage must track can.downloadRemainingInvoice exactly, + // not just "is this a sales owner". + it('hides the remaining-invoice manage button before the deal is ready', async () => { + api.tickets.get.mockResolvedValue({ + ticket: buildTicket({ + summary: { status: 'quotation_issued', fulfillmentStatus: null, createdById: 1 }, + }), + }); + + renderTicketDetailPage(salesOwnerUser); + await openTab(/^การเงิน/); + + await screen.findByText('สถานะการเงิน'); + expect(screen.queryByRole('button', { name: 'จัดการใบแจ้งหนี้ส่วนที่เหลือ' })).toBeNull(); + }); + }); + // ── Remaining-invoice download gate (stock-delivered deals) ───────────────────────────────── // // The gate used to be `fulfillmentStatus === 'GOODS_RECEIVED'`, a value only the import axis diff --git a/frontend/src/features/tickets/dealMoneyStatus.js b/frontend/src/features/tickets/dealMoneyStatus.js new file mode 100644 index 000000000..3ae971ffb --- /dev/null +++ b/frontend/src/features/tickets/dealMoneyStatus.js @@ -0,0 +1,25 @@ +// GLA-129: shared between DealMoneyStatusStrip and DealDocumentPipeline, which both need to answer +// "has this deal's deposit been requested/paid" — mirrors DealDepositPanel's own +// bypassesNotice/alreadyPaid predicates exactly (DealDepositPanel.jsx's `skipsNotice` check +// outranks its own paymentStatus reads, same priority order here). Extracted after review round 1 +// (2026-09-23) found the strip and the pipeline had each written this independently and diverged: +// the strip checked paymentStatus alone, so a CREDIT_CUSTOMER/WAIVED/NOT_REQUIRED deal that later +// reached AWAITING_FINAL_PAYMENT (TicketService#recordPayment skips DEPOSIT_PAID entirely for a +// bypass-policy deal) showed "มัดจำ: ชำระแล้ว" — a false statement, since no deposit was ever +// requested or paid on that deal. One implementation now, so THESE TWO surfaces cannot disagree — +// the same NOT_REQUIRED/WAIVED/CREDIT_CUSTOMER triple is still independently written out in +// DealDepositPanel.jsx, DealMoneyTimeline.jsx, accountActions.js and mockApi.js (pre-existing, +// out of scope here); consolidating those is a separate cleanup, not claimed by this change. +export const DEPOSIT_PAID_STATUSES = ['DEPOSIT_PAID', 'AWAITING_FINAL_PAYMENT', 'FULLY_PAID']; + +export function depositBypassed(policy) { + return ['NOT_REQUIRED', 'WAIVED', 'CREDIT_CUSTOMER'].includes(policy); +} + +/** `{ bypassed, paid, noticeIssued }` — the three deposit-status facts both callers key off. */ +export function depositStatus(summary) { + const bypassed = depositBypassed(summary.depositPolicy); + const paid = DEPOSIT_PAID_STATUSES.includes(summary.paymentStatus); + const noticeIssued = summary.paymentStatus === 'DEPOSIT_NOTICE_ISSUED'; + return { bypassed, paid, noticeIssued }; +} From 32f8325477669148b4bb64afc2dc95ea815e45ba Mon Sep 17 00:00:00 2001 From: "Yang.Pongburit" Date: Thu, 24 Sep 2026 21:13:19 +0700 Subject: [PATCH 003/204] =?UTF-8?q?fix(pricing):=20import-intake=20UX=20?= =?UTF-8?q?=E2=80=94=20pickup=20on=20detail/deal,=20price-gated=20on=20ema?= =?UTF-8?q?il,=20item=20preview,=20unclaimed=20badge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Frontend-only UX fixes to the import review/intake flow (owner asks 2026-09-24). No API/backend/authz change — pickup reuses the existing endpoint + gate, server enforces. - Price grid nudge (PricingRequestDetailPage): a factory's price inputs + ยืนยันราคาเสนอ stay locked (dimmed + a "send the email first" banner) while its request email is a DRAFT (not sent); they unlock once marked sent (REQUESTED). Grid stays visible (owner ruling 2026-08-16), just not fillable out of order. - Item preview before generate (PricingRequestDetailPage): the price section used to show a bare "ยังไม่มีราคาโรงงาน" before ร่างอีเมล was generated — now it previews the items to price, grouped by routed factory, read-only. - รับเรื่อง (pickup) on two surfaces that lacked it: the request page header (PricingRequestDetailPage) and the deal's คำขอราคา panel (PricingRequestPanel) — a SUBMITTED request could previously only be picked up from the คิวขอราคา queue. import + SUBMITTED only. - รอรับเรื่อง tab badge (PricingRequestQueuePage): shows the unclaimed count like งานของฉัน, reusing the unclaimed query already fetched for the empty-work prompt (no new fetch). Tests/build: lint 0 errors; PricingRequestDetailPage 155, PricingRequestPanel (pickup ×2), PricingRequestQueuePage 39 (unclaimed badge) — all green; frontend build ok. Co-Authored-By: Claude Opus 4.8 --- .../PricingRequestDetailPage.jsx | 104 ++++++++++++++++-- .../PricingRequestDetailPage.test.jsx | 55 +++++++++ .../pricingRequests/PricingRequestPanel.jsx | 24 +++- .../PricingRequestPanel.test.jsx | 21 ++++ .../PricingRequestQueuePage.jsx | 34 +++--- .../PricingRequestQueuePage.test.jsx | 21 ++++ 6 files changed, 236 insertions(+), 23 deletions(-) diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx index 9ae3a13f0..5f3300b69 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx @@ -40,6 +40,7 @@ import { canRecordCustomerQuotationOutcome, canSeePricingDecisionSalesView, canSeeRawPricingDecision, + canPickupPricingRequest, canStartCeoReview, canViewCustomerQuotation, isCustomerQuotationEditable, @@ -867,6 +868,10 @@ export function PricingRequestDetailPage({ user, showToast }) { ({ itemId, factory }) => api.pricingRequests.setItemFactory(pricingRequestId, itemId, { factory }), 'บันทึกโรงงานแล้ว', ); + // รับเรื่อง (pickup) from the request page itself — a SUBMITTED request landed here (e.g. from a + // notification link) with no way to claim it except going back to the คิวขอราคา queue. Same + // endpoint + canPickupPricingRequest gate as the queue and the deal panel; server is the authority. + const pickupRequest = useActionMutation(() => api.pricingRequests.pickup(pricingRequestId), 'รับเรื่องแล้ว'); const generateDrafts = useActionMutation(() => api.pricingRequests.generateFactoryEmailDrafts(pricingRequestId), 'สร้างร่างอีเมลแล้ว'); const updateQuote = useActionMutation(({ quote, draft }) => api.pricingRequests.updateFactoryQuote(quote.id, draft), 'บันทึกร่างอีเมลแล้ว'); // Manual-RFQ redesign: records that Import already sent this email themselves — see @@ -1321,6 +1326,20 @@ export function PricingRequestDetailPage({ user, showToast }) { const canSetItemFactory = isImport(user) && FACTORY_ROUTING_STATUSES.includes(summary?.status); const factoryQuotes = useMemo(() => factoryQuery.data ?? [], [factoryQuery.data]); const factoryGroups = useMemo(() => groupFactoryQuotesByFactory(factoryQuotes), [factoryQuotes]); + // Before any ร่างอีเมล has been generated there are no factory quotes, so the price section used + // to render a bare "ยังไม่มีราคาโรงงาน" — Import had no idea what they were about to price (owner + // UX ask 2026-09-24). This groups the request's OWN items by the factory each is routed to, so the + // section shows "what needs a price, per factory" up front; it is read-only preview only — the + // real price inputs still appear (and unlock per the email-sent nudge) once drafts are generated. + const pricePreviewGroups = useMemo(() => { + const map = new Map(); + (request?.items ?? []).forEach((item) => { + const name = itemFactoryName(item) || 'ยังไม่ได้ระบุโรงงาน'; + if (!map.has(name)) map.set(name, { key: name, factoryName: name, items: [] }); + map.get(name).items.push(item); + }); + return [...map.values()]; + }, [request]); const factoryItemCount = useMemo( () => factoryGroups.reduce((sum, group) => sum + (group.current.items?.length ?? 0), 0), [factoryGroups], @@ -1513,10 +1532,18 @@ export function PricingRequestDetailPage({ user, showToast }) { title={summary.requestCode} subtitle={`${summary.customerName ?? '-'}${summary.projectName ? ` · ${summary.projectName}` : ''}`} actions={( - + <> + {canPickupPricingRequest(user, summary) ? ( + + ) : null} + + )} /> @@ -1769,7 +1796,33 @@ export function PricingRequestDetailPage({ user, showToast }) { ) : null} > {factoryGroups.length === 0 ? ( -

ยังไม่มีราคาโรงงาน

+
+

+ ยังไม่ได้สร้างร่างอีเมลขอราคา — ด้านล่างคือรายการที่ต้องขอราคา จัดกลุ่มตามโรงงาน + {isImport(user) ? ' · กด “สร้างร่างอีเมล” ด้านบนเพื่อเริ่ม แล้วจึงกรอกราคาได้หลังกดส่งอีเมล' : ''} +

+ {pricePreviewGroups.map((group) => ( +
+
+ {group.factoryName} + ({group.items.length} รายการ) + กรอกราคาได้หลังส่งอีเมล +
+
+ {group.items.map((item) => { + const variant = [item.size, item.color, item.texture].filter(Boolean).join(' · '); + return ( +
+ {itemDisplayName(item)} + {variant ? {variant} : null} + · {item.requestedQty ?? '—'} {item.requestedUnit ?? ''} +
+ ); + })} +
+
+ ))} +
) : (
{/* Column headers (DESIGN.md §13's .table-head idiom: surface-muted band, overline @@ -1808,12 +1861,23 @@ export function PricingRequestDetailPage({ user, showToast }) { const dirty = Boolean(responseDrafts[current.id]); const editable = isImport(user) && current.current && ['DRAFT', 'REQUESTED', 'RESPONSE_RECEIVED', 'NEGOTIATING', 'READY_FOR_COSTING'].includes(current.status); + // Owner UX ask (2026-09-24): the price grid must not invite a quoted price before + // Import has actually SENT the request email to this factory — a DRAFT quote means + // "not sent yet" (the ร่างอีเมล → "ส่งแล้ว" action is what leaves DRAFT, → REQUESTED). + // So while DRAFT the ราคาที่เสนอ inputs stay locked (dimmed + a nudge to send first) + // and ยืนยันราคาเสนอ is withheld, without hard-hiding the grid — it stays the visible + // primary surface (owner ruling 2026-08-16), just not fillable out of order. + const emailSent = current.status !== 'DRAFT'; // READY_FOR_COSTING only offers ยืนยันราคาเสนอ again while dirty — see // confirmFactoryQuote's doc comment for why an undirtied re-click must not be // offered at all (it would either no-op-fail against markReady's own guard, or, // if this branch called receive() unconditionally, spuriously bump the revision). - const canConfirm = isImport(user) && current.current - && (['DRAFT', 'REQUESTED', 'RESPONSE_RECEIVED', 'NEGOTIATING'].includes(current.status) + // Withheld until the email is sent (emailSent): confirming a price before the + // request has gone out is exactly the out-of-order flow this nudge closes. DRAFT is + // the only !emailSent status the old list carried, so dropping it + AND emailSent is + // the whole change. + const canConfirm = isImport(user) && current.current && emailSent + && (['REQUESTED', 'RESPONSE_RECEIVED', 'NEGOTIATING'].includes(current.status) || (current.status === 'READY_FOR_COSTING' && dirty)); const canNegotiate = isImport(user) && current.status === 'RESPONSE_RECEIVED' && current.current; const canOpenEmailDraft = isImport(user) && current.status === 'DRAFT'; @@ -1881,6 +1945,17 @@ export function PricingRequestDetailPage({ user, showToast }) { ) : null}
+ {/* Nudge (owner UX ask 2026-09-24): while this factory's request email is still + a DRAFT (not sent), the price grid below is locked. Point Import at the + ร่างอีเมล → "ส่งแล้ว" action in the header rather than letting them type a + price out of order. Shown only to the role that can act (import, editable). */} + {editable && !emailSent ? ( +
+ + ส่งอีเมลขอราคาให้โรงงานนี้ก่อน จึงจะกรอกราคาที่เสนอได้ — กด “ร่างอีเมล” ด้านบน แล้วกด “ส่งแล้ว” +
+ ) : null} + {/* Per-factory control row — สกุลเงิน + หน่วยราคา, shared by every item below instead of a per-line picker (owner-supplied mockup). Static text once the response can no longer be edited here (not `editable`), so a read-only @@ -1969,7 +2044,7 @@ export function PricingRequestDetailPage({ user, showToast }) {
ราคาที่เสนอ (แก้ไข) - {editable ? ( + {editable && emailSent ? (
) : null}
+ ) : editable ? ( + // Import, but the request email is still a DRAFT — a dimmed, disabled + // stand-in so the column reads "you'll fill this after sending", not a + // usable field. The group-level banner above says why. + ) : ( {formatCurrency(line.rawUnitPrice, line.currency)} )} diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx index 43fc808f3..3ecdebf73 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx @@ -31,6 +31,7 @@ vi.mock('../../api/index.js', () => ({ attachmentUrl: (id) => `#attachment-${id}`, factoryQuoteAttachmentUrl: (id) => `#quote-attachment-${id}`, setItemFactory: vi.fn(), + pickup: vi.fn(), generateFactoryEmailDrafts: vi.fn(), updateFactoryQuote: vi.fn(), sendFactoryQuote: vi.fn(), @@ -790,6 +791,60 @@ describe('PricingRequestDetailPage Import factory-quote workflow', () => { expect(within(dialog).getByRole('button', { name: 'ส่งแล้ว' })).not.toBeNull(); }); + // Owner UX ask 2026-09-24: the price grid must not invite a quoted price before the request + // email has actually been sent to this factory (a DRAFT quote = not sent yet). Nudge, not hide: + // the grid stays visible with a locked/dimmed price input + a "send the email first" banner. + it('locks the price grid with a nudge while the factory email is still a DRAFT', async () => { + renderDetailPage({ user: importUser, factoryQuotes: [buildFactoryQuote()] }); // default status DRAFT + await waitForLoaded(); + await screen.findByText('SCG Ceramics'); + + expect(screen.getByTestId('pcr-await-email-91')).not.toBeNull(); // the nudge banner + // The price field (shared aria-label prefix) is the DISABLED stand-in, not an editable input. + expect(screen.getByLabelText(/^ราคาที่เสนอ/).disabled).toBe(true); + expect(screen.queryByRole('button', { name: 'ยืนยันราคาเสนอ' })).toBeNull(); // and no confirm + }); + + it('unlocks the price grid once the factory email is sent (REQUESTED)', async () => { + renderDetailPage({ user: importUser, factoryQuotes: [buildFactoryQuote({ status: 'REQUESTED' })] }); + await waitForLoaded(); + await screen.findByText('SCG Ceramics'); + + expect(screen.queryByTestId('pcr-await-email-91')).toBeNull(); // banner gone + expect(screen.getByLabelText(/^ราคาที่เสนอ/).disabled).toBe(false); // editable input back + expect(screen.getByRole('button', { name: 'ยืนยันราคาเสนอ' })).not.toBeNull(); + }); + + // #2 (owner ask 2026-09-24): before any ร่างอีเมล is generated the price section used to be a + // bare "ยังไม่มีราคาโรงงาน" — now it previews what needs pricing, grouped by the routed factory, + // read-only (no editable price input yet). + it('previews what needs pricing (grouped by factory) before any ร่างอีเมล is generated', async () => { + renderDetailPage({ user: importUser }); // no factoryQuotes → nothing generated yet + await waitForLoaded(); + + const preview = await screen.findByTestId('pcr-price-preview'); + expect(screen.getAllByTestId('pcr-price-preview-group').length).toBeGreaterThan(0); + expect(within(preview).getByText('SCG Ceramics')).not.toBeNull(); // the routed factory group + expect(screen.queryByLabelText(/^ราคาที่เสนอ/)).toBeNull(); // no editable price yet + }); + + // #1 (owner ask 2026-09-24): a SUBMITTED request opened here (e.g. from a link) had no รับเรื่อง + // affordance on the page itself — only in the คิวขอราคา queue. + it('lets an import user รับเรื่อง a SUBMITTED request from the request-page header', async () => { + api.pricingRequests.pickup.mockResolvedValue({ pricingRequest: buildRequest({ summary: { status: 'IMPORT_REVIEWING' } }) }); + renderDetailPage({ user: importUser, request: buildRequest({ summary: { status: 'SUBMITTED' } }) }); + await waitForLoaded(); + + fireEvent.click(screen.getByTestId('pcr-detail-pickup')); + await waitFor(() => expect(api.pricingRequests.pickup).toHaveBeenCalledWith(501)); + }); + + it('shows no รับเรื่อง on the request page once it is already picked up (IMPORT_REVIEWING)', async () => { + renderDetailPage({ user: importUser, request: buildRequest() }); // default IMPORT_REVIEWING + await waitForLoaded(); + expect(screen.queryByTestId('pcr-detail-pickup')).toBeNull(); + }); + it('records a factory response revision entry via receiveFactoryQuote with a fresh clientRequestId', async () => { const quote = buildFactoryQuote({ status: 'REQUESTED' }); renderDetailPage({ user: importUser, factoryQuotes: [quote] }); diff --git a/frontend/src/features/pricingRequests/PricingRequestPanel.jsx b/frontend/src/features/pricingRequests/PricingRequestPanel.jsx index 5516d5921..4409fc8fe 100644 --- a/frontend/src/features/pricingRequests/PricingRequestPanel.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestPanel.jsx @@ -10,7 +10,7 @@ import { Modal } from '../../components/common/Modal.jsx'; import { StatusBadge } from '../../components/common/StatusBadge.jsx'; import { formatThaiDate, pricingRequestStatusLabel } from '../../utils/format.js'; import { - canCancelPricingRequest, canCreatePricingRequest, + canCancelPricingRequest, canCreatePricingRequest, canPickupPricingRequest, canSubmitPricingRequest, canUpdatePricingRequest, pricingRequestRecipientLabel, } from './pricingRequestMeta.js'; import { PricingRequestCreateModal } from './PricingRequestCreateModal.jsx'; @@ -108,6 +108,14 @@ export const PricingRequestPanel = forwardRef(function PricingRequestPanel({ tic mutationFn: ({ id, reason }) => api.pricingRequests.cancel(id, { reason }), onSuccess: () => { invalidate(); setCancelDraft(null); }, }); + // Import รับเรื่อง (pickup) straight from the deal, not only from the คิวขอราคา queue — a + // SUBMITTED request on a deal an import user is looking at had no รับเรื่อง affordance here at + // all. Same api.pricingRequests.pickup + canPickupPricingRequest gate the queue page uses; the + // server (PricingRequestService.pickup: any import user, SUBMITTED only) stays the authority. + const pickupMutation = useMutation({ + mutationFn: (id) => api.pricingRequests.pickup(id), + onSuccess: invalidate, + }); const canCreate = canCreatePricingRequest(user, deal); @@ -158,8 +166,19 @@ export const PricingRequestPanel = forwardRef(function PricingRequestPanel({ tic - {(canUpdatePricingRequest(user, pr) || canSubmitPricingRequest(user, pr) || canCancelPricingRequest(user, pr)) ? ( + {(canPickupPricingRequest(user, pr) || canUpdatePricingRequest(user, pr) || canSubmitPricingRequest(user, pr) || canCancelPricingRequest(user, pr)) ? (
+ {canPickupPricingRequest(user, pr) ? ( + + ) : null} {canUpdatePricingRequest(user, pr) ? (
) : null} diff --git a/frontend/src/features/pricingRequests/PricingRequestPanel.test.jsx b/frontend/src/features/pricingRequests/PricingRequestPanel.test.jsx index d83cefccd..31694ed76 100644 --- a/frontend/src/features/pricingRequests/PricingRequestPanel.test.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestPanel.test.jsx @@ -19,6 +19,7 @@ vi.mock('../../api/index.js', async (importOriginal) => { update: vi.fn(), submit: vi.fn(), cancel: vi.fn(), + pickup: vi.fn(), // PricingRequestCreateModal (V69, review remediation COMMIT 4) fetches attachments // whenever it has a persisted id — including edit mode, which this file's "seed the // modal from request detail" test exercises. @@ -78,6 +79,26 @@ describe('PricingRequestPanel', () => { vi.clearAllMocks(); }); + // #1 (owner ask 2026-09-24): a SUBMITTED request waiting for import had no รับเรื่อง affordance + // on the deal itself — only in the คิวขอราคา queue. Same gate/endpoint the queue uses. + it('lets an import user รับเรื่อง a SUBMITTED request straight from the deal', async () => { + api.pricingRequests.listForTicket.mockResolvedValue({ items: [summary({ status: 'SUBMITTED' })] }); + api.pricingRequests.pickup.mockResolvedValue({ + pricingRequest: { summary: summary({ status: 'PICKED_UP' }), items: [], events: [] }, + }); + renderPanel({ user: importUser }); + + fireEvent.click(await screen.findByTestId('pcr-pickup-1')); + await waitFor(() => expect(api.pricingRequests.pickup).toHaveBeenCalledWith(1)); + }); + + it('shows no รับเรื่อง button to the sales owner (pickup is import-only)', async () => { + api.pricingRequests.listForTicket.mockResolvedValue({ items: [summary({ status: 'SUBMITTED' })] }); + renderPanel({ user: salesOwner }); + await screen.findByText('PCR-2026-0001'); + expect(screen.queryByTestId('pcr-pickup-1')).toBeNull(); + }); + // Ticket-detail IA rebuild Phase 1 clutter follow-up (FIX 1): this panel no // longer renders its own "สร้างคำขอราคา" button — TicketDetailPage's sticky // header CTA owns that action now (opening this panel's create modal via diff --git a/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx b/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx index ba47239cb..6a6f99060 100644 --- a/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx @@ -88,7 +88,7 @@ function byOldestFirst(a, b) { const TASK_TABS_BY_ROLE = { import: [ { key: 'MY_WORK', label: 'งานของฉัน', hasBadge: true, emptyTitle: 'ไม่มีงานที่คุณรับเรื่องค้างอยู่' }, - { key: 'UNCLAIMED', label: 'รอรับเรื่อง', emptyTitle: 'ไม่มีคำขอที่รอรับเรื่อง' }, + { key: 'UNCLAIMED', label: 'รอรับเรื่อง', hasBadge: true, emptyTitle: 'ไม่มีคำขอที่รอรับเรื่อง' }, { key: 'ALL', label: 'ทั้งหมด', emptyTitle: 'ไม่มีคำขอราคาในเงื่อนไขนี้' }, ], ceo: [ @@ -352,10 +352,10 @@ export function PricingRequestQueuePage({ user, showToast }) { // needs to know THIS count too (to offer "ไม่มีงานค้าง … ดูคำขอที่รอรับเรื่อง" instead of a // bare empty state, see showEmptyWorkPrompt below), so it must be known before its own // tab is ever opened, and import now always fetches this list once on load rather than - // only after opening the tab. No badge is added here on purpose (still exactly one badge - // on this page) — this is the SAME query/cache key as the (now-retired-as-a-default) - // SUBMITTED chip, so nothing is fetched here that this page didn't already sometimes fetch - // before. + // only after opening the tab. This count now ALSO shows as the รอรับเรื่อง tab's own badge + // (owner ask 2026-09-24 — matching งานของฉัน), reusing this exact query rather than adding a + // new fetch: it is the SAME query/cache key as the (now-retired-as-a-default) SUBMITTED chip, + // so nothing is fetched here that this page didn't already sometimes fetch before. // // ⚠️ Freshness trade-off (review finding, 2026-09-19): because `enabled` no longer flips // false->true when this tab is opened, react-query's own "refetch on a query becoming @@ -448,14 +448,22 @@ export function PricingRequestQueuePage({ user, showToast }) { // page defaults to `[]` before the first response, making an unloaded badge indistinguishable // from a genuinely-empty one). const badgeCount = hasTaskTabs && badgeQuery.isSuccess ? badgeRows.length : null; - const tabItems = taskTabs?.map((tab) => ({ - id: tab.key, - label: tab.label, - badge: tab.hasBadge ? badgeCount : undefined, - // Read as "
diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx index 3ecdebf73..2c1d96316 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx @@ -845,6 +845,22 @@ describe('PricingRequestDetailPage Import factory-quote workflow', () => { expect(screen.queryByTestId('pcr-detail-pickup')).toBeNull(); }); + // owner ask 2026-09-24: after ส่งแล้ว, Import can still reopen the RFQ email — read-only (view + + // copy), labelled ดูอีเมล; the ส่งแล้ว transition is gone. + it('reopens a SENT factory RFQ email read-only (ดูอีเมล, copy kept, no ส่งแล้ว)', async () => { + renderDetailPage({ user: importUser, factoryQuotes: [buildFactoryQuote({ status: 'REQUESTED' })] }); + await waitForLoaded(); + await screen.findByText('SCG Ceramics'); + + const openBtn = screen.getByTestId('pcr-open-email-draft-91'); + expect(openBtn.textContent).toContain('ดูอีเมล'); + fireEvent.click(openBtn); + + const dialog = await screen.findByTestId('factory-email-draft-modal'); + expect(within(dialog).getByTestId('pcr-copy-factory-email')).not.toBeNull(); // copy kept + expect(within(dialog).queryByTestId('pcr-mark-factory-email-sent')).toBeNull(); // no ส่งแล้ว + }); + it('records a factory response revision entry via receiveFactoryQuote with a fresh clientRequestId', async () => { const quote = buildFactoryQuote({ status: 'REQUESTED' }); renderDetailPage({ user: importUser, factoryQuotes: [quote] }); From b608f3753f4e477f12e4195457f93d23e02786e2 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Mon, 28 Sep 2026 00:36:23 +0700 Subject: [PATCH 005/204] fix(pricing-request): clamp DRAFT-quote lock to request send window; fix preview coverage gap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Opus review of #1062 found two frontend gaps in the factory-quote price section: - The DRAFT-quote lock/banner read only the quote's own status, never the pricing request's. FactoryQuoteService.send only succeeds while the request is in DRAFT_STATUSES, but receive()'s window is wider (RESPONSE_STATUSES, including READY_FOR_CEO_REVIEW) and explicitly still accepts a DRAFT quote. The lock/banner now apply only while the request's own status is inside send()'s window, reusing the existing FACTORY_ROUTING_STATUSES constant. This is a defensive guard mirroring the two backend windows, not a claim of a specific reproduction — no confirmed live path was found that lands a request there today (setItemFactory is a gap-fill only, and markReadyForCosting's auto-advance requires every item's quote to already be READY_FOR_COSTING) — but the UI must never show a "send first" banner pointing at an action that would 409 while receive() would accept it. - The pre-generate item preview was gated on factoryGroups.length === 0, so it vanished the instant any factory quote existed on the request, hiding an item whose factory was only just gap-filled by Import (setItemFactory — never a re-route of an already-set line, per its own Javadoc) after drafts were already generated for other items. The preview now covers exactly the items no current factory-quote group already lists, and renders alongside the quote grid rather than instead of it. Second-pass review fixes on top of the above: - canConfirm's disjunction still didn't offer ยืนยันราคาเสนอ for a DRAFT quote outside send()'s window — the input was unlocked but unsubmittable, so a typed price was silently unrecoverable. Added `current.status === 'DRAFT' && !inSendWindow` alongside the existing confirmable statuses, since receive() explicitly still accepts that combo. - The preview banner copy ("ยังไม่ได้สร้างร่างอีเมลขอราคา") was false in the mixed case where it renders above factory groups that already have a draft; it's now conditional on factoryGroups.length. - The panel's item count (factoryItemCount) summed only quoted items, so it undercounted in the mixed case; it now also counts previewed items. Third-pass review fix: FactoryEmailDraftModal's own canOfferSendActions and canEditFields flags had the identical gap the price grid had before the first review pass — they read only the QUOTE's status, never inSendWindow. In the state the branch now explicitly supports (request outside send()'s window, quote still DRAFT), the group header already showed "ดูอีเมล" (view-only) for this same reason, yet the modal itself still offered "บันทึกร่างอีเมล" (→ updateDraft, guarded by the same DRAFT_STATUSES → 409) and "ส่งแล้ว" (→ send → 409). inSendWindow is hoisted to component scope and passed into the modal; both flags now also require it. Also fixes a stale "computed below" comment in PricingRequestQueuePage.jsx. Co-Authored-By: Claude Sonnet 5 --- .../PricingRequestDetailPage.jsx | 137 +++++++++++++++--- .../PricingRequestDetailPage.test.jsx | 96 ++++++++++++ .../PricingRequestQueuePage.jsx | 2 +- 3 files changed, 214 insertions(+), 21 deletions(-) diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx index 4201a7aa3..85aa61005 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.jsx @@ -386,9 +386,16 @@ const FACTORY_ITEM_GRID = 'md:grid-cols-[minmax(150px,1.6fr)_minmax(120px,1.1fr) * about two `useDialogFocus` traps active together, and it matches how a user's attention actually * moves: finish the draft, then confirm the send. */ -function FactoryEmailDraftModal({ quote, draft, onChangeDraft, onClose, onSave, savePending, onCopy, onRequestSend }) { - const canOfferSendActions = quote.status === 'DRAFT'; - const canEditFields = quote.status === 'DRAFT'; +function FactoryEmailDraftModal({ quote, draft, onChangeDraft, onClose, onSave, savePending, onCopy, onRequestSend, inSendWindow }) { + // Opus review of #1062 (2026-09-28): these two used to read only the QUOTE's own status — the + // exact gap the price grid had before that review's fix. In the state that fix now explicitly + // supports (request outside FactoryQuoteService.send's DRAFT_STATUSES window, quote still DRAFT), + // the group header already shows "ดูอีเมล" (view-only) for this same reason, yet this modal would + // still offer "บันทึกร่างอีเมล" (→ updateDraft, guarded by the identical DRAFT_STATUSES → 409) and + // "ส่งแล้ว" (→ send → 409). Both must also require `inSendWindow` (passed in from the page, which + // hoists the same value the grid uses — see PricingRequestDetailPage's own comment on it). + const canOfferSendActions = quote.status === 'DRAFT' && inSendWindow; + const canEditFields = quote.status === 'DRAFT' && inSendWindow; return ( factoryQuery.data ?? [], [factoryQuery.data]); const factoryGroups = useMemo(() => groupFactoryQuotesByFactory(factoryQuotes), [factoryQuotes]); // Before any ร่างอีเมล has been generated there are no factory quotes, so the price section used @@ -1332,18 +1346,44 @@ export function PricingRequestDetailPage({ user, showToast }) { // UX ask 2026-09-24). This groups the request's OWN items by the factory each is routed to, so the // section shows "what needs a price, per factory" up front; it is read-only preview only — the // real price inputs still appear (and unlock per the email-sent nudge) once drafts are generated. + // + // Opus review of #1062 (2026-09-28): this used to be gated on `factoryGroups.length === 0`, so it + // was all-or-nothing — the moment ANY factory quote existed, the whole preview disappeared. But + // `canSetItemFactory` only ever GAP-FILLS a factory name on a line Sales left blank — never + // re-routes one that already has a factory (PricingRequestService#setItemFactory's own Javadoc: + // "deliberately a gap-FILL, never a re-route" — a line that already has a factory is refused with + // a 409) — while the request is still in FACTORY_ROUTING_STATUSES. If that happens AFTER drafts + // were already generated for other lines, the newly-filled item has no factory-quote row of its + // own (it never went through สร้างร่างอีเมล), so it showed up in neither `factoryGroups` nor the + // (suppressed) preview — invisible, with no price surface at all. Fixed by covering exactly the + // items no CURRENT factory quote already lists (below), independent of whether other items in the + // same request already have quotes. + const coveredItemIds = useMemo(() => { + const ids = new Set(); + factoryGroups.forEach((group) => { + (group.current.items ?? []).forEach((item) => ids.add(item.pricingRequestItemId)); + }); + return ids; + }, [factoryGroups]); const pricePreviewGroups = useMemo(() => { const map = new Map(); - (request?.items ?? []).forEach((item) => { - const name = itemFactoryName(item) || 'ยังไม่ได้ระบุโรงงาน'; - if (!map.has(name)) map.set(name, { key: name, factoryName: name, items: [] }); - map.get(name).items.push(item); - }); + (request?.items ?? []) + .filter((item) => !coveredItemIds.has(item.id)) + .forEach((item) => { + const name = itemFactoryName(item) || 'ยังไม่ได้ระบุโรงงาน'; + if (!map.has(name)) map.set(name, { key: name, factoryName: name, items: [] }); + map.get(name).items.push(item); + }); return [...map.values()]; - }, [request]); + }, [request, coveredItemIds]); + // Opus review of #1062 (2026-09-28): this used to count only `factoryGroups` (quoted items), so + // the panel title undercounted in the mixed case now covered by `pricePreviewGroups` — a request + // with 2 quoted items + 1 gap-filled-but-not-yet-quoted item showed "(2 รายการ)" above 3 rows. + // Count both: quoted items plus previewed-but-not-yet-quoted items. const factoryItemCount = useMemo( - () => factoryGroups.reduce((sum, group) => sum + (group.current.items?.length ?? 0), 0), - [factoryGroups], + () => factoryGroups.reduce((sum, group) => sum + (group.current.items?.length ?? 0), 0) + + pricePreviewGroups.reduce((sum, group) => sum + group.items.length, 0), + [factoryGroups, pricePreviewGroups], ); // Currency codes Import may pick from the per-factory สกุลเงิน select — real trade currencies // this business already reads elsewhere (fxRatesQuery above), not an invented list. THB always @@ -1796,11 +1836,32 @@ export function PricingRequestDetailPage({ user, showToast }) { ) : null} > - {factoryGroups.length === 0 ? ( + {/* Opus review of #1062 (2026-09-28): these two blocks used to be an if/else on + `factoryGroups.length === 0`, so the preview vanished the instant ANY factory quote + existed anywhere on the request — hiding an item whose factory was blank until Import + gap-filled it (canSetItemFactory, during FACTORY_ROUTING_STATUSES — never a re-route of + an already-set line, see setItemFactory's own Javadoc) AFTER drafts were already + generated for other items; that item has no quote row of its own. They now render + independently: the editable grid for whatever already has a quote, PLUS a preview for + whatever still doesn't — which can both be true at once. */} + {pricePreviewGroups.length > 0 ? (
+ {/* Opus review of #1062 (2026-09-28): "ยังไม่ได้สร้างร่างอีเมลขอราคา" (no draft has been + generated AT ALL) is false in the mixed case this block now also covers — it can + render ABOVE factory groups that already have a sent/answered quote, once another + item's factory was only just gap-filled. Text now depends on which case this is. */}

- ยังไม่ได้สร้างร่างอีเมลขอราคา — ด้านล่างคือรายการที่ต้องขอราคา จัดกลุ่มตามโรงงาน - {isImport(user) ? ' · กด “สร้างร่างอีเมล” ด้านบนเพื่อเริ่ม แล้วจึงกรอกราคาได้หลังกดส่งอีเมล' : ''} + {factoryGroups.length === 0 ? ( + <> + ยังไม่ได้สร้างร่างอีเมลขอราคา — ด้านล่างคือรายการที่ต้องขอราคา จัดกลุ่มตามโรงงาน + {isImport(user) ? ' · กด “สร้างร่างอีเมล” ด้านบนเพื่อเริ่ม แล้วจึงกรอกราคาได้หลังกดส่งอีเมล' : ''} + + ) : ( + <> + รายการที่ยังไม่ได้ขอราคาจากโรงงาน — ด้านล่างคือรายการที่ต้องขอราคาเพิ่ม จัดกลุ่มตามโรงงาน + {isImport(user) ? ' · กด “สร้างร่างอีเมล” ด้านบนเพื่อรวมรายการเหล่านี้เข้าไปในร่างอีเมล' : ''} + + )}

{pricePreviewGroups.map((group) => (
@@ -1824,7 +1885,8 @@ export function PricingRequestDetailPage({ user, showToast }) {
))}
- ) : ( + ) : null} + {factoryGroups.length > 0 ? (
{/* Column headers (DESIGN.md §13's .table-head idiom: surface-muted band, overline caption) — shared FACTORY_ITEM_GRID keeps every item row below aligned to these @@ -1868,7 +1930,27 @@ export function PricingRequestDetailPage({ user, showToast }) { // So while DRAFT the ราคาที่เสนอ inputs stay locked (dimmed + a nudge to send first) // and ยืนยันราคาเสนอ is withheld, without hard-hiding the grid — it stays the visible // primary surface (owner ruling 2026-08-16), just not fillable out of order. - const emailSent = current.status !== 'DRAFT'; + // + // Opus review of #1062 (2026-09-28): the lock above looked only at the QUOTE's own + // status, never the PRICING REQUEST's — but the "ส่งแล้ว" action it points at is + // FactoryQuoteService.send, which only succeeds while the pricing request itself is + // in DRAFT_STATUSES ({IMPORT_REVIEWING, AWAITING_FACTORY_RESPONSE} — + // FactoryQuoteService.java:47-49, guarded at :242). FactoryQuoteService.receive's + // window (RESPONSE_STATUSES, :50-53) is wider — it adds READY_FOR_CEO_REVIEW and + // explicitly still accepts a DRAFT quote (:402, :408). This clamp is a DEFENSIVE + // guard mirroring those two backend windows, not a claim of a specific sequence that + // reaches (request outside send()'s window) ∧ (quote still DRAFT) today — + // PricingRequestService#setItemFactory is a gap-FILL only (a line that already has a + // factory 409s), and markReadyForCosting's auto-advance requires every item's + // CURRENT quote to already be READY_FOR_COSTING, so no confirmed live path was found. + // But whatever combination does get a request there, the UI must never lock the grid + // behind a "send first" banner whose only action would 409 while receive() would + // accept the very same confirm. So the lock/banner apply only inside send()'s window; + // FACTORY_ROUTING_STATUSES above is that exact set (its own comment already notes it + // mirrors FactoryQuoteService.DRAFT_STATUSES), so it is reused rather than duplicated. + // `inSendWindow` itself is hoisted to component scope (above `canSetItemFactory`) — + // FactoryEmailDraftModal needs the identical value. + const emailSent = !inSendWindow || current.status !== 'DRAFT'; // READY_FOR_COSTING only offers ยืนยันราคาเสนอ again while dirty — see // confirmFactoryQuote's doc comment for why an undirtied re-click must not be // offered at all (it would either no-op-fail against markReady's own guard, or, @@ -1877,14 +1959,28 @@ export function PricingRequestDetailPage({ user, showToast }) { // request has gone out is exactly the out-of-order flow this nudge closes. DRAFT is // the only !emailSent status the old list carried, so dropping it + AND emailSent is // the whole change. + // + // Opus review of #1062 (2026-09-28): that DRAFT/emailSent history only ever + // ungated the price INPUT for a request past send()'s window (below) — it did not + // restore this disjunction, which still names only + // REQUESTED/RESPONSE_RECEIVED/NEGOTIATING/READY_FOR_COSTING&&dirty. So a request + // outside send()'s window with a still-DRAFT quote showed an ENABLED input with NO + // confirm button — a typed price was enterable but unsubmittable, silently lost. + // FactoryQuoteService.receive's window (RESPONSE_STATUSES, which includes + // READY_FOR_CEO_REVIEW) explicitly still accepts a DRAFT quote, so the button must + // offer exactly that confirm: add `current.status === 'DRAFT' && !inSendWindow`. const canConfirm = isImport(user) && current.current && emailSent && (['REQUESTED', 'RESPONSE_RECEIVED', 'NEGOTIATING'].includes(current.status) - || (current.status === 'READY_FOR_COSTING' && dirty)); + || (current.status === 'READY_FOR_COSTING' && dirty) + || (current.status === 'DRAFT' && !inSendWindow)); const canNegotiate = isImport(user) && current.status === 'RESPONSE_RECEIVED' && current.current; // Import can OPEN the RFQ email at any status (owner ask 2026-09-24): to draft+send // it while DRAFT, or just to VIEW what was actually sent afterwards. The modal is - // read-only once the quote leaves DRAFT (FactoryEmailDraftModal's canEditFields / - // canOfferSendActions), so opening it post-send can only view + copy, never re-edit. + // read-only once the quote leaves DRAFT OR the request leaves send()'s window + // (FactoryEmailDraftModal's canEditFields / canOfferSendActions — Opus review of + // #1062, 2026-09-28: DRAFT alone wasn't enough, matching the grid's own emailSent + // fix above), so opening it post-send, or outside that window, can only view + copy, + // never re-edit or re-send. const canOpenEmailDraft = isImport(user); // หน่วยราคา is a per-FACTORY control now, not per-line (owner-supplied mockup) — every // line in `draft.items` shares one unitBasis, so the first line speaks for the whole @@ -2193,7 +2289,7 @@ export function PricingRequestDetailPage({ user, showToast }) { ); })}
- )} + ) : null} ) : null} @@ -3551,6 +3647,7 @@ export function PricingRequestDetailPage({ user, showToast }) { })} savePending={updateQuote.isPending} onCopy={copyFactoryEmail} + inSendWindow={inSendWindow} onRequestSend={() => { const quote = emailModalQuote; const draft = emailDrafts[quote.id] ?? { emailTo: quote.emailTo ?? '', emailSubject: quote.emailSubject ?? '', emailBody: quote.emailBody ?? '', note: quote.note ?? '' }; diff --git a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx index 2c1d96316..899376314 100644 --- a/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestDetailPage.test.jsx @@ -815,6 +815,33 @@ describe('PricingRequestDetailPage Import factory-quote workflow', () => { expect(screen.getByRole('button', { name: 'ยืนยันราคาเสนอ' })).not.toBeNull(); }); + // Opus review of #1062 (2026-09-28): the lock above read only the QUOTE's own status, never the + // PRICING REQUEST's. FactoryQuoteService.send only succeeds while the request itself sits in + // DRAFT_STATUSES = {IMPORT_REVIEWING, AWAITING_FACTORY_RESPONSE} (FactoryQuoteService.java:47-49, + // guarded at :242). receive()'s window is wider (RESPONSE_STATUSES, :50-53, adds + // READY_FOR_CEO_REVIEW) and explicitly still accepts a DRAFT quote (:402, :408). This is a + // defensive guard, not a claim of a specific reproduction — PricingRequestService#setItemFactory + // is a gap-FILL only (a line that already has a factory 409s), so no confirmed live path was found + // that lands a DRAFT quote outside send()'s window today. But whatever combination does, the UI + // must never lock the grid behind a "send first" banner whose only action would 409 while + // receive() would accept the very same confirm — so outside send()'s window the grid must behave + // as it did before #1062: enabled, no banner, AND (since receive() explicitly still accepts a + // DRAFT quote there) the ยืนยันราคาเสนอ confirm button restored too — see the follow-up fix to + // canConfirm's own disjunction, which this test also covers. + it('does not lock the price grid for a surviving DRAFT quote once the request is past send()\'s window (READY_FOR_CEO_REVIEW)', async () => { + renderDetailPage({ + user: importUser, + request: buildRequest({ summary: { status: 'READY_FOR_CEO_REVIEW' } }), + factoryQuotes: [buildFactoryQuote()], // still DRAFT + }); + await waitForLoaded(); + await screen.findByText('SCG Ceramics'); + + expect(screen.queryByTestId('pcr-await-email-91')).toBeNull(); // no banner + expect(screen.getByLabelText(/^ราคาที่เสนอ/).disabled).toBe(false); // input enabled, not locked + expect(screen.getByRole('button', { name: 'ยืนยันราคาเสนอ' })).not.toBeNull(); // confirm restored too + }); + // #2 (owner ask 2026-09-24): before any ร่างอีเมล is generated the price section used to be a // bare "ยังไม่มีราคาโรงงาน" — now it previews what needs pricing, grouped by the routed factory, // read-only (no editable price input yet). @@ -828,6 +855,48 @@ describe('PricingRequestDetailPage Import factory-quote workflow', () => { expect(screen.queryByLabelText(/^ราคาที่เสนอ/)).toBeNull(); // no editable price yet }); + // Opus review of #1062 (2026-09-28): the preview used to be gated on `factoryGroups.length === + // 0`, so it vanished entirely the instant ANY factory quote existed anywhere on the request. An + // item whose factory was blank until Import gap-filled it (canSetItemFactory, still within + // FACTORY_ROUTING_STATUSES — never a re-route of an already-set line, per setItemFactory's own + // Javadoc) AFTER drafts were already generated for other items has no factory-quote row of its + // own, so it appeared in neither the (suppressed) preview nor the quote grid — invisible. The + // preview must now cover exactly the items no CURRENT factory quote already lists, independent of + // whether other items already have one — and its banner copy must not claim "no draft generated + // at all" in this mixed case, and the panel's item count must include these previewed items too. + it('previews an item with no factory-quote row yet even once other items already have one', async () => { + const base = buildRequest().items[0]; + const items = [ + { ...base, id: 1, resolvedFactoryName: 'SCG Ceramics' }, // A — quoted below + { ...base, id: 2, resolvedFactoryName: 'SCG Ceramics', model: 'A2' }, // B — quoted below + { ...base, id: 3, resolvedFactoryName: 'Cotto', model: 'C1' }, // C — gap-filled, not quoted yet + ]; + const quote = buildFactoryQuote({ + items: [ + { ...buildFactoryQuote().items[0], id: 911, pricingRequestItemId: 1 }, + { ...buildFactoryQuote().items[0], id: 912, pricingRequestItemId: 2 }, + ], + }); + const request = buildRequest({ items }); + renderDetailPage({ user: importUser, request, factoryQuotes: [quote] }); + await waitForLoaded(request); + + const preview = await screen.findByTestId('pcr-price-preview'); + expect(within(preview).getByText('Cotto')).not.toBeNull(); // C's factory shows + expect(within(preview).queryByText('SCG Ceramics')).toBeNull(); // A/B's factory does not + // Mixed case (A/B already quoted, C only just previewed): the banner must not claim no draft + // was ever generated — that would be false for A/B's already-drafted factory. + expect(within(preview).getByText(/รายการที่ยังไม่ได้ขอราคาจากโรงงาน/)).not.toBeNull(); + expect(within(preview).queryByText(/ยังไม่ได้สร้างร่างอีเมลขอราคา/)).toBeNull(); + + // A and B still render as their own quote-grid rows (locked, since request stays + // IMPORT_REVIEWING and the quote stays DRAFT) rather than being hidden by C's preview. + expect(screen.getAllByLabelText(/^ราคาที่เสนอ/).length).toBe(2); + + // Panel title counts all three items (2 quoted + 1 previewed), not just the quoted ones. + expect(screen.getByText('รายการสินค้า (3 รายการ)')).not.toBeNull(); + }); + // #1 (owner ask 2026-09-24): a SUBMITTED request opened here (e.g. from a link) had no รับเรื่อง // affordance on the page itself — only in the คิวขอราคา queue. it('lets an import user รับเรื่อง a SUBMITTED request from the request-page header', async () => { @@ -861,6 +930,33 @@ describe('PricingRequestDetailPage Import factory-quote workflow', () => { expect(within(dialog).queryByTestId('pcr-mark-factory-email-sent')).toBeNull(); // no ส่งแล้ว }); + // Opus review of #1062 (2026-09-28), second pass: FactoryEmailDraftModal's own two flags + // (canEditFields / canOfferSendActions) used to read only the QUOTE's status — the identical gap + // the price grid had before the first review pass, reachable through the exact same state (request + // outside send()'s window, quote still DRAFT). The group header already shows "ดูอีเมล" there + // (view-only, per the grid's emailSent fix), but the modal itself still offered "ส่งแล้ว" (→ + // FactoryQuoteService.send → 409) and "บันทึกร่างอีเมล" (→ updateDraft, guarded by the identical + // DRAFT_STATUSES → 409). Both now also require `inSendWindow`. + it('reopens the RFQ email modal read-only for a DRAFT quote once the request is past send()\'s window (READY_FOR_CEO_REVIEW)', async () => { + renderDetailPage({ + user: importUser, + request: buildRequest({ summary: { status: 'READY_FOR_CEO_REVIEW' } }), + factoryQuotes: [buildFactoryQuote()], // still DRAFT + }); + await waitForLoaded(); + await screen.findByText('SCG Ceramics'); + + const openBtn = screen.getByTestId('pcr-open-email-draft-91'); + expect(openBtn.textContent).toContain('ดูอีเมล'); // header already reads view-only here + fireEvent.click(openBtn); + + const dialog = await screen.findByTestId('factory-email-draft-modal'); + expect(within(dialog).queryByRole('button', { name: 'ส่งแล้ว' })).toBeNull(); // no send + expect(within(dialog).queryByRole('button', { name: 'บันทึกร่างอีเมล' })).toBeNull(); // no save + expect(within(dialog).getByTestId('pcr-copy-factory-email')).not.toBeNull(); // copy kept + expect(within(dialog).getByLabelText('อีเมลโรงงาน (ถ้ามี)').disabled).toBe(true); // fields locked + }); + it('records a factory response revision entry via receiveFactoryQuote with a fresh clientRequestId', async () => { const quote = buildFactoryQuote({ status: 'REQUESTED' }); renderDetailPage({ user: importUser, factoryQuotes: [quote] }); diff --git a/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx b/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx index 6a6f99060..ae00f6397 100644 --- a/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx +++ b/frontend/src/features/pricingRequests/PricingRequestQueuePage.jsx @@ -449,7 +449,7 @@ export function PricingRequestQueuePage({ user, showToast }) { // from a genuinely-empty one). const badgeCount = hasTaskTabs && badgeQuery.isSuccess ? badgeRows.length : null; // รอรับเรื่อง carries its OWN badge (owner ask 2026-09-24 — matching งานของฉัน) from the - // unclaimedCount already computed below for the empty-work prompt; badgeCount is the role's + // unclaimedCount already computed above for the empty-work prompt; badgeCount is the role's // first-tab count. unclaimedCount is import-only (its query is enabled for import alone) and // null for ceo, which has no รอรับเรื่อง tab anyway. const tabBadgeCount = (key) => (key === 'UNCLAIMED' ? unclaimedCount : badgeCount); From 12b3b7f591f1a6db9c694d96f9d470983a0d830b Mon Sep 17 00:00:00 2001 From: "Yang.Pongburit" Date: Thu, 24 Sep 2026 16:17:21 +0700 Subject: [PATCH 006/204] feat(fx): CEO console button to fetch BOT FX rates on demand MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FX rates only refreshed on the 18:00 scheduled BotFxFetchService.fetchDailyRates, so a CEO who just set BOT_FX_API_TOKEN had to wait for the schedule. Add a manual trigger. Backend: - BotFxFetchService: extract fetchNow() (returns FxFetchResult{updated,total,asOf, updatedCurrencies}) from the @Scheduled path; the schedule now delegates to it and swallows the "not configured" refusal as a warn (a cron must not throw). The manual path THROWS 503 on a missing token so the caller learns the config is incomplete. - FxRateController: POST /api/fx-rates/fetch-now, CEO-only (same write gate as upsert, since it writes rates). Frontend: - routes/hrApi/mockApi: fxRates.fetchNow(); mock SIMULATES a successful fetch (refreshes tracked rows to today, source=BOT) — it cannot call BOT, stated in a comment. - CeoSettingsPage: "ดึงเรตจาก BOT ตอนนี้" button in the FX panel header → fetch, toast "อัปเดต N/5 สกุล", refetch. Files: BotFxFetchService.java, FxRateController.java, FxRateControllerTest.java, CatalogPricingReadAuthzIntegrationTest.java (constructor arg), routes.js, hrApi.js, mockApi.js, CeoSettingsPage.jsx. Commands/tests: backend compile OK; FxRateControllerTest OK (added cases: ceo→2xx, every other role→403, no session→401); frontend lint 0 err, contract.test.js 6/6, build OK. Authz: new endpoint is a flat CEO-only role gate with no row-scoping — proven by the MockMvc decision cases above (same pattern as the existing upsert gate). No scope/WHERE filter, so no separate real-DB integration test required. Co-Authored-By: Claude Opus 4.8 --- .../co/glr/hr/pricing/BotFxFetchService.java | 39 ++++++++++++++++--- .../co/glr/hr/pricing/FxRateController.java | 18 ++++++++- ...atalogPricingReadAuthzIntegrationTest.java | 3 +- .../glr/hr/pricing/FxRateControllerTest.java | 31 ++++++++++++++- frontend/src/api/hrApi.js | 1 + frontend/src/api/mockApi.js | 22 +++++++++++ frontend/src/api/routes.js | 1 + .../features/ceoSettings/CeoSettingsPage.jsx | 30 +++++++++++++- 8 files changed, 136 insertions(+), 9 deletions(-) diff --git a/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java b/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java index 5b50914b9..5fac963d9 100644 --- a/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java +++ b/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java @@ -5,12 +5,15 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.math.BigDecimal; import java.time.LocalDate; +import java.util.ArrayList; import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.http.HttpStatus; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; import org.springframework.web.client.RestClient; +import th.co.glr.hr.common.ApiException; import th.co.glr.hr.config.AppProperties; /** @@ -42,18 +45,43 @@ public BotFxFetchService(FxRateRepository fxRates, AppProperties props, this.objectMapper = objectMapper; } + /** + * What a fetch changed: how many of the {@link #TRACKED_CURRENCIES} got a rate written, out of + * how many were attempted, for which BOT publication date. Returned by {@link #fetchNow} so the + * CEO console can report "อัปเดต N/5 สกุล" instead of guessing from a bare 200. + */ + public record FxFetchResult(int updated, int total, String asOf, List updatedCurrencies) {} + + /** + * The daily scheduled fetch. Delegates to {@link #fetchNow} and swallows its "not configured" + * refusal as a warn — a {@code @Scheduled} method must never throw. Behaviour with a configured + * token is byte-for-byte what it always was. + */ @Scheduled(cron = "0 0 18 * * *", zone = "Asia/Bangkok") public void fetchDailyRates() { + try { + fetchNow(); + } catch (ApiException e) { + log.warn("BOT FX scheduled fetch skipped: {}", e.getMessage()); + } + } + + /** + * Fetch today's BOT rates on demand (the CEO console's "ดึงเรตตอนนี้" button), returning what + * changed. Unlike the scheduled path this THROWS when the token is missing, so a manual caller + * is told the config is incomplete rather than silently doing nothing. + */ + public FxFetchResult fetchNow() { String token = props.getBot().getFxApiToken(); if (token == null || token.isBlank()) { - log.warn("BOT_FX_API_TOKEN not configured — skipping FX auto-fetch"); - return; + throw new ApiException(HttpStatus.SERVICE_UNAVAILABLE, + "ยังไม่ได้ตั้งค่า BOT_FX_API_TOKEN — ตั้งค่าก่อนจึงจะดึงอัตราจาก BOT ได้"); } LocalDate today = LocalDate.now(); String dateStr = today.toString(); // yyyy-MM-dd - int fetched = 0; + List updated = new ArrayList<>(); for (String currency : TRACKED_CURRENCIES) { try { String url = BOT_BASE_URL + "?start_period=" + dateStr + "&end_period=" + dateStr @@ -70,13 +98,14 @@ public void fetchDailyRates() { continue; } fxRates.upsertFromBot(currency, rate, today); - fetched++; + updated.add(currency); log.info("BOT FX: updated {} = {} THB ({})", currency, rate, dateStr); } catch (Exception e) { log.warn("BOT FX: failed to fetch {} — {}", currency, e.getMessage()); } } - log.info("BOT FX fetch completed: {}/{} currencies updated", fetched, TRACKED_CURRENCIES.size()); + log.info("BOT FX fetch completed: {}/{} currencies updated", updated.size(), TRACKED_CURRENCIES.size()); + return new FxFetchResult(updated.size(), TRACKED_CURRENCIES.size(), dateStr, updated); } private BigDecimal parseRate(String json) throws Exception { diff --git a/backend/src/main/java/th/co/glr/hr/pricing/FxRateController.java b/backend/src/main/java/th/co/glr/hr/pricing/FxRateController.java index d020d6ab5..2219fdfa7 100644 --- a/backend/src/main/java/th/co/glr/hr/pricing/FxRateController.java +++ b/backend/src/main/java/th/co/glr/hr/pricing/FxRateController.java @@ -8,6 +8,7 @@ import org.springframework.http.HttpStatus; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestMapping; @@ -58,10 +59,12 @@ public class FxRateController { private final FxRateRepository fxRates; private final SessionContext sessions; + private final BotFxFetchService botFx; - public FxRateController(FxRateRepository fxRates, SessionContext sessions) { + public FxRateController(FxRateRepository fxRates, SessionContext sessions, BotFxFetchService botFx) { this.fxRates = fxRates; this.sessions = sessions; + this.botFx = botFx; } /** Read is gated to {@link #READ_ROLES} — see that field for the ruling and its scope. */ @@ -88,6 +91,19 @@ Map upsert( return Map.of("fxRate", result); } + /** + * Trigger the BOT FX fetch on demand instead of waiting for the 18:00 schedule. CEO-only, the + * same gate as {@link #upsert} — this WRITES rates (via {@code BotFxFetchService.fetchNow}). + * Returns what changed so the console can report "อัปเดต N/5 สกุล"; a missing token surfaces as + * the service's own 503, not a silent no-op. + */ + @PostMapping("/fetch-now") + Map fetchNow(HttpSession session) { + UserPrincipal user = sessions.requireUser(session); + requireCeoRole(user); + return Map.of("result", botFx.fetchNow()); + } + private void requireCeoRole(UserPrincipal user) { if (!CEO_ROLES.contains(user.role())) { throw new ApiException(HttpStatus.FORBIDDEN, "เฉพาะ CEO เท่านั้น"); diff --git a/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java index 11265593f..541e831c1 100644 --- a/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java @@ -118,7 +118,8 @@ void wireRealCollaborators() { sessions), jsonMapper); priceConfigMvc = standalone( new PriceCalcConfigController(new PriceCalcConfigRepository(jdbc), sessions), jsonMapper); - fxMvc = standalone(new FxRateController(new FxRateRepository(jdbc), sessions), jsonMapper); + fxMvc = standalone(new FxRateController( + new FxRateRepository(jdbc), sessions, mock(th.co.glr.hr.pricing.BotFxFetchService.class)), jsonMapper); factoryMvc = standalone( new FactoryConfigController(new FactoryConfigRepository(jdbc), sessions), jsonMapper); diff --git a/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java b/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java index ec0bd5336..c3ff85e86 100644 --- a/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java +++ b/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java @@ -3,6 +3,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -45,9 +46,14 @@ class FxRateControllerTest { private static final List READ_DENIED = List.of("employee", "warehouse", "qc", "hr", "sales_manager", "account"); + /** Everyone but CEO — the write gate that /fetch-now shares with upsert. */ + private static final List WRITE_DENIED = + List.of("import", "sales", "sales_manager", "account", "hr", "employee", "warehouse", "qc"); + private final FxRateRepository fxRates = mock(FxRateRepository.class); + private final BotFxFetchService botFx = mock(BotFxFetchService.class); private final MockMvc mvc = MockMvcBuilders - .standaloneSetup(new FxRateController(fxRates, new SessionContext())) + .standaloneSetup(new FxRateController(fxRates, new SessionContext(), botFx)) .setControllerAdvice(new ApiExceptionHandler()) .build(); @@ -99,6 +105,29 @@ void salesCannotUpsertARate() throws Exception { .andExpect(status().isForbidden()); } + /** The manual FX fetch is a WRITE (it upserts rates), so it stays on the CEO write gate. */ + @Test + void ceoCanTriggerManualFxFetch() throws Exception { + when(botFx.fetchNow()) + .thenReturn(new BotFxFetchService.FxFetchResult(2, 5, "2026-09-24", List.of("USD", "EUR"))); + mvc.perform(post("/api/fx-rates/fetch-now").session(session("ceo"))) + .andExpect(status().is2xxSuccessful()); + } + + /** Wrong-way-round: everyone but CEO must be refused — the case that catches a widened gate. */ + @Test + void manualFxFetchIsForbiddenForEveryoneButCeo() throws Exception { + for (String role : WRITE_DENIED) { + mvc.perform(post("/api/fx-rates/fetch-now").session(session(role))) + .andExpect(status().isForbidden()); + } + } + + @Test + void manualFxFetchIsUnauthorizedWithoutASession() throws Exception { + mvc.perform(post("/api/fx-rates/fetch-now")).andExpect(status().isUnauthorized()); + } + private MockHttpSession session(String role) { MockHttpSession session = new MockHttpSession(); session.setAttribute(SessionContext.SESSION_USER_KEY, diff --git a/frontend/src/api/hrApi.js b/frontend/src/api/hrApi.js index afec09866..68f4dad14 100644 --- a/frontend/src/api/hrApi.js +++ b/frontend/src/api/hrApi.js @@ -635,6 +635,7 @@ export const api = { fxRates: { list: () => apiRequest(API_ROUTES.fxRates.list), upsert: (currency, payload) => apiRequest(API_ROUTES.fxRates.upsert(currency), { method: 'PUT', body: payload }), + fetchNow: () => apiRequest(API_ROUTES.fxRates.fetchNow, { method: 'POST' }), }, priceCalcConfigs: { list: () => apiRequest(API_ROUTES.priceCalcConfigs.list), diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 0bdf82f39..f20eb76d9 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -11443,6 +11443,28 @@ export const api = { mockFxRates.push(newRate); return delay({ fxRate: structuredClone(newRate) }); }, + // Mirrors FxRateController.fetchNow -> BotFxFetchService.fetchNow (CEO-only). The real path + // calls the Bank of Thailand API; the mock cannot, so this SIMULATES a successful fetch: + // it refreshes each tracked currency's row to today with source=BOT (clearing the staleness + // warning) without inventing new rates, and returns the same {result} shape. Not a rate + // computation — just plumbing so the button, toast and refetch are exercisable in mock mode. + async fetchNow() { + hasRole('ceo'); + const tracked = ['USD', 'EUR', 'JPY', 'CNY', 'GBP']; + const asOf = new Date().toISOString().slice(0, 10); + const now = new Date().toISOString(); + const updated = []; + for (const currency of tracked) { + const row = mockFxRates.find((r) => r.currency === currency); + if (!row) continue; + row.effectiveDate = asOf; + row.updatedAt = now; + row.fetchedAt = now; + row.source = 'BOT'; + updated.push(currency); + } + return delay({ result: { updated: updated.length, total: tracked.length, asOf, updatedCurrencies: updated } }); + }, }, // Mirrors PriceCalcConfigController (pricing/). diff --git a/frontend/src/api/routes.js b/frontend/src/api/routes.js index bada38889..11858b2d7 100644 --- a/frontend/src/api/routes.js +++ b/frontend/src/api/routes.js @@ -298,6 +298,7 @@ export const API_ROUTES = { fxRates: { list: '/api/fx-rates', upsert: (currency) => `/api/fx-rates/${currency}`, + fetchNow: '/api/fx-rates/fetch-now', }, priceCalcConfigs: { list: '/api/price-calc-configs', diff --git a/frontend/src/features/ceoSettings/CeoSettingsPage.jsx b/frontend/src/features/ceoSettings/CeoSettingsPage.jsx index 118f0dad1..c267e4f65 100644 --- a/frontend/src/features/ceoSettings/CeoSettingsPage.jsx +++ b/frontend/src/features/ceoSettings/CeoSettingsPage.jsx @@ -763,6 +763,20 @@ export function CeoSettingsPage({ showToast }) { onError: (e) => showToast('error', e.message || 'บันทึกไม่สำเร็จ'), }); + // Trigger the BOT FX fetch on demand instead of waiting for the 18:00 schedule + // (POST /api/fx-rates/fetch-now — CEO-only, same gate as the manual upsert above). + const fetchFxNowMutation = useMutation({ + mutationFn: () => api.fxRates.fetchNow(), + onSuccess: (data) => { + queryClient.invalidateQueries({ queryKey: queryKeys.fxRates() }); + const r = data?.result; + showToast('success', r + ? `ดึงเรตจาก BOT แล้ว — อัปเดต ${r.updated}/${r.total} สกุล (${r.asOf})` + : 'ดึงเรตจาก BOT แล้ว'); + }, + onError: (e) => showToast('error', e.message || 'ดึงเรตจาก BOT ไม่สำเร็จ'), + }); + // A single shared mutation acts on whichever currency row is being saved — // this reads the in-flight variables back out to know which row's button // should show a busy state (there's no per-row mutation instance). @@ -851,7 +865,21 @@ export function CeoSettingsPage({ showToast }) { {/* FX Rates */} - + fetchFxNowMutation.mutate()} + data-testid="fx-fetch-now" + > + {fetchFxNowMutation.isPending ? 'กำลังดึงเรต…' : 'ดึงเรตจาก BOT ตอนนี้'} + + )} + > {/* P0 fix follow-up (2026-09): this used to read "...ติดต่อผู้ดูแลระบบหากยังไม่เปิดใช้งาน การดึงอัตราอัตโนมัติ", which implied the CEO had to wait on an admin before costing would work at all. That is no longer true — FxResolver no longer requires source === From b425b12122b04f8d57876b004b7e43471e4128ae Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Mon, 28 Sep 2026 03:22:00 +0700 Subject: [PATCH 007/204] test(fx): regen API digests for POST /api/fx-rates/fetch-now; real-stack CEO-only authz case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /api/fx-rates/fetch-now was added without regenerating the two API-surface digests this repo relies on (docs/api/api-surface.json, docs/api/ui-reachable.json), the same trap PR #1061 hit — ApiSurfaceContractTest and serverContract.test.js were red on the stale digests. Regenerated both (+1 operation each, 359→360 / 299→300). Also adds a real-Postgres, wrong-way-round authz test for the new endpoint in CatalogPricingReadAuthzIntegrationTest: sales/import (both in FxRateController.READ_ROLES, proving the read-widening does not leak into this write) are refused and BotFxFetchService is proven never invoked for either; ceo is the positive control reaching the real controller + real FxRateRepository path. Also fixes a dangling {@link #READ_ROLES} javadoc reference in that same file (no such field on this test class) and replaces fully-qualified th.co.glr.hr.pricing.BotFxFetchService references with an import. Co-Authored-By: Claude Fable 5.1 --- ...atalogPricingReadAuthzIntegrationTest.java | 41 ++++++++++++++++++- docs/api/api-surface.json | 3 +- docs/api/ui-reachable.json | 3 +- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java index 541e831c1..f73d9254d 100644 --- a/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/catalog/CatalogPricingReadAuthzIntegrationTest.java @@ -1,7 +1,12 @@ package th.co.glr.hr.catalog; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -24,6 +29,7 @@ import th.co.glr.hr.common.ApiExceptionHandler; import th.co.glr.hr.factory.FactoryConfigController; import th.co.glr.hr.factory.FactoryConfigRepository; +import th.co.glr.hr.pricing.BotFxFetchService; import th.co.glr.hr.pricing.FxRateController; import th.co.glr.hr.pricing.FxRateRepository; import th.co.glr.hr.pricing.PriceCalcConfigController; @@ -90,6 +96,7 @@ class CatalogPricingReadAuthzIntegrationTest extends AbstractPostgresIntegration private MockMvc priceConfigMvc; private MockMvc fxMvc; private MockMvc factoryMvc; + private BotFxFetchService botFxFetchService; @BeforeEach void wireRealCollaborators() { @@ -118,8 +125,9 @@ void wireRealCollaborators() { sessions), jsonMapper); priceConfigMvc = standalone( new PriceCalcConfigController(new PriceCalcConfigRepository(jdbc), sessions), jsonMapper); + botFxFetchService = mock(BotFxFetchService.class); fxMvc = standalone(new FxRateController( - new FxRateRepository(jdbc), sessions, mock(th.co.glr.hr.pricing.BotFxFetchService.class)), jsonMapper); + new FxRateRepository(jdbc), sessions, botFxFetchService), jsonMapper); factoryMvc = standalone( new FactoryConfigController(new FactoryConfigRepository(jdbc), sessions), jsonMapper); @@ -238,6 +246,37 @@ void salesCannotWriteFxRatesDespiteTheReadWidening() throws Exception { .andExpect(status().isForbidden()); } + /** + * {@code POST /api/fx-rates/fetch-now} (added 2026-09-24, commit 94e258c9 — CEO console + * "ดึงเรตจาก BOT ตอนนี้" button) is gated exactly like {@link + * FxRateController#upsert}: CEO-only, because it writes rates too. Written wrong-way-round + * per CLAUDE.md: {@code sales} and {@code import} — both members of {@code + * FxRateController.READ_ROLES}, so + * this proves the read widening does not leak into the new write — are refused, and the + * refusal is proven to reach no further than the role gate by asserting {@code + * BotFxFetchService} was never invoked for either of them (a Mockito verification, not + * merely the 403 status). {@code ceo} is the positive control: it reaches the real write + * path and the service's result is echoed back through the real controller. + */ + @Test + void fetchNowIsCeoOnly_forbiddenRolesNeverInvokeTheBotFetch_ceoDoes() throws Exception { + for (String role : List.of("sales", "import")) { + fxMvc.perform(post("/api/fx-rates/fetch-now").session(sessionFor(role))) + .andExpect(status().isForbidden()); + } + verifyNoInteractions(botFxFetchService); + + when(botFxFetchService.fetchNow()).thenReturn(new BotFxFetchService.FxFetchResult( + 3, 5, LocalDate.now().toString(), List.of("USD", "EUR", "JPY"))); + + fxMvc.perform(post("/api/fx-rates/fetch-now").session(sessionFor("ceo"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.result.updated").value(3)) + .andExpect(jsonPath("$.result.total").value(5)); + + verify(botFxFetchService, times(1)).fetchNow(); + } + @Test void nonCostingRolesCannotReadTheSupplierDirectory() throws Exception { for (String role : NON_COSTING_ROLES) { diff --git a/docs/api/api-surface.json b/docs/api/api-surface.json index e36574326..79b3ab46d 100644 --- a/docs/api/api-surface.json +++ b/docs/api/api-surface.json @@ -1,7 +1,7 @@ { "$comment" : "GENERATED by ApiSurfaceContractTest — do not hand-edit. Regenerate: cd backend && ./mvnw -Dtest=ApiSurfaceContractTest -Dapi.surface.update=true test", "source" : "springdoc /v3/api-docs, generated from Spring's handler mappings", - "count" : 359, + "count" : 360, "deprecatedCount" : 3, "operations" : [ "DELETE /api/attachments/{id}", @@ -227,6 +227,7 @@ "POST /api/factory-quotes/{factoryQuoteId}/receive", "POST /api/factory-quotes/{factoryQuoteId}/send", "POST /api/factory-quotes/{factoryQuoteId}/start-negotiation", + "POST /api/fx-rates/fetch-now", "POST /api/holidays", "POST /api/holidays/fetch", "POST /api/import-requests/{id}/advance-step", diff --git a/docs/api/ui-reachable.json b/docs/api/ui-reachable.json index 4ecca9d7c..72c3f36fd 100644 --- a/docs/api/ui-reachable.json +++ b/docs/api/ui-reachable.json @@ -1,7 +1,7 @@ { "$comment": "GENERATED by src/api/serverContract.test.js — do not hand-edit. Regenerate: cd frontend && UI_REACHABLE_UPDATE=1 npx vitest run src/api/serverContract.test.js", "source": "endpoints reachable from a component call site (api..( outside src/api/ and outside tests), resolved through hrApi by driving every method", - "count": 299, + "count": 300, "operations": [ "DELETE /api/attachments/{id}", "DELETE /api/catalog/prices/{priceId}", @@ -185,6 +185,7 @@ "POST /api/factory-quotes/{factoryQuoteId}/receive", "POST /api/factory-quotes/{factoryQuoteId}/send", "POST /api/factory-quotes/{factoryQuoteId}/start-negotiation", + "POST /api/fx-rates/fetch-now", "POST /api/holidays", "POST /api/holidays/fetch", "POST /api/import-requests/{id}/advance-step", From 0310a15a1c97d8c5718710392b33468139281f80 Mon Sep 17 00:00:00 2001 From: waritctd <148909369+waritctd@users.noreply.github.com> Date: Mon, 28 Sep 2026 03:22:19 +0700 Subject: [PATCH 008/204] fix(fx): 10-min cooldown + Bangkok date on manual BOT fetch; honest 0/N toast; tests Opus review of the CEO-console manual FX fetch returned MERGE WITH FIXES. Applies each, mirroring the sibling BotHolidayFetchService/HolidaysTab implementation that had already solved the same problems: - B1 (blocking): BotFxFetchService.fetchNow() had no throttle, so repeated CEO-console clicks (5 BOT calls each) could exhaust BOT's ~100-calls/hour/token budget and make the 18:00 scheduled fetchDailyRates 429 silently. Added MANUAL_FETCH_COOLDOWN (10 min) with the same volatile-Instant/429-ApiException shape as BotHolidayFetchService, whose message frontend/.../holidayFetch.js's parseCooldownSeconds already parses. Guards the manual path only: fetchDailyRates now calls a new private doFetch() directly, never through fetchNow(), so a manual click can never suppress the scheduled fetch. - S3: doFetch() used LocalDate.now() (JVM default = UTC in prod), so a click at 01:00 Bangkok time queried/wrote yesterday's date. Added the same Clock.system(Asia/Bangkok) seam BotHolidayFetchService uses. - S1: CeoSettingsPage's onSuccess always fired a success toast; updated === 0 is the dominant case (BOT publishes late in the day) and also what a bad token looks like. Now branches: 0 -> info toast, otherwise -> the existing success wording. - S2: added frontend coverage for the fetch-now button (click-once, pending/disabled, success N/5, honest zero, 429 cooldown), modelled on HolidaysTab.test.jsx. - N2/N3: mockApi.js's hardcoded currency list now cites BotFxFetchService.TRACKED_CURRENCIES as its source; FxResolver's javadoc naming fetchDailyRates "the only writer" now also names the new manual writer. Mutation-checked both guards: disabling CeoSettingsPage's 0/N branch turns exactly one frontend test red; disabling BotFxFetchService's cooldown check turns exactly two backend tests red. Both restored and re-verified green. Co-Authored-By: Claude Fable 5.1 --- .../co/glr/hr/pricing/BotFxFetchService.java | 105 +++++++- .../java/th/co/glr/hr/pricing/FxResolver.java | 6 +- .../glr/hr/pricing/BotFxFetchServiceTest.java | 236 ++++++++++++++++++ .../glr/hr/pricing/FxRateControllerTest.java | 17 ++ frontend/src/api/mockApi.js | 3 + .../features/ceoSettings/CeoSettingsPage.jsx | 71 +++++- .../ceoSettings/CeoSettingsPage.test.jsx | 88 +++++++ 7 files changed, 511 insertions(+), 15 deletions(-) create mode 100644 backend/src/test/java/th/co/glr/hr/pricing/BotFxFetchServiceTest.java diff --git a/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java b/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java index 5fac963d9..bcc50da36 100644 --- a/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java +++ b/backend/src/main/java/th/co/glr/hr/pricing/BotFxFetchService.java @@ -4,11 +4,16 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.ObjectMapper; import java.math.BigDecimal; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; import java.time.LocalDate; +import java.time.ZoneId; import java.util.ArrayList; import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.stereotype.Service; @@ -23,26 +28,76 @@ * BotHolidayFetchService} uses; there is deliberately no fallback between the two (see {@code * AppProperties.Bot}'s javadoc for why). * Falls back silently when BOT has not published today's rate yet. + * + *

Review fix (2026-09-28), mirroring {@code BotHolidayFetchService} exactly — the + * CEO console's manual "ดึงเรตจาก BOT ตอนนี้" trigger ({@link #fetchNow}, added by commit 94e258c9) + * originally had no throttle of its own. BOT enforces the same ~100 calls/hour per token budget + * this class's sibling documents; 20 clicks (this class makes 5 BOT calls per {@link #fetchNow} + * invocation, one per {@link #TRACKED_CURRENCIES}) exhausts that budget, and the 18:00 {@link + * #fetchDailyRates} scheduled run would then 429 — silently, since the per-currency loop below + * catches {@code Exception} and only logs a WARN. {@link #MANUAL_FETCH_COOLDOWN} guards the manual + * path only, exactly as {@code BotHolidayFetchService.MANUAL_FETCH_COOLDOWN} does — see that + * class's javadoc for the full reasoning (in-memory/per-instance, not a distributed limiter, + * deliberately). {@link #fetchDailyRates} calls the shared fetch core directly and never touches + * the cooldown, so a manual click can never suppress the scheduled fetch. + * + *

Also picked up a {@link Clock} seam (Bangkok-zoned in production, exactly like {@code + * BotHolidayFetchService}) so {@link #fetchNow}'s "today" is always the Bangkok calendar day, not + * the JVM default (production runs UTC — no TZ set in the Dockerfile/render.yaml — so a CEO + * clicking at 01:00 Bangkok time, which is still 18:00 UTC the PREVIOUS day, would otherwise query + * BOT for and write {@code effective_date} as yesterday). */ @Service public class BotFxFetchService { private static final Logger log = LoggerFactory.getLogger(BotFxFetchService.class); + private static final ZoneId BANGKOK = ZoneId.of("Asia/Bangkok"); private static final String BOT_BASE_URL = "https://gateway.api.bot.or.th/Stat-ExchangeRate/v2/DAILY_AVG_EXG_RATE/"; private static final List TRACKED_CURRENCIES = List.of("USD", "EUR", "JPY", "CNY", "GBP"); + /** + * Minimum interval between {@link #fetchNow()} attempts — mirrors {@code + * BotHolidayFetchService.MANUAL_FETCH_COOLDOWN} exactly, including its reasoning: BOT's + * ~100-calls/hour-per-token budget, a separate token per API/environment (see {@code + * AppProperties.Bot}'s javadoc), and this guard existing solely to bound a human hammering the + * manual trigger, not as a distributed limiter (in-memory, per-JVM-instance state; a restart + * forgets it; N instances allow N times this rate — accepted, see the sibling class for why). + * This class's per-click cost is higher than the holiday fetcher's (5 BOT calls per {@link + * #fetchNow} invocation vs. its 1-2), so the same 10-minute window bounds a single instance to + * at most 6 manual attempts/hour = 30 BOT calls/hour from this path alone, still comfortably + * inside the 100/hour budget even stacked on the 18:00 scheduled run's own 5 calls. + */ + static final Duration MANUAL_FETCH_COOLDOWN = Duration.ofMinutes(10); + private final FxRateRepository fxRates; private final AppProperties props; private final RestClient restClient; private final ObjectMapper objectMapper; + private final Clock clock; + + /** When {@link #fetchNow()} last attempted a fetch, for {@link #MANUAL_FETCH_COOLDOWN}. Never + * touched by {@link #fetchDailyRates()} — see that method's javadoc for why. */ + private volatile Instant lastManualFetchAttemptAt; + @Autowired public BotFxFetchService(FxRateRepository fxRates, AppProperties props, RestClient.Builder restClientBuilder, ObjectMapper objectMapper) { + this(fxRates, props, restClientBuilder, objectMapper, Clock.system(BANGKOK)); + } + + /** + * Test-only seam: lets {@code BotFxFetchServiceTest} inject a fixed {@link Clock} so both the + * {@link #MANUAL_FETCH_COOLDOWN} and the Bangkok-date behaviour are deterministic instead of + * racing the real wall clock. Mirrors {@code BotHolidayFetchService}'s identical constructor. + */ + BotFxFetchService(FxRateRepository fxRates, AppProperties props, + RestClient.Builder restClientBuilder, ObjectMapper objectMapper, Clock clock) { this.fxRates = fxRates; this.props = props; this.restClient = restClientBuilder.build(); this.objectMapper = objectMapper; + this.clock = clock; } /** @@ -53,32 +108,66 @@ public BotFxFetchService(FxRateRepository fxRates, AppProperties props, public record FxFetchResult(int updated, int total, String asOf, List updatedCurrencies) {} /** - * The daily scheduled fetch. Delegates to {@link #fetchNow} and swallows its "not configured" - * refusal as a warn — a {@code @Scheduled} method must never throw. Behaviour with a configured - * token is byte-for-byte what it always was. + * The daily scheduled fetch. Calls {@link #doFetch()} directly, not {@link + * #fetchNow()} — exactly like {@code BotHolidayFetchService.scheduledFetch()} — so this + * never checks or updates {@link #MANUAL_FETCH_COOLDOWN}: a CEO clicking the manual button + * moments before 18:00 must never suppress that day's scheduled fetch. Swallows the "not + * configured" refusal as a warn — a {@code @Scheduled} method must never throw. Behaviour with a + * configured token is byte-for-byte what it always was. */ @Scheduled(cron = "0 0 18 * * *", zone = "Asia/Bangkok") public void fetchDailyRates() { try { - fetchNow(); + doFetch(); } catch (ApiException e) { log.warn("BOT FX scheduled fetch skipped: {}", e.getMessage()); } } /** - * Fetch today's BOT rates on demand (the CEO console's "ดึงเรตตอนนี้" button), returning what - * changed. Unlike the scheduled path this THROWS when the token is missing, so a manual caller - * is told the config is incomplete rather than silently doing nothing. + * Manual-trigger entry point for {@code FxRateController} (the CEO console's "ดึงเรตจาก BOT + * ตอนนี้" button) — see that controller for the role gate. Enforces {@link + * #MANUAL_FETCH_COOLDOWN}: a call within the cooldown of the previous manual attempt throws a + * 429 {@link ApiException} without making any BOT call, rather than silently + * doing nothing. The cooldown clock starts on the attempt itself (before the token check or any + * network call), so a misconfigured/blank token cannot be used to bypass it by retrying in a + * loop. Delegates to {@link #doFetch()} for the actual fetch, which THROWS a 503 when the token + * is missing, so a manual caller is told the config is incomplete rather than silently doing + * nothing. */ public FxFetchResult fetchNow() { + Instant now = clock.instant(); + Instant previous = lastManualFetchAttemptAt; + if (previous != null) { + Duration sinceLast = Duration.between(previous, now); + if (sinceLast.compareTo(MANUAL_FETCH_COOLDOWN) < 0) { + long secondsLeft = Math.max(1, MANUAL_FETCH_COOLDOWN.minus(sinceLast).toSeconds()); + throw new ApiException(HttpStatus.TOO_MANY_REQUESTS, + "รอสักครู่ก่อนเรียกดึงอัตราแลกเปลี่ยนจากธนาคารแห่งประเทศไทยอีกครั้ง (ประมาณ " + + secondsLeft + " วินาที)"); + } + } + lastManualFetchAttemptAt = now; + return doFetch(); + } + + /** + * Fetches today's (Bangkok calendar day — see {@link #clock}) BOT rates. The shared core both + * {@link #fetchDailyRates()} and {@link #fetchNow()} delegate to after their own, + * different, gating decisions — this method itself has no rate-limiting logic. + */ + private FxFetchResult doFetch() { String token = props.getBot().getFxApiToken(); if (token == null || token.isBlank()) { throw new ApiException(HttpStatus.SERVICE_UNAVAILABLE, "ยังไม่ได้ตั้งค่า BOT_FX_API_TOKEN — ตั้งค่าก่อนจึงจะดึงอัตราจาก BOT ได้"); } - LocalDate today = LocalDate.now(); + // Bangkok calendar day, not the JVM default -- production runs UTC (no TZ set in the + // Dockerfile/render.yaml), so LocalDate.now() at 01:00 Bangkok (= 18:00 UTC the PREVIOUS + // day) would query BOT for, and write effective_date as, yesterday. Mirrors + // BotHolidayFetchService's identical Clock-based fix. + LocalDate today = LocalDate.now(clock); String dateStr = today.toString(); // yyyy-MM-dd List updated = new ArrayList<>(); diff --git a/backend/src/main/java/th/co/glr/hr/pricing/FxResolver.java b/backend/src/main/java/th/co/glr/hr/pricing/FxResolver.java index f2a141949..22e49a9d4 100644 --- a/backend/src/main/java/th/co/glr/hr/pricing/FxResolver.java +++ b/backend/src/main/java/th/co/glr/hr/pricing/FxResolver.java @@ -31,8 +31,10 @@ * system) is direct evidence the gate fired exactly as that reasoning predicts. * *

A separate, weaker argument for the same conclusion is deliberately NOT what this rests on, - * and is named here only to correct it: {@code BotFxFetchService#fetchDailyRates} (the only writer - * of a BOT-sourced row, via {@link FxRateRepository#upsertFromBot}) returns early whenever + * and is named here only to correct it: {@code BotFxFetchService#fetchDailyRates} (the scheduled + * writer of a BOT-sourced row, via {@link FxRateRepository#upsertFromBot} — {@code POST + * /api/fx-rates/fetch-now}, added 2026-09-24, is the other, manual writer of the same BOT-sourced + * shape) returns early whenever * {@code BOT_FX_API_TOKEN} is unset, and {@code render.yaml} marks that key {@code sync: false} * with its own checked-in comment recording both BOT fetchers as no-op'ing in this environment. * That comment is exactly the shape of claim CLAUDE.md warns against reasoning from: {@code sync: diff --git a/backend/src/test/java/th/co/glr/hr/pricing/BotFxFetchServiceTest.java b/backend/src/test/java/th/co/glr/hr/pricing/BotFxFetchServiceTest.java new file mode 100644 index 000000000..c9ebc496e --- /dev/null +++ b/backend/src/test/java/th/co/glr/hr/pricing/BotFxFetchServiceTest.java @@ -0,0 +1,236 @@ +package th.co.glr.hr.pricing; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.ObjectMapper; +import java.math.BigDecimal; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.time.LocalDate; +import java.time.ZoneId; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.http.HttpStatus; +import org.springframework.web.client.RestClient; +import th.co.glr.hr.common.ApiException; +import th.co.glr.hr.config.AppProperties; + +/** + * Review fix (2026-09-28): {@link BotFxFetchService#fetchNow()} previously had no throttle at all, + * so 20 CEO-console clicks could exhaust BOT's ~100-calls/hour-per-token budget and make the 18:00 + * {@link BotFxFetchService#fetchDailyRates()} 429 silently. This mirrors {@code + * BotHolidayFetchServiceTest}'s cooldown/clock coverage exactly, against the sibling class's own + * {@link BotFxFetchService#MANUAL_FETCH_COOLDOWN} and Bangkok-zoned {@link Clock} seam. + * + *

No real HTTP is used anywhere in this class: {@link RestClient} is mocked at every step of its + * fluent chain (see {@link #mockRestClient}), so "no BOT call made" is provable by counting + * invocations of the mock, not merely inferred from a blank token short-circuiting before the loop. + */ +class BotFxFetchServiceTest { + + private static final ZoneId BANGKOK = ZoneId.of("Asia/Bangkok"); + + /** A BOT response carrying a real, parseable rate for every currency queried (the mocked + * {@link RestClient} returns this same body regardless of which currency was requested — the + * URI itself, captured separately, is what proves which currency/date was actually asked for). */ + private static final String VALID_BOT_JSON = """ + {"result": {"data": {"data_detail": [{"currency_id": "USD", "selling": "36.5000"}]}}} + """; + + /** + * (a) — the case B1 exists for: a second manual call inside the 10-minute window must be + * refused with a 429 carrying the remaining-seconds message {@code + * frontend/src/features/attendanceCalendar/holidayFetch.js}'s {@code parseCooldownSeconds} + * parses (the pattern is generic — any Thai message containing "N วินาที" — but this also pins + * the exact wording), and — the part a blank-token-only test could never prove — must make + * no BOT call at all for that second, refused attempt. A real token and a fully + * mocked {@link RestClient} chain are used specifically so this is provable by counting + * invocations, not merely inferred from the token check short-circuiting. + */ + @Test + void cooldownBlocksASecondImmediateManualCallAndMakesNoBotCall() { + MutableClock clock = new MutableClock(Instant.parse("2026-06-01T10:00:00Z")); + MockedRestClient rest = mockRestClient(VALID_BOT_JSON); + BotFxFetchService service = serviceWithClock(mock(FxRateRepository.class), rest.client, clock, "fake-token"); + + // First attempt: allowed through, and it does reach BOT (proves the mock is wired, not + // merely never called for an unrelated reason) -- one GET per TRACKED_CURRENCIES entry. + service.fetchNow(); + verify(rest.client, times(5)).get(); + + // Second call, same instant: refused before any further BOT interaction, and the message + // carries the remaining-seconds figure holidayFetch.js's parseCooldownSeconds parses. + assertThatThrownBy(service::fetchNow) + .isInstanceOf(ApiException.class) + .extracting(ex -> ((ApiException) ex).getStatus()) + .isEqualTo(HttpStatus.TOO_MANY_REQUESTS); + assertThatThrownBy(service::fetchNow) + .hasMessageMatching(".*\\(ประมาณ \\d+ วินาที\\).*"); + + // Still exactly 5 -- the refused calls above made no additional BOT request. + verify(rest.client, times(5)).get(); + } + + /** (b) — a call after the cooldown interval has elapsed proceeds normally, reaching BOT again. */ + @Test + void aCallAfterTheCooldownIntervalProceeds() { + MutableClock clock = new MutableClock(Instant.parse("2026-06-01T10:00:00Z")); + MockedRestClient rest = mockRestClient(VALID_BOT_JSON); + BotFxFetchService service = serviceWithClock(mock(FxRateRepository.class), rest.client, clock, "fake-token"); + service.fetchNow(); + + clock.advance(BotFxFetchService.MANUAL_FETCH_COOLDOWN.plusSeconds(1)); + + assertThatCode(service::fetchNow).doesNotThrowAnyException(); + verify(rest.client, times(10)).get(); // 5 currencies x 2 successful invocations + } + + /** + * (c) — the interaction B1 specifically calls out: {@link BotFxFetchService#fetchDailyRates()} + * must run its fetch logic even when a manual attempt happened moments earlier and would itself + * be well within {@link BotFxFetchService#MANUAL_FETCH_COOLDOWN}. Proven positively -- the + * scheduled call must still reach BOT (a real interaction count), not merely "did not throw", + * which could also be true of a silently-skipped no-op. + */ + @Test + void scheduledFetchStillRunsEvenImmediatelyAfterAManualAttempt() { + MutableClock clock = new MutableClock(Instant.parse("2026-06-01T10:00:00Z")); + MockedRestClient rest = mockRestClient(VALID_BOT_JSON); + BotFxFetchService service = serviceWithClock(mock(FxRateRepository.class), rest.client, clock, "fake-token"); + service.fetchNow(); // starts the manual cooldown + verify(rest.client, times(5)).get(); + + // Same instant as the manual call above -- deep inside the 10-minute cooldown window. + assertThatCode(service::fetchDailyRates).doesNotThrowAnyException(); + + // The scheduled path made its own 5 BOT calls despite the still-active manual cooldown. + verify(rest.client, times(10)).get(); + } + + /** + * S3 — the Bangkok-date fix. A fixed instant of 2026-09-27T18:30:00Z is 2026-09-28 01:30 + * Bangkok time (UTC+7): the requested BOT period AND the {@code effective_date} written via + * {@link FxRateRepository#upsertFromBot} must both be 2026-09-28, never 2026-09-27 (which is + * what {@code LocalDate.now()} under a UTC JVM default -- production's actual deployment, see + * CLAUDE.md -- would have produced). + */ + @Test + void aFetchAtOneThirtyAmBangkokTimeUsesTheBangkokCalendarDayNotTheUtcOne() { + Instant fixedInstant = Instant.parse("2026-09-27T18:30:00Z"); + MutableClock clock = new MutableClock(fixedInstant); + MockedRestClient rest = mockRestClient(VALID_BOT_JSON); + FxRateRepository repo = mock(FxRateRepository.class); + BotFxFetchService service = serviceWithClock(repo, rest.client, clock, "fake-token"); + + BotFxFetchService.FxFetchResult result = service.fetchNow(); + + assertThat(result.asOf()).isEqualTo("2026-09-28"); + + ArgumentCaptor uriCaptor = ArgumentCaptor.forClass(String.class); + verify(rest.uriSpec, times(5)).uri(uriCaptor.capture()); + assertThat(uriCaptor.getAllValues()).allSatisfy(uri -> { + assertThat(uri).contains("start_period=2026-09-28").contains("end_period=2026-09-28"); + assertThat(uri).doesNotContain("2026-09-27"); + }); + + verify(repo, times(5)).upsertFromBot( + anyString(), eq(new BigDecimal("36.5000")), eq(LocalDate.of(2026, 9, 28))); + } + + /** Missing/blank token still throws the pre-existing 503, and the cooldown attempt is still + * recorded even though the fetch itself never reached BOT -- mirrors {@code + * BotHolidayFetchServiceTest}'s equivalent case: a misconfigured token must not be a way to + * dodge the cooldown by retrying in a loop. */ + @Test + void aMissingTokenStillThrowsAndStillCountsAsAManualAttempt() { + MutableClock clock = new MutableClock(Instant.parse("2026-06-01T10:00:00Z")); + RestClient restClient = mock(RestClient.class); + BotFxFetchService service = serviceWithClock(mock(FxRateRepository.class), restClient, clock, ""); + + assertThatThrownBy(service::fetchNow) + .isInstanceOf(ApiException.class) + .extracting(ex -> ((ApiException) ex).getStatus()) + .isEqualTo(HttpStatus.SERVICE_UNAVAILABLE); + verifyNoInteractions(restClient); + + // The cooldown clock started on the attempt itself (before the token check), so an + // immediate second call is refused with 429, not another 503. + assertThatThrownBy(service::fetchNow) + .isInstanceOf(ApiException.class) + .extracting(ex -> ((ApiException) ex).getStatus()) + .isEqualTo(HttpStatus.TOO_MANY_REQUESTS); + verify(restClient, never()).get(); + } + + private BotFxFetchService serviceWithClock(FxRateRepository repo, RestClient restClient, Clock clock, String token) { + RestClient.Builder builder = mock(RestClient.Builder.class); + when(builder.build()).thenReturn(restClient); + AppProperties props = new AppProperties(); + props.getBot().setFxApiToken(token); + return new BotFxFetchService(repo, props, builder, new ObjectMapper(), clock); + } + + /** Bundles the mocked {@link RestClient} together with the URI-spec step of its fluent chain, + * since {@link #aFetchAtOneThirtyAmBangkokTimeUsesTheBangkokCalendarDayNotTheUtcOne} needs to + * capture the exact URI each currency was requested with. */ + private record MockedRestClient(RestClient client, RestClient.RequestHeadersUriSpec uriSpec) {} + + @SuppressWarnings({"unchecked", "rawtypes"}) + private MockedRestClient mockRestClient(String jsonBody) { + RestClient restClient = mock(RestClient.class); + RestClient.RequestHeadersUriSpec uriSpec = mock(RestClient.RequestHeadersUriSpec.class); + RestClient.RequestHeadersSpec headersSpec = mock(RestClient.RequestHeadersSpec.class); + RestClient.ResponseSpec responseSpec = mock(RestClient.ResponseSpec.class); + when(restClient.get()).thenReturn(uriSpec); + when(uriSpec.uri(anyString())).thenReturn(headersSpec); + when(headersSpec.header(anyString(), anyString())).thenReturn(headersSpec); + when(headersSpec.retrieve()).thenReturn(responseSpec); + when(responseSpec.body(String.class)).thenReturn(jsonBody); + return new MockedRestClient(restClient, uriSpec); + } + + /** A {@link Clock} a test can advance by hand, so the cooldown guard needs no real sleep. */ + private static final class MutableClock extends Clock { + private Instant instant; + private final ZoneId zone; + + MutableClock(Instant instant) { + this(instant, BANGKOK); + } + + private MutableClock(Instant instant, ZoneId zone) { + this.instant = instant; + this.zone = zone; + } + + void advance(Duration amount) { + instant = instant.plus(amount); + } + + @Override + public ZoneId getZone() { + return zone; + } + + @Override + public Clock withZone(ZoneId newZone) { + return new MutableClock(instant, newZone); + } + + @Override + public Instant instant() { + return instant; + } + } +} diff --git a/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java b/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java index c3ff85e86..ee35f4aed 100644 --- a/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java +++ b/backend/src/test/java/th/co/glr/hr/pricing/FxRateControllerTest.java @@ -10,12 +10,14 @@ import java.time.LocalDate; import java.util.List; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.mock.web.MockHttpSession; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import th.co.glr.hr.auth.SessionContext; import th.co.glr.hr.auth.UserPrincipal; +import th.co.glr.hr.common.ApiException; import th.co.glr.hr.common.ApiExceptionHandler; /** @@ -114,6 +116,21 @@ void ceoCanTriggerManualFxFetch() throws Exception { .andExpect(status().is2xxSuccessful()); } + /** + * B1 (review fix, 2026-09-28): {@code BotFxFetchService.fetchNow()} now throws a 429 {@link + * ApiException} when the CEO console's manual-fetch cooldown is still active. This proves the + * controller layer propagates that status verbatim rather than translating it into something + * else — {@code ApiExceptionHandler} is wired into this class's {@code mvc} exactly as + * production wires it, so a mismatch here would be a real regression, not a test artifact. + */ + @Test + void manualFxFetchPropagatesA429FromTheServiceAsIs() throws Exception { + when(botFx.fetchNow()).thenThrow(new ApiException(HttpStatus.TOO_MANY_REQUESTS, + "รอสักครู่ก่อนเรียกดึงอัตราแลกเปลี่ยนจากธนาคารแห่งประเทศไทยอีกครั้ง (ประมาณ 300 วินาที)")); + mvc.perform(post("/api/fx-rates/fetch-now").session(session("ceo"))) + .andExpect(status().isTooManyRequests()); + } + /** Wrong-way-round: everyone but CEO must be refused — the case that catches a widened gate. */ @Test void manualFxFetchIsForbiddenForEveryoneButCeo() throws Exception { diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index f20eb76d9..6611450f3 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -11450,6 +11450,9 @@ export const api = { // computation — just plumbing so the button, toast and refetch are exercisable in mock mode. async fetchNow() { hasRole('ceo'); + // Hardcodes BotFxFetchService.TRACKED_CURRENCIES's five currencies rather than importing it + // (this file has no access to backend source) -- contract.test.js's arity check cannot see + // this drift if the real list ever changes, so keep this in sync by hand. const tracked = ['USD', 'EUR', 'JPY', 'CNY', 'GBP']; const asOf = new Date().toISOString().slice(0, 10); const now = new Date().toISOString(); diff --git a/frontend/src/features/ceoSettings/CeoSettingsPage.jsx b/frontend/src/features/ceoSettings/CeoSettingsPage.jsx index c267e4f65..a306db125 100644 --- a/frontend/src/features/ceoSettings/CeoSettingsPage.jsx +++ b/frontend/src/features/ceoSettings/CeoSettingsPage.jsx @@ -11,6 +11,13 @@ import { Modal } from '../../components/common/Modal.jsx'; import { FormField, fieldErrorId } from '../../components/common/FormField.jsx'; import { Panel, PageStack } from '../../components/common/Layout.jsx'; import { SafeForm } from '../../components/common/SafeForm.jsx'; +// Shared with HolidaysTab's identical "ดึงข้อมูลจาก ธปท." cooldown affordance -- both +// BotFxFetchService and BotHolidayFetchService throw the same "...(ประมาณ N วินาที)" 429 shape +// (see BotFxFetchService.MANUAL_FETCH_COOLDOWN's review-fix javadoc), so this pure helper is +// reused across features rather than duplicated. Kept as a direct cross-feature import rather than +// moved to a shared location -- it is dependency-free (no React, no feature-specific imports) and +// re-exporting it would be a bigger diff than this one import line. +import { parseCooldownSeconds } from '../attendanceCalendar/holidayFetch.js'; function pctDisplay(val) { return val != null ? `${(Number(val) * 100).toFixed(2)}%` : '-'; @@ -763,6 +770,29 @@ export function CeoSettingsPage({ showToast }) { onError: (e) => showToast('error', e.message || 'บันทึกไม่สำเร็จ'), }); + // S1 (review fix, 2026-09-28): BOT publishes DAILY_AVG_EXG_RATE late in the day, so + // updated === 0 is the DOMINANT case for a fetch run any time before then, and is also what an + // invalid/expired token looks like -- never a green "success" toast for a 0/N no-op. Mirrors + // HolidaysTab's identical honest-zero-row handling. + // B1 (review fix): BotFxFetchService.fetchNow() now enforces a 10-minute manual-fetch cooldown + // and throws a 429 while it's active -- handled the same way HolidaysTab handles its own BOT + // fetcher's identical cooldown, via the shared parseCooldownSeconds helper. + const DEFAULT_FX_COOLDOWN_SECONDS = 600; // matches BotFxFetchService.MANUAL_FETCH_COOLDOWN (10 min) + const [fxCooldownUntil, setFxCooldownUntil] = useState(null); + const [fxCooldownMessage, setFxCooldownMessage] = useState(''); + const [fxNowTick, setFxNowTick] = useState(() => Date.now()); + + useEffect(() => { + if (!fxCooldownUntil) return undefined; + const timer = window.setInterval(() => setFxNowTick(Date.now()), 1000); + return () => window.clearInterval(timer); + }, [fxCooldownUntil]); + + const fxCooldownSecondsLeft = fxCooldownUntil ? Math.max(0, Math.ceil((fxCooldownUntil - fxNowTick) / 1000)) : 0; + useEffect(() => { + if (fxCooldownUntil && fxCooldownSecondsLeft <= 0) setFxCooldownUntil(null); + }, [fxCooldownSecondsLeft, fxCooldownUntil]); + // Trigger the BOT FX fetch on demand instead of waiting for the 18:00 schedule // (POST /api/fx-rates/fetch-now — CEO-only, same gate as the manual upsert above). const fetchFxNowMutation = useMutation({ @@ -770,11 +800,31 @@ export function CeoSettingsPage({ showToast }) { onSuccess: (data) => { queryClient.invalidateQueries({ queryKey: queryKeys.fxRates() }); const r = data?.result; - showToast('success', r - ? `ดึงเรตจาก BOT แล้ว — อัปเดต ${r.updated}/${r.total} สกุล (${r.asOf})` - : 'ดึงเรตจาก BOT แล้ว'); + if (r && r.updated === 0) { + // Never a success toast for a 0/N no-op -- the exact same failure shape HolidaysTab + // guards against for its own BOT fetcher (HTTP 200, nothing actually updated). + showToast('info', `ดึงเรตจาก BOT แล้ว แต่ยังไม่มีอัตราใหม่ (ณ ${r.asOf}) — ธปท. อาจยังไม่ประกาศเรตของวันนี้ ลองใหม่อีกครั้งหลัง 18:00`); + } else if (r) { + showToast('success', `ดึงเรตจาก BOT แล้ว — อัปเดต ${r.updated}/${r.total} สกุล (${r.asOf})`); + } else { + showToast('success', 'ดึงเรตจาก BOT แล้ว'); + } + }, + onError: (e) => { + if (e?.status === 429) { + const seconds = parseCooldownSeconds(e.message) ?? DEFAULT_FX_COOLDOWN_SECONDS; + const now = Date.now(); + setFxCooldownMessage(e.message || ''); + setFxCooldownUntil(now + seconds * 1000); + // fxNowTick's initial value is only set once, at mount -- by the time this error lands it + // can be arbitrarily stale (however long the request took), which would inflate the very + // first countdown render. Reset it here so the first render is accurate instead of + // waiting a tick for the interval above to correct it. + setFxNowTick(now); + return; + } + showToast('error', e.message || 'ดึงเรตจาก BOT ไม่สำเร็จ'); }, - onError: (e) => showToast('error', e.message || 'ดึงเรตจาก BOT ไม่สำเร็จ'), }); // A single shared mutation acts on whichever currency row is being saved — @@ -872,7 +922,7 @@ export function CeoSettingsPage({ showToast }) {