Commit Graph

8 Commits (2321854967ffce9e12a91d78cf7df7aeac9243b9)

Author SHA1 Message Date
nexxo 2321854967 Discard an unconfirmed registration, and sweep the abandoned ones
tests / pest (push) Failing after 8m17s Details
tests / assets (push) Successful in 19s Details
tests / release (push) Has been skipped Details
"Abmelden und neu registrieren" was advice that could not work. Signing out
frees nothing: the address is still held by the unique index, so registering
again with the address somebody meant fails — and every abandoned attempt
stayed in `users` for ever. Reported exactly that way, and it was right.

The button discards the account instead. Only an UNCONFIRMED one, and only
one with nothing behind it: the webhook creates a customer's login from a
paid checkout, so a user can sit on this page unconfirmed while already
having a contract, and deleting that is data loss dressed up as a
convenience. It refuses and says where to go. A confirmed account is closed,
not discarded — that is ConfirmCloseAccount, with an invoice history behind
it.

R23: confirmed in a modal. The modal mutates nothing; it dispatches the event
the page listens for, so the checks stay in the one place they already are
rather than being duplicated where they could drift.

And nothing has to be discarded by hand. clupilot:prune-unverified removes
registrations nobody confirmed after five days — long enough for somebody who
signed up on a Friday and found the mail in a spam folder on Monday, short
enough that a typo does not hold the correct address for a month. It skips a
confirmed account whatever its age, and any account with a customer record
behind it, matched on user_id AND on the address, because either link means
somebody is a customer. Every removal is logged with the address: a line that
disappears is what somebody asks about later, and a count answers nothing.

The deadline is said on the page as well as in the mail. Somebody who never
received the mail is looking at the page, and the deadline is the reason they
do not have to do anything about an attempt they abandon.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 14:08:02 +02:00
nexxo 3cf16ecd63 Name every property that decides what a price charges
The money gate compared amount, currency and interval and left the rest to a
filter that excluded two fields. A recurring, licensed, monthly price with
transform_quantity divide_by 10, at our exact figure, on our product, carrying
our addon key passed all five conditions. Modules bill BY quantity — a pack is
one item at quantity n — so a customer holding three would have been charged
one. A tiered price was excluded only by accident: Stripe reports unit_amount
null for it and we read that as 0, which fails the amount match unless the
caller's own figure is 0, and PlanPrices::ensure() has no zero guard.

activePricesFor() now also skips a non-empty transform_quantity and any
billing_scheme other than per_unit. Absent keys stay Stripe's defaults, which
is the direction that matters: read the other way round the filter would refuse
every legitimate price and turn recognition into a permanent no-op. The
contract docblock names the four properties instead of claiming to cover all of
them, and says the list is not everything Stripe can put on a price — only
what is known to change what one charges.

And a comment that says a gap is covered is worse than the gap. The sync said a
crash between Stripe creating a product and us storing its id gives back the
same product next run: true for twenty-four hours, false afterwards, and the
exact claim this branch was written to refute. There is no recognition step for
products at all, so an interrupted run leaves an orphan and the next run past
the expiry mints a second one — worse than a duplicate price, because
activePricesFor() then looks at the wrong product and recognition goes blind
for the whole family. Written down as a known gap at both createProduct call
sites, with the trade this branch chose: folding name and metadata into the key
means a renamed family now mints a second product where it used to answer 400,
and a blockade reaches a paying customer while a duplicate product does not.

The sweep no longer reports "created" for what it adopted — the count is taken
before ensure() runs, and telling those two apart is the whole point.

The test named after 2026-07-29 could not reproduce it: the fake's key ledger
was empty, so createPrice() could not have thrown and the test could not tell
"adoption prevented the 400" from "no 400 was possible". Seeded now with the
key today's call sends and a fingerprint over the metadata the pre-9da1358 call
sent. Remove the adopt() call from AddonPrices::ensure() and the test dies with
Stripe's own sentence instead of an assertion.

Also: the plan document still instructed migrate:fresh --env=testing. There is
no .env.testing here, so it targets the live development database; struck
through with the reason, not deleted, and a Korrekturen section records the
five defects the plan's own reviews found in it. Four comments corrected —
"below" that meant above, a counter claimed unable to tie that ties with a
planted price, the unique index's second meaning for remember()'s catch, and
that MariaDB DDL is not transactional so the dedupe commits before the index.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 13:50:20 +02:00
nexxo dba5df5d1e One row per Stripe price on the module side too
The plan register has had this since 2026_07_30_110000. The module register
never did, so two rows could claim one Price and archiving the one would have
withdrawn the Price the other was still selling — Block D of the real-run
handoff.

