docs(quick-261016-myi): Phase 18.1 re-review warnings fixed; disposition, coverage, summary

This commit is contained in:
Jakub Zych
2026-10-16 16:46:24 +02:00
parent 1788c50a85
commit 9098c87d04
4 changed files with 79 additions and 9 deletions

View File

@@ -2,13 +2,13 @@
| Finding | Severity | Disposition | Source |
|---------|----------|-------------|--------|
| WR-01 | warning | open | - |
| WR-02 | warning | open | - |
| WR-03 | warning | open | - |
| WR-04 | warning | open | - |
| WR-05 | warning | open | - |
| WR-06 | warning | open | - |
| WR-07 | warning | open | - |
| WR-01 | warning | fixed | quick 261016-myi |
| WR-02 | warning | fixed | quick 261016-myi |
| WR-03 | warning | fixed | quick 261016-myi |
| WR-04 | warning | fixed | quick 261016-myi |
| WR-05 | warning | fixed | quick 261016-myi |
| WR-06 | warning | fixed | quick 261016-myi |
| WR-07 | warning | fixed | quick 261016-myi |
| IN-01 | info | open | - |
| IN-02 | info | open | - |
| IN-03 | info | open | - |
@@ -16,3 +16,5 @@
| IN-05 | info | open | - |
_Recorded 2026-10-16T12:06:44Z after the gap-closure re-review (18.1-07..09)._
_WR-01..WR-07 fixed 2026-10-16 by quick task 261016-myi (pgsub 2c9e5ef, pgstripe 2e44508); see .planning/quick/261016-myi-fix-18-1-review-warnings/261016-myi-SUMMARY.md. IN-01..IN-05 stay open._

View File

@@ -9,7 +9,8 @@ Scope: the Stripe Billing surface the frozen PHP `SubscriptionGateway` contract
| customers.create with subscriber metadata | INTEGRATE | e-mail, name, subscriber_type and subscriber_id metadata; an idempotency key per subscriber and parameters in a 10-minute window, so concurrent checkouts create one customer (WR-02, plan 18.1-08) |
| customers.retrieve | OPT-OUT | not in the frozen PHP contract: the stored customer id is trusted, as PHP did |
| customers.update (sync e-mail or name changes) | OPT-OUT | not in the frozen PHP contract |
| checkout.sessions.create (mode subscription) | INTEGRATE | session_id placeholder in success URL; subscription_data.metadata; 10-min idempotency key (WR-03); price is the plan's own in that currency, never another's (CR-02); plan 18.1-08 |
| checkout.sessions.create (mode subscription) | INTEGRATE | session_id placeholder in success URL; subscription_data.metadata; the plan's own price in that currency (CR-02); no own idempotency key since quick 261016-myi (WR-04) |
| checkout.sessions.list + expire (open, subscription mode) | INTEGRATE | the guarded checkout expires the customer's open subscription sessions before a new one, so one payable session exists (quick 261016-myi, WR-04) |
| checkout.sessions.retrieve (after the success redirect) | OPT-OUT | not in the frozen PHP contract: activation comes from the signed customer.subscription.created event |
| billing_portal.sessions.create | INTEGRATE | |
| billing_portal.configurations (create or update) | OPT-OUT | not in the frozen PHP contract: the portal is configured per account in the Stripe dashboard |
@@ -20,10 +21,11 @@ Scope: the Stripe Billing surface the frozen PHP `SubscriptionGateway` contract
| subscriptions.update (resume or un-cancel through the API) | OPT-OUT | not in the frozen PHP contract: reactivation happens in the billing portal and arrives as customer.subscription.updated |
| subscriptions.update (plan change, proration) | OPT-OUT | not in the frozen PHP contract: plan changes go through the billing portal |
| subscriptions.list / subscriptions.search | OPT-OUT | not in the frozen PHP contract: reconcile walks local rows and retrieves each |
| products.create (plan_code and tier_level metadata) | INTEGRATE | |
| products.create (plan_code and tier_level metadata) | INTEGRATE | idempotency key over plan id, code, name, description and tier in a 10-min window; syncs of one plan also serialise on a plan lock (quick 261016-myi, WR-03) |
| products.update | INTEGRATE | |
| products archive or delete | OPT-OUT | not in the frozen PHP contract: deactivating a plan is the local is_active flag |
| prices.list by lookup_keys with active true | INTEGRATE | replaces PHP's prices.search with the same outcome (plan 18.1-03 decision, RESEARCH A2) |
| prices.retrieve (before a checkout) | INTEGRATE | a missing, inactive, foreign-currency or other-amount price is refused before the session (quick 261016-myi, WR-06) |
| prices.search | OPT-OUT | replaced by prices.list with lookup_keys, which is not eventually consistent; same result as PHP |
| prices.create (recurring, lookup_key <code>_<currency>, transfer_lookup_key) | INTEGRATE | |
| prices.update active false (archive the replaced price) | INTEGRATE | |

View File

