Skip to content

add alavara to integration_customers - #564

Open
annvelents wants to merge 1 commit into
mainfrom
fix/avalara-integration-customer-type-enum
Open

annvelents wants to merge 1 commit into
mainfrom
fix/avalara-integration-customer-type-enum

Conversation

@annvelents

Copy link
Copy Markdown
Contributor

No description provided.

annvelents pushed a commit that referenced this pull request Aug 28, 2026
…dian leads) (#566)

## OpenAPI Guardian sweep — 2026-08-17 (slice 2: customers,
billing_entities, organizations)

Automated spec sweep vs lago-api and the SDK clients.
**A human must review and merge — this agent never merges.**
`npm run build` and `npm run test` pass on this branch (0 errors, same 6
warnings as `main`; no new ones).

Weekly cadence this run, so the rotation slice was picked as ISO week %
8 (week 34 → slice 2) rather than the skill's twice-weekly formula. That
lands on the **same slice as the still-open #563**, so this sweep
deliberately covers only what #563 does not — see "Overlap with #563"
below.

### Fixed in this PR
| Type | Count |
|---|---|
| Typos / definitions | 5 |
| Required vs optional / nullability | 3 |
| Filters / query params / paths | 0 (slice-2 filters match the
controller) |

### Field-level evidence

**Typos and definitions**
- `src/schemas/CustomerObjectExtended.yaml` `error_details`: "processing
backgroud actions for the customers" → "processing background actions
for the customer".
- `src/schemas/CustomerObjectExtended.yaml`
`applicable_invoice_custom_sections`: curly apostrophe → straight (repo
convention), and "custom section" → "custom sections" — the field is an
array.
- `src/schemas/CustomerBaseObject.yaml` `sequential_id`: stray double
backtick (`` `sequential_id`` ``) closing the inline code span.
- `src/schemas/BillingEntityObject.yaml` `is_default`: the description
repeated itself and ended in a broken sentence ("…when invoice is
created. is the oldest active billing entity…"). Rewritten to one
accurate sentence — evidence: `organization.rb:110` `has_one
:default_billing_entity, -> { active.order(created_at: :asc) }`, and
`billing_entity_serializer.rb` computes `is_default` by comparing
against it.
- `src/schemas/AlertThresholdInput.yaml` + `AlertThresholdObject.yaml`
`value`: the unit was undocumented. Added the per-alert-type unit —
evidence: the `find_value` implementations in
`app/models/usage_monitoring/*_alert.rb`:
- `billable_metric_current_usage_units`,
`billable_metric_lifetime_usage_units` → `fee.units` (**units**)
- `wallet_credits_balance`, `wallet_credits_ongoing_balance` →
`wallet.credits_balance` / `credits_ongoing_balance` (**credits**)
- `current_usage_amount`, `billable_metric_current_usage_amount`,
`lifetime_usage_amount`, `wallet_balance_amount`,
`wallet_ongoing_balance_amount` → `*_cents` (**cents**)

**Required vs optional / nullability**
- `src/schemas/CustomerCreateInput.yaml` `account_type`:
`["string","null"]` with `null` in the enum → `string`, enum
`customer`/`partner`. Evidence: `customer.rb:55` `enum :account_type,
ACCOUNT_TYPES, suffix: :account, validate: true` — no `allow_nil`, in
deliberate contrast to `customer_type` on line 53 which uses `validate:
{allow_nil: true}`; and `db/structure.sql` has `account_type ... NOT
NULL DEFAULT 'customer'`. See the [BREAKING-DOC] note below.
- `src/schemas/CustomerCreateInput.yaml` `account_type` description:
added that the field is only applied when revenue share is enabled —
evidence: `customers/upsert_from_api_service.rb:80` assigns it only `if
customer.organization.revenue_share_enabled? && customer.editable?`;
otherwise the value is silently ignored.
- `src/schemas/InvoiceOneOffCreateInput.yaml` `fees[].units`:
`["string","null"]` → `["string","number","null"]`. Evidence:
`fees/one_off_service.rb:26` `units = fee[:units]&.to_f || 1`, so a JSON
number is accepted. Widening only. (Lead raised by the docs guardian in
getlago/lago-doc#642.)

### [BREAKING-DOC] flags
- **`CustomerCreateInput.account_type` no longer documents `null`.**
This tightens an input type, so a consumer generating a client from the
spec loses the ability to send `account_type: null`. The code proves
`null` is never a storable value, and sending it is either silently
ignored (revenue share off) or rejected by the inclusion validation
(revenue share on) — but flagging it so reviewers weigh it consciously.

### Docs-guardian leads triaged
Leads came from the "Suspected spec issues" sections of
getlago/lago-doc#642 (today) and #636.

| Lead | Verdict |
|---|---|
| `BillingEntityUpdateInput` missing its `billing_entity` request
wrapper (`billing_entities_controller.rb:105` does
`params.require(:billing_entity)`) | **Confirmed — already fixed in
#563**, not duplicated here |
| `InvoiceOneOffCreateInput.fees[].units` typed string-only while docs
pass integers | **Confirmed — fixed in this PR** |
| Wallet alert threshold `value` has no documented unit | **Confirmed —
fixed in this PR**, and extended to every alert type, not just the
wallet ones |

### SDK drift (spec is right — needs an `sdk-clients-update` run)
Cross-diffed slice 2 against all five clients. Nothing here indicates a
spec error; in every case `lago-api` confirms the spec.

| Resource | Client(s) | Divergence |
|---|---|---|
| customers | Ruby, Python, Go | Create params omit `account_type` and
`external_salesforce_id`; both are permitted in
`customers_controller.rb:125,147` |
| customers | Ruby, Python | `integration_customers[]` whitelist omits
`targeted_object` (permitted on line 159) |
| customers | Python | `CustomerResponse` omits `sequential_id`, `slug`,
`account_type` and `external_salesforce_id`, all always emitted by
`CustomerSerializer` |
| customers | Go | `CustomerInput` also omits `logo_url`;
`CustomerListInput` omits the `account_type[]`, `billing_entity_codes[]`
and `external_id` filters |
| customers | Rust | `CustomerPaymentProvider` enum has only
`stripe`/`adyen`/`gocardless`; the API also accepts `cashfree`,
`flutterwave`, `moneyhash` (Go has all six) |
| customers | Go | `IntegrationType` includes `okta`, which is not an
`integration_customers` type, and omits `avalara` |
| billing_entities | Ruby, Python, Go | Create/update params and
response models omit `einvoicing` (permitted in
`billing_entities_controller.rb:73,106`, serialized by
`BillingEntitySerializer`) |
| billing_entities | Python | Request models expose `logo_url`, but the
API permits `logo` (base64). Ruby and Go correctly send `logo` |
| billing_entities | Go | `BillingEntity` response struct omits `phone`
|
| organizations | Ruby, Python | Update params send `webhook_urls`,
which `organizations_controller.rb:51-76` does not permit — silently
dropped |
| organizations | Ruby, Python, Go | `slug` missing from both request
and response models |
| organizations | Python | `OrganizationResponse` omits
`default_currency` |
| billing_entities, organizations | Rust | Not modelled at all — known
partial-coverage gap by design, not drift |
| all | JavaScript | No drift possible: `openapi/client.ts` is generated
from the published spec at build time and is not committed |

### Needs human confirmation (not changed)
- **`GET /organizations` never returns `taxes`.**
`organizations_controller.rb:11` passes `include: %i[taxes]` while
`ModelSerializer#include?` reads `options[:includes]`; `update` on line
23 correctly passes `includes:`. This looks like a one-character bug in
**lago-api**, not in the spec, so nothing was changed here. If it is a
bug, the spec is already correct; if the omission is intentional,
`OrganizationObject.taxes` should be documented as update-only.
- **`GET /organizations/grpc_token`** exists in `config/routes.rb:201`
and returns `{organization: {grpc_token}}`, but is absent from the spec.
It looks internal (a JWT for the gRPC gateway), so it was not added.
Confirm whether it should be public.
- **`CustomersPaginated` over-promises.** It points at
`CustomerObjectExtended`, which carries
`applicable_invoice_custom_sections` and `error_details`, but
`customers_controller.rb:76` serializes the index with `includes:
%i[taxes integration_customers]` only, so those two keys never appear in
a list response. Fixing this properly needs a new intermediate schema
(base + `integration_customers` + `taxes` + `metadata`); left alone
rather than invent one unilaterally.
- **`CustomerBaseObject` under-declares `required`.**
`billing_entity_code`, `account_type`, `finalize_zero_amount_invoice`,
`skip_invoice_custom_sections` and `updated_at` are emitted
unconditionally by `CustomerSerializer` and are `NOT NULL` in
`db/structure.sql`, but are not in the `required` list. Adding them is
correct but tightens the response contract for generated clients, so it
is left for a human call.

### Overlap with #563
#563 (approved 2026-08-14, still unmerged) covers the same slice.
Everything it already fixes was deliberately left untouched here: the
billing-entity request/response wrappers, the `{code}` path param,
`external_salesforce_id`, `targeted_object`, the `integration_type` and
`payment_provider` enums, organization `slug` / `email_settings`, the
`GET /organizations` operation, the customers `external_id` filter, and
the `CustomerIntegratrion`/`CustomerPaymentProvidern` schema-key typos.
**#563 should merge first**; the only file both touch is
`src/schemas/CustomerCreateInput.yaml`, in non-adjacent hunks.

Also note: #564 adds `avalara` to `IntegrationCustomer.type`, which is
why that enum is untouched here.

### Deferred to next run
- `CustomerCreateInput.finalize_zero_amount_invoice` should also accept
`null` — `upsert_from_api_service.rb` does
`params[:finalize_zero_amount_invoice] || "inherit"` and
`valid_finalize_zero_amount_invoice?` returns true for `nil`. Deferred
only because the edit lands in the same hunk #563 modifies and would
conflict.
- `CustomerObjectExtended` redefines `metadata`, which
`CustomerBaseObject` already declares. Harmless duplication; a cleanup,
not a correctness fix.

### Process feedback for the retro
- The slice formula collided with an unmerged PR covering the same
slice. The skill has no rule for "an open guardian PR already covers
this slice"; the run's value came almost entirely from schemas outside
the previous sweep's diff, plus the docs-guardian leads. A cheap rule
would be: when the previous guardian PR is open and covers the same
resources, treat its diff as already-fixed and spend the run on leads
plus the untouched remainder (what this run did).
- Slack is unreachable from this runner (no Slack tooling and no
`SLACK_*` webhook), so the announcement is posted as a comment on this
PR instead. This is the second consecutive run to hit it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: lago-guardians[bot] <guardians@getlago.com>
@annvelents
annvelents force-pushed the fix/avalara-integration-customer-type-enum branch from e6e76d3 to afee656 Compare August 28, 2026 11:04

@sarkissianraffi sarkissianraffi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The addition itself is correct: the API's integration customer model maps avalara, so the enum was incomplete without it.

One blocker: the committed bundle was not regenerated. This repo publishes the root openapi.yaml, and everything downstream (the API reference, generated clients) reads that file, not src/. Ran npm run build on this branch: it produces exactly the one-line change missing from the committed bundle. Run the build and commit the regenerated openapi.yaml, and this is approved.

That gap also suggests the systemic fix: a CI step that runs the build and fails on a dirty openapi.yaml. A src-only PR merging cleanly while the published spec stays stale is the kind of drift nobody notices until a customer does.

One non-blocking parity question inline.

enum:
- netsuite
- anrok
- avalara

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: anrok also appears in the accounting_provider enum of CustomerIntegrationErrorObject and in InvoicePreviewInput's integration_type enum, and neither carries avalara. If avalara errors and previews flow through those surfaces too, they have the same gap this line fixes. Worth checking while you are in here; fine as a follow-up if preview does not support it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correcting my own note above, one half of it pointed the wrong way.

Anrok is a tax provider, not an accounting provider. Verified in the API code: INTEGRATION_TAX_TYPES is Anrok and Avalara, INTEGRATION_ACCOUNTING_TYPES is NetSuite and Xero, and tax provider errors ship as their own webhook (customer.tax_provider_error) with their own serializer, separate from the accounting error object.

Two consequences:

  1. Do NOT add avalara to CustomerIntegrationErrorObject.accounting_provider. The real issue there is pre-existing: anrok should not be in that enum at all, and the field arguably documents only the accounting error webhook. That cleanup deserves its own PR, out of scope here.
  2. The preview half is confirmed, not conditional: the invoice preview service maps avalara, so InvoicePreviewInput.integration_type (currently anrok only, description included) genuinely misses it. Fine as a follow-up.

The verdict on this PR is unchanged: regenerate the bundle and it is approved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants