From d5e6d354a0bc6fd06c5337759445f710999b77b6 Mon Sep 17 00:00:00 2001 From: Jeff Gibson Date: Fri, 18 Sep 2026 14:34:53 -0600 Subject: [PATCH] feat: add adjustment return tax and align dependency bumps Port invoice-tax fixes: ITEM_ADJ/REPAIR_ADJ return estimates, EXTERNAL_CHARGE product identity, alternate tax-line field names, and skip empty sales POSTs. Bump Jackson annotations/core/databind together and apply the other green Dependabot upgrades so CI stays green. Leave oss-parent at 0.146.x. --- .github/workflows/codeql-analysis.yml | 8 +- .github/workflows/release.yml | 9 +- docs/aviate-integration.md | 4 +- pom.xml | 16 +- .../killbill/KintsugiInvoicePluginApi.java | 62 +++-- .../internal/InvoiceRequestMapper.java | 159 +++++++++++-- .../internal/InvoiceTaxIdempotency.java | 63 +++++- .../killbill/internal/KintsugiTaxClient.java | 10 +- .../killbill/internal/TaxItemMapper.java | 13 +- .../internal/TestInvoiceRequestMapper.java | 212 +++++++++++++++++- .../internal/TestInvoiceTaxIdempotency.java | 50 ++++- .../internal/TestKintsugiTaxClient.java | 2 +- 12 files changed, 543 insertions(+), 65 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 76f0f7c..ecad5fe 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -16,13 +16,13 @@ jobs: contents: read security-events: write steps: - - uses: actions/checkout@v4 - - uses: actions/setup-java@v4 + - uses: actions/checkout@v7 + - uses: actions/setup-java@v6 with: distribution: temurin java-version: "11" - - uses: github/codeql-action/init@v3 + - uses: github/codeql-action/init@v4 with: languages: java - run: mvn -B -DskipTests package - - uses: github/codeql-action/analyze@v3 + - uses: github/codeql-action/analyze@v4 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e46f72a..9e78b04 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,11 +22,11 @@ jobs: steps: - name: Checkout code if: github.event.inputs.perform_version == '' - uses: actions/checkout@v2 + uses: actions/checkout@v7 - name: Checkout full repository # Required when performing an existing release. if: github.event.inputs.perform_version != '' - uses: actions/checkout@v2 + uses: actions/checkout@v7 with: fetch-depth: '0' - name: Setup git user @@ -38,9 +38,10 @@ jobs: git config --global user.name "Kill Bill core team" git config --global url."https://${BUILD_USER}:${BUILD_TOKEN}@github.com/".insteadOf "git@github.com:" - name: Configure Java - uses: actions/setup-java@v1 + uses: actions/setup-java@v6 with: java-version: 11 + distribution: temurin - name: Download Java dependencies # We do as much as we can, but it may not be enough (https://issues.apache.org/jira/browse/MDEP-82) run: | @@ -59,7 +60,7 @@ jobs: # Will be pushed as part of the release process, only if the release is successful git commit -m "pom.xml: update killbill-oss-parent to ${{ github.event.inputs.parent_version }}" - name: Configure settings.xml for release - uses: actions/setup-java@v3 + uses: actions/setup-java@v6 with: java-version: 11 distribution: temurin diff --git a/docs/aviate-integration.md b/docs/aviate-integration.md index 1211b64..68f36fa 100644 --- a/docs/aviate-integration.md +++ b/docs/aviate-integration.md @@ -1,6 +1,6 @@ # Aviate integration (0.1.0) -Kill Bill feedback on this plugin asks for enterprise parity with Aviate deployment patterns. Shipped in **0.1.0**. +Kill Bill feedback on [PR #1](https://github.com/killbill/killbill-kintsugi-plugin/pull/1) asks for enterprise parity with Aviate deployment patterns. Shipped in **0.1.0**. ## Deployment paths (Kill Bill team guidance) @@ -40,7 +40,7 @@ Documented in `InvoicePluginPropertyNames` for coordination with the Aviate plug ## Planned follow-ups * **Aviate Catalog tax codes** — resolve plan → product → Aviate tax code from tenant config or API. -* **Repairs and return documents** — `ITEM_ADJ` / `REPAIR_ADJ` as return lines; stable document IDs. +* ~~**Repairs and return documents** — `ITEM_ADJ` / `REPAIR_ADJ` as return lines; stable document IDs.~~ Shipped MVP: return estimate with `{invoiceId}:adj-return`, negative TAX linked to adj item; lenient skip when linked item missing. Follow-ups: TaxDate from original invoice, strict mode, CREDIT_ADJ policy. ## References diff --git a/pom.xml b/pom.xml index d429ba2..b0edaba 100644 --- a/pom.xml +++ b/pom.xml @@ -74,20 +74,28 @@ + + + com.fasterxml.jackson.core + jackson-annotations + 2.22 + com.fasterxml.jackson.core jackson-core - 2.18.6 + 2.22.2 com.fasterxml.jackson.core jackson-databind - 2.18.6 + 2.22.2 com.github.tomakehurst wiremock-standalone - 2.27.2 + 3.0.1 test @@ -214,7 +222,7 @@ org.apache.maven.plugins maven-surefire-plugin - 3.2.5 + 3.6.0 fast diff --git a/src/main/java/com/trykintsugi/killbill/KintsugiInvoicePluginApi.java b/src/main/java/com/trykintsugi/killbill/KintsugiInvoicePluginApi.java index e400f15..2561e96 100644 --- a/src/main/java/com/trykintsugi/killbill/KintsugiInvoicePluginApi.java +++ b/src/main/java/com/trykintsugi/killbill/KintsugiInvoicePluginApi.java @@ -38,11 +38,18 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; -/** Invoice plugin that delegates tax calculation to the Kintsugi tax API. */ +/** + * Invoice plugin that delegates tax calculation to the Kintsugi tax API. + * + *