It is also the net under the adoption step: at a VAT rate of nought both
treatments charge the same amount, and the check that refuses an already-claimed
Price is then the only thing keeping the two apart.

Duplicates are deleted, not archived. An archived row goes on claiming the id
and a unique index knows nothing about archived_at; the row rebuilds itself on
the next ensure(), and subscription_addons holds the Stripe id as text rather
than a foreign key, so no booking loses its price.

AddonPrices::remember()'s catch comment described the guard as the only thing
standing between two rows and one Price; now that the index exists, rewritten
to describe it as the net under that guard instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 12:47:34 +02:00
nexxo 40f7818757 Fix round 1: give the frozen-booking test real teeth
Two Important findings from review, both against the plan's own text.

The frozen-booking test could not fail for a defect this diff introduces.
Digging into why turned up two problems, not one. The two assertions the
review named were never exercised: sync-catalogue never writes
subscription_addons.stripe_price_id, and the orphan planted for the new
figure sits at a different amount than the booking's own price, so
archiveSuperseded's net_cents scoping was never going to reach it either.
Underneath that, the test's own orphan was inert regardless of any of that:
with no StripeAddonPrice row seeded first, AddonPrices::product() minted its
own Stripe Product instead of reusing 'prod_support', so the competing
orphan planted under that literal id sat on a product nothing ever asked
about. Fixed both — seeded the product the way the first two tests in this
file already do, added the three assertions the review specified, and a
comment on what the two original assertions do and do not prove.

Verified the fix actually bites, since a test whose teeth are asserted but
never demonstrated is what produced this finding in the first place. With
just the amount check in AdoptStripePrice disabled, the booking still
survived: the `claimed` callback this diff wires into AddonPrices::ensure()
caught it as a second, independent guard. Only disabling both together
reproduced the failure — the honest demonstration, not the single-line
sabotage first suggested, which this diff's own defense-in-depth absorbs.

The remember() comment credited a unique index stripe_addon_prices does not
have — that index sits on stripe_plan_prices, the plan side. Reworded to
name what actually keeps two rows off one Price id today (the `claimed`
callback), what 2026_07_31_210000_one_row_per_stripe_price adds as a net
under it, and why two rows sharing one Price is dangerous: archiving one —
the ordinary response to a superseded figure — would withdraw the very
Price the other row is still selling.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 11:45:50 +02:00
nexxo 97b655938e Ask before minting a module price
The sweep that died on 2026-07-29 left price_1TygdEC7u8NpJ8pOt3nsoyYw at Stripe
and no row here. This is the step that finds it: same money, same interval, our
metadata on it, unclaimed — taken over and brought up to date instead of
duplicated.

The comment on the key was the root of it. It read "keyed on what the Price IS",
and a key is not that: identity is product, amount, currency and interval, and
recognising those is AdoptStripePrice's job. Stripe forgets a key after a day;
it never was the guard against a duplicate.

A booking stays where it was sold. Adoption only ever matches an identical
amount, so a frozen booking cannot be moved by it — there is a test that says so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 11:24:05 +02:00
nexxo 9da1358802 Stop charging VAT to the customers who owe us none
A verified EU business outside Austria gets reverse charge — rate 0, no VAT
line, the note on the invoice — and was still charged 214,80 € for a 179,00 €
package, because one Stripe Price carried the domestic gross for everybody.
There is no VAT line on their document to reclaim, so it was a flat 20 %
surcharge on exactly the customers who read their invoices, and the document
then stated the whole 214,80 € as net at 0 %, so they self-accounted their own
VAT on a base a fifth too large.

A Stripe Price can ask who is buying after all: there are two per sellable
thing now, the domestic gross and the bare net, both live on one Product, and
the checkout picks by TaxTreatment. The rule is Austria B2B 20 %, other EU B2B
without VAT — a domestic business still pays the gross, reclaims it as input
tax, and the price on the website is still the price charged for them.

- stripe:sync-catalogue mints and archives both halves of every pair, per plan
  and per module; `reverse_charge` on stripe_plan_prices/stripe_addon_prices
  says which is which, and joining it on subscriptions.stripe_price_id says
  which one a running contract is billed on. A rate change moves the gross
  Price and leaves the net one alone, because the net owes nothing to the rate.
