feat(billing)!: catalogue-driven billing screen with product-key purchases on both rails

[anilcancakir]

The billing screen sells any app's catalogue: prices and calls to action come from the producer's products, both rails purchase by catalogue product key, and a store build sells, prices and discloses only what its own store carries. UI half of the reusable payments work, with fluttersdk/magic_payments#16 (client contract) and the magic-starter-laravel catalogue PR (backend rows).

What changes

• BREAKING: purchase by product key on both rails (checkout(productKey:), purchase(productKey, context:)).
• BREAKING: plan rows drive prices from products; MagicStarterPlan.monthly, annual and currency are removed. The first row is the free floor, a tier above it with no sellable product is custom, a web card shows the product's web price display, a store card the store's own localized price. Grandfathered products (sellable: false) stay in the rows so a held subscription can be ranked, and are never offered.
• Store builds: the 3.1.2 disclosure beside every store purchase button, only products with an id in this store are offered and priced, a tier the store does not carry says "Not available in this app." (never naming the web), cross-store and pending purchases are refused, the rail's lastChangeTiming decides between polling and "takes effect on ", and the wait is bounded.
• The free tier's Downgrade opens where the paid plan is cancelled (billing portal on the web, the store's subscriptions page for a store subscription on a matching device) and renders nothing otherwise; it used to fail with productUnavailable on every tap.
• Typed error copy per BillingErrorCode, and actionable account-deletion refusals.
• New and changed copy in en.stub; existing apps add the keys listed under ## [Unreleased].

CI note