Sales path: taxable lines → estimate/commit → positive TAX. + * Return path: untaxed {@code ITEM_ADJ}/{@code REPAIR_ADJ} → return + * estimate → negative TAX linked to the adj item. + */ public final class KintsugiInvoicePluginApi extends PluginInvoicePluginApi { private static final Logger LOGGER = LoggerFactory.getLogger(KintsugiInvoicePluginApi.class); @@ -80,9 +87,11 @@ public KintsugiAdditionalItemsResult getAdditionalInvoiceItems( return emptyResult(); } - if (InvoiceTaxIdempotency.allTaxableItemsAlreadyTaxed(invoice)) { + final boolean salesNeeded = !InvoiceTaxIdempotency.allTaxableItemsAlreadyTaxed(invoice); + final List untaxedAdjustments = InvoiceTaxIdempotency.untaxedAdjustmentItems(invoice); + if (!salesNeeded && untaxedAdjustments.isEmpty()) { LOGGER.debug( - "Skipping Kintsugi tax for invoice account {} — taxable lines already have TAX items", + "Skipping Kintsugi tax for invoice account {} — sales and adjustments already taxed", invoice.getAccountId()); return emptyResult(); } @@ -99,28 +108,49 @@ public KintsugiAdditionalItemsResult getAdditionalInvoiceItems( config, tenantApiKey, tenant.getApiSecret()); - final ObjectNode requestBody = InvoiceRequestMapper.toEstimateRequest( - invoice, - account, - dryRun, - invoiceContext.getTenantId() != null ? invoiceContext.getTenantId().toString() : null, - taxMetadata); + final String tenantIdStr = invoiceContext.getTenantId() != null + ? invoiceContext.getTenantId().toString() + : null; final KintsugiTaxClient client = new KintsugiTaxClient( config.getKintsugiUrl(), config.getHmacSecret(), tenantApiKey); - final List taxLines = - client.estimate(requestBody, !dryRun); - - final Map taxableById = TaxItemMapper.indexTaxableItems(invoice); - final List taxItems = TaxItemMapper.toTaxItems(invoice, taxLines, taxableById); + final Map itemsById = TaxItemMapper.indexTaxableItems(invoice); + final List taxItems = new ArrayList<>(); + + if (salesNeeded) { + final ObjectNode salesBody = InvoiceRequestMapper.toEstimateRequest( + invoice, account, dryRun, tenantIdStr, taxMetadata); + // Defense: never POST an empty sales document (adj/credit-only invoices). + if (salesBody.path("documents").path(0).path("line_items").size() > 0) { + final List salesTaxLines = + client.estimate(salesBody, !dryRun); + taxItems.addAll(TaxItemMapper.toTaxItems(invoice, salesTaxLines, itemsById)); + } + } + + if (!untaxedAdjustments.isEmpty()) { + final ObjectNode returnBody = InvoiceRequestMapper.toReturnEstimateRequest( + invoice, account, dryRun, tenantIdStr, taxMetadata, untaxedAdjustments); + if (returnBody != null) { + final List returnTaxLines = + client.estimate(returnBody, !dryRun); + taxItems.addAll(TaxItemMapper.toTaxItems(invoice, returnTaxLines, itemsById)); + } else { + LOGGER.debug( + "No return-tax lines for invoice account {} after lenient skip of unlinked adjs", + invoice.getAccountId()); + } + } LOGGER.info( - "Kintsugi returned {} tax line(s) for invoice account {}", + "Kintsugi returned {} tax line(s) for invoice account {} (salesNeeded={}, adjReturns={})", taxItems.size(), - invoice.getAccountId()); + invoice.getAccountId(), + salesNeeded, + untaxedAdjustments.size()); return new KintsugiAdditionalItemsResult(taxItems); } catch (InvoicePluginApiRetryException e) { diff --git a/src/main/java/com/trykintsugi/killbill/internal/InvoiceRequestMapper.java b/src/main/java/com/trykintsugi/killbill/internal/InvoiceRequestMapper.java index ac758c5..9257215 100644 --- a/src/main/java/com/trykintsugi/killbill/internal/InvoiceRequestMapper.java +++ b/src/main/java/com/trykintsugi/killbill/internal/InvoiceRequestMapper.java @@ -28,6 +28,7 @@ import java.math.BigDecimal; import java.math.RoundingMode; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.UUID; @@ -36,9 +37,8 @@ public final class InvoiceRequestMapper { private static final ObjectMapper MAPPER = new ObjectMapper(); - /** Default product labels for external charges without a plan name. */ - static final String EXTERNAL_CHARGE_CATEGORY = "Physical"; - static final String EXTERNAL_CHARGE_SUBCATEGORY = "General Physical"; + /** Shared product external id for plan-less {@code EXTERNAL_CHARGE} lines. */ + static final String EXTERNAL_CHARGE_PRODUCT_EXTERNAL_ID = "EXTERNAL_CHARGE"; private InvoiceRequestMapper() {} @@ -81,16 +81,7 @@ public static ObjectNode toEstimateRequest( if (item.getInvoiceItemType() != null) { line.put("item_type", item.getInvoiceItemType().name()); } - if (item.getPlanName() != null) { - line.put("plan_name", item.getPlanName()); - line.put("external_product_id", item.getPlanName()); - } - if (item.getPrettyProductName() != null) { - line.put("product_name", item.getPrettyProductName()); - } else if (item.getInvoiceItemType() == InvoiceItemType.EXTERNAL_CHARGE) { - line.put("product_category", EXTERNAL_CHARGE_CATEGORY); - line.put("product_subcategory", EXTERNAL_CHARGE_SUBCATEGORY); - } + putProductIdentity(line, item); final String taxCode = metadata.taxCodeForItem(item.getId()); if (taxCode != null) { line.put("tax_code", taxCode); @@ -122,7 +113,8 @@ public static ObjectNode toEstimateRequest( if (invoice.getInvoiceNumber() != null) { final ObjectNode document = (ObjectNode) root.path("documents").get(0); - document.put("invoice_number", invoice.getInvoiceNumber()); + // Emit invoice_number as a JSON string (not a number). + document.put("invoice_number", String.valueOf(invoice.getInvoiceNumber())); } return root; @@ -140,6 +132,10 @@ public static Map externalIdsForInvoice(final Invoice invoice) { return externalIds; } + /** + * Item types omitted from the sales estimate payload (TAX + adjustments). + * Return adjustments are mapped separately via {@link #toReturnEstimateRequest}. + */ public static boolean isSkippedItemType(final InvoiceItemType type) { return type == InvoiceItemType.TAX || type == InvoiceItemType.ITEM_ADJ @@ -147,6 +143,141 @@ public static boolean isSkippedItemType(final InvoiceItemType type) { || type == InvoiceItemType.REPAIR_ADJ; } + /** + * Return adjustment types sent as a return estimate. {@code CREDIT_ADJ} is excluded + * because it is not a return of a taxable line. + */ + public static boolean isReturnAdjustmentItemType(final InvoiceItemType type) { + return type == InvoiceItemType.ITEM_ADJ || type == InvoiceItemType.REPAIR_ADJ; + } + + /** + * Builds a return-tax estimate for untaxed {@code ITEM_ADJ}/{@code REPAIR_ADJ} lines. + * Document id is {@code {invoiceId}:adj-return} to distinguish returns from sales. + * + *

Lenient mode (MVP): adjustments without a resolvable linked taxable item are skipped. + */ + public static ObjectNode toReturnEstimateRequest( + final Invoice invoice, + final Account account, + final boolean dryRun, + final String tenantId, + final AccountTaxMetadata taxMetadata, + final List untaxedAdjustments) { + final AccountTaxMetadata metadata = taxMetadata != null ? taxMetadata : AccountTaxMetadata.empty(); + final Map itemsById = indexItemsById(invoice); + final ArrayNode lineItems = MAPPER.createArrayNode(); + + for (final InvoiceItem adj : untaxedAdjustments) { + if (adj.getId() == null) { + continue; + } + // Lenient: missing linked original → skip rather than fail the invoice. + if (adj.getLinkedItemId() == null || !itemsById.containsKey(adj.getLinkedItemId())) { + continue; + } + final InvoiceItem linked = itemsById.get(adj.getLinkedItemId()); + BigDecimal amount = adj.getAmount() != null ? adj.getAmount() : BigDecimal.ZERO; + if (amount.compareTo(BigDecimal.ZERO) > 0) { + amount = amount.negate(); + } + if (amount.compareTo(BigDecimal.ZERO) == 0) { + continue; + } + + final ObjectNode line = MAPPER.createObjectNode(); + line.put("external_id", adj.getId().toString()); + line.put("amount", formatAmount(amount)); + line.put("quantity", "1"); + line.put("kind", "item"); + line.put("invoice_item_id", adj.getId().toString()); + if (adj.getInvoiceItemType() != null) { + line.put("item_type", adj.getInvoiceItemType().name()); + } + if (adj.getDescription() != null) { + line.put("description", adj.getDescription()); + } else { + line.put("description", "Invoice item adjustment"); + } + // Same product identity rules as sales, from the linked taxable line. + putProductIdentity(line, linked); + final String taxCode = metadata.taxCodeForItem(linked.getId()); + if (taxCode != null) { + line.put("tax_code", taxCode); + } + line.put("linked_invoice_item_id", adj.getLinkedItemId().toString()); + lineItems.add(line); + } + + if (lineItems.isEmpty()) { + return null; + } + + final String documentId = (invoice.getId() != null ? invoice.getId().toString() : UUID.randomUUID().toString()) + + ":adj-return"; + final ObjectNode shipTo = resolveAddress(account, metadata); + final ObjectNode billTo = accountToAddress(account); + final ObjectNode customer = accountToCustomer(account, metadata); + final String transactionDate = formatInvoiceDate(invoice); + + final ObjectNode root = KintsugiTaxClient.buildEstimateRequest( + UUID.randomUUID().toString(), + invoice.getCurrency().toString(), + documentId, + invoice.getAccountId().toString(), + dryRun, + lineItems, + shipTo, + billTo, + customer, + transactionDate); + + if (tenantId != null && !tenantId.isBlank()) { + root.put("tenant_id", tenantId); + } + root.put("plugin_name", "killbill-kintsugi"); + + // Always attach document_kind; emit invoice_number as a JSON string. + final ObjectNode document = (ObjectNode) root.path("documents").get(0); + document.put("document_kind", "return"); + if (invoice.getInvoiceNumber() != null) { + document.put("invoice_number", String.valueOf(invoice.getInvoiceNumber())); + } + + return root; + } + + + /** + * Map plan/product fields the same way for sales and return lines. + * Plan-less {@code EXTERNAL_CHARGE} uses the shared sentinel product id. + */ + private static void putProductIdentity(final ObjectNode line, final InvoiceItem item) { + // Product key from plan when present; else shared EXTERNAL_CHARGE sentinel. + // prettyProductName is display-only and must not gate the sentinel. + final String planName = item.getPlanName(); + if (planName != null && !planName.isBlank()) { + line.put("plan_name", planName); + line.put("external_product_id", planName); + } else if (item.getInvoiceItemType() == InvoiceItemType.EXTERNAL_CHARGE) { + line.put("external_product_id", EXTERNAL_CHARGE_PRODUCT_EXTERNAL_ID); + } + final String prettyName = item.getPrettyProductName(); + if (prettyName != null && !prettyName.isBlank()) { + line.put("product_name", prettyName); + } + } + + private static Map indexItemsById(final Invoice invoice) { + final Map byId = new HashMap<>(); + for (final InvoiceItem item : invoice.getInvoiceItems()) { + if (item.getId() != null) { + byId.put(item.getId(), item); + } + } + return byId; + } + private static boolean shouldSkipItem(final InvoiceItem item) { return isSkippedItemType(item.getInvoiceItemType()); } diff --git a/src/main/java/com/trykintsugi/killbill/internal/InvoiceTaxIdempotency.java b/src/main/java/com/trykintsugi/killbill/internal/InvoiceTaxIdempotency.java index 51bc7b7..b388ade 100644 --- a/src/main/java/com/trykintsugi/killbill/internal/InvoiceTaxIdempotency.java +++ b/src/main/java/com/trykintsugi/killbill/internal/InvoiceTaxIdempotency.java @@ -20,18 +20,26 @@ import org.killbill.billing.invoice.api.InvoiceItem; import org.killbill.billing.invoice.api.InvoiceItemType; +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.UUID; -/** Detects when tax items were already generated for an invoice (re-invoice idempotency). */ +/** + * Detects when tax items were already generated for an invoice (re-invoice idempotency). + * + *

Sales coverage: every taxable line has a linked {@code TAX} item. + * Return coverage: every {@code ITEM_ADJ}/{@code REPAIR_ADJ} has a linked {@code TAX} + * (negative return tax), so a re-invoice does not tax an already-covered adjustment. + */ public final class InvoiceTaxIdempotency { private InvoiceTaxIdempotency() {} /** * Returns true when every taxable line on the invoice already has a linked {@code TAX} item. - * Avoids duplicate tax on a second plugin pass when TAX items are already linked. + * Adjustment lines are ignored for sales coverage (see {@link #untaxedAdjustmentItems}). */ public static boolean allTaxableItemsAlreadyTaxed(final Invoice invoice) { if (invoice.getInvoiceItems() == null || invoice.getInvoiceItems().isEmpty()) { @@ -39,7 +47,7 @@ public static boolean allTaxableItemsAlreadyTaxed(final Invoice invoice) { } final Set taxableItemIds = new HashSet<>(); - final Set taxedItemIds = new HashSet<>(); + final Set taxedItemIds = taxedLinkedItemIds(invoice); for (final InvoiceItem item : invoice.getInvoiceItems()) { if (item.getId() == null) { @@ -49,17 +57,56 @@ public static boolean allTaxableItemsAlreadyTaxed(final Invoice invoice) { continue; } if (item.getInvoiceItemType() == InvoiceItemType.TAX) { - if (item.getLinkedItemId() != null) { - taxedItemIds.add(item.getLinkedItemId()); - } - } else if (!InvoiceRequestMapper.isSkippedItemType(item.getInvoiceItemType())) { + continue; + } + if (!InvoiceRequestMapper.isSkippedItemType(item.getInvoiceItemType())) { taxableItemIds.add(item.getId()); } } + // No sales lines (adj/credit/tax-only invoice) → sales path is done; return path + // decides separately. Returning false here would POST an empty sales estimate + // and block return tax for ITEM_ADJ/REPAIR_ADJ-only invoices. if (taxableItemIds.isEmpty()) { - return false; + return true; } return taxedItemIds.containsAll(taxableItemIds); } + + /** + * Adjustment lines ({@code ITEM_ADJ}/{@code REPAIR_ADJ}) that do not yet have a linked TAX. + * Used so post-invoice adjustments still get return tax after sales lines are already taxed. + */ + public static List untaxedAdjustmentItems(final Invoice invoice) { + final List untaxed = new ArrayList<>(); + if (invoice.getInvoiceItems() == null) { + return untaxed; + } + final Set taxedItemIds = taxedLinkedItemIds(invoice); + for (final InvoiceItem item : invoice.getInvoiceItems()) { + if (!InvoiceRequestMapper.isReturnAdjustmentItemType(item.getInvoiceItemType())) { + continue; + } + if (item.getId() == null || taxedItemIds.contains(item.getId())) { + continue; + } + untaxed.add(item); + } + return untaxed; + } + + /** True when neither sales nor return tax work remains for this invoice. */ + public static boolean nothingLeftToTax(final Invoice invoice) { + return allTaxableItemsAlreadyTaxed(invoice) && untaxedAdjustmentItems(invoice).isEmpty(); + } + + private static Set taxedLinkedItemIds(final Invoice invoice) { + final Set taxedItemIds = new HashSet<>(); + for (final InvoiceItem item : invoice.getInvoiceItems()) { + if (item.getInvoiceItemType() == InvoiceItemType.TAX && item.getLinkedItemId() != null) { + taxedItemIds.add(item.getLinkedItemId()); + } + } + return taxedItemIds; + } } diff --git a/src/main/java/com/trykintsugi/killbill/internal/KintsugiTaxClient.java b/src/main/java/com/trykintsugi/killbill/internal/KintsugiTaxClient.java index 653d715..60822f7 100644 --- a/src/main/java/com/trykintsugi/killbill/internal/KintsugiTaxClient.java +++ b/src/main/java/com/trykintsugi/killbill/internal/KintsugiTaxClient.java @@ -81,8 +81,13 @@ public List estimate( httpClient.send(request, HttpResponse.BodyHandlers.ofString()); if (response.statusCode() < 200 || response.statusCode() >= 300) { + final String body = response.body(); + final String snippet = body == null + ? "" + : body.substring(0, Math.min(body.length(), 500)); throw new IllegalStateException( - "Kintsugi tax API returned HTTP " + response.statusCode()); + "Kintsugi tax API returned HTTP " + response.statusCode() + + (snippet.isEmpty() ? "" : ": " + snippet)); } return parseTaxLines(response.body()); @@ -106,8 +111,7 @@ static List parseTaxLines(final String responseJson) throws Excep continue; } for (final JsonNode line : lineItems) { - // Mosaic response uses line_external_id / rate_percentage; accept - // legacy external_id / rate for older fixtures and wiremocks. + // Prefer line_external_id / rate_percentage; fall back to external_id / rate. final String externalId = firstText(line, "line_external_id", "external_id"); final BigDecimal taxAmount = new BigDecimal(line.path("tax_amount").asText("0")); final String rateText = firstText(line, "rate_percentage", "rate"); diff --git a/src/main/java/com/trykintsugi/killbill/internal/TaxItemMapper.java b/src/main/java/com/trykintsugi/killbill/internal/TaxItemMapper.java index 1c28a33..acae2ad 100644 --- a/src/main/java/com/trykintsugi/killbill/internal/TaxItemMapper.java +++ b/src/main/java/com/trykintsugi/killbill/internal/TaxItemMapper.java @@ -80,9 +80,16 @@ private static InvoiceItem buildTaxItem( final InvoiceItem linkedItem, final LocalDate invoiceDate, final KintsugiTaxClient.TaxLineResult taxLine) { - final String description = taxLine.ratePercent() != null - ? String.format("Sales tax (%.2f%%)", taxLine.ratePercent()) - : "Sales tax"; + final String description; + if (taxLine.taxAmount().compareTo(BigDecimal.ZERO) < 0) { + description = taxLine.ratePercent() != null + ? String.format("Sales tax return (%.2f%%)", taxLine.ratePercent()) + : "Sales tax return"; + } else { + description = taxLine.ratePercent() != null + ? String.format("Sales tax (%.2f%%)", taxLine.ratePercent()) + : "Sales tax"; + } return PluginInvoiceItem.createTaxItem( linkedItem, invoice.getId(), diff --git a/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceRequestMapper.java b/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceRequestMapper.java index 481fca2..b801546 100644 --- a/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceRequestMapper.java +++ b/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceRequestMapper.java @@ -49,8 +49,8 @@ public void testFormatQuantityDefaultsToOne() { @Test(groups = "fast") public void testExternalChargeUsesDefaultProductLabels() { - Assert.assertEquals(InvoiceRequestMapper.EXTERNAL_CHARGE_CATEGORY, "Physical"); - Assert.assertEquals(InvoiceRequestMapper.EXTERNAL_CHARGE_SUBCATEGORY, "General Physical"); + Assert.assertEquals( + InvoiceRequestMapper.EXTERNAL_CHARGE_PRODUCT_EXTERNAL_ID, "EXTERNAL_CHARGE"); } @Test(groups = "fast") @@ -97,12 +97,13 @@ public void testToEstimateRequestMapsInvoiceFields() { final ObjectNode document = (ObjectNode) request.path("documents").get(0); Assert.assertTrue(document.path("dry_run").asBoolean()); Assert.assertEquals(document.path("transaction_date").asText(), "2026-01-15"); - Assert.assertEquals(document.path("invoice_number").asInt(), 42); + Assert.assertEquals(document.path("invoice_number").asText(), "42"); final ObjectNode line = (ObjectNode) document.path("line_items").get(0); Assert.assertEquals(line.path("external_id").asText(), itemId.toString()); - Assert.assertEquals(line.path("product_category").asText(), "Physical"); - Assert.assertEquals(line.path("product_subcategory").asText(), "General Physical"); + Assert.assertEquals(line.path("external_product_id").asText(), "EXTERNAL_CHARGE"); + Assert.assertFalse(line.has("product_category")); + Assert.assertFalse(line.has("product_subcategory")); } @Test(groups = "fast") @@ -271,4 +272,205 @@ public void testAviateShipToAddressOverridesAccountAddress() { document.path("customer").path("tax_registration_number").asText(), "12-3456789"); } + + @Test(groups = "fast") + public void testReturnEstimateUsesExternalChargeProductId() { + final UUID accountId = UUID.randomUUID(); + final UUID invoiceId = UUID.randomUUID(); + final UUID chargeId = UUID.randomUUID(); + final UUID adjId = UUID.randomUUID(); + + final Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(accountId); + Mockito.when(account.getExternalKey()).thenReturn("acct-ext"); + + final InvoiceItem charge = Mockito.mock(InvoiceItem.class); + Mockito.when(charge.getId()).thenReturn(chargeId); + Mockito.when(charge.getAmount()).thenReturn(new BigDecimal("100.00")); + Mockito.when(charge.getQuantity()).thenReturn(BigDecimal.ONE); + Mockito.when(charge.getInvoiceItemType()).thenReturn(InvoiceItemType.EXTERNAL_CHARGE); + Mockito.when(charge.getPlanName()).thenReturn(null); + Mockito.when(charge.getPrettyProductName()).thenReturn(null); + + final InvoiceItem adj = Mockito.mock(InvoiceItem.class); + Mockito.when(adj.getId()).thenReturn(adjId); + Mockito.when(adj.getAmount()).thenReturn(new BigDecimal("-25.00")); + Mockito.when(adj.getInvoiceItemType()).thenReturn(InvoiceItemType.ITEM_ADJ); + Mockito.when(adj.getLinkedItemId()).thenReturn(chargeId); + Mockito.when(adj.getDescription()).thenReturn(null); + + final Invoice invoice = Mockito.mock(Invoice.class); + Mockito.when(invoice.getId()).thenReturn(invoiceId); + Mockito.when(invoice.getAccountId()).thenReturn(accountId); + Mockito.when(invoice.getCurrency()).thenReturn(Currency.USD); + Mockito.when(invoice.getInvoiceDate()).thenReturn(new LocalDate(2026, 1, 15)); + Mockito.when(invoice.getInvoiceItems()).thenReturn(List.of(charge, adj)); + + final ObjectNode request = InvoiceRequestMapper.toReturnEstimateRequest( + invoice, account, true, null, AccountTaxMetadata.empty(), List.of(adj)); + + Assert.assertNotNull(request); + final ObjectNode document = (ObjectNode) request.path("documents").get(0); + Assert.assertEquals(document.path("document_kind").asText(), "return"); + Assert.assertEquals(document.path("id").asText(), invoiceId + ":adj-return"); + + final ObjectNode line = (ObjectNode) document.path("line_items").get(0); + Assert.assertEquals(line.path("external_id").asText(), adjId.toString()); + Assert.assertEquals(line.path("item_type").asText(), "ITEM_ADJ"); + Assert.assertEquals(line.path("external_product_id").asText(), "EXTERNAL_CHARGE"); + Assert.assertFalse(line.has("product_category")); + Assert.assertFalse(line.has("product_subcategory")); + Assert.assertFalse(line.has("plan_name")); + Assert.assertFalse(line.has("product_name")); + } + + @Test(groups = "fast") + public void testReturnEstimateInheritsPlanFromLinkedItem() { + final UUID accountId = UUID.randomUUID(); + final UUID invoiceId = UUID.randomUUID(); + final UUID chargeId = UUID.randomUUID(); + final UUID adjId = UUID.randomUUID(); + + final Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(accountId); + Mockito.when(account.getExternalKey()).thenReturn("acct-ext"); + + final InvoiceItem charge = Mockito.mock(InvoiceItem.class); + Mockito.when(charge.getId()).thenReturn(chargeId); + Mockito.when(charge.getAmount()).thenReturn(new BigDecimal("100.00")); + Mockito.when(charge.getInvoiceItemType()).thenReturn(InvoiceItemType.RECURRING); + Mockito.when(charge.getPlanName()).thenReturn("basic-monthly"); + Mockito.when(charge.getPrettyProductName()).thenReturn("Basic"); + + final InvoiceItem adj = Mockito.mock(InvoiceItem.class); + Mockito.when(adj.getId()).thenReturn(adjId); + Mockito.when(adj.getAmount()).thenReturn(new BigDecimal("-10.00")); + Mockito.when(adj.getInvoiceItemType()).thenReturn(InvoiceItemType.ITEM_ADJ); + Mockito.when(adj.getLinkedItemId()).thenReturn(chargeId); + + final Invoice invoice = Mockito.mock(Invoice.class); + Mockito.when(invoice.getId()).thenReturn(invoiceId); + Mockito.when(invoice.getAccountId()).thenReturn(accountId); + Mockito.when(invoice.getCurrency()).thenReturn(Currency.USD); + Mockito.when(invoice.getInvoiceDate()).thenReturn(new LocalDate(2026, 1, 15)); + Mockito.when(invoice.getInvoiceItems()).thenReturn(List.of(charge, adj)); + + final ObjectNode request = InvoiceRequestMapper.toReturnEstimateRequest( + invoice, account, true, null, AccountTaxMetadata.empty(), List.of(adj)); + + final ObjectNode line = (ObjectNode) request.path("documents").get(0).path("line_items").get(0); + Assert.assertEquals(line.path("plan_name").asText(), "basic-monthly"); + Assert.assertEquals(line.path("external_product_id").asText(), "basic-monthly"); + Assert.assertEquals(line.path("product_name").asText(), "Basic"); + Assert.assertFalse(line.has("product_category")); + } + + + @Test(groups = "fast") + public void testExternalChargeWithDisplayNameStillUsesSentinelProductId() { + final UUID accountId = UUID.randomUUID(); + final UUID invoiceId = UUID.randomUUID(); + final UUID itemId = UUID.randomUUID(); + + final Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(accountId); + Mockito.when(account.getExternalKey()).thenReturn("acct-ext"); + + final InvoiceItem item = Mockito.mock(InvoiceItem.class); + Mockito.when(item.getId()).thenReturn(itemId); + Mockito.when(item.getAmount()).thenReturn(new BigDecimal("50.00")); + Mockito.when(item.getQuantity()).thenReturn(BigDecimal.ONE); + Mockito.when(item.getInvoiceItemType()).thenReturn(InvoiceItemType.EXTERNAL_CHARGE); + Mockito.when(item.getPlanName()).thenReturn(null); + Mockito.when(item.getPrettyProductName()).thenReturn("One-time setup fee"); + + final Invoice invoice = Mockito.mock(Invoice.class); + Mockito.when(invoice.getId()).thenReturn(invoiceId); + Mockito.when(invoice.getAccountId()).thenReturn(accountId); + Mockito.when(invoice.getCurrency()).thenReturn(Currency.USD); + Mockito.when(invoice.getInvoiceDate()).thenReturn(new LocalDate(2026, 1, 15)); + Mockito.when(invoice.getInvoiceItems()).thenReturn(List.of(item)); + + final ObjectNode line = (ObjectNode) InvoiceRequestMapper.toEstimateRequest( + invoice, account, true, null).path("documents").get(0).path("line_items").get(0); + + Assert.assertEquals(line.path("external_product_id").asText(), "EXTERNAL_CHARGE"); + Assert.assertEquals(line.path("product_name").asText(), "One-time setup fee"); + Assert.assertFalse(line.has("plan_name")); + Assert.assertFalse(line.has("product_category")); + } + + @Test(groups = "fast") + public void testReturnExternalChargeWithDisplayNameStillUsesSentinelProductId() { + final UUID accountId = UUID.randomUUID(); + final UUID invoiceId = UUID.randomUUID(); + final UUID chargeId = UUID.randomUUID(); + final UUID adjId = UUID.randomUUID(); + + final Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(accountId); + Mockito.when(account.getExternalKey()).thenReturn("acct-ext"); + + final InvoiceItem charge = Mockito.mock(InvoiceItem.class); + Mockito.when(charge.getId()).thenReturn(chargeId); + Mockito.when(charge.getAmount()).thenReturn(new BigDecimal("100.00")); + Mockito.when(charge.getInvoiceItemType()).thenReturn(InvoiceItemType.EXTERNAL_CHARGE); + Mockito.when(charge.getPlanName()).thenReturn(null); + Mockito.when(charge.getPrettyProductName()).thenReturn("Hardware add-on"); + + final InvoiceItem adj = Mockito.mock(InvoiceItem.class); + Mockito.when(adj.getId()).thenReturn(adjId); + Mockito.when(adj.getAmount()).thenReturn(new BigDecimal("-25.00")); + Mockito.when(adj.getInvoiceItemType()).thenReturn(InvoiceItemType.ITEM_ADJ); + Mockito.when(adj.getLinkedItemId()).thenReturn(chargeId); + + final Invoice invoice = Mockito.mock(Invoice.class); + Mockito.when(invoice.getId()).thenReturn(invoiceId); + Mockito.when(invoice.getAccountId()).thenReturn(accountId); + Mockito.when(invoice.getCurrency()).thenReturn(Currency.USD); + Mockito.when(invoice.getInvoiceDate()).thenReturn(new LocalDate(2026, 1, 15)); + Mockito.when(invoice.getInvoiceItems()).thenReturn(List.of(charge, adj)); + + final ObjectNode line = (ObjectNode) InvoiceRequestMapper.toReturnEstimateRequest( + invoice, account, true, null, AccountTaxMetadata.empty(), List.of(adj) + ).path("documents").get(0).path("line_items").get(0); + + Assert.assertEquals(line.path("external_product_id").asText(), "EXTERNAL_CHARGE"); + Assert.assertEquals(line.path("product_name").asText(), "Hardware add-on"); + Assert.assertFalse(line.has("product_category")); + } + + + @Test(groups = "fast") + public void testBlankPlanNameFallsBackToExternalChargeSentinel() { + final UUID accountId = UUID.randomUUID(); + final UUID invoiceId = UUID.randomUUID(); + final UUID itemId = UUID.randomUUID(); + + final Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(accountId); + Mockito.when(account.getExternalKey()).thenReturn("acct-ext"); + + final InvoiceItem item = Mockito.mock(InvoiceItem.class); + Mockito.when(item.getId()).thenReturn(itemId); + Mockito.when(item.getAmount()).thenReturn(new BigDecimal("10.00")); + Mockito.when(item.getQuantity()).thenReturn(BigDecimal.ONE); + Mockito.when(item.getInvoiceItemType()).thenReturn(InvoiceItemType.EXTERNAL_CHARGE); + Mockito.when(item.getPlanName()).thenReturn(" "); + Mockito.when(item.getPrettyProductName()).thenReturn(null); + + final Invoice invoice = Mockito.mock(Invoice.class); + Mockito.when(invoice.getId()).thenReturn(invoiceId); + Mockito.when(invoice.getAccountId()).thenReturn(accountId); + Mockito.when(invoice.getCurrency()).thenReturn(Currency.USD); + Mockito.when(invoice.getInvoiceDate()).thenReturn(new LocalDate(2026, 1, 15)); + Mockito.when(invoice.getInvoiceItems()).thenReturn(List.of(item)); + + final ObjectNode line = (ObjectNode) InvoiceRequestMapper.toEstimateRequest( + invoice, account, true, null).path("documents").get(0).path("line_items").get(0); + + Assert.assertEquals(line.path("external_product_id").asText(), "EXTERNAL_CHARGE"); + Assert.assertFalse(line.has("plan_name")); + } + } diff --git a/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceTaxIdempotency.java b/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceTaxIdempotency.java index f40718f..fa7f56e 100644 --- a/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceTaxIdempotency.java +++ b/src/test/java/com/trykintsugi/killbill/internal/TestInvoiceTaxIdempotency.java @@ -61,7 +61,7 @@ public void testReturnsFalseWhenOnlySomeLinesTaxed() { } @Test(groups = "fast") - public void testIgnoresAdjustmentItemsWhenCheckingCoverage() { + public void testIgnoresAdjustmentItemsWhenCheckingSalesCoverage() { final UUID chargeId = UUID.randomUUID(); final InvoiceItem charge = chargeItem(chargeId, new BigDecimal("100")); final InvoiceItem tax = taxItem(chargeId, new BigDecimal("8.25")); @@ -74,6 +74,26 @@ public void testIgnoresAdjustmentItemsWhenCheckingCoverage() { final Invoice invoice = invoiceWithItems(charge, tax, adjustment); Assert.assertTrue(InvoiceTaxIdempotency.allTaxableItemsAlreadyTaxed(invoice)); + Assert.assertEquals(InvoiceTaxIdempotency.untaxedAdjustmentItems(invoice).size(), 1); + Assert.assertFalse(InvoiceTaxIdempotency.nothingLeftToTax(invoice)); + } + + @Test(groups = "fast") + public void testUntaxedAdjustmentsEmptyWhenReturnTaxLinkedToAdj() { + final UUID chargeId = UUID.randomUUID(); + final UUID adjId = UUID.randomUUID(); + final InvoiceItem charge = chargeItem(chargeId, new BigDecimal("100")); + final InvoiceItem salesTax = taxItem(chargeId, new BigDecimal("8.25")); + final InvoiceItem adjustment = Mockito.mock(InvoiceItem.class); + Mockito.when(adjustment.getId()).thenReturn(adjId); + Mockito.when(adjustment.getAmount()).thenReturn(new BigDecimal("-10")); + Mockito.when(adjustment.getInvoiceItemType()).thenReturn(InvoiceItemType.ITEM_ADJ); + final InvoiceItem returnTax = taxItem(adjId, new BigDecimal("-0.83")); + final Invoice invoice = invoiceWithItems(charge, salesTax, adjustment, returnTax); + + Assert.assertTrue(InvoiceTaxIdempotency.allTaxableItemsAlreadyTaxed(invoice)); + Assert.assertTrue(InvoiceTaxIdempotency.untaxedAdjustmentItems(invoice).isEmpty()); + Assert.assertTrue(InvoiceTaxIdempotency.nothingLeftToTax(invoice)); } @Test(groups = "fast") @@ -102,6 +122,34 @@ private static InvoiceItem chargeItem(final UUID id, final BigDecimal amount) { return item; } + + @Test(groups = "fast") + public void testAdjOnlyInvoiceHasNoSalesWorkRemaining() { + final UUID adjId = UUID.randomUUID(); + final InvoiceItem adjustment = Mockito.mock(InvoiceItem.class); + Mockito.when(adjustment.getId()).thenReturn(adjId); + Mockito.when(adjustment.getAmount()).thenReturn(new BigDecimal("-10")); + Mockito.when(adjustment.getInvoiceItemType()).thenReturn(InvoiceItemType.ITEM_ADJ); + final Invoice invoice = invoiceWithItems(adjustment); + + Assert.assertTrue(InvoiceTaxIdempotency.allTaxableItemsAlreadyTaxed(invoice)); + Assert.assertEquals(InvoiceTaxIdempotency.untaxedAdjustmentItems(invoice).size(), 1); + Assert.assertFalse(InvoiceTaxIdempotency.nothingLeftToTax(invoice)); + } + + @Test(groups = "fast") + public void testCreditOnlyInvoiceHasNoSalesOrReturnWork() { + final InvoiceItem credit = Mockito.mock(InvoiceItem.class); + Mockito.when(credit.getId()).thenReturn(UUID.randomUUID()); + Mockito.when(credit.getAmount()).thenReturn(new BigDecimal("-5")); + Mockito.when(credit.getInvoiceItemType()).thenReturn(InvoiceItemType.CREDIT_ADJ); + final Invoice invoice = invoiceWithItems(credit); + + Assert.assertTrue(InvoiceTaxIdempotency.allTaxableItemsAlreadyTaxed(invoice)); + Assert.assertTrue(InvoiceTaxIdempotency.untaxedAdjustmentItems(invoice).isEmpty()); + Assert.assertTrue(InvoiceTaxIdempotency.nothingLeftToTax(invoice)); + } + private static InvoiceItem taxItem(final UUID linkedId, final BigDecimal amount) { final InvoiceItem item = Mockito.mock(InvoiceItem.class); Mockito.when(item.getId()).thenReturn(UUID.randomUUID()); diff --git a/src/test/java/com/trykintsugi/killbill/internal/TestKintsugiTaxClient.java b/src/test/java/com/trykintsugi/killbill/internal/TestKintsugiTaxClient.java index ce29ddc..2e5b2f7 100644 --- a/src/test/java/com/trykintsugi/killbill/internal/TestKintsugiTaxClient.java +++ b/src/test/java/com/trykintsugi/killbill/internal/TestKintsugiTaxClient.java @@ -91,7 +91,7 @@ public void testParseTaxLinesMapsResponse() throws Exception { } @Test(groups = "fast") - public void testParseTaxLinesMapsMosaicResponseFields() throws Exception { + public void testParseTaxLinesMapsLineExternalIdResponseFields() throws Exception { final String json = "{\"documents\":[{\"line_items\":[" + "{\"line_external_id\":\"line-1\",\"tax_amount\":\"8.25\"," + "\"rate_percentage\":\"8.250000000\",\"taxable_amount\":\"100.00\"}"