- A status that changes after the sale converges: a verification (or a lapse)
  makes stripe:reprice-subscriptions move the contract onto the other Price
  with PRORATE_NONE, because the term is already paid for. The module items
  follow the same way.
- Downstream follows: the invoice total equals what was taken, the proof
  register expects the figure this customer is actually charged (so a correct
  reverse-charge sale is no longer flagged as a mismatch, and an overcharged
  one is), the setup fee obeys the same rule, and the booking page quotes what
  it will charge.
- StripeClient gained activatePrice(). Archived Prices were brought back in our
  own table and left inactive at Stripe, so a rate that moved and moved back
  pointed the catalogue at a Price no checkout could be opened for.

An unverified number is still charged the gross — an unchecked string must
never be a discount — and where the net Price is missing the checkout refuses
rather than reaching for the domestic one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-30 02:15:41 +02:00
nexxo f02e86769b Charge the price the website shows, and hand a withdrawal back in full
tests / pest (push) Failing after 9m21s Details
tests / assets (push) Successful in 21s Details
tests / release (push) Has been skipped Details
Stripe was charging the catalogue's NET figure while the document added the
domestic rate on top: a customer quoted 214,80 paid 179,00 and was then invoiced
for VAT nobody had collected. Under para 11(12) UStG that VAT is owed to the tax
office whether or not it ever arrived, so every document issued created a
liability against revenue that did not contain it.

The Stripe Price now carries the GROSS figure. price_cents stays net - it is
frozen onto every contract and PlanChange prorates against it, so redefining it
would corrupt every pro-rata sum ever computed. Only the amount at the till
moved, and it is formed by the one call TaxTreatment already answers for the
price sheet and for the invoice, so displayed, charged and invoiced cannot drift
apart.

Stripe's automatic_tax is deliberately not used. TaxTreatment is the single tax
authority here, and a second rate computed by Stripe would take 19 % from a
German consumer while our document said 20 %.

A Stripe Price cannot be edited, so a changed figure means a new Price and the
old one archived - which stops it being SOLD and leaves every subscription
already on it billing the old amount for ever. stripe:reprice-subscriptions is
the second half, and a command of its own because it touches live money.

A withdrawing consumer is refunded in full, as the owner decided. That is more
generous than FAGG para 16 requires, and it makes the express-request consent the
statute hinges on irrelevant - so the gate is gone rather than left looking
load-bearing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 22:42:02 +02:00
nexxo 8f1630ba91 Bill a booked module every month, and put it on the invoice
A module was never an item on the Stripe subscription, so it was charged in
the month it was booked and never again. And the renewal document was written
from our own contract snapshot, so it would not have said so even if the money
had been taken. Two halves of one fault, closed together.

Modules are items now. stripe:sync-catalogue mirrors a Product and a Price per
module on both terms — a Stripe Price carries its own interval, and a booking
frozen at an older figure needs its own Price, so they are keyed by module,
amount, currency and interval in stripe_addon_prices. Booking adds an item with
always_invoice: prorated by days AND charged there and then, which is the
invoice the owner wants for a module booked mid-term. A second storage pack is
a quantity on one item, not a second item. Cancelling takes the item off with
no proration — no credit, and the next cycle is simply smaller — while the
module itself is kept until the period end on subscription_addons.cancels_at
and ended by clupilot:end-cancelled-addons.

Documents are built from Stripe's own invoice lines. Stripe is what charged the
customer; a document assembled from our figures would state a sum that is not
the one taken. So every paid Stripe invoice gets one document with one number,
idempotent on stripe_invoice_id — a cycle carrying package and every module
together, a mid-period booking carrying just its prorated line, an upgrade
carrying its proration. Only the wording is ours: each line is named from the
catalogue rather than printed as a Price id, and a line nobody can name stays on
the document under Stripe's own description and is logged. The checkout's own
invoice is still the one exception; that purchase already has a document.

Stripe being away never undoes a booking that has already reached the machine.
The failure is parked on the contract in stripe_addon_sync and swept by the
same clupilot:sync-stripe-subscriptions that retries a plan change, which finds
its work from the bookings themselves rather than from the marker. A granted
contract has no Stripe subscription and is not touched at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-29 20:22:02 +02:00