This branch compiles only against the unreleased magic_payments contract (#16). The Published graph job stays red until magic_payments 0.0.8 is on pub.dev; the floor moves to ^0.0.8 in the release PR, and this PR merges after that publish.

Gates (local, against the magic_payments branch)

• dart format --set-exit-if-changed lib test: 0 changed.
• flutter analyze: no issues.
• flutter test: 1881 passed, 1 skipped (pre-existing).

Review

Per-wave reviews, a final code review and two independent oracle passes; every finding is fixed on this branch (the last pass: the unsold sentence never on the held tier's own card, and no floor exit once the subscription stopped renewing or on the other store's device).

[kodizm]

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The design holds together and I found no merge-blocking defect in the code I read, but CI has not run the tests, so nothing here is verified green. One contract choice can silently price a real tier as "Free".

Major

lib/src/ui/views/teams/magic_starter_billing_view.dart (_isFloor) (correctness): the floor is chosen only by position (plans.first). A catalogue with no free tier, so the first row is a paid tier with sellable products, would get that tier labelled "Free" with no purchase button. A subscriber to it would also read renewal_free in _renewalLine. The CHANGELOG documents "first row is the free floor", and the Laravel producer may guarantee it, but this package is meant for any app's catalogue. Requiring plans.first.sellableProducts.isEmpty as well would make the wrong case impossible rather than merely documented.

Minor

lib/src/http/controllers/magic_starter_billing_controller.dart (MagicStarterEntitlementSnapshot) (correctness): currentPeriodEnd is part of the snapshot that confirms a purchase. If the held subscription renews while a pending purchase is being polled, that renewal is reported as confirmed and the gate reopens. The window is narrow and the 60 s cap bounds it, so I'm noting it rather than treating it as a defect.

Tests

The PR adds broad coverage for the changed behaviour: product/plan decoding, the wire fixtures, the controller wait and polling, view cards, the disclosure and floor exit, and deletion refusals. None of it has run in CI on this commit (see below).

CI

• Lint & Test: failure (the process exited 1; the details are in the job log, which I can't read). This job clones magic_payments from its default branch, so it most likely fails for the same unreleased-contract reason, not only Published graph as the PR description says. Either way, analyze, format and tests are unconfirmed for this head.
• Published graph: failure, as the description expects until magic_payments 0.0.8 is published.
• Auto-merge low-risk Dependabot PRs: skipped.

I reviewed the controller, model, config and view diffs and the profile controller. I skimmed the test files and did not read them line by line.

[anilcancakir]

Thanks. Round one, per finding:

Major, _isFloor by position only: fixed in f048125. lib/src/ui/views/teams/magic_starter_billing_view.dart _isFloor now requires plans.first.id == plan.id && plan.sellableProducts.isEmpty, so a catalogue with no free tier keeps its first paid tier priced and purchasable; _isCustom and _renewalLine read the same predicate. New test "a paid first row is not the floor: it keeps its price and its button" mounts the producer's own fixture rows minus the free row; it failed before the fix (Found 1 widget with text "Free") and passes now. The producer side already refuses a floor with a sellable product (magic-starter-laravel BillingCatalogueTest::test_a_floor_with_a_sellable_product_is_refused_naming_the_floor), so on its rows both tests agree. CHANGELOG wording updated.

Minor, currentPeriodEnd in the snapshot: kept, deliberately. It is the only field that moves for a Play base-plan switch the backend can name only by the bare subscription id: there product is null before and after (the bare id names a tier, never a product), plan and provider are unchanged, so dropping the period end would leave that purchase waiting for the 60 s cap and reporting "still processing" for a change that did land. A renewal landing inside the same poll window confirms early, at worst, which the cap already bounds; I prefer that to a confirmed purchase reported as stuck. The record's doc comment names this trade.

CI: both jobs resolve magic_payments from pub.dev (the published 0.0.7), so both stay red until magic_payments 0.0.8 (fluttersdk/magic_payments#16) publishes; this PR merges only after that, with CI re-run. Local, against the magic_payments branch: flutter analyze no issues, dart format --set-exit-if-changed lib test 0 changed, flutter test 1882 passed (1 pre-existing skip).

@kodizm please re-review.

[kodizm]

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

Both of my earlier findings are now settled, and the two new commits add no new defect. Merge still waits on CI, which has not run the tests on this head.

What changed since my last review: f048125 fixes the floor finding, and e5ca815 only adds a doc comment.

• _isFloor (Major): fixed. lib/src/ui/views/teams/magic_starter_billing_view.dart:354 now requires plans.first.id == plan.id && plan.sellableProducts.isEmpty. _isCustom (:367) is built from that predicate, so a catalogue whose first row is paid stays priced and purchasable. It is not treated as custom either, because it has sellable products. The other call sites (:693, :874, :2241) go through the same predicate. The CHANGELOG wording is updated too.
• currentPeriodEnd in the snapshot (Minor): settled by your reply. You keep it because a Play base-plan switch named by the bare subscription id changes no other field. The doc comment on MagicStarterEntitlementSnapshot now records that trade-off.

Tests

The new test "a paid first row is not the floor: it keeps its price and its button" covers the fix. The rest of the coverage is the same as in my last review, and none of it has run in CI on this commit.

CI

• Lint & Test: failure (the process exited 1; the details are in the job log, which I can't read). Your reply says both jobs resolve from pub.dev, but .github/workflows/ci.yml:37 clones magic_payments from its default branch and points an override at it. So this job goes green once magic_payments#16 is merged, not when it is published.
• Published graph: failure, as expected until magic_payments 0.0.8 is on pub.dev.
• Auto-merge low-risk Dependabot PRs: skipped.

This re-review read only the diffs of f048125 and e5ca815 and the call sites of _isFloor and _isCustom. The rest of the PR stands on my earlier pass.

[kodizm]

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

The new commits fix the floor finding correctly and add a regression test for it. I found nothing new, but CI is still running on this head.

Changes since my last review (incremental, e34698d to e5ca815):

• _isFloor now also requires plan.sellableProducts.isEmpty.
• The snapshot's doc comment explains why currentPeriodEnd is part of the comparison.
• The CHANGELOG wording is updated.
• There is one new view test.

On the floor fix: _isFloor is the only place that decides the floor. _isCustom, _renewalLine, _billingNote and the card builder all call it, and nothing else in lib reads plans.first. So when a catalogue has no free tier, its first paid tier now keeps its price and its button everywhere on the screen.

On currentPeriodEnd: your answer settles it. A Play base-plan switch named only by its bare subscription id changes no other field, so keeping the period end in the snapshot is a deliberate trade, and the 60 s cap bounds it.

Tests

The new test, "a paid first row is not the floor: it keeps its price and its button", builds a catalogue from the producer's rows minus the free row. It checks that no "Free" label appears, that 290.00 USD is shown, and that a Downgrade button renders on the first paid tier. That covers the fix directly.

CI

CI was still running when I checked a second time, so this head is not confirmed green.

• Lint & Test: in progress.
• Published graph: in progress. The PR description expects this job to fail until magic_payments 0.0.8 is published.
• Auto-merge low-risk Dependabot PRs: skipped.

[anilcancakir]

Two more fixes from a cross-repo review of the batch, in 54e5a68:

• assets/stubs/install/en.stub:106 billing.errors.managed_elsewhere said "managed in another store or on the web". It reaches a store build's toast, and naming the web there is what App Store 3.1.3 forbids. Now "This subscription is managed elsewhere. Change it where you bought it." New test "no shipped billing error sentence names the web" reads every billing.errors.* sentence from the stub; it failed on the old copy (billing.errors.managed_elsewhere) and passes now.
• lib/src/configuration/magic_starter_config.dart termsUrl() / privacyUrl() returned '' for a blank value, so an empty env variable rendered a legal link that opens nothing on the register and billing screens. A blank value now reads as unset; test "a blank url reads as unset, so no dead link renders" failed before (Actual: '').
• CHANGELOG lines added. Local: flutter analyze no issues, flutter test 1884 passed. On the previous head, after magic_payments#16 merged, the CI rerun is green for Lint & Test; Published graph stays red until magic_payments 0.0.8 is on pub.dev.

@kodizm please re-review.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[kodizm]

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

54e5a68 makes both fixes correctly, and I found no defect in it. One sentence that this PR adds still names the web on a store build, which goes against the rule this commit sets.

What changed since my last review (incremental, e5ca815 to 54e5a68): the managed_elsewhere copy, blank legal URLs now read as unset, two tests and two CHANGELOG lines.

• Blank legal URLs: _filledUrl in lib/src/configuration/magic_starter_config.dart:366 returns null for an empty or whitespace-only value. This covers every caller: hasLegalLinks(), the register view (magic_starter_register_view.dart:312) and the billing disclosure (magic_starter_billing_view.dart:1095). They all already treat a link as present only when it is non-null, so an empty value no longer renders a dead link anywhere.
• managed_elsewhere: the new sentence is fine. The test that reads it from the stub guards every billing.errors.* sentence.

Minor

assets/stubs/install/en.stub:424 (correctness, uncertain impact): social.subscription_stripe_no_link, added in this PR, says "Cancel that subscription on the web". magic_starter_profile_controller.dart:216 shows it for a card-billed team whenever the host has no accountDeletionUrl, and nothing stops that from happening on a store build. I'm not sure 3.1.3 covers a cancellation sentence in an account-deletion refusal. Still, it breaks the "never name the web on a store build" rule this commit sets, and the new test only checks billing.errors.*, so it would not catch this. Something like "Cancel that subscription where you set it up" would make the copy match the rule.

Tests

The config test "a blank url reads as unset, so no dead link renders" covers the URL change. The stub-scan test "no shipped billing error sentence names the web" covers the copy change, but only within billing.errors.*.

CI

• Lint & Test: success on 54e5a68.
• codecov/patch: success.
• Published graph: failure (the process exited 1). The PR description expects this until magic_payments 0.0.8 is on pub.dev.
• Auto-merge low-risk Dependabot PRs: skipped.

This re-review read only the five files changed in 54e5a68 and the callers of termsUrl, privacyUrl and managed_elsewhere. The rest of the PR stands on my earlier passes.

[anilcancakir]

Minor fixed in d1f48a9.

• assets/stubs/install/en.stub:424 social.subscription_stripe_no_link now reads "One of your teams is billed by card. Cancel that subscription where you set it up, then delete your account."
• The stub-scan test is now "no shipped billing error or subscription refusal names the web" and also reads every social.subscription_* sentence; it failed on the old copy (subscription_stripe_no_link) and passes now. The delete-account widget test that quoted the old sentence is updated.
• CHANGELOG line extended. Local: flutter analyze no issues, flutter test 1884 passed, format 0 changed.

@kodizm please re-review.

[kodizm]

Note

Kodizm (AI-generated). May contain mistakes; verify before acting.

d1f48a9 fixes my last Minor finding correctly, and I found nothing new in it. Lint & Test is still running on this head, so it is not confirmed green yet.

What changed since my last review (incremental, 54e5a68 to d1f48a9): one copy change in the stub, a wider stub-scan test, an updated quote in the delete-account test, and a longer CHANGELOG line.

• social.subscription_stripe_no_link: fixed. assets/stubs/install/en.stub:424 now reads "Cancel that subscription where you set it up". No other sentence in the stub or in lib still says "on the web" in copy a user would see. The only call site is still magic_starter_profile_controller.dart:216. The controller test at magic_starter_profile_controller_test.dart:890 reads the sentence from the stub through _shipped(...), so it follows the new wording without an edit.
• The scan test now covers the refusals. It collects every top-level social.subscription_* key; I confirmed that social is a top-level key in the stub, and the scan picks up six sentences. It also asserts that subscription_stripe_no_link is among them, so if that key were renamed the test would fail instead of silently passing.

Tests

"no shipped billing error or subscription refusal names the web" covers the copy change. The delete-account widget test quotes the new sentence.

CI

• Lint & Test: still in progress on d1f48a9 after two reads.
• Published graph: failure (the process exited 1; the details are in the job log, which I can't read). The PR description expects this until magic_payments 0.0.8 is on pub.dev.
• Auto-merge low-risk Dependabot PRs: skipped.

This re-review read only the four files changed in d1f48a9 and the callers of subscription_stripe_no_link. The rest of the PR stands on my earlier passes.