diff --git a/backend/src/main/java/th/co/glr/hr/factory/FactoryConfigController.java b/backend/src/main/java/th/co/glr/hr/factory/FactoryConfigController.java index 5af41aeea..dd98e1f4e 100644 --- a/backend/src/main/java/th/co/glr/hr/factory/FactoryConfigController.java +++ b/backend/src/main/java/th/co/glr/hr/factory/FactoryConfigController.java @@ -28,8 +28,9 @@ public class FactoryConfigController { private static final String[] READ_ROLES = { "ceo", "import" }; /** Names only (W3-03 follow-up, owner ruling 2026-10-29): the owning sales rep and sales_manager pick a - * factory for a factory-less line when creating the IR. No email/currency/unit leaves this route. */ - private static final String[] NAME_READ_ROLES = { "ceo", "import", "sales", "sales_manager" }; + * factory for a factory-less line when creating the IR; account opens /catalog, whose factory filter + * reads this. No email/currency/unit leaves this route. */ + private static final String[] NAME_READ_ROLES = { "ceo", "import", "sales", "sales_manager", "account" }; public record FactoryNameDto(long id, String factoryName) {} diff --git a/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestRequests.java b/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestRequests.java index 3d6e3b0ad..27bcc725d 100644 --- a/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestRequests.java +++ b/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestRequests.java @@ -32,14 +32,31 @@ private ImportRequestRequests() {} public record CreateImportRequestsRequest( String clientRequestId, @Valid List newFactoryCountries, - @Valid List itemFactories + @Valid List itemFactories, + @Valid List leadTimes ) { /** Pre-picker shape: no per-line factory choices. */ public CreateImportRequestsRequest(String clientRequestId, List newFactoryCountries) { - this(clientRequestId, newFactoryCountries, null); + this(clientRequestId, newFactoryCountries, null, null); + } + + public CreateImportRequestsRequest(String clientRequestId, List newFactoryCountries, + List itemFactories) { + this(clientRequestId, newFactoryCountries, itemFactories, null); } } + /** + * One factory NAME's caller-supplied import lead time, for a new factory whose lead time can be + * derived from neither the quotation nor the country default (owner ruling 10-29). Same bounds as + * {@link SetLeadTimeRequest}; min must not exceed max. + */ + public record NewFactoryLeadTimeInput( + @NotBlank String factoryName, + @NotNull @Min(1) @Max(365) Integer minDays, + @NotNull @Min(1) @Max(365) Integer maxDays + ) {} + /** * Sales' factory choice for ONE deal line that has no factory (a free-text line on a direct * quotation — W3-03 follow-up, owner ruling 2026-10-29). Applied only to a line that resolves to diff --git a/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestService.java b/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestService.java index dbb67e01b..43fb3d169 100644 --- a/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestService.java +++ b/backend/src/main/java/th/co/glr/hr/importrequest/ImportRequestService.java @@ -210,13 +210,17 @@ public ImportRequestDto get(long id, UserPrincipal actor) { } /** - * Raises one DRAFT per FACTORY on the deal that does not already have a live one, resolving each + * Raises AND ISSUES one ใบขอซื้อ per FACTORY on the deal that does not already have a live one (owner + * ruling 10-29: no DRAFT is left behind; each row comes back ISSUED with a doc number and step + * CONTACTED, and the first one moves the deal to PROCUREMENT). All-or-nothing: a missing lead time, + * an empty item list or the payment gate refuses the whole call with a 409 before anything is + * written. Resolving each * line's factory per {@link ImportRequestQueryRepository#factoryResolutionCandidates}'s cascade — * see that method's Javadoc for the order and {@link #resolveFactoryGroups} for the match-or-create * step (owner decision 2, revised 09-18: auto-create now requires {@code request}'s {@code * newFactoryCountries}). * - *

Idempotent in the way that matters: a factory with a DRAFT or an ISSUED form is skipped + *

Idempotent in the way that matters: a factory with a (leftover) DRAFT or an ISSUED form is skipped * rather than duplicated, so calling this twice does not produce two forms for the same factory — * and the two partial unique indexes in V184 refuse it in the database even if this check were * wrong. Refuses outright if every factory is already covered, rather than returning "created @@ -233,6 +237,13 @@ public List createDrafts(long ticketId, CreateImportRequestsRe "สร้างใบขอซื้อได้หลังจากยืนยันคำสั่งซื้อแล้ว (ORDER_RECEIVED) เท่านั้น"); } + // The payment gate applies at create now (widened gate, same as issue()) and refuses FIRST, so a + // deal with no paid deposit is never walked through the country / lead-time prompts. + ticketService.requireImportRequestIssuable(requireTicketSummary(ticketId), true); + + // 400 before anything is written (the picker and auto-create below both write). + Map suppliedLeadTimes = + validatedLeadTimes(request); applyItemFactoryChoices(ticketId, request); List groups = resolveFactoryGroups(ticketId, actor.id(), request); if (groups.isEmpty()) { @@ -248,32 +259,54 @@ public List createDrafts(long ticketId, CreateImportRequestsRe // CR-1 (D, B-R4/B-R5): each factory's IR takes the lead time on the deal's current QUOTATION items. List quotationLeadTimes = repository.currentQuotationLeadTimes(ticketId); - int created = 0; + + // ALL-OR-NOTHING (owner ruling 10-29): create also ISSUES, so every issue check runs for every + // NEW factory BEFORE the first write. (Not left to the @Transactional rollback alone: it is + // inert in the hand-wired integration suite, and the checks are cheap.) + record NewRow(FactoryGroup group, LeadTimeDefaults.Range derived, LeadTimeDefaults.Range leadTime) { } + List newRows = new ArrayList<>(); + List missingLeadTime = new ArrayList<>(); for (FactoryGroup group : groups) { if (alreadyCovered.contains(group.factoryId())) { continue; } LeadTimeDefaults.Range derived = leadTimeFromQuotation(quotationLeadTimes, group.factoryId(), group.items()); - LeadTimeDefaults.Range leadTime = derived; + LeadTimeDefaults.Range leadTime = derived != null ? derived : LeadTimeDefaults.forCountry(group.countryCode()); + if (group.items().isEmpty()) { + throw new ApiException(HttpStatus.CONFLICT, "ใบขอซื้อยังไม่มีรายการสินค้า"); + } if (leadTime == null) { - leadTime = LeadTimeDefaults.forCountry(group.countryCode()); + // Owner ruling 10-29: a caller-supplied lead time (keyed by factory name) fills it in. + var supplied = suppliedLeadTimes.get(normalizeFactoryName(group.factoryName())); + if (supplied == null) { + missingLeadTime.add(group.factoryName()); + continue; + } + leadTime = new LeadTimeDefaults.Range(supplied.minDays(), supplied.maxDays()); } + newRows.add(new NewRow(group, derived, leadTime)); + } + if (!missingLeadTime.isEmpty()) { + // Parseable by the panel (like the missing-country prefix); lists ONLY the factories still lacking one. + throw new ApiException(HttpStatus.CONFLICT, + MISSING_LEAD_TIME_PREFIX + String.join(", ", missingLeadTime)); + } + if (newRows.isEmpty()) { + throw new ApiException(HttpStatus.CONFLICT, + "ทุกโรงงานในดีลนี้มีใบขอซื้อที่ยังใช้งานอยู่แล้ว"); + } + for (NewRow n : newRows) { + FactoryGroup group = n.group(); long id = stored.insertDraft(ticketId, group.factoryId(), group.factoryName(), group.brandLabel(), stored.highestVersion(ticketId, group.factoryId()) + 1, - snapshot, leadTime == null ? null : leadTime.minDays(), - leadTime == null ? null : leadTime.maxDays(), null, // S-E: nothing to carry, fresh draft - null, // REVIEW ROUND 3, item 2: nothing to carry either -- a fresh draft's own - // requiredByNote comes from the deal snapshot only, same as before. + snapshot, n.leadTime().minDays(), n.leadTime().maxDays(), null, // S-E: nothing to carry, fresh row + null, // REVIEW ROUND 3, item 2: nothing to carry either -- requiredByNote comes from the deal snapshot only. actor.id(), actor.name()); stored.replaceItems(id, group.items()); - if (derived != null) { - stored.setDerivedLeadTime(id, derived.minDays(), derived.maxDays()); + if (n.derived() != null) { + stored.setDerivedLeadTime(id, n.derived().minDays(), n.derived().maxDays()); } - created++; - } - if (created == 0) { - throw new ApiException(HttpStatus.CONFLICT, - "ทุกโรงงานในดีลนี้มีใบขอซื้อที่ยังใช้งานอยู่แล้ว"); + issueStoredDraft(requireStored(id), null, snapshot, actor); } return withPageCounts(stored.findByTicket(ticketId)); } @@ -546,6 +579,17 @@ public ImportRequestDto issue(long id, IssueImportRequestRequest request, UserPr String override = request == null || request.docNumber() == null || request.docNumber().isBlank() ? null : request.docNumber().strip(); + return issueStoredDraft(draft, override, dealSnapshot, actor); + } + + /** + * The write half of {@link #issue}: everything AFTER the issuability checks (status, items, lead + * time, payment gate). Shared with {@link #createDrafts}, which pre-checks every new factory and + * then issues each freshly inserted row through here, so the two paths cannot drift. + */ + private ImportRequestDto issueStoredDraft(ImportRequestDto draft, String override, + TicketSnapshot dealSnapshot, UserPrincipal actor) { + long id = draft.id(); // An override does NOT advance the sequence. A number typed ahead of the counter therefore // cannot be minted a second time later — ux_import_request_doc_number refuses the duplicate — // and this pre-check turns that race into a clear message instead of a constraint violation. @@ -1105,6 +1149,29 @@ public void setRequiredByNote(long ticketId, SetRequiredByNoteRequest request, U private record FactoryGroup(long factoryId, String factoryName, String countryCode, String brandLabel, List items) {} + /** 409 prefix the panel parses to open its lead-time popup (mirrors the missing-country prefix). */ + public static final String MISSING_LEAD_TIME_PREFIX = "กรุณาระบุระยะเวลานำเข้าให้กับโรงงาน: "; + + /** Supplied per-factory lead times keyed by normalized name; same bounds as setLeadTime, else 400. */ + private static Map validatedLeadTimes( + CreateImportRequestsRequest request) { + Map out = new LinkedHashMap<>(); + if (request == null || request.leadTimes() == null) { + return out; + } + for (var in : request.leadTimes()) { + if (in == null || in.factoryName() == null || in.factoryName().isBlank() + || in.minDays() == null || in.maxDays() == null + || in.minDays() < 1 || in.maxDays() > 365 || in.minDays() > in.maxDays()) { + throw new ApiException(HttpStatus.BAD_REQUEST, + "ระยะเวลานำเข้าต้องเป็น 1–365 วัน และขั้นต่ำต้องไม่เกินขั้นสูง" + + (in == null || in.factoryName() == null ? "" : " (โรงงาน " + in.factoryName() + ")")); + } + out.put(normalizeFactoryName(in.factoryName()), in); + } + return out; + } + /** A caller-supplied country for a factory about to be auto-created, already validated. */ private record ValidatedCountry(String code, String other) {} 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 2219fdfa7..2005a8345 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 @@ -24,8 +24,9 @@ public class FxRateController { /** * Read gate, owner ruling 2026-08-02 — widened from issue #388's {@code ceo}/{@code import} - * gate by exactly one role, to {@code sales}. NOT opened to every authenticated session: the - * owner's words were "should only be to sale", and this set is the whole of that ruling. + * gate by exactly one role, to {@code sales}, and to {@code sales_manager} by the 2026-10-29 + * ruling below. NOT opened to every authenticated session: the owner's words were "should + * only be to sale", and this set is the whole of those two rulings. * *

The trigger was the deal-create modal's "ราคาตั้ง (ประมาณการ)" estimate (V112): a rep who * picks a catalog item needs the FX rate to convert its price to THB, and every catalog row is @@ -45,6 +46,11 @@ public class FxRateController { * number already visible to this caller, so it survives its trigger. Re-narrowing it is a * separate owner decision, not a tidy-up that follows from #748. * + *

Widened again, owner ruling 2026-10-29 (role check RC-01): {@code sales_manager} + * joins the set — the deal page converts SM's foreign-currency prices to THB, which needs this + * rate. The "sales_manager cannot create a deal" argument above no longer decides the gate. + * Nothing else changes; writes stay CEO-only. + * *

The reasoning is deliberately narrower than #388's: an FX rate only converts a number that * is ALREADY visible to this caller — {@code /api/catalog/prices} is open to any authenticated * user per the 2026-08-01 ruling that closed #388 — and on its own reveals nothing about landed @@ -55,7 +61,7 @@ public class FxRateController { *

If a test pinning this set fails, someone has re-narrowed or re-widened the gate: reopen * the ruling with the owner rather than editing the test to match the code. */ - private static final Set READ_ROLES = Set.of("ceo", "import", "sales"); + private static final Set READ_ROLES = Set.of("ceo", "import", "sales", "sales_manager"); private final FxRateRepository fxRates; private final SessionContext sessions; 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 f73d9254d..43fbc598a 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 @@ -66,15 +66,14 @@ * (2026-07-16). *

  • {@code GET /api/catalog/prices} → open to any authenticated user, per the owner's * 2026-08-01 ruling closing #388: #205's "browsable" covers the supplier purchase price.
    • - *
  • {@code GET /api/fx-rates} → {@code ceo}/{@code import}/{@code sales}, per the - * owner's 2026-08-02 ruling (see {@code FxRateController.READ_ROLES}): an FX rate only + *
  • {@code GET /api/fx-rates} → {@code ceo}/{@code import}/{@code sales}/ + * {@code sales_manager}, per the owner's 2026-08-02 ruling (sales) and 2026-10-29 + * ruling RC-01 (sales_manager) — see {@code FxRateController.READ_ROLES}: an FX rate only * converts a number already visible via the open catalog-prices read, and reveals nothing - * about landed cost or margin on its own — but the ruling was "should only be to sale", so - * this is one role wider than #388, NOT open to every authenticated session.
    • + * about landed cost or margin on its own. NOT open to every authenticated session. *
  • {@code GET /api/price-calc-configs}, {@code GET /api/factory-configs} → ceo/import - * ({@code RAW_COSTING_ROLES}), unchanged. The FX widening is deliberately narrower than - * #388's catalog ruling: it does not extend to the margin policy or the supplier - * directory.
    • + * ({@code RAW_COSTING_ROLES}), unchanged. The FX widening does not extend to the margin + * policy or this supplier-directory endpoint. * */ class CatalogPricingReadAuthzIntegrationTest extends AbstractPostgresIntegrationTest { @@ -211,20 +210,29 @@ void salesCanReadFxRatesPerOwnerRuling20260802() throws Exception { .andExpect(jsonPath("$.fxRates[?(@.currency == 'XTS')].rateToThb").exists()); } + /** + * Owner ruling 2026-10-29 (role check RC-01): the deal page converts SM's foreign-currency + * prices to THB, so the FX read widens to {@code sales_manager}. Writes stay CEO-only. + */ + @Test + void salesManagerCanReadFxRatesPerOwnerRuling20261029() throws Exception { + fxMvc.perform(get("/api/fx-rates").session(sessionFor("sales_manager"))) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.fxRates[?(@.currency == 'XTS')].rateToThb").exists()); + } + /** * THE OTHER HALF OF THE SAME RULING, and the half that actually catches a regression. The * ruling was "should only be to sale" — it did NOT open the rate to every authenticated * session. Without this case the suite stays green with the gate wide open, which is precisely * the failure mode a read-gate widening has: you find out in production. * - *

    {@code sales_manager} is in this list on purpose. It browses the deal pipeline but cannot - * create a deal ({@code canCreateTickets} is {@code ['sales']}, and {@code TicketService.create} - * gates on {@code Set.of("sales")}), so it never reaches the modal that needs the rate. Adding - * it here would be widening beyond what the owner authorised. + *

    {@code sales_manager} left this list by the owner's ruling of 2026-10-29 (role check + * RC-01) and is pinned by {@link #salesManagerCanReadFxRatesPerOwnerRuling20261029}. */ @Test void everyRoleOutsideTheRulingIsStillRefusedTheFxRead() throws Exception { - for (String role : List.of("employee", "warehouse", "qc", "hr", "sales_manager", "account")) { + for (String role : List.of("employee", "warehouse", "qc", "hr", "account")) { fxMvc.perform(get("/api/fx-rates").session(sessionFor(role))) .andExpect(status().isForbidden()); } diff --git a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java index a3024f289..1cb8e3979 100644 --- a/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/dealquotation/DealQuotationConfirmOrderIntegrationTest.java @@ -882,11 +882,14 @@ void confirmOrder_onCatalogueLines_copiesCatalogPriceIdAndFactoryToEveryTicketIt } @Test - void confirmOrder_onCatalogueLines_letsImportCreateThePerFactoryIrDraft_insteadOf409() { + void confirmOrder_onCatalogueLines_letsSalesCreateAndIssueThePerFactoryIr_insteadOf409() { long priceB = insertCatalogProduct("W3 Factory B", "IT", "W3-B", new BigDecimal("10"), "EUR", "per_sqm"); long ticketId = pipelineTicket(salesActor); DealQuotationDto approved = approvedQuotation(ticketId, catalogueLineDraft(priceB, "Line B")); confirmer.confirmOrderFromDirectQuotation(approved.id(), salesActor); + // create also issues now, so the payment gate applies: the deposit has been received. + jdbc.update("UPDATE sales.ticket SET payment_status = 'DEPOSIT_PAID' WHERE ticket_id = :id", + Map.of("id", ticketId)); th.co.glr.hr.importrequest.ImportRequestService importRequests = new th.co.glr.hr.importrequest.ImportRequestService( new th.co.glr.hr.importrequest.ImportRequestQueryRepository(jdbc), @@ -899,6 +902,7 @@ void confirmOrder_onCatalogueLines_letsImportCreateThePerFactoryIrDraft_insteadO assertThat(drafts).hasSize(1); assertThat(drafts.get(0).factoryName()).isEqualTo("W3 Factory B"); + assertThat(drafts.get(0).status()).isEqualTo("ISSUED"); } /** Guard (green before and after): a free-text line has no catalogue item, so it keeps factory NULL. */ diff --git a/backend/src/test/java/th/co/glr/hr/factory/FactoryConfigControllerTest.java b/backend/src/test/java/th/co/glr/hr/factory/FactoryConfigControllerTest.java index 314eaf5d9..81265b64c 100644 --- a/backend/src/test/java/th/co/glr/hr/factory/FactoryConfigControllerTest.java +++ b/backend/src/test/java/th/co/glr/hr/factory/FactoryConfigControllerTest.java @@ -64,7 +64,8 @@ void listIsUnauthorizedWithoutASession() throws Exception { void namesAreReadableByIrCreators_andExposeOnlyIdAndName() throws Exception { when(repo.findAll()).thenReturn(List.of( new FactoryConfigDto(7L, "Rex Ceramics", "rfq@rex.example", "EUR", "sqm", "IT", null))); - for (String role : List.of("ceo", "import", "sales", "sales_manager")) { + // account added 2026-10-29 (owner ruling): it can open /catalog, whose factory filter reads this. + for (String role : List.of("ceo", "import", "sales", "sales_manager", "account")) { mvc.perform(get("/api/factory-configs/names").session(session(role))) .andExpect(status().is2xxSuccessful()) .andExpect(content().string(containsString("Rex Ceramics"))) @@ -75,7 +76,7 @@ void namesAreReadableByIrCreators_andExposeOnlyIdAndName() throws Exception { @Test void namesAreForbiddenForOtherRoles() throws Exception { when(repo.findAll()).thenReturn(List.of()); - for (String role : List.of("employee", "warehouse", "qc", "hr", "account")) { + for (String role : List.of("employee", "warehouse", "qc", "hr")) { mvc.perform(get("/api/factory-configs/names").session(session(role))) .andExpect(status().isForbidden()); } diff --git a/backend/src/test/java/th/co/glr/hr/importdeal/ImportDealAuthzIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importdeal/ImportDealAuthzIntegrationTest.java index af1e736c4..880d1440a 100644 --- a/backend/src/test/java/th/co/glr/hr/importdeal/ImportDealAuthzIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importdeal/ImportDealAuthzIntegrationTest.java @@ -347,8 +347,8 @@ void importCannotWriteDelivery() { // ── fixtures ────────────────────────────────────────────────────────────────────────────────── private ImportRequestDto issueOneImportRequest() { - List drafts = importRequestService.createDrafts(ticketId, null, ceoUser); - return importRequestService.issue(drafts.get(0).id(), null, ceoUser); + // createDrafts creates AND issues (10-29), so the created row is already ISSUED. + return importRequestService.createDrafts(ticketId, null, ceoUser).get(0); } private long firstItemId() { diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/DraftSeed.java b/backend/src/test/java/th/co/glr/hr/importrequest/DraftSeed.java new file mode 100644 index 000000000..76fa6cbc6 --- /dev/null +++ b/backend/src/test/java/th/co/glr/hr/importrequest/DraftSeed.java @@ -0,0 +1,53 @@ +package th.co.glr.hr.importrequest; + +import java.util.List; +import java.util.Map; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import th.co.glr.hr.auth.UserPrincipal; +import th.co.glr.hr.importrequest.ImportRequestDtos.ImportRequestDto; + +/** + * Test-only seed for leftover DRAFT rows. Since the 10-29 ruling {@code createDrafts} creates AND + * ISSUES, so tests of the DRAFT lifecycle (edit, delete, first-issue checks) need a DRAFT that the + * service no longer produces. This writes one per factory of the deal's lines (matched by + * {@code ticket_item.factory} name) straight through the repository, with the country lead-time + * default, exactly as the old create did. Skips factories that already have a live form. + */ +final class DraftSeed { + + private DraftSeed() { } + + static List seed(NamedParameterJdbcTemplate jdbc, long ticketId, UserPrincipal actor) { + ImportRequestRepository stored = new ImportRequestRepository(jdbc); + var snapshot = new ImportRequestQueryRepository(jdbc).loadTicketSnapshot(ticketId).orElseThrow(); + List> factories = jdbc.queryForList(""" + SELECT f.factory_id, f.name, f.country, + string_agg(DISTINCT ti.brand, ', ') AS brands, min(ti.sort_order) AS first_sort + FROM sales.ticket_item ti + JOIN price_catalog.factories f ON f.name = ti.factory + WHERE ti.ticket_id = :t + GROUP BY f.factory_id, f.name, f.country + ORDER BY min(ti.sort_order) + """, Map.of("t", ticketId)); + for (Map f : factories) { + long factoryId = ((Number) f.get("factory_id")).longValue(); + if (stored.findByTicket(ticketId).stream().anyMatch(r -> r.factoryId() == factoryId + && ImportRequestStatus.isLive(r.status()))) { + continue; + } + var lead = LeadTimeDefaults.forCountry((String) f.get("country")); + long id = stored.insertDraft(ticketId, factoryId, (String) f.get("name"), (String) f.get("brands"), + stored.highestVersion(ticketId, factoryId) + 1, snapshot, + lead == null ? null : lead.minDays(), lead == null ? null : lead.maxDays(), null, null, + actor.id(), actor.name()); + jdbc.update(""" + INSERT INTO sales.import_request_item + (import_request_id, ticket_item_id, seq, code, size, qty, unit, brand, model) + SELECT :id, ti.item_id, row_number() OVER (ORDER BY ti.sort_order), ti.model, ti.size, + ti.qty, ti.unit, ti.brand, ti.model + FROM sales.ticket_item ti WHERE ti.ticket_id = :t AND ti.factory = :name + """, Map.of("id", id, "t", ticketId, "name", f.get("name"))); + } + return stored.findByTicket(ticketId); + } +} diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestEmailDraftIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestEmailDraftIntegrationTest.java index dc2813c0b..7fe00496c 100644 --- a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestEmailDraftIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestEmailDraftIntegrationTest.java @@ -90,7 +90,7 @@ void issue_generatesAnEmailDraft_toTheFactorysAddress_withNoPricingContent() { insertFactory("Email Factory", "IT", "sales@emailfactory.example"); long ticketId = insertTicket("EM-ISSUE"); insertItem(ticketId, "Email Factory", "Lithos Nero", "60x60 cm", "100", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issued = service.issue(id, null, owner); @@ -141,9 +141,9 @@ void issue_bodyUsesTheNumberedBlockLayout_skippingBlankFieldsAndNeverPrintingNul "60x60 cm", "308.000", "pcs", 0, "Grigio", "Matte"); insertItemWithBrandModelColor(ticketId, "Terrazzo Co", "Block Layout Factory", "Terrazzo White", "30x60 cm", "10", "pcs", 1, null, null); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); - - ImportRequestDto issued = service.issue(id, null, owner); + // Real create (it now issues): these two pin the line prefill (colour/surface, catalog code) + // that only the service's own resolution produces, which a seeded row would not carry. + ImportRequestDto issued = service.createDrafts(ticketId, null, owner).get(0); String body = issued.emailBody(); assertThat(body).contains("1. Living Ceramics Lithos Nero") @@ -177,7 +177,7 @@ void issue_bodyKeepsABusinessTypedNote_whenItDiffersFromTheAutoDerivedOne() { // Business hand-edits the auto-derived note to something else before drafts are raised is not // reachable pre-createDrafts in this flow (the note is only ever set AT creation), so this // exercises the draft-stage item edit via update() instead. - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto draft = service.get(id, owner); java.util.List edited = draft.items().stream() .map(it -> new ImportRequestRequests.ImportRequestItemInput(it.ticketItemId(), it.code(), @@ -209,9 +209,9 @@ void issue_bodyShowsTheCodeSuffix_whenItGenuinelyDiffersFromTheModel() { .addValue("model", "Lithos Nero Hand-Typed").addValue("size", "60x60 cm") .addValue("qty", new BigDecimal("10")).addValue("unit", "pcs") .addValue("catalogPriceId", catalogPriceId)); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); - - ImportRequestDto issued = service.issue(id, null, owner); + // Real create (it now issues): these two pin the line prefill (colour/surface, catalog code) + // that only the service's own resolution produces, which a seeded row would not carry. + ImportRequestDto issued = service.createDrafts(ticketId, null, owner).get(0); assertThat(issued.emailBody()) .contains("1. Living Ceramics Lithos Nero Hand-Typed (Code: CAT-CODE-777)"); } @@ -227,7 +227,7 @@ void issue_attachedFilename_matchesTheDownloadRoutesOwnFilenameHelper() { insertFactory("Filename Tie Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-FILENAME-TIE"); insertItem(ticketId, "Filename Tie Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issued = service.issue(id, null, owner); String expectedFilename = ImportRequestService.downloadFileName(issued.docNumber(), issued.id(), @@ -249,7 +249,7 @@ void issue_emailRequiredByLine_matchesWhatActuallyGetsSnapshottedOntoTheForm() { insertItem(ticketId, "Required By Consistency Factory", "Line", "60x60", "10", "pcs", 0); // The draft is raised BEFORE the deal-level value is set, so the draft's OWN requiredByNote // starts (and stays) blank until #issue's re-snapshot fallback fires. - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThat(service.get(id, owner).requiredByNote()).isNull(); service.setRequiredByNote(ticketId, new SetRequiredByNoteRequest("Within 30 days"), owner); @@ -270,7 +270,7 @@ void issue_emailSubjectAndBody_neverNameTheCustomerOrTheProject() { insertFactory("Privacy Factory", "IT", "sales@privacyfactory.example"); long ticketId = insertTicket("EM-PRIVACY"); insertItem(ticketId, "Privacy Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issued = service.issue(id, null, owner); @@ -289,7 +289,7 @@ void issue_withNoFactoryEmailOnFile_leavesEmailToNull_butStillDraftsTheBody() { insertFactory("No Email Factory", "IT", null); long ticketId = insertTicket("EM-NOEMAIL"); insertItem(ticketId, "No Email Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issued = service.issue(id, null, owner); @@ -303,7 +303,7 @@ void revision_regeneratesTheEmailDraft() { insertFactory("Revision Email Factory", "IT", "old@example.com"); long ticketId = insertTicket("EM-REVISE"); insertItem(ticketId, "Revision Email Factory", "Line A", "60x60", "10", "pcs", 0); - long v1 = service.createDrafts(ticketId, null, owner).get(0).id(); + long v1 = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issuedV1 = service.issue(v1, null, owner); assertThat(issuedV1.emailBody()).contains("Line A"); @@ -328,7 +328,7 @@ void setLeadTime_neverClobbersAHandEditedUnsentEmailBody() { insertFactory("Hand Edit Preserve Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-HANDEDIT-PRESERVE"); insertItem(ticketId, "Hand Edit Preserve Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); ImportRequestDto handEdited = service.updateEmailDraft(id, new UpdateEmailDraftRequest( @@ -352,7 +352,7 @@ void setLeadTime_regeneratesAnUneditedEmail_butAlwaysSignsAsTheOriginalIssuer_no insertFactory("Original Signer Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-ORIG-SIGNER"); insertItem(ticketId, "Original Signer Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issued = service.issue(id, null, owner); assertThat(issued.emailBody()).contains(owner.email()); @@ -374,7 +374,7 @@ void emailDraft_isEditableByOwningRepCeoOrImport_notByANonOwningRep() { insertFactory("Edit Email Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-EDIT"); insertItem(ticketId, "Edit Email Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); ImportRequestDto afterOwner = service.updateEmailDraft(id, @@ -406,7 +406,7 @@ void emailDraft_isNotEditableOnceMarkedSent() { insertFactory("Sent Email Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-SENT-EDIT"); insertItem(ticketId, "Sent Email Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); service.markEmailSent(id, importUser); @@ -431,7 +431,7 @@ void updateEmailDraftSql_refusesADraftRow_directlyAtTheRepository() { insertFactory("Repo Guard Draft Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-REPO-DRAFT"); insertItem(ticketId, "Repo Guard Draft Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); // never issued + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); // never issued ImportRequestRepository repo = new ImportRequestRepository(jdbc); assertThat(repo.updateEmailDraft(id, "sneaky@example.com", null, null)).isZero(); @@ -445,7 +445,7 @@ void updateEmailDraftSql_refusesAnAlreadySentRow_directlyAtTheRepository() { insertFactory("Repo Guard Sent Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-REPO-SENT"); insertItem(ticketId, "Repo Guard Sent Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); service.markEmailSent(id, importUser); @@ -461,7 +461,7 @@ void markEmailSentSql_refusesADraftRow_directlyAtTheRepository() { insertFactory("Repo Guard MarkSent Draft Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-REPO-MARK-DRAFT"); insertItem(ticketId, "Repo Guard MarkSent Draft Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); // never issued + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); // never issued ImportRequestRepository repo = new ImportRequestRepository(jdbc); assertThat(repo.markEmailSent(id, importUser.id(), importUser.name())).isZero(); @@ -479,7 +479,7 @@ void markEmailSent_isAllowedToImportCeoAndOwningRep_notToANonOwningRepOrSalesMan insertItem(ticketId, "Mark Sent Factory", "Line", "60x60", "10", "pcs", 0); // Non-owning rep and sales_manager: refused. - long draftForOther = service.createDrafts(ticketId, null, owner).get(0).id(); + long draftForOther = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(draftForOther, null, owner); assertThatThrownBy(() -> service.markEmailSent(draftForOther, otherSalesRep)) .isInstanceOfSatisfying(ApiException.class, @@ -505,7 +505,7 @@ void markEmailSent_allowedToTheOwningRep_andToCeo() { insertFactory("Mark Sent Factory 2", "IT", "a@example.com"); long ticketId = insertTicket("EM-MARK2"); insertItem(ticketId, "Mark Sent Factory 2", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); ImportRequestDto afterOwner = service.markEmailSent(id, owner); @@ -517,7 +517,7 @@ void markEmailSent_recordsATicketEvent() { insertFactory("Event Factory", "IT", "a@example.com"); long ticketId = insertTicket("EM-EVENT"); insertItem(ticketId, "Event Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); service.markEmailSent(id, importUser); diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestFactoryResolutionIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestFactoryResolutionIntegrationTest.java index 61a16d49d..ab22a0105 100644 --- a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestFactoryResolutionIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestFactoryResolutionIntegrationTest.java @@ -250,14 +250,21 @@ void aNewFactoryWithZzAndCountryOther_createsARowWithCountryOtherSet() { long ticketId = insertTicket("RESOLV-3-ZZ-TEXT"); insertItem(ticketId, "AnyBrand", "ZZ With Text Line", "60x60", "10", "pcs", 0, "โรงงานลึกลับ2"); - List drafts = service.createDrafts(ticketId, + // 10-29: create now ISSUES, and 'ZZ' has no lead-time default, so the call is refused (409, + // nothing created). The factory row is still auto-created before that refusal; in production the + // @Transactional rolls it back, but this hand-wired service has no tx proxy, so it is visible + // here and is what pins country/country_other on the auto-create path. + assertThatThrownBy(() -> service.createDrafts(ticketId, new CreateImportRequestsRequest(null, List.of(new NewFactoryCountryInput("โรงงานลึกลับ2", "ZZ", "Farawaystan"))), - owner); + owner)) + .isInstanceOfSatisfying(ApiException.class, + e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); + assertThat(service.list(ticketId, owner)).isEmpty(); Map row = jdbc.queryForMap(""" - SELECT country, country_other FROM price_catalog.factories WHERE factory_id = :id - """, Map.of("id", drafts.get(0).factoryId())); + SELECT country, country_other FROM price_catalog.factories WHERE name = :n + """, Map.of("n", "โรงงานลึกลับ2")); assertThat(row.get("country")).isEqualTo("ZZ"); assertThat(row.get("country_other")).isEqualTo("Farawaystan"); } @@ -325,10 +332,7 @@ void twoFactoriesSharingABrand_bothIssueIndependently() { .containsExactlyInAnyOrder(factoryA, factoryB); assertThat(drafts).extracting(ImportRequestDto::brand) .containsExactly("SharedBrand", "SharedBrand"); - - for (ImportRequestDto d : drafts) { - assertThat(service.issue(d.id(), null, owner).status()).isEqualTo(ImportRequestStatus.ISSUED); - } + assertThat(drafts).allSatisfy(d -> assertThat(d.status()).isEqualTo(ImportRequestStatus.ISSUED)); } // ── F-SM-001 line prefill (owner decision 09-18 #3 §A) ────────────────────────────────────── @@ -406,9 +410,6 @@ void rollup_firesOnlyOnceEveryFactoryIsReceived_notBefore() { insertItem(ticketId, "Brand", "Line B", "60x60", "10", "pcs", 1, "Rollup Factory B"); List drafts = service.createDrafts(ticketId, null, owner); - for (ImportRequestDto d : drafts) { - service.issue(d.id(), null, owner); - } assertThat(fulfillmentStatus(ticketId)).isEqualTo(FulfilmentStatus.IR_ISSUED); ImportRequestDto a = drafts.stream().filter(d -> "Rollup Factory A".equals(d.factoryName())) @@ -439,7 +440,6 @@ void rollup_onAMixedPartiallyDeliveredDeal_writesOnlyTheEvent_neverTouchesStatus insertItem(ticketId, "Brand", "Import Line", "60x60", "10", "pcs", 0, "Mixed Deal Factory"); long irId = service.createDrafts(ticketId, null, owner).get(0).id(); - service.issue(irId, null, owner); assertThat(fulfillmentStatus(ticketId)).isEqualTo(FulfilmentStatus.IR_ISSUED); // Simulate: while this factory's goods were still in transit, the deal's stock-sourced lines @@ -463,7 +463,7 @@ void legacyMarkIrSent_isRefusedOnceTheDealIsTrackedPerFactory() { insertFactory("Legacy Refusal Factory", "IT", null); long ticketId = insertTicket("RESOLV-7"); insertItem(ticketId, "Brand", "Line", "60x60", "10", "pcs", 0, "Legacy Refusal Factory"); - service.issue(service.createDrafts(ticketId, null, owner).get(0).id(), null, owner); + service.createDrafts(ticketId, null, owner); assertThat(tickets.hasLiveImportRequests(ticketId)).isTrue(); assertThatThrownBy(() -> ticketService.markIrSent(ticketId, importUser)) @@ -478,7 +478,7 @@ void legacyIssueImportRequest_isRefusedOnceTheDealIsTrackedPerFactory() { insertFactory("Legacy Issue Refusal Factory", "IT", null); long ticketId = insertTicket("RESOLV-8"); insertItem(ticketId, "Brand", "Line", "60x60", "10", "pcs", 0, "Legacy Issue Refusal Factory"); - service.issue(service.createDrafts(ticketId, null, owner).get(0).id(), null, owner); + service.createDrafts(ticketId, null, owner); // The legacy one-shot action itself 409s once fulfillment_status is already IR_ISSUED — // recordFirstImportRequestIssued sets exactly that, so the deal cannot tell which path diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeFromPricingRequestIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeFromPricingRequestIntegrationTest.java index a169a23cb..58abdc213 100644 --- a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeFromPricingRequestIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeFromPricingRequestIntegrationTest.java @@ -413,7 +413,6 @@ private long[] issuedPrRouteIr(String tag, Integer min, Integer max) { if (min != null) { assertThat(v1.leadTimeMinDays()).isEqualTo(min); } - service.issue(v1.id(), null, owner); return new long[] {v1.id(), pr, q}; } diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeIntegrationTest.java index 467b1c468..23f740531 100644 --- a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestLeadTimeIntegrationTest.java @@ -97,25 +97,123 @@ void draftAutofillsLeadTimeFromTheFactorysCountry_th() { assertAutofill("TH", 3, 7); } - /** 'ZZ' (อื่นๆ) and any other unmapped code get NO default — the rep must type it in. */ + /** + * 'ZZ' (อื่นๆ) and any other unmapped code get NO default. Create now ISSUES, so with no lead time + * it is refused outright (409 naming the factory) and writes nothing — the rep must type it in on + * the factory first / the deal's quotation. + */ + @Test + void createDrafts_isRefusedNamingTheFactory_forZzAndUnmappedCodes() { + insertFactory("ZZ Factory", "ZZ", "Farawaystan"); + insertFactory("Unmapped Factory", "PL", null); + + for (String name : new String[] {"ZZ Factory", "Unmapped Factory"}) { + long ticket = insertTicket("LT-NULL-" + name.charAt(0)); + insertItem(ticket, name, "Line", 0); + assertThatThrownBy(() -> service.createDrafts(ticket, null, owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).contains(name); + }); + assertThat(service.list(ticket, owner)).isEmpty(); + } + } + + // ── 10-29 ruling B: lead-time popup contract (missing lead time is askable, not a dead end) ── + + private static final String MISSING_LEAD_TIME_PREFIX = "กรุณาระบุระยะเวลานำเข้าให้กับโรงงาน: "; + + private long dealWithThreeFactories() { + insertFactory("LT Good Factory", "IT", null); + insertFactory("LT Missing PL", "PL", null); + insertFactory("LT Missing ZZ", "ZZ", "Farawaystan"); + long ticket = insertTicket("LT-B-" + System.nanoTime()); + insertItem(ticket, "LT Good Factory", "Line G", 0); + insertItem(ticket, "LT Missing PL", "Line P", 1); + insertItem(ticket, "LT Missing ZZ", "Line Z", 2); + return ticket; + } + + private static ImportRequestRequests.CreateImportRequestsRequest withLeadTimes( + ImportRequestRequests.NewFactoryLeadTimeInput... inputs) { + return new ImportRequestRequests.CreateImportRequestsRequest(null, null, null, List.of(inputs)); + } + + @Test + void createDrafts_409ListsOnlyTheFactoriesWithNoLeadTime_andWritesNothing() { + long ticket = dealWithThreeFactories(); + + assertThatThrownBy(() -> service.createDrafts(ticket, null, owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).startsWith(MISSING_LEAD_TIME_PREFIX) + .contains("LT Missing PL").contains("LT Missing ZZ").doesNotContain("LT Good Factory"); + }); + assertThat(service.list(ticket, owner)).isEmpty(); + } + @Test - void draftLeavesLeadTimeNull_forZzAndUnmappedCodes() { - long zzFactory = insertFactory("ZZ Factory", "ZZ", "Farawaystan"); - long unmappedFactory = insertFactory("Unmapped Factory", "PL", null); - - long ticketZz = insertTicket("LT-ZZ"); - insertItem(ticketZz, "ZZ Factory", "Line", 0); - ImportRequestDto zzDraft = service.createDrafts(ticketZz, null, owner).get(0); - assertThat(zzDraft.leadTimeMinDays()).isNull(); - assertThat(zzDraft.leadTimeMaxDays()).isNull(); - - long ticketOther = insertTicket("LT-PL"); - insertItem(ticketOther, "Unmapped Factory", "Line", 0); - ImportRequestDto otherDraft = service.createDrafts(ticketOther, null, owner).get(0); - assertThat(otherDraft.leadTimeMinDays()).isNull(); - assertThat(otherDraft.leadTimeMaxDays()).isNull(); - - assertThat(zzFactory).isNotEqualTo(unmappedFactory); // sanity: two distinct rows used + void createDrafts_withOnlySomeLeadTimesSupplied_409ListsTheRest_andWritesNothing() { + long ticket = dealWithThreeFactories(); + + assertThatThrownBy(() -> service.createDrafts(ticket, + withLeadTimes(new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", 20, 30)), owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).startsWith(MISSING_LEAD_TIME_PREFIX) + .contains("LT Missing ZZ").doesNotContain("LT Missing PL").doesNotContain("LT Good Factory"); + }); + assertThat(service.list(ticket, owner)).isEmpty(); + } + + @Test + void createDrafts_withASuppliedLeadTime_issuesEveryRow_usingIt_andKeepsTheDefaultsElsewhere() { + long ticket = dealWithThreeFactories(); + + var holder = new java.util.concurrent.atomic.AtomicReference>(List.of()); + org.assertj.core.api.Assertions.assertThatCode(() -> holder.set(service.createDrafts(ticket, + withLeadTimes(new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", 20, 30), + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing ZZ", 5, 9)), owner))) + .doesNotThrowAnyException(); + + assertThat(holder.get()).hasSize(3).allSatisfy(r -> assertThat(r.status()).isEqualTo(ImportRequestStatus.ISSUED)); + assertThat(byName(holder.get(), "LT Missing PL").leadTimeMinDays()).isEqualTo(20); + assertThat(byName(holder.get(), "LT Missing PL").leadTimeMaxDays()).isEqualTo(30); + assertThat(byName(holder.get(), "LT Missing ZZ").leadTimeMinDays()).isEqualTo(5); + assertThat(byName(holder.get(), "LT Good Factory").leadTimeMinDays()).isEqualTo(75); // IT default + } + + @Test + void createDrafts_withABadSuppliedLeadTime_is400_andWritesNothing() { + long ticket = dealWithThreeFactories(); + var ok = new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing ZZ", 5, 9); + for (var bad : List.of( + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", 0, 30), // min < 1 + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", 40, 30), // min > max + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", 10, 366), // max > 365 + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", null, 30))) { // absent + assertThatThrownBy(() -> service.createDrafts(ticket, withLeadTimes(bad, ok), owner)) + .isInstanceOfSatisfying(ApiException.class, + e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST)); + assertThat(service.list(ticket, owner)).isEmpty(); + } + } + + @Test + void createDrafts_ignoresASuppliedLeadTime_forAFactoryThatAlreadyHasADefault() { + long ticket = dealWithThreeFactories(); + + List created = service.createDrafts(ticket, + withLeadTimes(new ImportRequestRequests.NewFactoryLeadTimeInput("LT Good Factory", 1, 2), + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing PL", 20, 30), + new ImportRequestRequests.NewFactoryLeadTimeInput("LT Missing ZZ", 5, 9)), owner); + + assertThat(byName(created, "LT Good Factory").leadTimeMinDays()).isEqualTo(75); + assertThat(byName(created, "LT Good Factory").leadTimeMaxDays()).isEqualTo(90); + } + + private static ImportRequestDto byName(List rows, String name) { + return rows.stream().filter(r -> name.equals(r.factoryName())).findFirst().orElseThrow(); } private void assertAutofill(String isoCode, int expectedMin, int expectedMax) { @@ -135,7 +233,7 @@ void issue_isRefused_whileLeadTimeIsUnset() { insertFactory("No Default Factory", "PL", null); long ticketId = insertTicket("LT-ISSUE-NULL"); insertItem(ticketId, "No Default Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThatThrownBy(() -> service.issue(id, null, owner)) .isInstanceOfSatisfying(ApiException.class, @@ -154,7 +252,7 @@ void draftLeadTime_isEditableByTheOwningRepOrCeo_notByANonOwningRepOrImport() { insertFactory("Editable Draft Factory", "PL", null); long ticketId = insertTicket("LT-DRAFT-EDIT"); insertItem(ticketId, "Editable Draft Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); // Owning rep: allowed. ImportRequestDto afterOwner = service.update(id, new UpdateImportRequestRequest( @@ -183,7 +281,7 @@ void issuedLeadTime_isEditableByImportOrCeo_notBySales() { insertFactory("Editable Issued Factory", "IT", null); // autofilled 75-90, satisfies issue gate long ticketId = insertTicket("LT-ISSUED-EDIT"); insertItem(ticketId, "Editable Issued Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); // Import: allowed. @@ -209,7 +307,7 @@ void setLeadTime_refusesAMinGreaterThanMax() { insertFactory("Bad Range Factory", "IT", null); long ticketId = insertTicket("LT-BADRANGE"); insertItem(ticketId, "Bad Range Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); assertThatThrownBy(() -> service.setLeadTime(id, new SetLeadTimeRequest(50, 10), importUser)) @@ -222,7 +320,7 @@ void draftUpdate_refusesOneFieldWithoutTheOther() { insertFactory("Partial Field Factory", "IT", null); long ticketId = insertTicket("LT-PARTIAL"); insertItem(ticketId, "Partial Field Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThatThrownBy(() -> service.update(id, new UpdateImportRequestRequest( null, null, null, null, null, null, null, null, null, 30, null, null), owner)) @@ -237,7 +335,7 @@ void revision_carriesLeadTimeForward_notReautofilled() { insertFactory("Revision Factory", "IT", null); // would autofill 75-90 long ticketId = insertTicket("LT-REVISE"); insertItem(ticketId, "Revision Factory", "Line", 0); - long v1 = service.createDrafts(ticketId, null, owner).get(0).id(); + long v1 = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); // Import/CEO hand-refine the estimate AFTER issue. service.issue(v1, null, owner); service.setLeadTime(v1, new SetLeadTimeRequest(60, 65), importUser); @@ -261,7 +359,7 @@ void setLeadTime_reDerivesTheAutoVesselEtaNote_andRegeneratesTheUnsentEmailBody( insertFactory("ETA Redrive Factory", "IT", null); // autofills 75-90 long ticketId = insertTicket("LT-ETA-REDERIVE"); insertItem(ticketId, "ETA Redrive Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issued = service.issue(id, null, owner); String autoNoteFor75to90 = issued.vesselEtaNote(); assertThat(autoNoteFor75to90).isNotBlank(); @@ -283,7 +381,7 @@ void setLeadTime_neverOverwritesACeoTypedCustomVesselEtaNote() { insertFactory("ETA Custom Factory", "IT", null); long ticketId = insertTicket("LT-ETA-CUSTOM"); insertItem(ticketId, "ETA Custom Factory", "Line", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(id, null, owner); ImportRequestDto ceoTyped = service.update(id, new UpdateImportRequestRequest( null, null, null, "Vessel confirmed by phone, mid-December", null, null, null, null, @@ -305,7 +403,7 @@ void revision_carriesVesselEtaNoteForward_andReDerivesOnlyIfStillAutoDerived() { insertFactory("ETA Revise Factory", "IT", null); // autofills 75-90 long ticketId = insertTicket("LT-ETA-REVISE"); insertItem(ticketId, "ETA Revise Factory", "Line", 0); - long v1 = service.createDrafts(ticketId, null, owner).get(0).id(); + long v1 = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issuedV1 = service.issue(v1, null, owner); assertThat(issuedV1.vesselEtaNote()).isNotBlank(); @@ -333,7 +431,7 @@ void revision_thenEditingLeadTime_thenIssue_showsTheNewRange_notThePredecessors( insertFactory("ETA Revise Then Edit Factory", "IT", null); // autofills 75-90 long ticketId = insertTicket("LT-ETA-REVISE-EDIT"); insertItem(ticketId, "ETA Revise Then Edit Factory", "Line", 0); - long v1 = service.createDrafts(ticketId, null, owner).get(0).id(); + long v1 = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); ImportRequestDto issuedV1 = service.issue(v1, null, owner); assertThat(issuedV1.vesselEtaNote()).contains("75").contains("90"); @@ -363,7 +461,7 @@ void revision_carriesACeoTypedCustomVesselEtaNoteForward_untouched() { insertFactory("ETA Revise Custom Factory", "IT", null); long ticketId = insertTicket("LT-ETA-REVISE-CUSTOM"); insertItem(ticketId, "ETA Revise Custom Factory", "Line", 0); - long v1 = service.createDrafts(ticketId, null, owner).get(0).id(); + long v1 = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); service.issue(v1, null, owner); service.update(v1, new UpdateImportRequestRequest( null, null, null, "Confirmed by phone, ETA mid-Jan", null, null, null, null, @@ -381,7 +479,7 @@ void expectedArrival_isDerivedFromIssueDatePlusLeadTime_andNullUntilIssued() { insertFactory("Arrival Factory", "IT", null); long ticketId = insertTicket("LT-ARRIVAL"); insertItem(ticketId, "Arrival Factory", "Line", 0); - ImportRequestDto draft = service.createDrafts(ticketId, null, owner).get(0); + ImportRequestDto draft = DraftSeed.seed(jdbc, ticketId, owner).get(0); // Null while DRAFT — no issueDate yet even though lead time is already autofilled. assertThat(draft.expectedArrivalFrom()).isNull(); diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestPaymentGateIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestPaymentGateIntegrationTest.java index e1ed5c503..8bd1497aa 100644 --- a/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestPaymentGateIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importrequest/ImportRequestPaymentGateIntegrationTest.java @@ -82,7 +82,7 @@ void secondFactorysFirstIssue_isAllowed_afterTheDealHasAdvancedToFullyPaid() { insertItem(ticketId, "Pay Gate Factory A", "Line A", "60x60", "10", "pcs", 0); insertItem(ticketId, "Pay Gate Factory B", "Line B", "60x60", "10", "pcs", 1); - List drafts = service.createDrafts(ticketId, null, owner); + List drafts = DraftSeed.seed(jdbc, ticketId, owner); long aId = byFactory(drafts, factoryA).id(); long bId = byFactory(drafts, factoryB).id(); assertThat(service.issue(aId, null, owner).status()).isEqualTo(ImportRequestStatus.ISSUED); @@ -99,7 +99,7 @@ void aRevisionsIssue_isAllowed_afterTheDealHasAdvancedToAwaitingFinalPayment() { insertFactory("Pay Gate Revise Factory"); long ticketId = insertTicket("GATE-AWAITING", "DEPOSIT_PAID"); insertItem(ticketId, "Pay Gate Revise Factory", "Line", "60x60", "10", "pcs", 0); - long v1 = service.createDrafts(ticketId, null, owner).get(0).id(); + long v1 = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThat(service.issue(v1, null, owner).status()).isEqualTo(ImportRequestStatus.ISSUED); setPaymentStatus(ticketId, "AWAITING_FINAL_PAYMENT"); @@ -117,13 +117,53 @@ void issue_isStillRefused_onADealWithNoDepositAtAll() { insertFactory("Pay Gate No Deposit Factory"); long ticketId = insertTicket("GATE-NODEPOSIT", null); insertItem(ticketId, "Pay Gate No Deposit Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThatThrownBy(() -> service.issue(id, null, owner)) .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); } + /** 10-29: the payment gate now applies at CREATE (create issues). Nothing is written when refused. */ + @Test + void createDrafts_isRefused409_andWritesNothing_onADealWithNoDepositYet() { + insertFactory("Pay Gate Create Factory"); + long ticketId = insertTicket("GATE-CREATE-NODEP", null); + insertItem(ticketId, "Pay Gate Create Factory", "Line", "60x60", "10", "pcs", 0); + + assertThatThrownBy(() -> service.createDrafts(ticketId, null, owner)) + .isInstanceOfSatisfying(ApiException.class, + e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT)); + + assertThat(service.list(ticketId, owner)).isEmpty(); + assertThat(new TicketRepository(jdbc).findById(ticketId).orElseThrow().summary() + .fulfillmentStatus()).isNull(); + } + + /** + * Review fix: the payment gate refuses FIRST. A deal with no paid deposit whose line also names a + * brand-new factory (no country, no lead time) must get the gate's 409, not the missing-country + * 409 that would walk sales through both popups before refusing — and nothing is written (no + * auto-created factory either). + */ + @Test + void createDrafts_paymentGateRefusesBeforeTheCountryAndLeadTimePrompts() { + long ticketId = insertTicket("GATE-FIRST", null); + insertItem(ticketId, "Brand New Gate Factory Co", "Line", "60x60", "10", "pcs", 0); + Long factoriesBefore = jdbc.queryForObject( + "SELECT count(*) FROM price_catalog.factories", Map.of(), Long.class); + + assertThatThrownBy(() -> service.createDrafts(ticketId, null, owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).contains("รับชำระมัดจำ").doesNotContain("ประเทศ"); + }); + + assertThat(service.list(ticketId, owner)).isEmpty(); + assertThat(jdbc.queryForObject("SELECT count(*) FROM price_catalog.factories", Map.of(), Long.class)) + .isEqualTo(factoriesBefore); + } + /** * Cheap nit (REVIEW ROUND 3): {@code CUSTOMER_CONFIRMED} on a deposit-REQUIRED policy (the * default — {@code insertTicket} never overrides {@code deposit_policy}, so this ticket carries @@ -139,7 +179,7 @@ void issue_isStillRefused_onCustomerConfirmed_whenDepositPolicyIsRequired() { insertFactory("Pay Gate Customer Confirmed Factory"); long ticketId = insertTicket("GATE-CUSTCONF", "CUSTOMER_CONFIRMED"); insertItem(ticketId, "Pay Gate Customer Confirmed Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThatThrownBy(() -> service.issue(id, null, owner)) .isInstanceOfSatisfying(ApiException.class, @@ -157,7 +197,7 @@ void issue_isAllowed_onADraftTicket_whenTheDepositIsPaid() { long ticketId = insertTicket("GATE-DRAFT-PAID", "DEPOSIT_PAID"); setTicketStatus(ticketId, "draft"); insertItem(ticketId, "Pay Gate Draft Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThat(service.issue(id, null, owner).status()).isEqualTo(ImportRequestStatus.ISSUED); } @@ -179,7 +219,7 @@ void issue_isStillRefused_onADraftTicket_withNoDeposit_orOnlyCustomerConfirmed() long ticketId = insertTicket("GATE-DR-" + (paymentStatus == null ? "NULL" : "CC"), paymentStatus); setTicketStatus(ticketId, "draft"); insertItem(ticketId, "Pay Gate Draft Refused Factory", "Line", "60x60", "10", "pcs", 0); - long id = service.createDrafts(ticketId, null, owner).get(0).id(); + long id = DraftSeed.seed(jdbc, ticketId, owner).get(0).id(); assertThatThrownBy(() -> service.issue(id, null, owner)) .isInstanceOfSatisfying(ApiException.class, diff --git a/backend/src/test/java/th/co/glr/hr/importrequest/StoredImportRequestIntegrationTest.java b/backend/src/test/java/th/co/glr/hr/importrequest/StoredImportRequestIntegrationTest.java index 57e685fe8..ac1ed1399 100644 --- a/backend/src/test/java/th/co/glr/hr/importrequest/StoredImportRequestIntegrationTest.java +++ b/backend/src/test/java/th/co/glr/hr/importrequest/StoredImportRequestIntegrationTest.java @@ -147,13 +147,6 @@ void list_ordersByFactoryName_notByTheNullableBrandSnapshot() { void createDrafts_isOnePerFactory_andSkipsFactoriesAlreadyCovered() { List first = service.createDrafts(ticketId, null, owner); assertThat(first).hasSize(2); - assertThat(first).allSatisfy(r -> { - assertThat(r.status()).isEqualTo(ImportRequestStatus.DRAFT); - assertThat(r.docNumber()).isNull(); - assertThat(r.issueDate()).isNull(); - // DRAFT carries no progress step yet (chk_import_request_step_pairing). - assertThat(r.importStep()).isNull(); - }); assertThat(first).extracting(ImportRequestDto::factoryId) .containsExactlyInAnyOrder(padanaFactoryId, leaFactoryId); assertThat(byFactory(first, padanaFactoryId).items()).extracting(i -> i.code()) @@ -166,9 +159,46 @@ void createDrafts_isOnePerFactory_andSkipsFactoriesAlreadyCovered() { assertThat(service.list(ticketId, owner)).hasSize(2); } + // ── 10-29 ruling: createDrafts creates AND ISSUES, all-or-nothing ──────────────────────────── + + @Test + void createDrafts_issuesEveryFactoryInOneStep_andMovesTheDealToProcurement() { + List created = service.createDrafts(ticketId, null, owner); + + assertThat(created).hasSize(2); + assertThat(created).allSatisfy(r -> { + assertThat(r.status()).isEqualTo(ImportRequestStatus.ISSUED); + assertThat(r.docNumber()).matches("IR\\d{2}\\d{3}"); + assertThat(r.importStep()).isEqualTo(ImportRequestStep.CONTACTED); + }); + assertThat(created).extracting(ImportRequestDto::docNumber).doesNotHaveDuplicates(); + // The tab and ImportFulfilmentPage read this: the ticket must now carry a fulfilment status. + var summary = new TicketRepository(jdbc).findById(ticketId).orElseThrow().summary(); + assertThat(summary.fulfillmentStatus()).isEqualTo("IR_ISSUED"); + assertThat(summary.salesStage()).isEqualTo(DealStage.PROCUREMENT); + } + + @Test + void createDrafts_isAllOrNothing_whenOneNewFactoryHasNoLeadTime() { + // country PL has no LeadTimeDefaults entry -> null lead time -> cannot be issued. + jdbc.update("INSERT INTO price_catalog.factories (name, country, default_currency) " + + "VALUES ('No Lead Time Factory', 'PL', 'EUR')", Map.of()); + insertItem(ticketId, "No Lead Time Factory", "Line", "60x60 cm", "5", "pcs", 3); + + assertThatThrownBy(() -> service.createDrafts(ticketId, null, owner)) + .isInstanceOfSatisfying(ApiException.class, e -> { + assertThat(e.getStatus()).isEqualTo(HttpStatus.CONFLICT); + assertThat(e.getMessage()).contains("No Lead Time Factory"); + }); + + assertThat(service.list(ticketId, owner)).isEmpty(); + assertThat(new TicketRepository(jdbc).findById(ticketId).orElseThrow().summary() + .fulfillmentStatus()).isNull(); + } + @Test void issue_mintsTheNumberAndFreezesTheDate_andSetsTheFirstStepToContacted() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); ImportRequestDto issued = service.issue(id, null, owner); @@ -187,7 +217,7 @@ void issue_mintsTheNumberAndFreezesTheDate_andSetsTheFirstStepToContacted() { @Test void issue_withAnOverride_doesNotBurnASequenceValue() { - List drafts = service.createDrafts(ticketId, null, owner); + List drafts = DraftSeed.seed(jdbc, ticketId, owner); int seqBefore = sequenceValue(); service.issue(byFactory(drafts, padanaFactoryId).id(), @@ -201,7 +231,7 @@ void issue_withAnOverride_doesNotBurnASequenceValue() { @Test void issue_withAnAlreadyUsedOverride_isRefused() { - List drafts = service.createDrafts(ticketId, null, owner); + List drafts = DraftSeed.seed(jdbc, ticketId, owner); service.issue(byFactory(drafts, padanaFactoryId).id(), new IssueImportRequestRequest("IR69068"), owner); @@ -218,7 +248,7 @@ void issue_withAnAlreadyUsedOverride_isRefused() { */ @Test void revise_leavesThePreviousVersionIssuedUntilTheReplacementIsIssued_andCarriesTheStepForward() { - long v1 = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long v1 = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(v1, null, owner); service.advanceStep(v1, new AdvanceImportStepRequest(ImportRequestStep.ORDERED, null, null), importUser); @@ -242,7 +272,7 @@ void revise_leavesThePreviousVersionIssuedUntilTheReplacementIsIssued_andCarries /** A revision cannot even be prepared once the predecessor has reached RECEIVED. */ @Test void revise_isRefusedOnceThePredecessorHasReceivedTheGoods() { - long v1 = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long v1 = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(v1, null, owner); for (String step : List.of(ImportRequestStep.ORDERED, ImportRequestStep.PICKED_UP, ImportRequestStep.IN_TRANSIT, ImportRequestStep.AWAITING_CUSTOMS, ImportRequestStep.RECEIVED)) { @@ -256,7 +286,7 @@ void revise_isRefusedOnceThePredecessorHasReceivedTheGoods() { @Test void issuedForm_bodyIsFrozenEvenToTheOwningRep() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, null, owner); // Body: refused to import outright — FORBIDDEN, not CONFLICT, because import is not even in @@ -285,7 +315,7 @@ void issuedForm_bodyIsFrozenEvenToTheOwningRep() { */ @Test void issuedFormFooter_isWritableByCeoOnly_ownerRepImportAndSalesManagerAllRefused() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); ImportRequestDto issued = service.issue(id, null, owner); // vesselEtaNote is auto-prefilled by the SYSTEM at issue (owner decision 09-18 #3 §A) — a // service-internal write, unaffected by 09-19's actor-role gate. Captured here so the @@ -320,7 +350,7 @@ void issuedFormFooter_isWritableByCeoOnly_ownerRepImportAndSalesManagerAllRefuse @Test void deleteDraft_removesADraftButNeverAnIssuedForm() { - List drafts = service.createDrafts(ticketId, null, owner); + List drafts = DraftSeed.seed(jdbc, ticketId, owner); long lea = byFactory(drafts, leaFactoryId).id(); long padana = byFactory(drafts, padanaFactoryId).id(); service.issue(padana, null, owner); @@ -336,7 +366,7 @@ void deleteDraft_removesADraftButNeverAnIssuedForm() { @Test void aStoredFormRendersFromItsOwnSnapshot_notFromTheDeal() throws Exception { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, new IssueImportRequestRequest("IR69068"), owner); jdbc.update("UPDATE sales.ticket SET customer_name = 'ลูกค้าใหม่' WHERE ticket_id = :id", @@ -354,7 +384,7 @@ void aStoredFormRendersFromItsOwnSnapshot_notFromTheDeal() throws Exception { */ @Test void aStoredFormsPrintedHeader_namesItsOwnFactory() throws Exception { - long padanaId = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long padanaId = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(padanaId, new IssueImportRequestRequest("IR-S-B-1"), owner); String text = textOf(service.renderStored(padanaId, importUser)); @@ -370,7 +400,7 @@ void aStoredFormsPrintedHeader_namesItsOwnFactory() throws Exception { */ @Test void factoryCopy_omitsCustomerNameAndDepositDate_internalCopyKeepsBoth() throws Exception { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, new IssueImportRequestRequest("IR-OWNER-0919-2"), owner); String internal = textOf(service.renderStored(id, importUser, false)); @@ -394,7 +424,7 @@ void factoryCopy_omitsCustomerNameAndDepositDate_internalCopyKeepsBoth() throws */ @Test void uniquePartialIndex_refusesASecondIssuedRowForTheSameTicketAndFactory_evenBypassingTheService() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, new IssueImportRequestRequest("IR-UNIQ-1"), owner); assertThatThrownBy(() -> jdbc.update(""" @@ -412,7 +442,7 @@ void uniquePartialIndex_refusesASecondIssuedRowForTheSameTicketAndFactory_evenBy @Test void advanceStep_forwardSkipsAllowed_butNeverBackwards() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, null, owner); // Skip ORDERED and PICKED_UP entirely — forward skips are allowed by owner decision. @@ -434,7 +464,7 @@ void advanceStep_forwardSkipsAllowed_butNeverBackwards() { @Test void advanceStep_isRefusedToSalesAndSalesManager_butAllowedToCeo() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, null, owner); assertThatThrownBy(() -> service.advanceStep(id, @@ -461,7 +491,7 @@ void nonOwningSalesRepCannotCreateIssueOrRevise() { e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.FORBIDDEN)); assertThat(stored.findByTicket(ticketId)).isEmpty(); - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); assertThatThrownBy(() -> service.issue(id, null, otherSalesRep)) .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.FORBIDDEN)); @@ -480,7 +510,7 @@ void importCannotCreateIssueOrRevise() { e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.FORBIDDEN)); assertThat(stored.findByTicket(ticketId)).isEmpty(); - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); assertThatThrownBy(() -> service.issue(id, null, importUser)) .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.FORBIDDEN)); @@ -509,7 +539,7 @@ void salesManagerIsReadOnly() { .isInstanceOfSatisfying(ApiException.class, e -> assertThat(e.getStatus()).isEqualTo(HttpStatus.FORBIDDEN)); - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, null, owner); assertThatThrownBy(() -> service.update(id, new UpdateImportRequestRequest( @@ -528,7 +558,7 @@ void salesManagerIsReadOnly() { @Test void ceoCanRaiseAndIssue() { - long id = byFactory(service.createDrafts(ticketId, null, ceoUser), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, ceoUser), padanaFactoryId).id(); assertThat(service.issue(id, null, ceoUser).status()).isEqualTo(ImportRequestStatus.ISSUED); } @@ -546,7 +576,7 @@ void ceoCanRaiseAndIssue() { void requiredByNote_isSetByTheOwningRep_andSnapshottedOntoTheFormAtCreation() { service.setRequiredByNote(ticketId, new SetRequiredByNoteRequest("Within 21/5/26"), owner); - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); assertThat(service.get(id, owner).requiredByNote()).isEqualTo("Within 21/5/26"); } @@ -557,7 +587,7 @@ void requiredByNote_isSetByTheOwningRep_andSnapshottedOntoTheFormAtCreation() { */ @Test void requiredByNote_isEditableOnTheDraftByOwningRepOrCeo_notByANonOwningRepOrImport() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); ImportRequestDto afterOwner = service.update(id, new UpdateImportRequestRequest( null, null, null, null, null, null, null, null, null, null, null, "Within 10 days"), owner); @@ -589,7 +619,7 @@ void requiredByNote_isReSnapshottedAtIssue_onlyWhenStillBlankOnTheDraft() { // ONE createDrafts call raises BOTH factories' drafts at once (setUp's ticket already has // items for both) — capture both ids from it rather than calling createDrafts a second // time, which would find every factory already covered and 409. - List drafts = service.createDrafts(ticketId, null, owner); + List drafts = DraftSeed.seed(jdbc, ticketId, owner); long blankId = byFactory(drafts, padanaFactoryId).id(); long typedId = byFactory(drafts, leaFactoryId).id(); assertThat(service.get(blankId, owner).requiredByNote()).isNull(); @@ -618,7 +648,7 @@ void requiredByNote_isReSnapshottedAtIssue_onlyWhenStillBlankOnTheDraft() { */ @Test void requiredByNote_typedOnTheFormBeforeItsFirstIssue_survivesARevision() { - long v1 = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long v1 = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.update(v1, new UpdateImportRequestRequest( null, null, null, null, null, null, null, null, null, null, null, "Before Songkran"), owner); @@ -642,7 +672,7 @@ void requiredByNote_typedOnTheFormBeforeItsFirstIssue_survivesARevision() { */ @Test void nonOwningSalesRepCannotReadListOrDownloadAnotherRepsIr() throws Exception { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, new IssueImportRequestRequest("IR-S3-READ"), owner); assertThatThrownBy(() -> service.get(id, otherSalesRep)) @@ -671,7 +701,7 @@ void nonOwningSalesRepCannotReadListOrDownloadAnotherRepsIr() throws Exception { */ @Test void importAndSalesManagerCannotDeleteDraft() { - long lea = byFactory(service.createDrafts(ticketId, null, owner), leaFactoryId).id(); + long lea = byFactory(DraftSeed.seed(jdbc, ticketId, owner), leaFactoryId).id(); assertThatThrownBy(() -> service.deleteDraft(lea, importUser)) .isInstanceOfSatisfying(ApiException.class, @@ -694,7 +724,7 @@ void importAndSalesManagerCannotDeleteDraft() { */ @Test void everyMutatingMethod_isRefusedOnceTheDealIsInactive() { - long padana = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long padana = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(padana, null, owner); long lea = byFactory(service.list(ticketId, owner), leaFactoryId).id(); // A THIRD, not-yet-covered factory, added before markLost — so the post-lost createDrafts @@ -760,7 +790,7 @@ void everyMutatingMethod_isRefusedOnceTheDealIsInactive() { */ @Test void revision_doesNotMoveTheArrivalAnchor() { - long v1 = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long v1 = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.update(v1, new UpdateImportRequestRequest( null, null, null, null, null, null, null, null, null, 10, 20, null), owner); ImportRequestDto issuedV1 = service.issue(v1, null, owner); @@ -782,7 +812,7 @@ void revision_doesNotMoveTheArrivalAnchor() { @Test void advanceStep_refusesAFutureEventDate_andADateBeforeTheFirstIssue() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); ImportRequestDto issued = service.issue(id, null, owner); assertThatThrownBy(() -> service.advanceStep(id, new AdvanceImportStepRequest( @@ -802,7 +832,7 @@ void advanceStep_refusesAFutureEventDate_andADateBeforeTheFirstIssue() { * not the raw uppercase code. */ @Test void advanceStep_eventMessageUsesThaiStepLabels() { - long id = byFactory(service.createDrafts(ticketId, null, owner), padanaFactoryId).id(); + long id = byFactory(DraftSeed.seed(jdbc, ticketId, owner), padanaFactoryId).id(); service.issue(id, null, owner); service.advanceStep(id, new AdvanceImportStepRequest(ImportRequestStep.ORDERED, null, "ทดสอบ"), importUser); 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 ee35f4aed..9961efab8 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 @@ -25,8 +25,9 @@ * {@code ceo}/{@code import} gate by exactly one role, to {@code sales}, so the deal-create modal's * "ราคาตั้ง (ประมาณการ)" estimate can convert a catalog price to THB for a plain {@code sales} * session. It was NOT opened to every authenticated session — the owner's words were "should only - * be to sale". See {@link FxRateController#READ_ROLES}'s javadoc for the full reasoning, including - * why this does NOT relax {@code PriceCalcConfigController}. + * be to sale". The owner widened it once more on 2026-10-29 (role check RC-01), to + * {@code sales_manager}. See {@link FxRateController#READ_ROLES}'s javadoc for the full reasoning, + * including why this does NOT relax {@code PriceCalcConfigController}. * *

    This class proves the DECISION (which branch the role check takes). * {@code CatalogPricingReadAuthzIntegrationTest} proves the ENFORCEMENT on real Postgres through the @@ -36,17 +37,16 @@ * suite that only asserts "sales can read" stays green with the gate wide open. */ class FxRateControllerTest { - /** The whole of the 2026-08-02 ruling. Anything added here is a new ruling, not a refactor. */ - private static final List READ_ALLOWED = List.of("ceo", "import", "sales"); - /** - * Everyone else. {@code sales_manager} belongs here deliberately: it browses the deal pipeline - * but cannot create a deal ({@code canCreateTickets} is {@code ['sales']} and - * {@code TicketService.create} gates on {@code Set.of("sales")}), so it never reaches the modal - * that needs the rate. + * The 2026-08-02 ruling plus {@code sales_manager}, owner ruling 2026-10-29 (role check RC-01: + * the deal page converts foreign-currency prices to THB for SM too). Anything added here is a + * new ruling, not a refactor. */ + private static final List READ_ALLOWED = List.of("ceo", "import", "sales", "sales_manager"); + + /** Everyone else. */ private static final List READ_DENIED = - List.of("employee", "warehouse", "qc", "hr", "sales_manager", "account"); + List.of("employee", "warehouse", "qc", "hr", "account"); /** Everyone but CEO — the write gate that /fetch-now shares with upsert. */ private static final List WRITE_DENIED = diff --git a/frontend/e2e-real/helpers/sales.js b/frontend/e2e-real/helpers/sales.js index cd922f4df..3269a3e43 100644 --- a/frontend/e2e-real/helpers/sales.js +++ b/frontend/e2e-real/helpers/sales.js @@ -977,8 +977,8 @@ export async function waiveDeposit(sessions, ticketId, { policy = 'NOT_REQUIRED' } /** - * POST /api/tickets/{id}/import-requests — ImportRequestController.createDrafts: one DRAFT ใบขอซื้อ - * per FACTORY (V184). This is how every deal starts its import now: the one-click legacy + * POST /api/tickets/{id}/import-requests — ImportRequestController.createDrafts: one ใบขอซื้อ, + * created AND ISSUED in the same call (owner ruling 2026-10-29), per FACTORY (V184). This is how every deal starts its import now: the one-click legacy * `POST /api/tickets/{id}/import-request` (issueImportRequest) answers 409 for every deal (W3-04, * owner ruling 2026-10-29). Needs the deal's OWNING sales rep (or ceo) and DealStage >= ORDER_RECEIVED; * each line must resolve to a factory (PCR item factory, catalogue link or typed name), else 409 diff --git a/frontend/e2e-real/uat-sales-journeys.spec.js b/frontend/e2e-real/uat-sales-journeys.spec.js index 6ed4324c7..8b738f011 100644 --- a/frontend/e2e-real/uat-sales-journeys.spec.js +++ b/frontend/e2e-real/uat-sales-journeys.spec.js @@ -17,7 +17,6 @@ import { declareStockCoverage, expectNeverEntered, expectRoute, - issueImportRequestDraft, logActivity, readDeal, resolveFactory, @@ -171,12 +170,10 @@ test.describe('@uat-sales pipeline journeys — Cases A-D', () => { // it is written against the controllers (ImportRequestController) and has not been executed. let importRequests; await test.step('S10 -> S12 (-> PROCUREMENT): auto, the first per-factory ใบขอซื้อ is issued — S11 (DEPOSIT_RECEIVED) is a real, structural skip here, not an oversight', async () => { - const drafts = await createImportRequestDrafts(sessions, deal.id); - expect(drafts.length, 'one DRAFT per factory').toBeGreaterThan(0); - importRequests = []; - for (const draft of drafts) { - importRequests.push(await issueImportRequestDraft(sessions, draft.id)); - } + // Create ISSUES one ใบขอซื้อ per factory in the same call (10-29): nothing left to issue. + importRequests = await createImportRequestDrafts(sessions, deal.id); + expect(importRequests.length, 'one ISSUED ใบขอซื้อ per factory').toBeGreaterThan(0); + for (const ir of importRequests) expect(ir.status).toBe('ISSUED'); const ticket = await readDeal(sessions, 'sales', deal.id); expect(ticket.summary.salesStage).toBe('PROCUREMENT'); expect(ticket.summary.fulfillmentStatus).toBe('IR_ISSUED'); diff --git a/frontend/src/api/mockApi.js b/frontend/src/api/mockApi.js index 02f28a458..9370030fc 100644 --- a/frontend/src/api/mockApi.js +++ b/frontend/src/api/mockApi.js @@ -12367,6 +12367,21 @@ export const api = { if (dealStageIndex(ticket.salesStage) < dealStageIndex('ORDER_RECEIVED')) { fail('สร้างใบขอซื้อได้หลังจากยืนยันคำสั่งซื้อแล้ว (ORDER_RECEIVED) เท่านั้น', 409); } + // Payment gate FIRST (same predicate as issue() below), before any prompt-producing check. + if (!(['DEPOSIT_NOTICE_ISSUED', 'DEPOSIT_PAID', 'AWAITING_FINAL_PAYMENT', 'FULLY_PAID'].includes(ticket.paymentStatus) + || (depositBypassesNotice(ticket) && ticket.paymentStatus === 'CUSTOMER_CONFIRMED'))) { + fail('ออกใบขอนำเข้า (IR) ได้เฉพาะเมื่อรับชำระมัดจำแล้ว (หรือได้รับการยกเว้นมัดจำ) เท่านั้น', 409); + } + // Supplied per-factory lead times (10-29): 400 on a bad value, before anything is written. + const suppliedLeadTimes = new Map(); + for (const lt of request?.leadTimes ?? []) { + const okInt = (v) => Number.isInteger(v) && v >= 1 && v <= 365; + if (!lt?.factoryName?.trim() || !okInt(lt.minDays) || !okInt(lt.maxDays) || lt.minDays > lt.maxDays) { + fail('ระยะเวลานำเข้าต้องเป็น 1–365 วัน และขั้นต่ำต้องไม่เกินขั้นสูง', 400); + } + suppliedLeadTimes.set(lt.factoryName.trim().replace(/\s+/g, ' ').toLowerCase(), lt); + } + const suppliedFor = (g) => suppliedLeadTimes.get(String(g.factoryName).trim().replace(/\s+/g, ' ').toLowerCase()); mockApplyItemFactoryChoices(ticket, request); const groups = mockResolveFactoryGroups(ticket, request); if (groups.length === 0) { @@ -12375,11 +12390,22 @@ export const api = { const alreadyCovered = new Set(mockImportRequests .filter((r) => r.ticketId === ticket.id && (r.status === 'DRAFT' || r.status === 'ISSUED')) .map((r) => r.factoryId)); - let created = 0; const now = new Date().toISOString(); - for (const group of groups) { - if (alreadyCovered.has(group.factoryId)) continue; - const leadTime = mockLeadTimeDefaults(group.countryCode); + // 10-29: create also ISSUES, all-or-nothing -- check every NEW factory before writing any row + // (the payment gate itself is re-checked per row by issue() below, same predicate). + const newGroups = groups.filter((g) => !alreadyCovered.has(g.factoryId)); + const missingLeadTime = newGroups + .filter((g) => !mockLeadTimeDefaults(g.countryCode) && !suppliedFor(g)) + .map((g) => g.factoryName); + if (missingLeadTime.length > 0) { + fail(`กรุณาระบุระยะเวลานำเข้าให้กับโรงงาน: ${missingLeadTime.join(', ')}`, 409); + } + if (newGroups.length === 0) fail('ทุกโรงงานในดีลนี้มีใบขอซื้อที่ยังใช้งานอยู่แล้ว', 409); + const newIds = []; + for (const group of newGroups) { + const supplied = suppliedFor(group); + const leadTime = mockLeadTimeDefaults(group.countryCode) + ?? (supplied ? { min: supplied.minDays, max: supplied.maxDays } : null); const id = mockImportRequestSeq++; mockImportRequests.push({ id, ticketId: ticket.id, ticketCode: ticket.code, @@ -12400,9 +12426,10 @@ export const api = { id: `${id}-${i + 1}`, importRequestId: id, ticketItemId: it.ticketItemId, seq: i + 1, ...it, })), }); - created++; + newIds.push(id); } - if (created === 0) fail('ทุกโรงงานในดีลนี้มีใบขอซื้อที่ยังใช้งานอยู่แล้ว', 409); + // Reuse the mock's issue(): doc number, step CONTACTED, IR_ISSUED + PROCUREMENT on the first. + for (const id of newIds) await api.storedImportRequests.issue(id); return delay({ importRequests: mockImportRequestsForTicket(ticket.id) }); }, async listForTicket(ticketId) { @@ -12927,7 +12954,7 @@ export const api = { }, // Mirrors FactoryConfigController#names: id + name only, for the IR creators too. async names() { - hasRole('ceo', 'import', 'sales', 'sales_manager'); + hasRole('ceo', 'import', 'sales', 'sales_manager', 'account'); return delay({ factories: mockPriceImportFactories.map((f) => ({ id: f.factoryId, factoryName: f.name })) }); }, // sendEmail is retired — POST /api/tickets/{id}/factory-emails/send no longer exists (factory @@ -12938,12 +12965,13 @@ export const api = { // Owner ruling 2026-08-02: list() widened from #388's ceo/import gate by exactly one role, to // sales, so the deal-create modal's ราคาตั้ง estimate can convert a catalog price to THB for a // plain rep. NOT opened to every authenticated session — the ruling was "should only be to sale". - // See FxRateController.READ_ROLES for the full reasoning; it does NOT extend to priceCalcConfigs - // below, which is the real margin policy. upsert stays CEO-only, unchanged. + // sales_manager added 2026-10-29 (RC-01). See FxRateController.READ_ROLES for the full + // reasoning; it does NOT extend to priceCalcConfigs below, which is the real margin policy. + // upsert stays CEO-only, unchanged. // Reminder: this mock's authz is NOT authoritative — the Java service is (see CLAUDE.md). fxRates: { async list() { - hasRole('ceo', 'import', 'sales'); + hasRole('ceo', 'import', 'sales', 'sales_manager'); return delay({ fxRates: structuredClone(mockFxRates) }); }, async upsert(currency, payload) { diff --git a/frontend/src/api/mockApi.storedImportRequestRollup.test.js b/frontend/src/api/mockApi.storedImportRequestRollup.test.js index b44af51a6..c26872e45 100644 --- a/frontend/src/api/mockApi.storedImportRequestRollup.test.js +++ b/frontend/src/api/mockApi.storedImportRequestRollup.test.js @@ -146,7 +146,7 @@ async function driveTwoFactoryTicketToOrderReceived() { describe('mockApi storedImportRequests — S3: rollup requires EVERY required factory issued+received', () => { it( - 'does NOT roll the deal up to GOODS_RECEIVED while a second required factory is still only DRAFT', + 'does NOT roll the deal up to GOODS_RECEIVED while a second required factory is issued but not yet received', async () => { const { ticketId } = await driveTwoFactoryTicketToOrderReceived(); @@ -159,15 +159,15 @@ describe('mockApi storedImportRequests — S3: rollup requires EVERY required fa // given role), so this used to switch to 'ceo' for the call and is no longer allowed to. await api.tickets.setDepositPolicy(ticketId, { policy: 'NOT_REQUIRED', reason: 'ทดสอบ S3' }); - const { importRequests: drafts } = await api.storedImportRequests.createDrafts(ticketId, {}); - expect(drafts).toHaveLength(2); // Panaria SpA + REFIN. - const panariaDraft = drafts.find((r) => r.factoryName === 'Panaria SpA'); - const refinDraft = drafts.find((r) => r.factoryName === 'REFIN'); - expect(panariaDraft).toBeTruthy(); - expect(refinDraft).toBeTruthy(); + // 10-29: create also ISSUES, so both rows come back ISSUED. + const { importRequests: created } = await api.storedImportRequests.createDrafts(ticketId, {}); + expect(created).toHaveLength(2); // Panaria SpA + REFIN. + const panariaIssued = created.find((r) => r.factoryName === 'Panaria SpA'); + const refinIssued = created.find((r) => r.factoryName === 'REFIN'); + expect(panariaIssued?.status).toBe('ISSUED'); + expect(refinIssued?.status).toBe('ISSUED'); - // Issue and fully advance ONLY Panaria SpA — REFIN stays a DRAFT, never issued. - const { importRequest: panariaIssued } = await api.storedImportRequests.issue(panariaDraft.id, {}); + // Fully advance ONLY Panaria SpA — REFIN stays issued but not received. await api.auth.login({ role: 'import' }); for (const target of ['ORDERED', 'PICKED_UP', 'IN_TRANSIT', 'AWAITING_CUSTOMS']) { await api.storedImportRequests.advanceStep(panariaIssued.id, { targetStep: target }); @@ -185,9 +185,7 @@ describe('mockApi storedImportRequests — S3: rollup requires EVERY required fa expect(afterPanaria.fulfillmentStatus).not.toBe('GOODS_RECEIVED'); expect(afterPanaria.fulfillmentStatus).toBe('IR_ISSUED'); - // Now issue and receive REFIN too — every required factory has an ISSUED row, all RECEIVED. - await api.auth.login({ role: 'sales' }); - const { importRequest: refinIssued } = await api.storedImportRequests.issue(refinDraft.id, {}); + // Now receive REFIN too — every required factory has an ISSUED row, all RECEIVED. await api.auth.login({ role: 'import' }); for (const target of ['ORDERED', 'PICKED_UP', 'IN_TRANSIT', 'AWAITING_CUSTOMS', 'RECEIVED']) { await api.storedImportRequests.advanceStep(refinIssued.id, { targetStep: target }); @@ -201,7 +199,7 @@ describe('mockApi storedImportRequests — S3: rollup requires EVERY required fa }); describe('mockApi storedImportRequests — S4: bypass issue gate requires EXACTLY CUSTOMER_CONFIRMED', () => { - it('issue() succeeds once a bypass-policy deal is genuinely CUSTOMER_CONFIRMED', async () => { + it('create (which now issues) succeeds once a bypass-policy deal is genuinely CUSTOMER_CONFIRMED', async () => { const { ticketId } = await driveTwoFactoryTicketToOrderReceived(); // driveTwoFactoryTicketToOrderReceived's own confirmOrder already leaves paymentStatus at // CUSTOMER_CONFIRMED — setting a bypass policy on TOP of that is exactly the "bypass-policy @@ -212,9 +210,9 @@ describe('mockApi storedImportRequests — S4: bypass issue gate requires EXACTL await api.tickets.setDepositPolicy(ticketId, { policy: 'WAIVED', reason: 'ทดสอบ S4' }); const { importRequests: drafts } = await api.storedImportRequests.createDrafts(ticketId, {}); - const draft = drafts[0]; - const { importRequest: issued } = await api.storedImportRequests.issue(draft.id, {}); - expect(issued.status).toBe('ISSUED'); + // 10-29: the payment gate now applies at create, which also issues. + expect(drafts.length).toBeGreaterThan(0); + expect(drafts[0].status).toBe('ISSUED'); }); }); diff --git a/frontend/src/features/catalog/CatalogSearchPage.jsx b/frontend/src/features/catalog/CatalogSearchPage.jsx index 03fd34cc2..4addffc22 100644 --- a/frontend/src/features/catalog/CatalogSearchPage.jsx +++ b/frontend/src/features/catalog/CatalogSearchPage.jsx @@ -88,7 +88,10 @@ export function CatalogSearchPage({ user, showToast }) { const inputRef = useRef(null); useEffect(() => { - api.priceImport.factories().then(setFactories).catch(() => {}); + // Names-only read (RC-03): /api/price-import/factories is ceo/import and carries supplier emails. + api.factoryConfigs.names() + .then((res) => setFactories((res.factories ?? []).map((f) => ({ factoryId: f.id, name: f.factoryName })))) + .catch(() => {}); inputRef.current?.focus(); }, []); diff --git a/frontend/src/features/catalog/CatalogSearchPage.test.jsx b/frontend/src/features/catalog/CatalogSearchPage.test.jsx index 9fe4a59f8..3b2c68993 100644 --- a/frontend/src/features/catalog/CatalogSearchPage.test.jsx +++ b/frontend/src/features/catalog/CatalogSearchPage.test.jsx @@ -11,6 +11,9 @@ vi.mock('../../api/index.js', () => ({ priceImport: { factories: vi.fn(), }, + factoryConfigs: { + names: vi.fn(), + }, catalog: { prices: vi.fn(), }, @@ -24,7 +27,10 @@ vi.mock('../../hooks/useIsMobile.js', () => ({ describe('CatalogSearchPage audit copy', () => { beforeEach(() => { vi.clearAllMocks(); - api.priceImport.factories.mockResolvedValue([]); + // Mocked the way Java answers sales/sales_manager: PriceImportController#factories is + // ceo/import only; FactoryConfigController#names is open to every role that can open /catalog. + api.priceImport.factories.mockRejectedValue(Object.assign(new Error('forbidden'), { status: 403 })); + api.factoryConfigs.names.mockResolvedValue({ factories: [] }); api.catalog.prices.mockResolvedValue({ items: [] }); }); @@ -34,4 +40,14 @@ describe('CatalogSearchPage audit copy', () => { expect(await screen.findByText('ราคาจากรายการราคาที่ใช้งานล่าสุด')).not.toBeNull(); expect(screen.queryByText(/price list|ACTIVE/i)).toBeNull(); }); + + // RC-03 (role check 2026-10-29, owner ruling): the factory filter was silently empty for sales + // and sales_manager because it read the ceo/import-only /api/price-import/factories. + it('fills the factory filter from the names-only endpoint', async () => { + api.factoryConfigs.names.mockResolvedValue({ factories: [{ id: 42, factoryName: 'Mutina' }] }); + render(); + + const option = await screen.findByRole('option', { name: 'Mutina' }); + expect(option.getAttribute('value')).toBe('42'); + }); }); diff --git a/frontend/src/features/dashboard/HrOverview.jsx b/frontend/src/features/dashboard/HrOverview.jsx index 6eb94464f..435af7ad7 100644 --- a/frontend/src/features/dashboard/HrOverview.jsx +++ b/frontend/src/features/dashboard/HrOverview.jsx @@ -7,7 +7,7 @@ import { Button } from '../../components/common/Button.jsx'; import { StatCard } from '../../components/common/StatCard.jsx'; import { PageHeader } from '../../components/common/PageHeader.jsx'; import { PageStack, Panel, StatGrid } from '../../components/common/Layout.jsx'; -import { formatThaiDate, payrollStatusLabel } from '../../utils/format.js'; +import { bangkokMonthStartIso, formatThaiDate, payrollStatusLabel } from '../../utils/format.js'; import { divisions, findDivision } from '../../data/referenceData.js'; import { ActionQueue } from './ActionQueue.jsx'; @@ -74,9 +74,12 @@ function MonitorRow({ label, value, onClick }) { export function HrOverview({ employees, dashboardSummary }) { const navigate = useNavigate(); + // PayrollController requires payrollMonth (RC-04); Bangkok month, not UTC. Own key suffix: + // PayrollPage caches a different shape under payrollCurrent(month). + const payrollMonth = bangkokMonthStartIso().slice(0, 7); const payrollQuery = useQuery({ - queryKey: queryKeys.payrollCurrent(), - queryFn: () => api.payroll.current().then((response) => response?.period ?? null).catch(() => null), + queryKey: [...queryKeys.payrollCurrent(payrollMonth), 'overview'], + queryFn: () => api.payroll.current({ payrollMonth }).then((response) => response?.period ?? null), enabled: !!api.payroll?.current, }); const payrollPeriod = payrollQuery.data ?? null; @@ -152,9 +155,11 @@ export function HrOverview({ employees, dashboardSummary }) { { key: 'notifications', label: 'แจ้งเตือนยังไม่อ่าน', value: stats.unreadNotifications }, ]; - const payrollDetail = payrollPeriod - ? `${formatThaiDate(payrollPeriod.payrollMonth)} · ${payrollStatusLabel(payrollPeriod.status).label}` - : 'ยังไม่เริ่มรอบเงินเดือนเดือนนี้'; + const payrollDetail = payrollQuery.isError + ? 'โหลดรอบเงินเดือนไม่สำเร็จ' + : payrollPeriod + ? `${formatThaiDate(payrollPeriod.payrollMonth)} · ${payrollStatusLabel(payrollPeriod.status).label}` + : 'ยังไม่เริ่มรอบเงินเดือนเดือนนี้'; return ( diff --git a/frontend/src/features/dashboard/HrOverview.test.jsx b/frontend/src/features/dashboard/HrOverview.test.jsx index fbfd1cae2..72cde6711 100644 --- a/frontend/src/features/dashboard/HrOverview.test.jsx +++ b/frontend/src/features/dashboard/HrOverview.test.jsx @@ -1,9 +1,11 @@ import React from 'react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; import { MemoryRouter } from 'react-router-dom'; -import { render, screen, within } from '@testing-library/react'; +import { render, screen, waitFor, within } from '@testing-library/react'; import { describe, expect, it, vi } from 'vitest'; import { HrOverview } from './HrOverview.jsx'; +import { api } from '../../api/index.js'; +import { queryKeys } from '../../api/queryKeys.js'; globalThis.React = React; @@ -47,10 +49,9 @@ const dashboardSummary = { notifications: { unread: 5 }, }; -function renderOverview(summary = dashboardSummary) { - const queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, - }); +function renderOverview(summary = dashboardSummary, queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, +})) { return render( @@ -81,6 +82,49 @@ describe('HrOverview', () => { expect(screen.getByText('ไปที่หน้าเงินเดือน').closest('button')).not.toBeNull(); }); + // RC-04 (role check 2026-10-29): PayrollController#currentOrPreview requires payrollMonth and + // 400s without it. 2026-10-31T18:00Z is already 1 Nov in Bangkok, so a UTC month would send + // the wrong period on the last evening of every month. + it('asks for the current Bangkok payroll month', async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date('2026-10-31T18:00:00Z')); + try { + api.payroll.current.mockClear(); + renderOverview(); + await waitFor(() => expect(api.payroll.current).toHaveBeenCalledWith({ payrollMonth: '2026-11' })); + } finally { + vi.useRealTimers(); + } + }); + + // Review of RC-04: PayrollPage caches { period, suggestionsByEmployee, ... } under + // payrollCurrent(month). The landing must not overwrite that entry with a bare period, or the + // payroll page opened within staleTime renders an empty table. + it('leaves the payroll page cache entry for the same month untouched', async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + vi.setSystemTime(new Date('2026-10-15T05:00:00Z')); + try { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const payrollPageEntry = { period: { id: 7, status: 'PROCESSED' }, suggestionsByEmployee: {} }; + queryClient.setQueryData(queryKeys.payrollCurrent('2026-10'), payrollPageEntry); + api.payroll.current.mockClear(); + renderOverview(dashboardSummary, queryClient); + await waitFor(() => expect(api.payroll.current).toHaveBeenCalled()); + await waitFor(() => expect(queryClient.isFetching()).toBe(0)); + expect(queryClient.getQueryData(queryKeys.payrollCurrent('2026-10'))).toBe(payrollPageEntry); + } finally { + vi.useRealTimers(); + } + }); + + // A failed payroll read used to render as "not started", which is how RC-04's 400 hid. + it('says the payroll period failed to load instead of claiming it has not started', async () => { + api.payroll.current.mockRejectedValueOnce(Object.assign(new Error('bad request'), { status: 400 })); + renderOverview(); + expect(await screen.findByText('โหลดรอบเงินเดือนไม่สำเร็จ')).not.toBeNull(); + expect(screen.queryByText('ยังไม่เริ่มรอบเงินเดือนเดือนนี้')).toBeNull(); + }); + it('shows today attendance counts', () => { renderOverview(); const panel = screen.getByText('การลงเวลาวันนี้').closest('section'); diff --git a/frontend/src/features/fulfilment/ImportFulfilmentPage.jsx b/frontend/src/features/fulfilment/ImportFulfilmentPage.jsx index 39df45d37..4c46b3012 100644 --- a/frontend/src/features/fulfilment/ImportFulfilmentPage.jsx +++ b/frontend/src/features/fulfilment/ImportFulfilmentPage.jsx @@ -202,16 +202,14 @@ export function ImportFulfilmentPage({ user, showToast }) { let completesDeal = false; if (targetStep === 'RECEIVED') { try { - const [{ ticket: freshTicket }, { importRequests: freshRows }] = await Promise.all([ - api.tickets.get(ticketId), - api.storedImportRequests.listForTicket(ticketId), - ]); - const fs = freshTicket?.summary?.fulfillmentStatus; + // importDeals.get, not tickets.get: TicketService#get refuses role import (RC-02). + const { deal: freshDeal } = await api.importDeals.get(ticketId); + const fs = freshDeal?.fulfillmentStatus; if (fs === 'GOODS_RECEIVED') { completesDeal = true; } else if (fs === 'PARTIALLY_DELIVERED') { // A factory still in DRAFT means the server's allFactoriesReceived declined the rollup. - const all = freshRows ?? []; + const all = freshDeal?.importRequests ?? []; const issued = all.filter((r) => r.status === 'ISSUED'); completesDeal = issued.length > 0 && issued.every((r) => r.importStep === 'RECEIVED') && !all.some((r) => r.status === 'DRAFT'); diff --git a/frontend/src/features/fulfilment/ImportFulfilmentPage.test.jsx b/frontend/src/features/fulfilment/ImportFulfilmentPage.test.jsx index eff2f68cf..e405376b1 100644 --- a/frontend/src/features/fulfilment/ImportFulfilmentPage.test.jsx +++ b/frontend/src/features/fulfilment/ImportFulfilmentPage.test.jsx @@ -21,6 +21,7 @@ vi.mock('../../api/index.js', async (importOriginal) => { ...actual, api: { tickets: { list: vi.fn(), get: vi.fn() }, + importDeals: { get: vi.fn() }, storedImportRequests: { listForTicket: vi.fn(), advanceStep: vi.fn(), @@ -102,7 +103,10 @@ describe('ImportFulfilmentPage (per-factory, stored aggregate)', () => { beforeEach(() => { vi.clearAllMocks(); api.tickets.list.mockResolvedValue({ tickets: [ticket()] }); - api.tickets.get.mockResolvedValue({ ticket: { summary: { fulfillmentStatus: 'IR_ISSUED' } } }); + // RC-02 (role check 2026-10-29): TicketService#get refuses import outright, so the deal read + // is mocked the way Java answers it; the page reads import's own projection instead. + api.tickets.get.mockRejectedValue(Object.assign(new Error('ไม่มีสิทธิ์เข้าถึงรายการนี้'), { status: 403 })); + api.importDeals.get.mockResolvedValue({ deal: { fulfillmentStatus: 'IR_ISSUED' } }); mockRowsByTicket({ 1: [row()] }); api.storedImportRequests.advanceStep.mockResolvedValue({ importRequest: row({ importStep: 'PICKED_UP' }) }); }); @@ -493,7 +497,7 @@ describe('ImportFulfilmentPage (per-factory, stored aggregate)', () => { // after the advance actually lands. describe('X4 — completion toast reflects the refetched ticket, not a client-side guess', () => { it('shows the ordinary step-advanced toast, never the "รับครบทุกโรงงาน" claim, when the refetched ticket is NOT GOODS_RECEIVED', async () => { - api.tickets.get.mockResolvedValue({ ticket: { summary: { fulfillmentStatus: 'IR_ISSUED' } } }); + api.importDeals.get.mockResolvedValue({ deal: { fulfillmentStatus: 'IR_ISSUED' } }); mockRowsByTicket({ 1: [row({ id: 1, importStep: 'AWAITING_CUSTOMS' })] }); api.storedImportRequests.advanceStep.mockResolvedValue({ importRequest: row({ id: 1, importStep: 'RECEIVED' }) }); const showToast = vi.fn(); @@ -506,7 +510,7 @@ describe('ImportFulfilmentPage (per-factory, stored aggregate)', () => { }); it('shows the completion toast when the refetched ticket is genuinely GOODS_RECEIVED', async () => { - api.tickets.get.mockResolvedValue({ ticket: { summary: { fulfillmentStatus: 'GOODS_RECEIVED' } } }); + api.importDeals.get.mockResolvedValue({ deal: { fulfillmentStatus: 'GOODS_RECEIVED' } }); mockRowsByTicket({ 1: [row({ id: 1, importStep: 'AWAITING_CUSTOMS' })] }); api.storedImportRequests.advanceStep.mockResolvedValue({ importRequest: row({ id: 1, importStep: 'RECEIVED' }) }); const showToast = vi.fn(); @@ -519,15 +523,14 @@ describe('ImportFulfilmentPage (per-factory, stored aggregate)', () => { it('shows the completion toast for a PARTIALLY_DELIVERED deal once the refetched rows confirm every issued row is RECEIVED', async () => { api.tickets.list.mockResolvedValue({ tickets: [ticket({ fulfillmentStatus: 'PARTIALLY_DELIVERED' })] }); - api.tickets.get.mockResolvedValue({ ticket: { summary: { fulfillmentStatus: 'PARTIALLY_DELIVERED' } } }); + // The confirmation read is import's own projection, which carries the post-advance rows; + // listForTicket still answers with the pre-advance rows, so a toast decided from it is wrong. + api.importDeals.get.mockResolvedValue({ deal: { + fulfillmentStatus: 'PARTIALLY_DELIVERED', + importRequests: [row({ id: 1, importStep: 'RECEIVED' })], + } }); mockRowsByTicket({ 1: [row({ id: 1, importStep: 'AWAITING_CUSTOMS' })] }); - api.storedImportRequests.advanceStep.mockImplementationOnce(async () => { - // The advance's own refetch (listForTicket, called directly by this page's onSuccess to - // decide the toast) must see the POST-advance state, exactly like a real advanceStep call - // landing before the follow-up read does — same pattern as the B3 test above. - mockRowsByTicket({ 1: [row({ id: 1, importStep: 'RECEIVED' })] }); - return { importRequest: row({ id: 1, importStep: 'RECEIVED' }) }; - }); + api.storedImportRequests.advanceStep.mockResolvedValue({ importRequest: row({ id: 1, importStep: 'RECEIVED' }) }); const showToast = vi.fn(); renderPage(showToast); const card = await screen.findByTestId('fulfilment-deal'); @@ -538,7 +541,10 @@ describe('ImportFulfilmentPage (per-factory, stored aggregate)', () => { it('does not claim completion for a PARTIALLY_DELIVERED deal while another issued row is still short of RECEIVED', async () => { api.tickets.list.mockResolvedValue({ tickets: [ticket({ fulfillmentStatus: 'PARTIALLY_DELIVERED' })] }); - api.tickets.get.mockResolvedValue({ ticket: { summary: { fulfillmentStatus: 'PARTIALLY_DELIVERED' } } }); + api.importDeals.get.mockResolvedValue({ deal: { + fulfillmentStatus: 'PARTIALLY_DELIVERED', + importRequests: [row({ id: 1, importStep: 'RECEIVED' }), row({ id: 2, factoryName: 'B', importStep: 'IN_TRANSIT' })], + } }); mockRowsByTicket({ 1: [row({ id: 1, importStep: 'AWAITING_CUSTOMS' }), row({ id: 2, factoryName: 'B', importStep: 'IN_TRANSIT' })], }); @@ -554,13 +560,13 @@ describe('ImportFulfilmentPage (per-factory, stored aggregate)', () => { it('does not claim completion for a PARTIALLY_DELIVERED deal while another factory is still a DRAFT', async () => { api.tickets.list.mockResolvedValue({ tickets: [ticket({ fulfillmentStatus: 'PARTIALLY_DELIVERED' })] }); - api.tickets.get.mockResolvedValue({ ticket: { summary: { fulfillmentStatus: 'PARTIALLY_DELIVERED' } } }); const draftB = row({ id: 2, factoryName: 'B', status: 'DRAFT', importStep: null }); + api.importDeals.get.mockResolvedValue({ deal: { + fulfillmentStatus: 'PARTIALLY_DELIVERED', + importRequests: [row({ id: 1, importStep: 'RECEIVED' }), draftB], + } }); mockRowsByTicket({ 1: [row({ id: 1, importStep: 'AWAITING_CUSTOMS' }), draftB] }); - api.storedImportRequests.advanceStep.mockImplementationOnce(async () => { - mockRowsByTicket({ 1: [row({ id: 1, importStep: 'RECEIVED' }), draftB] }); - return { importRequest: row({ id: 1, importStep: 'RECEIVED' }) }; - }); + api.storedImportRequests.advanceStep.mockResolvedValue({ importRequest: row({ id: 1, importStep: 'RECEIVED' }) }); const showToast = vi.fn(); renderPage(showToast); const card = await screen.findByTestId('fulfilment-deal'); diff --git a/frontend/src/features/payroll/PayrollPage.jsx b/frontend/src/features/payroll/PayrollPage.jsx index f0444185a..72b155512 100644 --- a/frontend/src/features/payroll/PayrollPage.jsx +++ b/frontend/src/features/payroll/PayrollPage.jsx @@ -19,7 +19,7 @@ import { TaxAllowanceDrilldown } from '../taxAllowance/TaxAllowanceDrilldown.jsx import { useDialogFocus } from '../../hooks/useDialogFocus.js'; import { useIsMobile, useMediaQuery } from '../../hooks/useIsMobile.js'; import { cn } from '../../utils/cn.js'; -import { formatMoney, formatShortDate, formatThaiMonthYearFromMonthInputValue, payrollStatusLabel as statusInfo } from '../../utils/format.js'; +import { bangkokMonthStartIso, formatMoney, formatShortDate, formatThaiMonthYearFromMonthInputValue, payrollStatusLabel as statusInfo } from '../../utils/format.js'; // Reproduces `.panel` for the detail sidebar, which must stay a real