Skip to content

fix(integrations): fall back when shipping is blank [INT-36]#5509

Merged
ancorcruz merged 3 commits into
mainfrom
fix/int-36-claude-shipping-address-empty-string-fallback
May 21, 2026
Merged

fix(integrations): fall back when shipping is blank [INT-36]#5509
ancorcruz merged 3 commits into
mainfrom
fix/int-36-claude-shipping-address-empty-string-fallback

Conversation

@mikeh-lago
Copy link
Copy Markdown
Contributor

⚠️ Local validation incomplete (Mode B): the runner has no Postgres/Redis/ClickHouse available, so RSpec was not run locally. Rubocop was run on all 10 changed files and is green. Validation delegated to CI — please wait for CI to be green before reviewing.

Summary

Customers created via the UI without a shipping address persist empty strings (rather than NULL) in shipping_* columns. The Anrok and Avalara tax-integration payloads fell back from customer.shipping_* to billing customer.address_* using bare ||, which does not catch "" in Ruby — so the integrations were receiving empty addresses and failing.

Fix: switch the fallback from || to .presence || in the Anrok and Avalara payload builders. "".presence is nil, so the fallback chain now does what it was always meant to do.

Files touched

Production:

  • app/services/integrations/aggregator/taxes/invoices/payloads/anrok.rb
  • app/services/integrations/aggregator/taxes/invoices/payloads/avalara.rb
  • app/services/integrations/aggregator/taxes/credit_notes/payloads/anrok.rb
  • app/services/integrations/aggregator/taxes/credit_notes/payloads/avalara.rb
  • app/services/integrations/aggregator/contacts/payloads/avalara.rb (Avalara contacts is part of the same tax flow)

Tests: matching describe "shipping address fallback" block added to each of the 5 specs, asserting empty-string → fallback, nil → fallback, and non-empty shipping preserved.

What was NOT done and why

  • No data migration on customers. The Linear description proposed a raw UPDATE customers SET shipping_* = NULLIF(...). Per @yohan's 2026-05-11 comment on INT-36, "a data migration here is unnecessary. We should update the integration code with customer.shipping_address_line1.presence || customer.address_line1." That is the direction implemented here. If a backfill is later judged necessary (it likely isn't, since the integration code now handles "" correctly), it should be done as a batched migration job in app/jobs/database_migrations/ per Julien's earlier comment — not as a single unbatched UPDATE on the customers hot table.
  • NetSuite contacts payload was not touched. Contacts::Payloads::Netsuite uses customer.shipping_address_line1&.first(N) and does not have a fallback-to-billing pattern. It is not a tax integration and is outside this ticket's scope (the ticket explicitly says "issues with our tax integrations"). If NetSuite needs similar handling, that is a separate ticket.
  • No frontend change. Stopping the UI from persisting "" in the first place is a sensible follow-up but is outside this ticket and a different team (Front).

Performance notes

This is a pure expression-level change in payload builders that are already called once per integration request. Adding .presence is O(1) per field. No new DB queries, no allocations in a loop, no hot-path impact. Section 4 bis questions all answered "no concern."

How a reviewer can verify locally

bundle exec rspec spec/services/integrations/aggregator/taxes/invoices/payloads/anrok_spec.rb \
                  spec/services/integrations/aggregator/taxes/invoices/payloads/avalara_spec.rb \
                  spec/services/integrations/aggregator/taxes/credit_notes/payloads/anrok_spec.rb \
                  spec/services/integrations/aggregator/taxes/credit_notes/payloads/avalara_spec.rb \
                  spec/services/integrations/aggregator/contacts/payloads/avalara_spec.rb
bundle exec rubocop app/services/integrations/aggregator/{taxes,contacts}/

Linear

INT-36 — Data Migration, "" in shipping address issue


🤖 Generated by the ai-augmented pilot (Claude Code, Opus 4.7). Direction sourced from @yohan's 2026-05-11 comment on INT-36.

## Context

Customers created via the UI without a shipping address persist
empty strings (rather than NULL) in shipping_* columns. The Anrok
and Avalara tax-integration payloads fell back from shipping_* to
billing address_* using `||`, which does not catch "", so the
integrations received empty addresses and failed.

INT-36.

## Description

Switch the shipping-to-billing fallback in the Anrok and Avalara
payload builders (tax invoices, tax credit notes, and Avalara
contacts) from `||` to `.presence ||`. No data backfill: per
Yohan's 2026-05-11 comment, the right fix lives in the integration
code, not in a customers-table migration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mikeh-lago mikeh-lago requested review from ancorcruz and osmarluz May 15, 2026 15:45
Copy link
Copy Markdown
Contributor

@ancorcruz ancorcruz left a comment

Choose a reason for hiding this comment

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

LGTM.

We could DRY this pattern up across 5 payload builders exposing a #effective_shipping_address helper in Customer model that does the .presence || fallback.

Do you think it would make sense in the Customer create/update services cleaning the attributes to ensure no "" is saved for new customers or updates?