@@ -0,0 +1,29 @@
---
quick_id: 261016-myi
slug: fix-18-1-review-warnings
date: 2026-10-16
repos: [sm-paymentgatewaysubscriptions-plugin, sm-pgstripe-plugin]
source: .planning/phases/18.1-payment-gateway-subscriptions/18.1-REVIEW.md (WR-01..WR-07)
---
# Quick 261016-myi: fix the Phase 18.1 re-review warnings WR-01..WR-07
No push, tag or release. Plugin repos commit first; disposition and summary here afterwards.
## Tasks
1. **WR-01** `Service.Cancel` takes `SubscriptionLockKey` around the gateway call and the local flag write.
2. **WR-02** `ApplyDeleted` fires `SubscriptionCanceled` only on a transition into canceled; ordering tests count canceled == 1.
3. **WR-03** `SyncPlan` (detached path) runs under a per-plan session lock on a fresh copy of the row; `V1Products.Create` carries an idempotency key.
4. **WR-04** Guarded checkout expires the customer's open subscription-mode Checkout Sessions inside the subscriber lock (optional gateway interface, implemented by pgstripe).
5. **WR-05** Plan sync prices every currency the plan lists that the gateway supports, not the fixed usd/eur/pln list.
6. **WR-06** The gateway retrieves the price before the session and refuses (`ErrNoPriceForCurrency`) when amount, currency or active differ from the plan.
7. **WR-07** `incomplete` no longer blocks a checkout; the guarded checkout cancels the subscriber's incomplete subscriptions at the gateway inside the lock.
## Verify
`go vet ./...` and `go test ./...` (Docker) green in both plugin repos.
## Added during execution
8. **G-18.1-8** pgsub cannot boot in a `summer build` app: `bootService` required `*gorm.DB` at Boot, but the generated main.go activates plugins before any command opens the database. Build the service through `lagoon.OnDatabase`; add a test whose host activates plugins before publishing the database.

View File

@@ -0,0 +1,37 @@
---
quick_id: 261016-myi
slug: fix-18-1-review-warnings
status: complete
date: 2026-10-16
repos:
sm-paymentgatewaysubscriptions-plugin: [2c9e5ef, ab46d6f]
sm-pgstripe-plugin: [2e44508]
---
# Quick 261016-myi: Phase 18.1 re-review warnings WR-01..WR-07 and G-18.1-8
Nothing pushed, tagged or released.
| Item | Fix | Commit |
|------|-----|--------|
| WR-01 | `Service.Cancel` runs the gateway call and the `cancel_at_period_end` write under `SubscriptionLockKey` | pgsub 2c9e5ef |
| WR-02 | `ApplyDeleted` fires `SubscriptionCanceled` only on the transition into canceled (deviation from PHP's unconditional fire); ordering tests count canceled == 1 | pgsub 2c9e5ef, pgstripe 2e44508 |
| WR-03 | `SyncPlan` (nil tx) runs under the new `PlanLockKey` and re-reads the plan; `V1Products.Create` carries a `pgsub-product-` idempotency key | pgsub 2c9e5ef, pgstripe 2e44508 |
| WR-04 | `CheckoutGuarded` expires the customer's open subscription sessions through the new optional `contracts.CheckoutSessionExpirer` (pgstripe `ExpireOpenCheckoutSessions`); the session create drops its own idempotency key (it would replay an expired session); `Apply` logs at Error when a subscriber ends up with two open subscriptions | pgsub 2c9e5ef, pgstripe 2e44508 |
| WR-05 | Plan sync prices usd/eur/pln plus every supported currency the plan prices (own currency, `currency_prices` keys) | pgstripe 2e44508 |
| WR-06 | The checkout retrieves its price and refuses a missing, inactive, foreign-currency or other-amount price; `ErrNoPriceForCurrency` wraps the new `contracts.ErrPriceUnavailable`; the customer API answers 422 | pgsub 2c9e5ef, pgstripe 2e44508 |
| WR-07 | `incomplete` no longer blocks a checkout (`ScopeBlocksCheckout`, `Service.CheckoutBlocker`); the guarded checkout cancels the subscriber's incomplete subscriptions at the gateway first | pgsub 2c9e5ef |
| G-18.1-8 | `bootService` builds the service through `lagoon.OnDatabase`; `TestBootBeforeDatabase` activates before publishing the database (fails on the old boot) | pgsub ab46d6f |
Other boot-time lookups checked: pgsub's API, admin controllers and pgstripe (root and subscriptions) all look up `*gorm.DB` and `*classes.Service` lazily at request or job time.
## Verification
- `go vet ./...` and `go test ./...` (Docker) green in sm-paymentgatewaysubscriptions-plugin and sm-pgstripe-plugin.
- New tests: pgsub `TestCheckoutIncompleteAndOpenSessions`, `TestCancelTakesSubscriptionLock`, `TestSyncPlanSerialised`, `TestCheckoutPriceUnavailable`, `TestBootBeforeDatabase`; pgstripe `TestReviewFixesGateway`.
- api-coverage gate passes after the COVERAGE.md update (checkout.sessions list/expire, prices.retrieve, products.create key).
## Notes
- pgstripe now uses pgsub API added here (`contracts.CheckoutSessionExpirer`, `contracts.ErrPriceUnavailable`); it builds through the go.work replace. A release must tag pgsub first and bump pgstripe's `require`.
- `.planning/STATE.md` was not updated: it carries another session's uncommitted edits.