ancorcruz added 2 commits May 18, 2026 19:22
# Context

The previous commit on this branch fixed the bug where UI-created
customers persist empty strings (not NULL) for unset shipping fields
by switching the per-field fallback from `||` to `.presence ||` in
five tax-integration payload builders. That repeated the same
expression for every address field across every builder.

# Description

Introduce `Customer#effective_shipping_address`, returning a hash of
the six address fields with a per-field `.presence || billing`
fallback. The five payload builders (Anrok and Avalara, for invoices
and credit notes, plus Avalara contacts) now read from this hash
instead of repeating the fallback expression per field. Each builder
still maps the hash keys to its own payload field names (`zip` vs
`zipcode`, `region` vs `state`) so the per-integration contracts are
preserved.
@ancorcruz ancorcruz marked this pull request as ready for review May 18, 2026 19:08
@ancorcruz ancorcruz merged commit a3ee48b into main May 21, 2026
12 checks passed
@ancorcruz ancorcruz deleted the fix/int-36-claude-shipping-address-empty-string-fallback branch May 21, 2026 07:44
D1353L pushed a commit that referenced this pull request May 26, 2026
> ⚠️ **Local validation incomplete (Mode B):** the runner has no
Postgres/Redis/ClickHouse available, so RSpec was not run locally.
Rubocop was run on all 10 changed files and is green. Validation
delegated to CI — please wait for CI to be green before reviewing.

## Summary

Customers created via the UI without a shipping address persist **empty
strings** (rather than `NULL`) in `shipping_*` columns. The Anrok and
Avalara tax-integration payloads fell back from `customer.shipping_*` to
billing `customer.address_*` using bare `||`, which does **not** catch
`""` in Ruby — so the integrations were receiving empty addresses and
failing.

Fix: switch the fallback from `||` to `.presence ||` in the Anrok and
Avalara payload builders. `"".presence` is `nil`, so the fallback chain
now does what it was always meant to do.

## Files touched

Production:
-
`app/services/integrations/aggregator/taxes/invoices/payloads/anrok.rb`
-
`app/services/integrations/aggregator/taxes/invoices/payloads/avalara.rb`
-
`app/services/integrations/aggregator/taxes/credit_notes/payloads/anrok.rb`
-
`app/services/integrations/aggregator/taxes/credit_notes/payloads/avalara.rb`
- `app/services/integrations/aggregator/contacts/payloads/avalara.rb`
(Avalara contacts is part of the same tax flow)

Tests: matching `describe "shipping address fallback"` block added to
each of the 5 specs, asserting empty-string → fallback, `nil` →
fallback, and non-empty shipping preserved.

## What was NOT done and why

- **No data migration on `customers`.** The Linear description proposed
a raw `UPDATE customers SET shipping_* = NULLIF(...)`. Per @yohan's
2026-05-11 comment on INT-36, *"a data migration here is unnecessary. We
should update the integration code with
`customer.shipping_address_line1.presence || customer.address_line1`."*
That is the direction implemented here. If a backfill is later judged
necessary (it likely isn't, since the integration code now handles `""`
correctly), it should be done as a batched migration job in
`app/jobs/database_migrations/` per Julien's earlier comment — not as a
single unbatched UPDATE on the `customers` hot table.
- **NetSuite contacts payload was not touched.**
`Contacts::Payloads::Netsuite` uses
`customer.shipping_address_line1&.first(N)` and does not have a
fallback-to-billing pattern. It is not a tax integration and is outside
this ticket's scope (the ticket explicitly says "issues with our tax
integrations"). If NetSuite needs similar handling, that is a separate
ticket.
- **No frontend change.** Stopping the UI from persisting `""` in the
first place is a sensible follow-up but is outside this ticket and a
different team (Front).

## Performance notes

This is a pure expression-level change in payload builders that are
already called once per integration request. Adding `.presence` is O(1)
per field. No new DB queries, no allocations in a loop, no hot-path
impact. Section 4 bis questions all answered "no concern."

## How a reviewer can verify locally

```
bundle exec rspec spec/services/integrations/aggregator/taxes/invoices/payloads/anrok_spec.rb \
                  spec/services/integrations/aggregator/taxes/invoices/payloads/avalara_spec.rb \
                  spec/services/integrations/aggregator/taxes/credit_notes/payloads/anrok_spec.rb \
                  spec/services/integrations/aggregator/taxes/credit_notes/payloads/avalara_spec.rb \
                  spec/services/integrations/aggregator/contacts/payloads/avalara_spec.rb
bundle exec rubocop app/services/integrations/aggregator/{taxes,contacts}/
```

## Linear

[INT-36 — Data Migration, "" in shipping address
issue](https://linear.app/getlago/issue/INT-36/data-migration-in-shipping-address-issue)

---

🤖 Generated by the `ai-augmented` pilot (Claude Code, Opus 4.7).
Direction sourced from @yohan's 2026-05-11 comment on INT-36.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Ancor Cruz <hello@ancorcruz.com>
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.

3 participants