Compare commits
23 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
450c535452 | ||
|
|
ab9f385db8 | ||
|
|
2dd5fb3bbb | ||
|
|
06e2e60ce1 | ||
|
|
90b2d4e23b | ||
|
|
6878be4e6a | ||
|
|
5f1245f1e4 | ||
|
|
0e5c56b5ae | ||
|
|
0dbf5e14b0 | ||
|
|
6e14522a84 | ||
|
|
17398fe87c | ||
|
|
013890c3d6 | ||
|
|
bc5685eb93 | ||
|
|
9890aff66d | ||
|
|
bad37d1d3d | ||
|
|
6ae1c1c077 | ||
|
|
8a04416b0c | ||
|
|
9098c87d04 | ||
|
|
1788c50a85 | ||
|
|
52d149b8f1 | ||
|
|
f56ebd0828 | ||
|
|
646e51356e | ||
|
|
d9ccc8275e |
@@ -1156,7 +1156,7 @@ Plans:
|
||||
4. A host test proves checkout, activation, `invoice.paid`, `past_due` with its grace period, cancel and duplicate-event idempotency against a Stripe fake.
|
||||
5. The new code has unit tests, delivered in the phase's last plan.
|
||||
|
||||
**Plans:** 8/9 plans executed
|
||||
**Plans:** 9/9 plans executed
|
||||
|
||||
Plans:
|
||||
**Wave 1**
|
||||
@@ -1180,7 +1180,7 @@ Plans:
|
||||
- [x] 18.1-08-PLAN.md — Checkout, admin sync and layer: no cross-currency price (CR-02), first-write-wins customer id and idempotency keys (WR-02), guarded checkout for any open subscription (WR-03), framework `pact.AdminBulkAction.Detached` and plan sync outside transactions (WR-01), buttons recover after Back (WR-07)
|
||||
|
||||
**Gap closure wave 3** *(blocked on 18.1-07, 18.1-08)*
|
||||
- [ ] 18.1-09-PLAN.md — Unit tests last for the gap closure across four repositories, the extended gate, VALIDATION rows, layer 0.2.1, final push approval (framework v0.1.27, layer v0.2.1, optional first plugin tags)
|
||||
- [x] 18.1-09-PLAN.md — Unit tests last for the gap closure across four repositories, the extended gate, VALIDATION rows, layer 0.2.1, final push approval (framework v0.1.27, layer v0.2.1, optional first plugin tags)
|
||||
|
||||
### Phase 19: Chat and ChatChannels
|
||||
|
||||
@@ -1385,7 +1385,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5 → 6 → 7 → 8 →
|
||||
| 16.3. Backend admin personal API tokens with scopes (INSERTED) | 4/4 | Complete | 2026-10-12 |
|
||||
| 17. Blog companions | 6/6 | Complete | 2026-10-13 |
|
||||
| 18. Payment Gateway | 8/8 | Complete | 2026-10-14 |
|
||||
| 18.1. Payment Gateway Subscriptions | 8/9 | In Progress| |
|
||||
| 18.1. Payment Gateway Subscriptions | 9/9 | In Progress| |
|
||||
| 19. Chat and ChatChannels | 0/TBD | Not started | - |
|
||||
| 19.1. Chat satellites | 0/TBD | Not started | - |
|
||||
| 19.2. Chat polish against WavePath clients | 0/TBD | Not started | - |
|
||||
|
||||
@@ -900,6 +900,7 @@ Recent decisions affecting current work:
|
||||
| 261015-w7f | Framework docs since v0.1.14 + Upgrading to 0.2, release v0.2.0 (plugins, layers), deploy summercms.io | 2026-10-15 | a204d38 | [261015-w7f-update-framework-docs-for-phase-19-3-rel](./quick/261015-w7f-update-framework-docs-for-phase-19-3-rel/) |
|
||||
| 36 | summercms.io copy Alpha 0.1 → 0.2 (vue b5d6dbb, app 6a13a87), deployed | 2026-10-15 | 853ea1f | — |
|
||||
| 261016-0fm | Admin minors for v0.2.1: theme switch, language switch, settings nav on form pages | 2026-10-16 | 66fd4ae | [261016-0fm-admin-minors-for-v0-2-1-theme-switch-lan](./quick/261016-0fm-admin-minors-for-v0-2-1-theme-switch-lan/) |
|
||||
| 261016-s8x | PaymentGateway: banner bottom margin and hide disabled sections from admin navigation | 2026-10-16 | ab9f385 | [261016-s8x-paymentgateway-banner-bottom-margin-and-](./quick/261016-s8x-paymentgateway-banner-bottom-margin-and-/) |
|
||||
|
||||
## Deferred Items
|
||||
|
||||
|
||||
@@ -254,7 +254,7 @@ The plan proposed framework tag `v0.1.27`. Since the plan was written, `v0.1.27`
|
||||
**Push sheet.** Every repository was 0 commits behind its origin after `git fetch origin --tags`. Unpushed commits:
|
||||
- summercms.go master: 27 ahead before this SUMMARY.
|
||||
- This phase's commits: plans 07 to 09.
|
||||
- Other sessions' committed work, approved for this push: quick-261016-0fm (admin theme and locale preferences, the dist rebuild for v0.2.1), quick-261015-w7f (docs since v0.1.14, deployed on v0.2.0), the fast task "summercms.io Alpha 0.2 copy", and `9921768` docs(19.3-05).
|
||||
- Other sessions' committed work, approved for this push: quick-261016-0fm (admin theme and locale preferences, the dist rebuild for v0.2.1), the closing docs of quick-261015-w7f (summercms.io deployed on v0.2.0), and the fast task "summercms.io Alpha 0.2 copy". (Correction made after the release: an earlier version of this line also named `9921768` docs(19.3-05). That commit was already on origin as `v0.1.27`.)
|
||||
- pgsub main: 7 ahead (plans 07 to 09).
|
||||
- pgstripe main: 8 ahead (plans 07 to 09).
|
||||
- layer main: 3 ahead (plans 08 and 09).
|
||||
@@ -267,6 +267,44 @@ The plan proposed framework tag `v0.1.27`. Since the plan was written, `v0.1.27`
|
||||
|
||||
Replace lines stay in both plugins. Nothing is force-pushed. The pushed hashes and tag objects are recorded in the release record below, which is added after the pushes.
|
||||
|
||||
### Release record
|
||||
|
||||
This is what was done, in the order of the push sheet. Nothing was force-pushed. Every repository was 0 commits behind its origin right before its push (`git fetch origin --tags`, then `rev-list --count HEAD..origin/<branch>`).
|
||||
|
||||
**Framework**
|
||||
- master pushed: `f3e8a98..f64ae79`. `f64ae79` is this SUMMARY's commit.
|
||||
- Annotated tag `v0.2.1` created on `f64ae79` (tag object `f067801`) and pushed. `v0.1.27` and `v0.2.0` are untouched.
|
||||
|
||||
**pgsub**
|
||||
- Commit `41b68f6` raises the require to `git.golem15.com/golem15/summercms v0.2.1`; the replace onto `../summercms.go` is unchanged.
|
||||
- `go mod tidy` added the framework's indirect dependencies since v0.1.26: aws-sdk-go-v2 (S3), go-redis, otter, google/wire, go.uber.org/atomic. This is +24 lines in each of go.mod and go.sum, matching the framework's own go.mod; no direct dependency was added.
|
||||
- vet and the `-short` tests pass.
|
||||
- main pushed: `472b75b..41b68f6`.
|
||||
- Annotated tag `v0.1.0` created on `41b68f6` (tag object `8b6a1f0`) and pushed.
|
||||
|
||||
**pgstripe**
|
||||
- Commit `2a427fd` raises the requires to summercms `v0.2.1` and sm-paymentgatewaysubscriptions-plugin `v0.1.0`; the replace lines are unchanged.
|
||||
- tidy added the same indirect dependencies as in pgsub. vet and `-short` pass.
|
||||
- main pushed: `baf3c55..2a427fd`.
|
||||
- Annotated tag `v0.1.0` created on `2a427fd` (tag object `da39829`) and pushed.
|
||||
|
||||
**Layer**
|
||||
- main pushed: `30046f5..22368c5`.
|
||||
- Annotated tag `v0.2.1` created on `22368c5` (tag object `b7a6fe7`, package.json 0.2.1) and pushed.
|
||||
|
||||
No GOPROXY or GOPRIVATE resolution was needed during tidy, because the replace lines resolve the framework and pgsub locally.
|
||||
|
||||
**Check:** after the pushes, each local branch equals its origin branch:
|
||||
|
||||
| Repository | Branch | Hash | Tag (`ls-remote ^{}`) |
|
||||
|------------|--------|------|-----------------------|
|
||||
| summercms.go | master | `f64ae79`, before the later docs commits, which are pushed afterwards | `v0.2.1` → `f64ae79` |
|
||||
| pgsub | main | `41b68f6` | `v0.1.0` → `41b68f6` |
|
||||
| pgstripe | main | `2a427fd` | `v0.1.0` → `2a427fd` |
|
||||
| layer | main | `22368c5` | `v0.2.1` → `22368c5` |
|
||||
|
||||
`grep "summercms v0.2.1"` finds a line in both plugin go.mod files, and `grep "sm-paymentgatewaysubscriptions-plugin v0.1.0"` finds one in pgstripe's go.mod.
|
||||
|
||||
**Open item:** the layer's `scripts/check-git-install.sh` still targets `vue-journal-layer`, so an install check of layer `v0.2.1` needs that script fixed first. See `deferred-items.md`.
|
||||
|
||||
## Decisions Made
|
||||
|
||||
@@ -66,6 +66,7 @@ Research (`18.1-RESEARCH.md` § Premise Shifts S-1..S-10) found premises that no
|
||||
- **D-20 (S-8, S-9):** The Stripe subscriptions subpackage is its own party plugin, `golem15.pgstripesubscriptions`, at module path `git.golem15.com/golem15/sm-pgstripe-plugin/subscriptions`. It gets its own importable or package-local Stripe fake; P18's root `stripefake_test.go` stays unchanged.
|
||||
- **D-21:** Behaviour defaults: keep PHP's `cancel_at_period_end` mapping (a manual test-mode check decides whether `cancel_at` from the billing portal also needs handling); out-of-order events behave as PHP does; the job reads both pre-basil and basil payload shapes; customer `GET /subscription` returns the latest subscription with `has_access` and `in_grace_period` flags.
|
||||
- **D-21 amendment (2026-10-14, gap closure 18.1-07, user decision on verification human item 1):** out-of-order events no longer behave as PHP; created and updated events apply the subscription Stripe holds, serialised per subscription, and a deletion before its subscription is stored is retried. D-21's other defaults are unchanged.
|
||||
- **D-21 amendment (2026-10-16, quick 261016-nqk, UAT test 7 / gap G-18.1-7):** the test-mode check settled the `cancel_at` question. On API `2026-09-30.endive` a billing-portal cancel reports `cancel_at` (the period end) with `cancel_at_period_end` false; an API cancel reports both. pgstripe now stores `cancel_at_period_end` true when Stripe reports that flag or a `cancel_at` date on an active or trialing subscription; clearing `cancel_at` maps back to false and fires `reactivated`. The row keeps PHP's single flag; `cancel_at` itself is not stored.
|
||||
- **D-22 (plan split, 6 plans):** (1) framework: cabana flat reorder + `taglist`; (2) pgsub core tracer in the new repo (created with `tea repos create --owner golem15`); (3) pgstripe secret columns + import mapping + `subscriptions` plugin (gateway, both webhook routes, River job, host test); (4) pgsub admin screens + customer API; (5) `vue-paymentgateway-layer` pieces (tag `v0.2.0` only on go-ahead); (6) unit tests across all repos + `scripts/check-phase18.1.sh`. Plan 2's repo is committed before plan 3's `go.mod` points at it. No push, tag or deploy without the user's go-ahead.
|
||||
|
||||
### Claude's Discretion
|
||||
|
||||
@@ -0,0 +1,20 @@
|
||||
# Phase 18.1: Code Review Disposition
|
||||
|
||||
| Finding | Severity | Disposition | Source |
|
||||
|---------|----------|-------------|--------|
|
||||
| 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 | - |
|
||||
| IN-04 | info | open | - |
|
||||
| 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._
|
||||
@@ -1,434 +1,307 @@
|
||||
---
|
||||
phase: 18.1-payment-gateway-subscriptions
|
||||
reviewed: 2026-10-14T19:22:09Z
|
||||
reviewed: 2026-10-16T12:04:37Z
|
||||
depth: standard
|
||||
files_reviewed: 96
|
||||
review_kind: re-review after gap closure (plans 18.1-07, 18.1-08, 18.1-09)
|
||||
files_reviewed: 44
|
||||
files_reviewed_list:
|
||||
- summercms.go/admin/src/components/form/PreviewField.vue
|
||||
- summercms.go/admin/src/components/form/fields/TaglistField.vue
|
||||
- summercms.go/admin/src/components/form/registry.ts
|
||||
- summercms.go/admin/src/components/reorder/ReorderBranch.vue
|
||||
- summercms.go/admin/src/components/reorder/tree.ts
|
||||
- summercms.go/admin/src/views/ReorderView.vue
|
||||
- summercms.go/modules/cabana/admin_openapi.go
|
||||
- summercms.go/modules/cabana/bulk_detached_test.go
|
||||
- summercms.go/modules/cabana/crud.go
|
||||
- summercms.go/modules/cabana/field_file.go
|
||||
- summercms.go/modules/cabana/field_taglist.go
|
||||
- summercms.go/modules/cabana/form_schema.go
|
||||
- summercms.go/modules/cabana/list_schema.go
|
||||
- summercms.go/modules/cabana/registry.go
|
||||
- summercms.go/modules/cabana/relation_child.go
|
||||
- summercms.go/modules/cabana/relation_form.go
|
||||
- summercms.go/modules/cabana/reorder.go
|
||||
- summercms.go/modules/cabana/schema_types.go
|
||||
- summercms.go/modules/cabana/settings.go
|
||||
- summercms.go/modules/pact/capabilities.go
|
||||
- summercms.go/scripts/check-phase10.sh
|
||||
- summercms.go/scripts/check-phase18.1.sh
|
||||
- sm-paymentgatewaysubscriptions-plugin/.gitignore
|
||||
- sm-paymentgatewaysubscriptions-plugin/admin.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/admin_navigation.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/admin_permissions.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/api/errors.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/admin_sync_detached_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/api/json.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/api/plans.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/api/subscription.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/api/urls.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/assets/js/fields.js
|
||||
- sm-paymentgatewaysubscriptions-plugin/boot.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/apply_events_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/checkout_guard_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/classes/apply.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/classes/lock.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/classes/service.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/classes/user_subscribable.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/config/config.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/contracts/gateway.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/contracts/registry.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/contracts/subscriber.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/admin_registry.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/base.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans/config_form.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans/config_list.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans/config_reorder.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/_subscription_plan.htm
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/_subscription_subscriber.htm
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/config_filter.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/config_form.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions/config_list.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/events/events.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/field_types.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/go.mod
|
||||
- sm-paymentgatewaysubscriptions-plugin/go.work
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/feature_list.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/filters.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/processed_webhook_event.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/registry.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/core_smoke_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/customer_api_authz_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/customer_api_smoke_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/fixture_plugin_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/checkout_price_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/subscription.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/subscription/columns.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/subscription/fields.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/subscriptionplan/columns.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/models/subscriptionplan/fields.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/plugin.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/routes.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/updates/202610150001_create_golem15_pgsub_baseline.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/updates/202610150002_add_stripe_customer_id_to_users.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/updates/registry.go
|
||||
- sm-pgstripe-plugin/client.go
|
||||
- sm-pgstripe-plugin/config/config.yaml
|
||||
- sm-pgstripe-plugin/console/import_winter.go
|
||||
- sm-pgstripe-plugin/go.mod
|
||||
- sm-pgstripe-plugin/go.work
|
||||
- sm-pgstripe-plugin/models/settings.go
|
||||
- sm-pgstripe-plugin/models/settings/fields.yaml
|
||||
- sm-pgstripe-plugin/subscriptions/config/config.yaml
|
||||
- sm-paymentgatewaysubscriptions-plugin/plan_resolution_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/service_api_test.go
|
||||
- sm-paymentgatewaysubscriptions-plugin/sync_subscription_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/decode.go
|
||||
- sm-pgstripe-plugin/subscriptions/decode_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/fresh_state_smoke_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/gateway.go
|
||||
- sm-pgstripe-plugin/subscriptions/gateway_hardening_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/gateway_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/host_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/job.go
|
||||
- sm-pgstripe-plugin/subscriptions/plugin.go
|
||||
- sm-pgstripe-plugin/subscriptions/sync_plan.go
|
||||
- sm-pgstripe-plugin/subscriptions/webhook.go
|
||||
- sm-pgstripe-plugin/updates/202610150001_add_subscription_webhook_secrets.go
|
||||
- vue-paymentgateway-layer/.playground/app/pages/subscriptions/index.vue
|
||||
- vue-paymentgateway-layer/app/assets/css/pg-base.css
|
||||
- sm-pgstripe-plugin/subscriptions/job_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/ordering_test.go
|
||||
- sm-pgstripe-plugin/subscriptions/stripefake_test.go
|
||||
- vue-paymentgateway-layer/app/components/Pg/ManageSubscriptionButton.vue
|
||||
- vue-paymentgateway-layer/app/components/Pg/MySubscription.vue
|
||||
- vue-paymentgateway-layer/app/components/Pg/Plans.vue
|
||||
- vue-paymentgateway-layer/app/components/Pg/SubscribeButton.vue
|
||||
- vue-paymentgateway-layer/app/composables/usePlans.ts
|
||||
- vue-paymentgateway-layer/app/composables/useSubscription.ts
|
||||
- vue-paymentgateway-layer/app/composables/useSubscriptionsApi.ts
|
||||
- vue-paymentgateway-layer/app/utils/subscribe.ts
|
||||
- vue-paymentgateway-layer/app/utils/subscriptionsClient.ts
|
||||
- vue-paymentgateway-layer/nuxt.config.ts
|
||||
- vue-paymentgateway-layer/package.json
|
||||
- vue-paymentgateway-layer/tests/subscribe.test.ts
|
||||
- vue-paymentgateway-layer/tests/subscriptionComponents.test.ts
|
||||
findings:
|
||||
critical: 2
|
||||
critical: 0
|
||||
warning: 7
|
||||
info: 8
|
||||
total: 17
|
||||
info: 5
|
||||
total: 12
|
||||
status: issues_found
|
||||
---
|
||||
|
||||
# Phase 18.1: Code Review Report
|
||||
# Phase 18.1: Code Review Report (re-review after gap closure)
|
||||
|
||||
**Reviewed:** 2026-10-14T19:22:09Z
|
||||
**Reviewed:** 2026-10-16T12:04:37Z
|
||||
**Depth:** standard
|
||||
**Files Reviewed:** 96 (four repositories; every path below carries its repository prefix)
|
||||
**Files Reviewed:** 44 (four repositories; every path carries its repository prefix)
|
||||
**Status:** issues_found
|
||||
|
||||
## Summary
|
||||
|
||||
The review covered the cabana taglist field and flat reorder (framework and admin SPA), the pgsub core, its admin screens and customer API, the Stripe subscription gateway with its webhook, River job and plan sync, and the Nuxt layer pieces.
|
||||
This is the re-review after the gap closure. Plans 18.1-07, 18.1-08 and 18.1-09 targeted CR-01, CR-02, WR-01, WR-02, WR-03, WR-04, WR-06 and WR-07 of the first review (2026-10-14). Scope:
|
||||
- summercms.go: commits after `929cfd8`;
|
||||
- pgsub: commits after `472b75b`;
|
||||
- pgstripe: commits after `baf3c55`;
|
||||
- the layer: commits after `30046f5`.
|
||||
|
||||
The framework half is in good shape. The taglist save path normalises and checks values at every save site before any transaction. The flat reorder writes only the sort column, and a submitted parent is refused. The webhook verifies the signature on the raw body before decoding it, uses a separate secret per endpoint, and claims the processed-event marker inside the transaction that applies the event. The customer API never takes a subscriber id, a subscription id or a redirect URL from the request.
|
||||
Called code outside the list was read where a finding depends on it: `lagoon.Transaction`, cabana's toolbar action path, `sync_plan.go`, conga's default attempts, and the pre-fix `subscriptionplans.go` and `SyncPlan`.
|
||||
|
||||
Two money-relevant defects block shipping:
|
||||
Both blockers are closed. The webhook job now retrieves the subscription's state from Stripe outside any transaction. It does this under a per-subscription session advisory lock, and the claim-first marker transaction is unchanged. Checkout no longer falls back to another currency's price.
|
||||
|
||||
1. **CR-01: stale webhook data can overwrite newer data.** The webhook job applies the subscription snapshot carried in each Stripe event, with nothing to tell an old event from a new one. Stripe delivers events out of order, and River runs and retries jobs concurrently. A paid subscription can drop back to `incomplete` and lose access. A deleted subscription can come back as active.
|
||||
2. **CR-02: checkout can charge a different currency or amount than the user was shown.** The customer API checks the currency against the plan's local prices. The gateway then silently falls back to `stripe_price_id`, which plan sync sets to the USD price.
|
||||
The lock implementation holds up under tracing:
|
||||
- **Pinned connection.** GORM's `Connection` keeps the `*sql.Conn`. The `NewDB` session keeps the `ConnPool`, and `lagoon.Transaction` begins its transaction on that same connection.
|
||||
- **Unlock and discard.** The unlock runs with `context.WithoutCancel`. A failed lock or unlock marks the connection bad through `Conn.Raw` returning `driver.ErrBadConn`, so database/sql closes it and the server releases the lock.
|
||||
- **Framework change.** The cabana `Detached` branch keeps the list scope, the empty and partial-selection rules and the permission check. The new field is `json:"-"` and opt-in, so existing consumers (the `users` bulk actions in the apps) compile and behave as before.
|
||||
- **Layer.** The bfcache restore works in all three components, and the listener is removed on unmount.
|
||||
|
||||
The warnings cover:
|
||||
- Stripe calls made while holding a database transaction and row locks.
|
||||
- Duplicate Stripe customers and duplicate subscriptions under concurrent or repeated checkout.
|
||||
- Subscriptions that are dropped after a plan is repriced.
|
||||
- A grace period inherited from PHP that grants about a full unpaid period.
|
||||
- A lost-update risk in SyncSubscription.
|
||||
- Subscribe and manage buttons in the layer that stay disabled after the browser Back button.
|
||||
No new blocker was found. The seven warnings are:
|
||||
- **Residual gaps in fixed findings (WR-01, WR-04, WR-06).** `Service.Cancel` still writes a gateway-owned column without the subscription lock, so the spurious `reactivated` of CR-01 sequence 3 is still reachable through a race. The open-subscription guard cannot see unpaid Checkout Sessions. Checkout bills the last-synced amount, not the amount shown.
|
||||
- **New behaviour introduced by the fixes (WR-02, WR-03, WR-05, WR-07):**
|
||||
- fresh-state retrieval can fire `SubscriptionCanceled` twice;
|
||||
- the detached plan sync dropped the row lock that serialised concurrent syncs of one plan;
|
||||
- the strict price map makes plans in gbp, cad, aud or chf impossible to buy once they are synced;
|
||||
- treating `incomplete` as open locks a user out of checkout for up to about 23 hours.
|
||||
|
||||
Byte compatibility: the webhook's five plain-text answers, their order, the event names, the table, column and constraint names and the permission codes match what the SUMMARYs claim. The documented exception is `text/plain` instead of Laravel's `text/html`. CR-01, CR-02 (the usd/eur/pln loop and its fallback) and WR-05 are defects inherited from the frozen PHP source. They are reported anyway, because the Go port adds concurrent workers and a new customer API that make them easier to reach.
|
||||
## Resolution of the first review's findings
|
||||
|
||||
| Finding | Status | Evidence |
|
||||
|---|---|---|
|
||||
| CR-01 stale webhook snapshots | **Resolved** (one residual race, WR-01 below) | `sm-pgstripe-plugin/subscriptions/job.go:206-244` retrieves fresh state under `classes.WithSessionLock`. `apply.go:191-195` retries an unknown deletion. Sequences 1 to 3 are covered by `TestFreshStateSmoke` and `TestEventOrdering`. `Service.Cancel` does not take the lock (WR-01). |
|
||||
| CR-02 cross-currency checkout | **Resolved** for currency (amount residual, WR-06; side effect, WR-05) | `models/subscription_plan.go:129-144` `CheckoutPriceID` has no fallback. The API answers 422 at `api/subscription.go:128-131`, and the gateway refuses with `ErrNoPriceForCurrency` at `gateway.go:199-202`. The yen is dropped from `supportedCurrencies` (`gateway.go:44`). |
|
||||
| WR-01 Stripe calls inside the bulk transaction | **Resolved** (regression, WR-03) | `controllers/subscriptionplans.go:55` sets `Detached: true`. The branch is at `crud.go:370-373, 412-435`. `SyncPlan(ctx, nil, plan)` saves with `context.WithoutCancel` (`service.go:597-605`), and a failed save becomes a failure line. |
|
||||
| WR-02 duplicate Stripe customers | **Resolved** | `user_subscribable.go` writes the first value only and re-reads the stored one. `gateway.go:163-164` sets an idempotency key on the customer create. `ensureCustomer` stores through `WithoutCancel` (`service.go:631`). |
|
||||
| WR-03 already-subscribed guard | **Partially resolved** (WR-04 and WR-07 below) | `ScopeOpen` (`models/subscription.go:114-121`), `CheckoutGuarded` under `SubscriberLockKey` (`service.go:319-344`), and a checkout idempotency key (`gateway.go:245-246`). Unpaid open sessions are still not seen. |
|
||||
| WR-04 subscriptions on archived prices | **Resolved** | `apply.go:327-339` resolves the plan by price, then product, then plan code. `decode.go` carries `product_id`/`plan_code`. |
|
||||
| WR-05 grace period from the advanced period end | **Open (deferred)** | `models/subscription.go:149`, unchanged and kept PHP-identical by user decision (`deferred-items.md`). Not counted again below. |
|
||||
| WR-06 SyncSubscription lost update | **Resolved** | `service.go:418-488`: under the lock, `FOR UPDATE` re-read, `Select` of the gateway-owned columns, plan refreshed on the retriever path. |
|
||||
| WR-07 buttons stuck after Back | **Resolved** | `subscribe.ts:57-64, 66-71, 138, 195`. All three components wire `watchPageRestore` in `onMounted` and remove it in `onBeforeUnmount`. `MySubscription` also refreshes its data. |
|
||||
| IN-01 to IN-08 | **Open (deferred)** | `deferred-items.md`. Not counted again below. |
|
||||
|
||||
## Narrative Findings (AI reviewer)
|
||||
|
||||
## Critical Issues
|
||||
|
||||
### CR-01: Webhook job applies stale event snapshots with no ordering guard, so a newer subscription state can be overwritten by an older event
|
||||
|
||||
**Files:**
|
||||
- `sm-pgstripe-plugin/subscriptions/job.go:119-147, 155-170, 187-205`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:28-126` (lookup and lock at 52, unconditional status write at 96, reactivated diff at 138), `apply.go:170-205` (unknown row at 183)
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:308-330`
|
||||
|
||||
**Issue:** `work` maps `data.object` from the Stripe event into `contracts.Update`, and `Apply` writes that status, period and cancel flag onto the row as they are. Neither the event's `created` timestamp nor any version is decoded (the `event` struct in `decode.go:30-36` has only id, type and data), so nothing stops an older event from winning. Stripe does not guarantee delivery order, and River runs several workers and retries failed jobs with backoff. Several realistic sequences break:
|
||||
|
||||
1. **Checkout race on a new subscription.** Stripe Checkout emits `customer.subscription.created` (status `incomplete`) and `customer.subscription.updated` (status `active`) within milliseconds.
|
||||
- Both jobs run `SELECT ... FOR UPDATE`, which finds no row and so locks nothing, and both INSERT.
|
||||
- If the `updated` job wins the insert, it stores `active` and fires `activated`. The `created` job fails on the unique index on `stripe_subscription_id`, rolls back its marker and is retried.
|
||||
- On retry it finds the row and writes `incomplete` over `active`. No event fires.
|
||||
- The customer has paid but `HasAccess` is false until Stripe's next update, typically the next renewal a month later.
|
||||
2. **Deleted before created.** If the `created` job is retrying (for example "no plan holds price id" until an admin syncs the plan) and `customer.subscription.deleted` arrives:
|
||||
- `ApplyDeleted` finds no row, logs a warning and returns `nil, nil`, so the deletion marker is committed.
|
||||
- The `created` retry then inserts the row as `active`.
|
||||
- The canceled subscription grants access with no end, and `activated` fires.
|
||||
3. **Spurious `reactivated`.** `Service.Cancel` writes `cancel_at_period_end = true` locally (`service.go:324-325`). An older queued `customer.subscription.updated` carrying `cancel_at_period_end: false` then meets `previousCancel && !sub.CancelAtPeriodEnd && active` (`apply.go:138`) and fires `SubscriptionReactivated` for a cancellation the user never withdrew.
|
||||
|
||||
PHP's job had the same snapshot semantics. The Go port adds concurrent workers and retry-with-backoff, which makes the race in sequence 1 likely on every Checkout rather than rare.
|
||||
|
||||
**Fix:** Apply authoritative state rather than the snapshot, as Stripe recommends. In the job, outside the transaction, retrieve the subscription from Stripe (the gateway already has the code in `SyncSubscription`) and feed that into `Apply`, keeping the claim-first transaction. Alternatively, decode `event.created` and refuse any update older than the last one applied. That needs a stored per-subscription marker, such as a column or a side table keyed by `stripe_subscription_id`. Also treat an unknown row in `ApplyDeleted` as retryable instead of committing the marker:
|
||||
|
||||
```go
|
||||
// decode.go
|
||||
type event struct {
|
||||
ID string `json:"id"`
|
||||
Type string `json:"type"`
|
||||
Created int64 `json:"created"`
|
||||
Data struct{ Object json.RawMessage `json:"object"` } `json:"data"`
|
||||
}
|
||||
|
||||
// job.go: route() for created/updated
|
||||
fresh, err := p.gateway().Retrieve(ctx, s.ID) // outside tx, before lagoon.Transaction
|
||||
...
|
||||
_, err = svc.Apply(ctx, tx, fresh.update())
|
||||
|
||||
// apply.go: ApplyDeleted
|
||||
if len(rows) == 0 {
|
||||
return nil, fmt.Errorf("golem15.paymentgatewaysubscriptions: apply deleted %s: subscription not stored yet", subscriptionID) // retried
|
||||
}
|
||||
```
|
||||
|
||||
### CR-02: Checkout can charge a Stripe price in a different currency or amount than the customer API shows and validated
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:112-119, 140-149`
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:128`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go:109-119`
|
||||
- `sm-pgstripe-plugin/subscriptions/sync_plan.go:15, 55-69`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/json.go:62-75`
|
||||
|
||||
**Issue:** `Checkout` accepts a currency when `priced(plan, currency)` and `SupportsCurrency(currency)` are both true. `priced` only looks at the local `currency_prices` and `price_cents`/`currency`. The gateway then picks the Stripe price with `StripePriceIDFor(currency)`, which silently falls back to `stripe_price_id` when `stripe_prices` has no entry for that currency.
|
||||
|
||||
`SyncPlanToGateway` creates prices only for `usd`, `eur` and `pln` and sets `stripe_price_id` to the USD price. A plan whose `currency` is any other supported code (`gbp`, `cad`, `aud`, `chf`, `jpy`; the admin form accepts any 3-letter text) therefore goes like this:
|
||||
- `/plans` lists it as, for example, `29.99 GBP`.
|
||||
- Checkout passes the 422 gate.
|
||||
- Stripe Checkout runs on the **USD** price created from the same `price_cents`: `$29.99` instead of `£29.99`.
|
||||
|
||||
The same happens when `currency_prices` names a currency the plan was never synced in, or when an admin typed a `stripe_price_id` for one currency and the customer asks for another.
|
||||
|
||||
JPY is wrong a second way. `money.FromMinor` divides by 100 for every currency, so `/plans` shows `¥5` for `price_cents: 500`, while a JPY price would bill `¥500`.
|
||||
|
||||
**Fix:** Before calling the gateway, require a gateway price that belongs to the requested currency, and answer 422 otherwise. Never fall back across currencies:
|
||||
|
||||
```go
|
||||
// api/subscription.go, after the priced/SupportsCurrency check
|
||||
if !gatewayPriced(plan, currency) {
|
||||
writeValidation(w, map[string][]string{"currency": {msgCurrencyInvalid}})
|
||||
return
|
||||
}
|
||||
|
||||
func gatewayPriced(plan *models.SubscriptionPlan, currency string) bool {
|
||||
if plan.StripePrices.Valid {
|
||||
if id, ok := plan.StripePrices.Data[strings.ToLower(currency)]; ok && id != "" {
|
||||
return true
|
||||
}
|
||||
return false // synced plan without this currency: never fall back to stripe_price_id
|
||||
}
|
||||
// unsynced plan with a hand-entered price id: only its own currency
|
||||
return plan.StripePriceID != nil && *plan.StripePriceID != "" && strings.EqualFold(plan.Currency, currency)
|
||||
}
|
||||
```
|
||||
|
||||
Also make `CreateCheckoutSession` refuse the fallback in the same way (`ErrNoPriceForCurrency`), because other callers of `Service.Checkout` hit the same path. Either drop `jpy` from `supportedCurrencies` or handle zero-decimal currencies in `planPrices`.
|
||||
|
||||
## Warnings
|
||||
|
||||
### WR-01: Plan sync calls Stripe while holding the bulk-action transaction and row locks; a rollback loses the created Stripe ids
|
||||
### WR-01: `Service.Cancel` writes `cancel_at_period_end` without the subscription lock, so a racing webhook job still fires a spurious `SubscriptionReactivated` (CR-01 sequence 3 residual)
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go:130-153, 155-178`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:362-388`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:373-395`
|
||||
- `sm-pgstripe-plugin/subscriptions/job.go:219-243`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:143`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/lock.go:25-28`
|
||||
|
||||
**Issue:** cabana's `BulkAction` opens a transaction, locks the selected plans with `FOR UPDATE` and runs `runSyncSelected` inside it (`summercms.go/modules/cabana/crud.go:369-388`). `syncOne` then makes one product call and up to three price list, create and archive calls per plan, all while the transaction and the row locks are held. This contradicts the `contracts.Gateway` doc ("no transaction is held across a provider call").
|
||||
**Issue:** The lock's documented rule is "every writer of the gateway state of that subscription takes it", and plan 07 made that the established pattern. `Service.Cancel` (used by `POST /subscription/cancel`) calls Stripe and then runs `UPDATE ... SET cancel_at_period_end = true` without `SubscriptionLockKey`. One interleaving reproduces the original sequence 3:
|
||||
1. A webhook job for an unrelated `customer.subscription.updated` takes the lock and retrieves the subscription from Stripe (`cancel_at_period_end: false`).
|
||||
2. The user cancels. Stripe sets the flag, and the local UPDATE commits `true`.
|
||||
3. The job opens its transaction. `Apply` reads the row `FOR UPDATE` with `previousCancel = true`, writes `false` and fires `SubscriptionReactivated` (`apply.go:143`).
|
||||
|
||||
Stripe can create a product and its prices and then the transaction can still roll back:
|
||||
- the admin's request is cancelled or times out (the context is shared);
|
||||
- a lock wait or statement times out;
|
||||
- the commit fails.
|
||||
|
||||
In each case the saved `stripe_product_id`/`stripe_prices` are lost, and the next sync creates a duplicate product. `runSyncAll` does the same per plan inside `lagoon.Transaction`, discards the transaction error (`_ = lagoon.Transaction(...)`, line 173), and has already counted the plan as `synced`.
|
||||
|
||||
**Fix:**
|
||||
1. Make the bulk action collect only the plan ids.
|
||||
2. Run each plan's gateway call outside any transaction, then save the ids in a short transaction of its own, the way `ensureCustomer` already does.
|
||||
3. In `runSyncAll`, check the returned error and count a failed commit as a failure:
|
||||
Stripe's own update event later restores `true`, but the listeners have already received a reactivation the user never asked for. Until then the row shows no scheduled cancellation. The window is the time between the job's Stripe response and its `SELECT ... FOR UPDATE`. It is short, but nothing closes it.
|
||||
|
||||
**Fix:** Take the subscription lock around the gateway call and the local write:
|
||||
```go
|
||||
if err := lagoon.Transaction(ctx, db, func(ctx context.Context, tx *gorm.DB) error { ... }); err != nil {
|
||||
tally.failures = append(tally.failures, plan.Code+": save failed")
|
||||
func (s *Service) Cancel(ctx context.Context, sub *models.Subscription, atPeriodEnd bool) error {
|
||||
...
|
||||
if sub.StripeSubscriptionID == nil || *sub.StripeSubscriptionID == "" { /* existing gateway refusal */ }
|
||||
return WithSessionLock(ctx, s.db, SubscriptionLockKey(*sub.StripeSubscriptionID), func(conn *gorm.DB) error {
|
||||
if err := gw.CancelSubscription(ctx, sub, atPeriodEnd); err != nil { ... }
|
||||
if !atPeriodEnd { return nil }
|
||||
return conn.WithContext(ctx).Model(&models.Subscription{}).Where("id = ?", sub.ID).
|
||||
Update("cancel_at_period_end", true).Error
|
||||
})
|
||||
}
|
||||
```
|
||||
Add a `TestEventOrdering` case that blocks the fake's retrieve, runs `Cancel` and then releases the retrieve.
|
||||
|
||||
### WR-02: Concurrent or retried checkouts create duplicate Stripe customers, and the last write wins
|
||||
### WR-02: With fresh-state retrieval, `SubscriptionCanceled` fires twice when a created/updated job runs after Stripe canceled the subscription but before its deletion job
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:392-411`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/user_subscribable.go:89`
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:76-108`
|
||||
- `sm-pgstripe-plugin/subscriptions/job.go:221-241`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:161-165, 208-211`
|
||||
- `sm-pgstripe-plugin/subscriptions/ordering_test.go:115-152`, `fresh_state_smoke_test.go:130-170`
|
||||
|
||||
**Issue:** `ensureCustomer` checks `sub.CustomerID()` on a subscriber loaded before the call, creates a Stripe customer without an idempotency key, and then runs `UPDATE users SET stripe_customer_id = ? WHERE id = ?` unconditionally. Two parallel checkout requests (a double click in two tabs, or a client retry after a timeout) each create a customer, and the second UPDATE overwrites the first. If the user completes the session tied to the overwritten customer:
|
||||
- the subscription lives under a customer the app no longer knows;
|
||||
- `POST /subscription/portal` opens the other customer's empty portal;
|
||||
- the user cannot manage or cancel that subscription from the site.
|
||||
**Issue:** A created or updated job now applies Stripe's current status. If Stripe already canceled the subscription, that status is `canceled`. `fireStateChangeEvents` sees a change to canceled and fires `SubscriptionCanceled`. The deletion job then runs `ApplyDeleted`, which fires `SubscriptionCanceled` unconditionally (`apply.go:208-211`, the PHP rule). Two sequences lead there:
|
||||
- **CR-01 sequence 2, the case plan 07 targeted:** the deletion is retried with `ErrSubscriptionNotStored`, the late created event stores `canceled` and fires once, and the retried deletion fires again.
|
||||
- **An immediate cancel while an update job is queued, retrying, or simply wins the lock first.**
|
||||
|
||||
**Fix:** Write the id only when none is stored, and re-read on conflict. Also pass an idempotency key derived from alias and id to `V1Customers.Create`:
|
||||
Under PHP's snapshot semantics a created or updated snapshot practically never carried `canceled`, so the unconditional fire was harmless. The fresh path changes that. Both ordering tests assert `activated == 0` but never count `canceled`, so the double fire passes the gate. Listeners such as a "your subscription ended" mail or an access revocation that writes an audit row run twice.
|
||||
|
||||
```go
|
||||
res := tx.Exec(`UPDATE users SET stripe_customer_id = ? WHERE id = ? AND (stripe_customer_id IS NULL OR stripe_customer_id = '')`, id, u.id)
|
||||
if res.RowsAffected == 0 { /* re-read the stored id and use it; optionally delete the orphan customer */ }
|
||||
```
|
||||
**Fix:** In `ApplyDeleted`, fire only on a transition (`previous := sub.StripeStatus` before the write; fire when `previous != models.StatusCanceled`). Record it as a deviation next to the D-21 amendment. Then add `rec.Count(events.SubscriptionCanceledEventName) == 1` to both `deleted_before_created` subtests.
|
||||
|
||||
```go
|
||||
params.SetIdempotencyKey("pgsub-customer-" + s.MorphAlias() + "-" + strconv.FormatUint(uint64(s.Key()), 10))
|
||||
```
|
||||
|
||||
### WR-03: Checkout's "Already subscribed" guard misses non-active live subscriptions and is racy, so users can be billed twice
|
||||
### WR-03: The detached plan sync lost the row lock that serialised concurrent syncs of one plan; parallel syncs create duplicate Stripe products and prices
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:87-93, 198-205`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go:48-57, 86-101, 162-176`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:584-611`
|
||||
- `summercms.go/modules/cabana/crud.go:412-435`
|
||||
- `sm-pgstripe-plugin/subscriptions/sync_plan.go:92-117` (called)
|
||||
|
||||
**Issue:** Checkout refuses with 409 only when `Active()` (active or trialing) finds a row. A user whose subscription is `past_due` (still in grace and still billed by Stripe's retries), `unpaid`, `incomplete` or `paused` can start a second Checkout. A completed second Checkout gives two live Stripe subscriptions on one customer, both charging.
|
||||
**Issue:** Before plan 08, `sync_to_stripe` ran inside cabana's transaction with the selected plans locked `FOR UPDATE`, so two admins, or two tabs, syncing the same plan were serialised. The detached branch loads the plans without locks (`loadScoped(..., false)`), and `SyncPlan` takes no lock of its own. Two concurrent syncs of a plan with no `stripe_product_id` each call `V1Products.Create`, which carries no idempotency key. Each then lists the prices by lookup key, finds none, and creates its own. The last `Updates` wins, and the other product and its prices stay active and orphaned in Stripe. This is the duplicate-product outcome WR-01 set out to prevent.
|
||||
|
||||
The check is also not atomic. Two parallel requests both pass it and both get a session URL; the layer's busy guard protects only one tab. `POST /subscription/cancel` then cancels only the newest active row, leaving the other one billing.
|
||||
|
||||
**Fix:**
|
||||
- Refuse checkout while any subscription of the subscriber is in a non-terminal status (anything except `canceled` and `incomplete_expired`). Send `past_due`/`unpaid` users to the portal instead.
|
||||
- Serialize per subscriber, either with `SELECT ... FOR UPDATE` on the users row or `pg_advisory_xact_lock(hashtext('pgsub:user:'||id))` around the check, or with a Stripe idempotency key per user and plan for a short window.
|
||||
|
||||
### WR-04: A subscription whose price was archived by a later sync (or whose plan code changed) is never stored, although the customer is billed
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:72-85, 300-323`
|
||||
- `sm-pgstripe-plugin/subscriptions/sync_plan.go:141-151`
|
||||
|
||||
**Issue:** `planForPrice` resolves a plan only from the current `stripe_price_id` or the current `stripe_prices` values. Re-syncing a repriced plan archives the old price and replaces the ids in `stripe_prices`. A Checkout session created before the resync (valid for 24 h) and completed after it produces a subscription on the old price. Its `customer.subscription.created`/`updated` events then fail with "no plan holds price id", are retried for conga's default attempts and are discarded. The webhook already answered 200, so Stripe never redelivers. The customer is charged and the app has no subscription row. Changing a plan's `code` (editable in the form) has the same effect for new checkouts on the old lookup key.
|
||||
|
||||
**Fix:** Fall back to the `plan_code` metadata the gateway writes on every Checkout subscription (`gateway.go:153`). Carry it in `SubscriptionArgs`, add `PlanCode` to `contracts.Update`, and in `Apply` use `ActivePlanByCode` (or any plan by code) when `planForPrice` returns nil:
|
||||
The toolbar `sync_all_to_stripe` never locked. It has no confirm dialog (`subscriptionplans.go:59-63`), so a double click starts two full syncs.
|
||||
|
||||
**Fix:** Serialise per plan with the existing helper, and sync from a fresh copy of the row:
|
||||
```go
|
||||
if plan == nil && u.PlanCode != "" {
|
||||
plan, err = planByCode(db, u.PlanCode)
|
||||
func (s *Service) SyncPlan(ctx context.Context, tx *gorm.DB, plan *models.SubscriptionPlan) (map[string]string, error) {
|
||||
...
|
||||
if tx == nil {
|
||||
var out map[string]string
|
||||
err := WithSessionLock(ctx, s.db, lockKeyPrefix+"plan:"+strconv.FormatUint(uint64(plan.ID), 10), func(conn *gorm.DB) error {
|
||||
if err := conn.WithContext(ctx).First(plan, plan.ID).Error; err != nil { return err } // fresh stripe ids
|
||||
var err error
|
||||
out, err = s.syncPlanLocked(ctx, conn, plan)
|
||||
return err
|
||||
})
|
||||
return out, err
|
||||
}
|
||||
...
|
||||
}
|
||||
```
|
||||
Also give `V1Products.Create` an idempotency key over the plan id, code and name, in the same window as the customer key.
|
||||
|
||||
### WR-05: The grace period is computed from the already advanced period end, so a past_due subscription keeps access for about a full unpaid period plus grace_period_days
|
||||
### WR-04: The open-subscription guard cannot see unpaid Checkout Sessions, so a subscriber can still complete two Checkouts and be billed twice (WR-03 partially resolved)
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/models/subscription.go:137-142`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:319-344`
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:245-246`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/checkout_guard_test.go` ("two checkouts of one subscriber never overlap and share one customer")
|
||||
|
||||
**Issue:** `InGracePeriod` grants access while `now < current_period_end + graceDays`. When Stripe renews, it moves `current_period_start`/`current_period_end` to the new period before the renewal invoice is paid. A `past_due` subscription therefore already carries the end of the period that was not paid for. With monthly billing and the default 7 days, `has_access` stays true for about 37 days after the failed payment, not 7. PHP had the same formula (inherited), but `has_access` is now exposed through the new customer API and `Service.HasAccess`.
|
||||
**Issue:** `CheckoutGuarded` refuses only when a stored row is open. A Checkout Session creates no row until the customer pays and the webhook is applied. The subscriber lock serialises the two requests but does not make the second one fail: the test named above asserts that both checkouts succeed. The Stripe idempotency key returns the same session only when every parameter matches within the same 10-minute window. So these still produce two open sessions for one customer:
|
||||
- two tabs with different plans or currencies;
|
||||
- the same plan more than 10 minutes apart (sessions stay valid for 24 h);
|
||||
- a second click in the gap between payment and webhook processing.
|
||||
|
||||
**Fix:** Base grace on the start of the unpaid period (or on the timestamp of the move to `past_due`):
|
||||
Completing both produces two live Stripe subscriptions, both billing. That is the outcome WR-03 described.
|
||||
|
||||
**Fix:** Before creating a new session for a customer, expire that customer's open subscription-mode sessions inside the subscriber lock (Stripe: list Checkout Sessions with `customer` and `status=open`, then `Expire`). Optionally, when `Apply` stores a new subscription for a subscriber who already has another open row, log at Error or fire a dedicated event so the duplicate is caught.
|
||||
|
||||
### WR-05: Once synced, plans priced in gbp, cad, aud or chf can no longer be bought, although `/plans` lists them and `SupportsCurrency` accepts them
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go:129-144`
|
||||
- `sm-pgstripe-plugin/subscriptions/sync_plan.go:15, 55-69` (called)
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:44`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:121-131`
|
||||
|
||||
**Issue:** `CheckoutPriceID` lets a non-empty `stripe_prices` map decide alone. Plan sync fills that map only for usd, eur and pln, and it overwrites `stripe_price_id` with the USD price. A plan with `currency: GBP` therefore:
|
||||
1. sells correctly while it carries a hand-entered GBP `stripe_price_id`;
|
||||
2. after an admin presses "Sync to Stripe" (reported as successful), loses its GBP price id and gets a map without `gbp`;
|
||||
3. answers 422 "unavailable" to every checkout from then on, while `/plans` still shows `29.99 GBP` with a Subscribe button.
|
||||
|
||||
The same happens to any `currency_prices` key outside the three sync currencies. This is no longer a wrong charge, but the strict map combined with the unchanged fixed sync list makes a supported currency unsellable without any visible error.
|
||||
|
||||
**Fix:** Sync every currency the plan prices that the gateway supports, instead of the fixed list:
|
||||
```go
|
||||
return now.Before(s.CurrentPeriodStart.AddDate(0, 0, graceDays))
|
||||
currencies := []string{strings.ToLower(plan.Currency)}
|
||||
for code := range plan.CurrencyPrices.Data { currencies = append(currencies, strings.ToLower(code)) }
|
||||
// dedupe, keep only g.SupportsCurrency(code), then upsertPrice for each
|
||||
```
|
||||
At least report the currencies the plan lists but has no price for as a sync failure line. Alternatively, have `/plans` mark a price unbuyable when `CheckoutPriceID` is empty.
|
||||
|
||||
If PHP parity must be kept, document the behaviour in the README and the API contract.
|
||||
|
||||
### WR-06: SyncSubscription saves a stale, unlocked row in full and never refreshes the plan
|
||||
### WR-06: Checkout bills the amount of the last sync, not the price the API shows; an edit of `price_cents` or `currency_prices` without a re-sync charges the old amount (CR-02 amount residual)
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:335-352`
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:263-306`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/models/subscription_plan.go:129-144`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/json.go:113-126`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:121-131`
|
||||
|
||||
**Issue:** The caller hands in a row loaded earlier without a lock. The gateway changes some fields in memory, and the service then runs `Save(sub)`, which writes every column. Changes that `Apply` made between the load and the save are overwritten with the stale values: `subscription_plan_id` after a plan switch, `subscriber_*`, `stripe_customer_id` and `canceled_at`. The gateway also ignores the retrieved price, so a plan change made in the portal is never reconciled by a sync.
|
||||
**Issue:** `/plans` renders `currency_prices`/`price_cents` from the row (`planPrices`). The checkout gate (`priced`) checks the same local amounts. The Stripe price comes from the `stripe_prices` id, whose `unit_amount` was fixed at the last sync. Nothing ties the two together: there is no save hook on the plan form, and `CheckoutPriceID` never compares amounts. An admin who raises a plan from 29.99 to 39.99 and forgets to press Sync leaves customers who are shown 39.99 being billed 29.99, and the reverse after a price cut. CR-02 was "charge a different currency or amount than the user was shown". The currency half is fixed; the amount half is not.
|
||||
|
||||
**Fix:**
|
||||
1. Re-load the row `FOR UPDATE` inside the short transaction.
|
||||
2. Copy only the gateway-owned fields onto it.
|
||||
3. Map the retrieved price to a plan with `planForPrice`.
|
||||
**Fix:** One of:
|
||||
1. Store the synced amount per currency next to the id (for example `stripe_prices` values as `{id, amount}`, or a parallel `stripe_price_amounts` map). Have `CheckoutPriceID` return "" when `PriceCentsFor(currency)` differs, which gives a 422 and an admin log line naming the plan.
|
||||
2. In the gateway, `V1Prices.Retrieve(priceID)` before the session and refuse with `ErrNoPriceForCurrency` when `UnitAmount != plan.PriceCentsFor(currency)`, `Currency` differs or `Active` is false.
|
||||
3. Clear `stripe_prices`/`stripe_price_id` in a `FormBeforeUpdate` when a price field changes.
|
||||
|
||||
Or `Select(...)` only the columns the gateway sets.
|
||||
|
||||
### WR-07: After a redirect the subscribe and manage buttons stay disabled for good, including after the browser Back button
|
||||
### WR-07: Treating `incomplete` as open locks a user out of checkout for up to about 23 hours after an abandoned or failed first payment
|
||||
|
||||
**Files:**
|
||||
- `vue-paymentgateway-layer/app/utils/subscribe.ts:74, 85, 115, 125, 134`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/models/subscription.go:114-121`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:92-98`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:325-332`
|
||||
|
||||
**Issue:** `state.redirecting` is set to true before `location.assign` and never reset. When the user goes to Stripe Checkout or the portal and presses Back, browsers with a back/forward cache restore the page with its JavaScript state. The button then shows "Subscribing..." and stays disabled, and `cancel()` returns null without a request. The only way out is a full reload. Leaving Checkout with Back is a common path.
|
||||
**Issue:** `ScopeOpen` excludes only `canceled` and `incomplete_expired`. A subscription whose first invoice failed, or which needs 3-D Secure the customer abandoned, is stored as `incomplete` from the `customer.subscription.created` event. Stripe expires it after about 23 h. Until then every checkout answers 409, and the layer offers only the billing portal (`SubscribeButton.vue:56`). The portal is not a reliable way to finish the first payment of an incomplete subscription, so a customer with a declined card cannot retry with another card the same day. The other statuses that block (`past_due`, `unpaid`, `paused`) have an invoice the portal can settle; `incomplete` is the exception.
|
||||
|
||||
**Fix:** Reset the state when the page is shown again:
|
||||
|
||||
```ts
|
||||
if (typeof window !== 'undefined') {
|
||||
window.addEventListener('pageshow', (e) => { if ((e as PageTransitionEvent).persisted) { state.redirecting = false; state.busy = false } })
|
||||
}
|
||||
```
|
||||
|
||||
Register the listener in the components' `onMounted` and remove it in `onBeforeUnmount`.
|
||||
**Fix:** Leave `incomplete` out of the checkout guard. Stripe expires it, and an incomplete subscription never charges again on its own. If double billing through a later-completed incomplete subscription is a concern, have the guarded checkout cancel the subscriber's `incomplete` subscriptions at Stripe, inside the lock, before it creates the new session.
|
||||
|
||||
## Info
|
||||
|
||||
### IN-01: The per-user throttle key of the customer API never applies
|
||||
|
||||
**File:** `sm-paymentgatewaysubscriptions-plugin/routes.go:54-60, 77-82`
|
||||
|
||||
**Issue:** `throttle:paymentgatewaysubscriptions-api` is group middleware, and it runs before `pgapi.RequireJWT`. Nothing sets the bouncer principal until the jwt guard runs (`bouncer/jwt.go:63`), so `bouncer.User` is always empty when the bucket key is computed. Every request is keyed by client IP, and the documented "120 per minute per frontend user" is not what happens. The order API has the same pattern.
|
||||
|
||||
**Fix:** Run the guard before the throttle on the four Bearer routes. Alternatively, drop the per-user branch and correct the comment and README.
|
||||
|
||||
### IN-02: The `metadata_app` filter lets untagged events from other applications through
|
||||
|
||||
**File:** `sm-pgstripe-plugin/subscriptions/job.go:136`
|
||||
|
||||
**Issue:** `want != "" && got != "" && got != want` skips only events tagged with a different app. On a shared Stripe account, an application that writes no tag (including the PHP apps before cutover) gets every subscription event applied. Each fails on "no plan holds price id" and is retried until it is discarded, which floods the error logs.
|
||||
|
||||
**Fix:** When `metadata_app` is set, skip (and mark) untagged events too, or make that behaviour a config option.
|
||||
|
||||
### IN-03: The admin sync result shows the raw Stripe error text
|
||||
|
||||
**File:** `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go:86-92`
|
||||
|
||||
**Issue:** `cause.Error()` (the Stripe message) is put into the toast. Elsewhere the gateway deliberately logs only Stripe's type, code and request id, because "the message may echo request data".
|
||||
|
||||
**Fix:** Map `*stripe.Error` to a localized line naming the code, or reuse the `failed()` attributes.
|
||||
|
||||
### IN-04: The plans table decides "free" differently from the server
|
||||
|
||||
**File:** `vue-paymentgateway-layer/app/components/Pg/Plans.vue:35-38, 78-81`
|
||||
|
||||
**Issue:** The server treats a plan as free when `price_cents == 0` (checkout answers 422, `api/subscription.go:105-108`). The layer treats it as free when every listed price is zero. A plan with `price_cents: 0` and non-zero `currency_prices` shows a Subscribe button that always fails with "unavailable". A plan with mixed zero and non-zero prices shows `0.00` with a Subscribe button.
|
||||
|
||||
**Fix:** Expose a `free` boolean in `PlanJSON`, or apply the same rule on both sides.
|
||||
|
||||
### IN-05: TaglistField leaves a typed separator in the input when the draft is empty
|
||||
|
||||
**File:** `summercms.go/admin/src/components/form/fields/TaglistField.vue:86-99`
|
||||
|
||||
**Issue:** Typing `,` (or a space in space mode) into an empty input leaves `draft` at `''`, which is unchanged, so Vue does not patch the input. The separator stays visible until the next keystroke or blur.
|
||||
|
||||
**Fix:** Write the remaining text back to the element (`(event.target as HTMLInputElement).value = rest`) after splitting.
|
||||
|
||||
### IN-06: Taglist required message is not localized; `customTags: false` without options is accepted at boot
|
||||
|
||||
**File:** `summercms.go/modules/cabana/field_taglist.go:300, 314, 252-263`
|
||||
|
||||
**Issue:** The required error is a hard-coded English string, while the other taglist errors use phrase keys. A field with `customTags: false` and no `options` compiles and then refuses every tag at save time.
|
||||
|
||||
**Fix:** Use the localized required message. Fail boot when `customTags: false` has no options.
|
||||
|
||||
### IN-07: Test-mode and live-mode Stripe customer ids share `users.stripe_customer_id`
|
||||
### IN-01: The `ErrSubscriptionNotStored` retry lasts only conga's 3 attempts; a deletion of a subscription that is never stored ends as a discarded job with error logs
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/updates/202610150002_add_stripe_customer_id_to_users.go:18-21`
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:80-82`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/apply.go:191-195`
|
||||
- `summercms.go/modules/conga/client.go:18-19` (called)
|
||||
|
||||
**Issue:** A customer created while `test_mode` is on is stored in the same column and reused after switching to live. Live checkout and portal calls then fail with "No such customer" and the user gets a permanent 502. Nothing recovers from it.
|
||||
**Issue:** River retries the deletion 3 times with backoff, roughly 20 s in total. CR-01 sequence 2 is actually closed by the fresh retrieval of the late created event, not by this retry. A subscription that never gets a row (pre-cutover PHP subscriptions, or the untagged events of another application, IN-02 of the first review) now produces three Error lines and a discarded River job per deletion, where it used to leave a marker.
|
||||
|
||||
**Fix:** When Stripe answers `resource_missing` for the stored customer, clear it and create a new one. Alternatively, store the customer per mode.
|
||||
**Fix:** On the last attempt (`river.JobFromContext(ctx).Attempt >= MaxAttempts`), claim the marker and log at Warn instead of failing. Alternatively, document that the discarded jobs are expected.
|
||||
|
||||
### IN-08: Repository hygiene of the new plugin module
|
||||
### IN-02: The session lock holds a pooled connection while it waits, and its holder needs a second one; with a capped pool or a re-entrant listener this stalls until the job timeout
|
||||
|
||||
**Files:**
|
||||
- `sm-paymentgatewaysubscriptions-plugin/go.work:9`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/go.mod:5-7, 132-134`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/lock.go:63-88`
|
||||
- `sm-pgstripe-plugin/subscriptions/gateway.go:114-128`
|
||||
- `sm-paymentgatewaysubscriptions-plugin/classes/service.go:325-339`
|
||||
|
||||
**Issue:**
|
||||
- The committed `go.work` replaces `sm-user-plugin` with `../sm-summercmsio-app/...`, which names a consuming application from a shared core plugin repository.
|
||||
- `go.mod` requires `sm-paymentgateway-plugin v0.0.0`, which resolves only through the module's own `replace` directives. Consumers ignore those, so every consumer must add its own replace.
|
||||
- The tagged `summercms v0.1.26` requirement is shadowed by `replace ../summercms.go` in every local build.
|
||||
**Issue:** Each waiter keeps a pinned connection blocked in `pg_advisory_lock`. The holder needs another connection: `LoadSettings` in `client()`, and in `CheckoutGuarded` the `Open`, `Subscriber` and `ensureCustomer` calls on `s.db`.
|
||||
- **Capped pool.** With `MaxOpenConns` at or below the number of concurrent jobs for one subscription, the holder starves until River's job timeout. Each timeout uses one of only 3 attempts, so an event can be discarded.
|
||||
- **Re-entrant listener.** An event listener that calls `Service.SyncSubscription` for the same subscription waits on a lock its own job holds. That also lasts until the context deadline.
|
||||
|
||||
**Fix:** Point the workspace at a neutral path or leave `go.work` untracked. Require tagged versions of the sibling plugins once they exist.
|
||||
lagoon sets no cap today, so the default deployment is unaffected (plan 07 noted this).
|
||||
|
||||
**Fix:** Document the pool requirement (at least 2 x workers). Consider `SET lock_timeout` on the pinned connection before the lock call, or `pg_try_advisory_lock` with a River snooze. Note in the events doc that listeners must not call `SyncSubscription` or `Cancel` for the subscription they are handling.
|
||||
|
||||
### IN-03: A plan changed between the handler's price check and the gateway call answers 502 instead of 422
|
||||
|
||||
**File:** `sm-paymentgatewaysubscriptions-plugin/api/subscription.go:128-131, 141-148`
|
||||
|
||||
**Issue:** `ErrNoPriceForCurrency` from the gateway (for example, the plan was re-synced without that currency in between) goes through `writeGatewayError` and becomes 502 "provider". It is logged without the error.
|
||||
|
||||
**Fix:** Map `errors.Is(err, ErrNoPriceForCurrency)` to the same 422 `currency` validation answer. That needs a gateway-neutral sentinel in `contracts`, because pgsub must not import pgstripe.
|
||||
|
||||
### IN-04: The component wiring spec asserts source text, not behaviour
|
||||
|
||||
**File:** `vue-paymentgateway-layer/tests/subscriptionComponents.test.ts:423-440`
|
||||
|
||||
**Issue:** The test "the three components wire watchPageRestore on mount and remove it on unmount" matches regexes against the `.vue` sources. A refactor that keeps the strings but breaks the wiring, for example by registering on a different target, still passes. A correct rewrite, such as a composable `usePageRestore(flow)`, fails.
|
||||
|
||||
**Fix:** Mount each component with `@vue/test-utils` (or the Nuxt test utils) and dispatch a `pageshow` with `persisted: true` on `window`. Assert that the button is enabled again and that `removeEventListener` runs on unmount.
|
||||
|
||||
### IN-05: `/plans` still shows yen amounts divided by 100
|
||||
|
||||
**File:** `sm-paymentgatewaysubscriptions-plugin/api/json.go:122, 126`
|
||||
|
||||
**Issue:** Dropping `jpy` from `supportedCurrencies` stops yen checkouts, but a plan whose `currency` is JPY, or whose `currency_prices` include `jpy`, is still listed through `money.FromMinor` as one hundredth of its price (¥5 for 500). This was the second half of CR-02's yen note.
|
||||
|
||||
**Fix:** Leave out of `planPrices` any currency the stripe gateway does not support, or refuse `jpy` in the plan form's validation.
|
||||
|
||||
---
|
||||
|
||||
_Reviewed: 2026-10-14T19:22:09Z_
|
||||
_Reviewed: 2026-10-16T12:04:37Z_
|
||||
_Reviewer: Claude (msd-code-reviewer)_
|
||||
_Depth: standard_
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
---
|
||||
phase: "18.1"
|
||||
slug: "payment-gateway-subscriptions"
|
||||
status: verified
|
||||
# threats_open = count of OPEN threats at or above workflow.security_block_on severity (the blocking gate)
|
||||
threats_open: 0
|
||||
asvs_level: 1
|
||||
created: "2026-10-16"
|
||||
---
|
||||
|
||||
# Phase 18.1 — Security
|
||||
|
||||
> Per-phase security contract: threat register, accepted risks, and audit trail.
|
||||
|
||||
Scope: plans 18.1-01..09 plus quick tasks 261016-myi and 261016-nqk. Code in summercms.go (cabana, pact, admin), sm-paymentgatewaysubscriptions-plugin, sm-pgstripe-plugin (root and subscriptions), vue-paymentgateway-layer.
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|---------------|
|
||||
| Stripe → webhook | Signed events at /webhook/stripe/subscriptions, verified per mode | subscription and invoice state, ids (no PII kept in job args) |
|
||||
| App → Stripe API | Customer, checkout, portal, subscription, product and price calls | e-mail and name on customer create; secret key (encrypted at rest) |
|
||||
| Browser (layer) → customer API | JWT Bearer calls for the logged-in user's own subscription | plan code, currency; checkout/portal URLs back |
|
||||
| Admin SPA → cabana | Plans and subscriptions screens behind manage_plans / manage_subscriptions | plan data, subscriber e-mail/name (read-only) |
|
||||
| Webhook job → database | Claim-first transaction under a per-subscription session lock | subscription rows, processed-event markers |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation | Status |
|
||||
|-----------|----------|-----------|----------|-------------|------------|--------|
|
||||
| T-18.1-01 | Tampering | taglist save | medium | mitigate | cabana field_taglist.go type/allow-list/required checks (422) on all save paths; TestTaglistFieldSmoke | closed |
|
||||
| T-18.1-02 | Tampering (XSS) | TaglistField.vue, PreviewField | high | mitigate | no v-html; TaglistField.test.ts renders script/onerror as text | closed |
|
||||
| T-18.1-03 | Tampering | flat reorder POST | medium | mitigate | reorder.go parent/duplicate 422, stale set 409, sort column only; TestReorderFlatSmoke | closed |
|
||||
| T-18.1-04 | EoP | reorder and taglist routes | low | accept | reorderPost under s.protect; taglist rides existing saves (AR-18.1-01) | closed |
|
||||
| T-18.1-05 | Tampering | Apply with unknown subscriber_type | high | mitigate | apply.go errors, stores nothing (ErrUnknownSubscriberType); core smoke unknown_alias | closed |
|
||||
| T-18.1-06 | Repudiation/Tampering | duplicate or concurrent deliveries | high | mitigate | ClaimEvent ON CONFLICT in tx, unique event_id; job_test two concurrent jobs fire once | closed |
|
||||
| T-18.1-07 | Info Disclosure | Apply/ApplyInvoice/Checkout logs | medium | mitigate | PII-free contracts.Invoice; no e-mail in slog calls; provider text never logged | closed |
|
||||
| T-18.1-08 | Tampering | mass assignment | medium | mitigate | Fillable + Rules; subscription saves refused; TestAdminSubscriptionsSmoke/read_only | closed |
|
||||
| T-18.1-09 | Tampering | users.stripe_customer_id writes | medium | mitigate | targeted single-column UPDATE; ResolveMany read-only; TestUserSubscribable | closed |
|
||||
| T-18.1-10 | Spoofing | webhook handler | high | mitigate | body cap, ValidatePayload (hmac.Equal) before decode; wrong_secret/missing_header tests | closed |
|
||||
| T-18.1-11 | Tampering | replay | high | mitigate | 300 s tolerance; Already processed check; claim-first job; duplicate_event test | closed |
|
||||
| T-18.1-12 | Tampering/Repudiation | concurrent duplicate jobs | high | mitigate | claim inside lagoon.Transaction; job_test.go:187 | closed |
|
||||
| T-18.1-13 | Info Disclosure | river_job args and logs | medium | mitigate | minimal args struct; TestJobArgsMinimal key and PII leak lists | closed |
|
||||
| T-18.1-14 | Info Disclosure | subscription signing secrets | high | mitigate | lagoon.Encrypted, json:"-", password fields; only mode logged; settings/secrets tests | closed |
|
||||
| T-18.1-15 | Spoofing | wrong-mode secret | medium | mitigate | secret by test_mode only, no payment-secret fallback; live_secret_in_test_mode | closed |
|
||||
| T-18.1-16 | DoS | webhook floods, large bodies | medium | mitigate | 60/min/IP shared bucket, 1 MiB cap, cheap check, handled() filter; tests | closed |
|
||||
| T-18.1-17 | Tampering | cross-application events | medium | mitigate | app tag on objects; foreignApp skip keeps marker; app_tag test | closed |
|
||||
| T-18.1-18 | Tampering | import-winter target column | medium | mitigate | webhookSecretTarget validated before writes; maskSecret; target_* tests | closed |
|
||||
| T-18.1-19 | EoP | missing secret accepting events | high | mitigate | empty secret answers 500 before signature check; missing_secret test | closed |
|
||||
| T-18.1-20 | Spoofing | checkout and portal return URLs | high | mitigate | api/urls.go config-only URLs, refuses // javascript: userinfo; TestRedirectURLs | closed |
|
||||
| T-18.1-21 | EoP | IDOR on subscription routes | high | mitigate | subscriber from JWT on every handler; new cancelIncomplete/expire scoped ForSubscriber; cross_user_isolation, TestCustomerAPIAuthz | closed |
|
||||
| T-18.1-22 | Tampering | admin plan sync abuse | medium | mitigate | manage_plans enforced (403 tests); toolbar action run not audit-logged with admin id (AR-18.1-03) | closed |
|
||||
| T-18.1-23 | Info Disclosure | subscriber name/e-mail (preview, list, API) | medium | mitigate | manage_subscriptions via protect on list/show/partials incl. ListExtendRecords/FormRecordTitle; Subscriber json:"-"; authz tests | closed |
|
||||
| T-18.1-24 | DoS | customer API floods | medium | mitigate | 120/min bucket; cheap 409/404/422 before gateway (residual: per-IP key, no 429 test) | closed |
|
||||
| T-18.1-25 | Tampering | bulk sync rollback losing ids | high | mitigate | syncOne never errors; ids saved before gateway error; bulk_sync_partial_failure | closed |
|
||||
| T-18.1-26 | Spoofing (CSRF) | customer API | medium | mitigate | RequireJWT Bearer only, no cookies; bearer_required | closed |
|
||||
| T-18.1-27 | Info Disclosure | unknown subscriber_type on preview | low | mitigate | danger notice + Error log; alias #id fallback; unknown_alias_is_visible | closed |
|
||||
| T-18.1-28 | Spoofing | layer open redirect | high | mitigate | isNavigableURL; only checkout_url/portal_url navigated; client sends no URL | closed |
|
||||
| T-18.1-29 | Info Disclosure | layer token handling | medium | mitigate | token from app hook as Bearer, nothing stored; base URL from runtime config | closed |
|
||||
| T-18.1-30 | Tampering (XSS) | PgPlans, PgMySubscription | high | mitigate | no v-html in subscription components; subscriptionComponents.test.ts | closed |
|
||||
| T-18.1-31 | Tampering | double submit | low | mitigate | busy/redirecting guard; subscribe.test.ts | closed |
|
||||
| T-18.1-32 | Repudiation | gate false green | high | mitigate | check-phase18.1.sh detectors, --self-test, PHASE181_REQUIRE_POSTGRES; gate run exit 0 | closed |
|
||||
| T-18.1-33 | Info Disclosure | tests leaking secrets or calling Stripe | medium | mitigate | BackendOverride fakes; no api.stripe.com or real keys in tests | closed |
|
||||
| T-18.1-34 | Tampering | publishing red or unreviewed code | medium | mitigate | tags only after green gate; post-gate fixes unpushed pending go-ahead | closed |
|
||||
| T-18.1-35 | Tampering | stale snapshot overwrite (CR-01) | high | mitigate | fresh RetrieveSubscription under session lock; Cancel under lock; TestFreshStateSmoke, TestCancelTakesSubscriptionLock | closed |
|
||||
| T-18.1-36 | EoP | delete-before-create | high | mitigate | ErrSubscriptionNotStored rolls back marker; deleted_before_created (canceled once) | closed |
|
||||
| T-18.1-37 | Repudiation | unresolved plan after reprice | medium | mitigate | price, product, code resolution; loud failure; plan_resolution_test.go | closed |
|
||||
| T-18.1-38 | Tampering | SyncSubscription lost update | medium | mitigate | lock + FOR UPDATE re-read + gateway columns only; TestSyncSubscriptionFresh | closed |
|
||||
| T-18.1-39 | DoS | leaked lock or slow retrieve | low | mitigate | unlock via WithoutCancel, conn discarded on failure (residual: IN-02 pool starvation) | closed |
|
||||
| T-18.1-40 | Info Disclosure | product_id and plan_code in job args | low | accept | ids and a code only, PII refused by TestJobArgsMinimal (AR-18.1-02) | closed |
|
||||
| T-18.1-41 | Tampering | cross-currency billing (CR-02) | high | mitigate | CheckoutPriceID no fallback, 422; gateway refuses; checkPrice amount check; TestCheckoutPriceUnavailable | closed |
|
||||
| T-18.1-42 | Tampering | duplicate Stripe customers | medium | mitigate | idempotency key; first write wins; TestCustomerIDFirstWriteWins | closed |
|
||||
| T-18.1-43 | Tampering | second billing subscription (WR-03) | high | mitigate | 409 CheckoutBlocker; CheckoutGuarded under SubscriberLockKey; cancelIncomplete + ExpireOpenCheckoutSessions (residual AR-18.1-04) | closed |
|
||||
| T-18.1-44 | DoS | plan sync holding locks across Stripe calls | medium | mitigate | Detached bulk action; per-plan PlanLockKey; TestAdminSyncDetached, TestSyncPlanSerialised | closed |
|
||||
| T-18.1-45 | EoP | detached bulk bypassing scope/permissions | medium | mitigate | protect + allowAction before BulkAction; loadScoped; TestDetachedBulkAction | closed |
|
||||
| T-18.1-46 | DoS | idempotent replay of failures | low | mitigate | 10-min window + parameter hash; keys never logged; customer/product creates only | closed |
|
||||
| T-18.1-47 | DoS | bfcache-stuck buttons | low | mitigate | watchPageRestore wired in three components; UAT row 4 | closed |
|
||||
| T-18.1-48 | Repudiation | gate false green on gap-closure tests | high | mitigate | plan 07-09 test names required by gate (residual: quick-task names not pinned) | closed |
|
||||
| T-18.1-49 | Info Disclosure | tests calling Stripe or leaking secrets | medium | mitigate | same fakes in review_fixes_test.go | closed |
|
||||
| T-18.1-50 | Tampering | publishing order, immovable tags | medium | mitigate | requires point at existing tags; unreleased HEAD not published | closed |
|
||||
| T-18.1-SC | Tampering | npm/pip/cargo installs (all plans) | high | mitigate | no new third-party modules or lockfile changes in 18.1 or the quick tasks | closed |
|
||||
|
||||
*Status: open · closed · open — below high threshold (non-blocking)*
|
||||
*Severity: critical > high > medium > low — only open threats at or above workflow.security_block_on count toward threats_open*
|
||||
*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)*
|
||||
|
||||
Post-plan attack surface from the quick tasks, mapped to existing entries: `pact.ListExtendRecords` and `pact.FormRecordTitle` (T-18.1-23, gated by protect); `contracts.CheckoutSessionExpirer`, `ExpireOpenCheckoutSessions`, `cancelIncomplete` (T-18.1-21, T-18.1-43); `checkPrice` and `contracts.ErrPriceUnavailable` (T-18.1-41); `contracts.SubscriberBatchResolver.ResolveMany` (T-18.1-09, T-18.1-23).
|
||||
|
||||
---
|
||||
|
||||
## Accepted Risks Log
|
||||
|
||||
| Risk ID | Threat Ref | Rationale | Accepted By | Date |
|
||||
|---------|------------|-----------|-------------|------|
|
||||
| AR-18.1-01 | T-18.1-04 | Reorder POST runs under the controller's protect; taglist adds no route and rides the existing saves | plan 18.1-01 (verified by audit) | 2026-10-16 |
|
||||
| AR-18.1-02 | T-18.1-40 | product_id and plan_code are Stripe ids and a plan code, not PII; TestJobArgsMinimal pins the key list and refuses PII | plan 18.1-07 (verified by audit) | 2026-10-16 |
|
||||
| AR-18.1-03 | T-18.1-22 | Toolbar admin actions (sync_all_to_stripe) are not audit-logged with the admin id on success; bulk actions are. The action needs manage_plans and pgsub logs each synced plan. Follow-up todo: admin action audit log (plugin or cabana extension) | user | 2026-10-16 |
|
||||
| AR-18.1-04 | T-18.1-43 | A Checkout Session already paid but whose webhook is not yet processed is complete, not open, so it is not expired; a second checkout in that window can bill twice. Detected by warnDuplicateOpen (Error log). Replaces the old 10-minute idempotency key, which covered only identical parameters | user (UAT row 5, quick 261016-myi) | 2026-10-16 |
|
||||
| AR-18.1-05 | T-18.1-39 | Session advisory locks hold a pooled connection; a capped MaxOpenConns or a re-entrant listener could starve the pool (review IN-02, open) | audit residual | 2026-10-16 |
|
||||
| AR-18.1-06 | T-18.1-24 | The customer API throttle keys on client IP in practice (JWT check runs inside the handler); no 429 test for this bucket | audit residual | 2026-10-16 |
|
||||
| AR-18.1-07 | T-18.1-48 | check-phase18.1.sh does not pin the quick-task test names, so their removal would not fail the gate | audit residual | 2026-10-16 |
|
||||
|
||||
*Accepted risks do not resurface in future audit runs.*
|
||||
|
||||
---
|
||||
|
||||
## Security Audit Trail
|
||||
|
||||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||||
|------------|---------------|--------|------|--------|
|
||||
| 2026-10-16 | 51 | 50 (+1 accepted: T-18.1-22) | 0 | msd-security-auditor (ASVS 1, block_on high) |
|
||||
|
||||
---
|
||||
|
||||
## Sign-Off
|
||||
|
||||
- [x] All threats have a disposition (mitigate / accept / transfer)
|
||||
- [x] Accepted risks documented in Accepted Risks Log
|
||||
- [x] `threats_open: 0` confirmed
|
||||
- [x] `status: verified` set in frontmatter
|
||||
|
||||
**Approval:** verified 2026-10-16
|
||||
@@ -1,44 +1,130 @@
|
||||
---
|
||||
status: testing
|
||||
status: complete
|
||||
phase: 18.1-payment-gateway-subscriptions
|
||||
source: [18.1-VERIFICATION.md]
|
||||
started: 2026-10-14T19:30:26.848Z
|
||||
updated: 2026-10-14T19:30:26.848Z
|
||||
updated: 2026-10-16T17:00:00Z
|
||||
---
|
||||
|
||||
## Current Test
|
||||
|
||||
number: 1
|
||||
name: Decide on review findings CR-01 (out-of-order webhook snapshots) and CR-02 (checkout currency fallback to stripe_price_id) before the production cutover
|
||||
expected: |
|
||||
Either accept them as PHP parity (D-21 and the frozen getStripePriceId fallback), recorded as overrides or a backlog item, or open a decimal gap-closure phase or quick task: CR-01 by retrieving the subscription from Stripe in the job (or an event.created watermark) and retrying ApplyDeleted on an unknown row; CR-02 by refusing a currency without its own entry in stripe_prices in api/subscription.go and in CreateCheckoutSession
|
||||
awaiting: user response
|
||||
[testing complete]
|
||||
|
||||
## Tests
|
||||
|
||||
### 1. Decide on review findings CR-01 (out-of-order webhook snapshots) and CR-02 (checkout currency fallback to stripe_price_id) before the production cutover
|
||||
expected: Either accept them as PHP parity (D-21 and the frozen getStripePriceId fallback), recorded as overrides or a backlog item, or open a decimal gap-closure phase or quick task: CR-01 by retrieving the subscription from Stripe in the job (or an event.created watermark) and retrying ApplyDeleted on an unknown row; CR-02 by refusing a currency without its own entry in stripe_prices in api/subscription.go and in CreateCheckoutSession
|
||||
result: [pending]
|
||||
result: pass
|
||||
note: CR-01 fixed in plan 18.1-07 (fresh-state retrieval, deletion retry), CR-02 in 18.1-08 (currency without own price refused); both covered by tests
|
||||
|
||||
### 2. Live Stripe test-mode checkout end to end: test-mode keys and the subscription webhook secret in pgstripe settings, `stripe listen --forward-to <host>/webhook/stripe/subscriptions`, run checkout from the layer playground (PgPlans -> PgSubscribeButton) with the user subscriber
|
||||
expected: Stripe Checkout opens on the plan's price in the shown currency; after payment the webhook answers 200 'Webhook received', the job stores the subscription as active, golem15.pgsub.subscription.activated fires once, GET /subscription answers has_access true; observe whether customer.subscription.created arrives as incomplete followed by updated active (the CR-01 race window)
|
||||
result: [pending]
|
||||
result: pass
|
||||
reason: duplicate of test 6; the same check passed live as test 6
|
||||
|
||||
### 3. Billing-portal cancellation (D-21 cancel_at question): open the portal through PgManageSubscriptionButton, cancel the subscription, inspect the delivered customer.subscription.updated
|
||||
expected: If Stripe sends cancel_at_period_end: true, the row gets cancel_at_period_end true and PgMySubscription shows the will-cancel notice. If it sends only cancel_at (newer API versions), the row keeps cancel_at_period_end false and the cancellation is invisible; that would need a follow-up mapping
|
||||
result: [pending]
|
||||
result: pass
|
||||
reason: duplicate of test 7; the same check passed live as test 7
|
||||
|
||||
### 4. Browser pass through the admin screens and the layer components
|
||||
expected: Taglist chips (Enter, comma, remove, suggestions, read-only) on the plan form; flat drag reorder of plans writes sort_order; bulk sync_to_stripe and toolbar sync_all_to_stripe toasts with per-plan failures; Subscriptions list filters and the preview subscriber partial; the layer's plans table, subscribe redirect, my-subscription cancel through PgConfirm, portal redirect; press Back from Stripe Checkout and check whether the subscribe button is stuck on 'Subscribing...' (review WR-07, unfixed in app/utils/subscribe.ts)
|
||||
result: [pending]
|
||||
result: pass
|
||||
retest: "retested 2026-10-16 after quick 261016-nqk: subscriber e-mail column, title "Subscription #1 (Pro)", Gateway/Status shown; all pass"
|
||||
first_result: issue
|
||||
reported: "Taglist pass; reorder pass; sync actions pass; playground subscribe, Back from Checkout and 409 pass. Subscriptions list: shows subscriber ID, expected some sort of field: type: relation select: email so we could easily identify subscriber. Otherwise works. Cancel through PgConfirm not run (blocked by test 7)."
|
||||
severity: minor
|
||||
note: "Part E re-run 2026-10-16 after un-cancel in the portal: E: Pass, everything works. Shows it will end 16.11 and Stripe portal says it will cancel at 16.11. Stripe reports cancel_at_period_end=true plus cancel_at for the API cancel; only the portal cancel sends cancel_at alone (G-18.1-7)."
|
||||
|
||||
### 5. Disposition of the re-review warnings (18.1-REVIEW.md, 2026-10-16; 18.1-REVIEW-DISPOSITION.md lists all open) before the Horoskopia cutover
|
||||
expected: Each of WR-01 to WR-07 is accepted (backlog item or deferred-items entry) or scheduled as a quick task or decimal phase. The money- and listener-relevant ones: WR-02 SubscriptionCanceled fires twice in the deleted-before-created sequence (ordering_test.go never counts canceled); WR-01 Service.Cancel race; WR-03 concurrent plan syncs can create duplicate Stripe products (row lock lost with Detached); WR-04 unpaid open Checkout Sessions not refused; WR-06 checkout bills the last-synced amount, not the shown one; WR-05 gbp/cad/aud/chf plans become unbuyable after a sync; WR-07 incomplete blocks checkout for about 23 h
|
||||
result: pass
|
||||
note: all seven fixed in quick task 261016-myi (pgsub 2c9e5ef, pgstripe 2e44508); WR-02 now fires canceled once (deviation from PHP)
|
||||
|
||||
### 6. Live Stripe test-mode checkout end to end: test-mode keys and the subscription webhook secret in pgstripe settings, `stripe listen --forward-to <host>/webhook/stripe/subscriptions`, checkout from the layer playground (PgPlans -> PgSubscribeButton) with the user subscriber
|
||||
expected: Checkout opens on the plan's price in the shown currency; after payment the webhook answers 200 'Webhook received'; the job logs source=stripe (fresh retrieval, not snapshot), stores the subscription active; golem15.pgsub.subscription.activated fires once even when created(incomplete) and updated(active) arrive together; GET /subscription answers has_access true. A second Subscribe answers 409
|
||||
result: pass
|
||||
note: source=stripe logged, activated fired once, second subscribe refused
|
||||
|
||||
### 7. Billing-portal cancellation (D-21 cancel_at question): open the portal through PgManageSubscriptionButton, cancel the subscription, inspect the delivered customer.subscription.updated and the retrieved subscription
|
||||
expected: If Stripe reports cancel_at_period_end true, the row gets it and PgMySubscription shows the will-cancel notice. If it reports only cancel_at, the row keeps cancel_at_period_end false and the cancellation is invisible; that needs a follow-up mapping
|
||||
result: pass
|
||||
retest: "retested 2026-10-16 after quick 261016-nqk: portal un-cancel then cancel shows will end 16.11; pass"
|
||||
first_result: issue
|
||||
reported: "Fail. Subscription canceled on Stripe, playground still showing 'Days until renewal 30'"
|
||||
severity: major
|
||||
|
||||
### 8. pgsub boots in an application built with `summer build`
|
||||
expected: A summer-built app with pgsub and pgstripe runs migrate, serve and admin:create
|
||||
result: pass
|
||||
retest: "demo app rebuilt without the boot overlay after quick 261016-myi boots, migrates and serves; UAT 4, 6, 7 ran on it"
|
||||
first_result: issue
|
||||
reported: "Found while setting up the UAT demo app: every command fails with 'party: boot golem15.paymentgatewaysubscriptions: ... boot service: database is not published' (boot.go bootService looks up *gorm.DB during Boot; generated main.go activates plugins before the database is published)"
|
||||
severity: blocker
|
||||
|
||||
## Summary
|
||||
|
||||
total: 4
|
||||
passed: 0
|
||||
total: 8
|
||||
passed: 8
|
||||
issues: 0
|
||||
pending: 4
|
||||
pending: 0
|
||||
skipped: 0
|
||||
blocked: 0
|
||||
|
||||
## Gaps
|
||||
|
||||
- gap_id: G-18.1-8
|
||||
truth: "A summer-built app with pgsub boots: migrate, serve and admin:create run"
|
||||
status: resolved
|
||||
resolved_by: quick 261016-myi (pgsub ab46d6f, TestBootBeforeDatabase)
|
||||
resolved_at: 2026-10-16
|
||||
reason: "Demo app setup: boot service: database is not published"
|
||||
severity: blocker
|
||||
test: 8
|
||||
root_cause: "sm-paymentgatewaysubscriptions-plugin boot.go bootService requires *gorm.DB at Boot; internal/build main.go runs party.Activate before commands open the database; test hosts publish the DB before activation so tests miss it"
|
||||
artifacts:
|
||||
- path: "../sm-paymentgatewaysubscriptions-plugin/boot.go"
|
||||
issue: "app.Lookup[*gorm.DB] at Boot"
|
||||
missing:
|
||||
- "Build the service in lagoon.OnDatabase; test with plugins activated before the DB is published"
|
||||
debug_session: ""
|
||||
|
||||
- gap_id: G-18.1-4
|
||||
truth: "The admin subscriptions list and form identify the subscriber by a readable value (e-mail / display name), not only type and id"
|
||||
status: resolved
|
||||
resolved_by: quick 261016-nqk (user retest pass 2026-10-16)
|
||||
resolved_at: 2026-10-16
|
||||
reason: "User reported: Shows subscriber ID, expected some sort of field: type: relation select: email so we could easily identify subscriber"
|
||||
severity: minor
|
||||
test: 4
|
||||
root_cause: "models/subscription/columns.yaml and fields.yaml render subscriber_type and subscriber_id only; PHP's subscriber_display_name accessor was deliberately dropped. The subscriber registry (contracts.SubscriberResolver) can already resolve display data (used by the _subscription_subscriber header partial)"
|
||||
artifacts:
|
||||
- path: "../sm-paymentgatewaysubscriptions-plugin/models/subscription/columns.yaml"
|
||||
issue: "no subscriber display column"
|
||||
- path: "../sm-paymentgatewaysubscriptions-plugin/models/subscription/fields.yaml"
|
||||
issue: "subscriber shown as type + id"
|
||||
missing:
|
||||
- "A subscriber column (e-mail / display name resolved through the subscriber registry, batched for the list page) on the list, and the same value on the form; searchable by e-mail if feasible"
|
||||
debug_session: ""
|
||||
|
||||
- gap_id: G-18.1-7
|
||||
truth: "A cancellation in the Stripe billing portal shows as will-cancel (cancel_at_period_end true) on the row and in PgMySubscription"
|
||||
status: resolved
|
||||
resolved_by: quick 261016-nqk (user retest pass 2026-10-16)
|
||||
resolved_at: 2026-10-16
|
||||
reason: "User reported: Subscription canceled on Stripe, playground still showing 'Days until renewal 30'"
|
||||
severity: major
|
||||
test: 7
|
||||
root_cause: "On API 2026-09-30.endive the portal cancel returns cancel_at_period_end=false with cancel_at=<period end> and canceled_at set (verified on sub_1URCbD4Ba6xZslcbFdpiwvHX). pgstripe maps only CancelAtPeriodEnd (subscriptions/decode.go:165, job.go:295, gateway.go:422), so the row keeps false and the layer shows a renewing subscription; canceled_at is stored on an active row"
|
||||
artifacts:
|
||||
- path: "../sm-pgstripe-plugin/subscriptions/decode.go"
|
||||
issue: "cancel_at not decoded/mapped"
|
||||
- path: "../sm-pgstripe-plugin/subscriptions/job.go"
|
||||
issue: "fresh-state mapping ignores cancel_at"
|
||||
- path: "../sm-pgstripe-plugin/subscriptions/gateway.go"
|
||||
issue: "RetrieveSubscription mapping ignores cancel_at"
|
||||
missing:
|
||||
- "Map a pending cancellation: cancel_at_period_end = s.CancelAtPeriodEnd || (s.CancelAt set while active/trialing), in snapshot and fresh paths; un-cancel (cancel_at cleared) maps back to false so reactivated fires"
|
||||
- "Tests for both payload shapes (cancel_at_period_end and cancel_at-only) and for un-cancel; amend D-21 note"
|
||||
- "Re-run UAT 7 and the site-side cancel (test 4 part E) in the demo"
|
||||
debug_session: ""
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
---
|
||||
phase: 18.1-payment-gateway-subscriptions
|
||||
verified: 2026-10-14T19:35:00Z
|
||||
verified: 2026-10-16T15:45:06Z
|
||||
status: human_needed
|
||||
score: 5/5 roadmap success criteria verified (plan must-haves 41/41 verified; 0 present-behavior-unverified)
|
||||
score: 5/5 roadmap success criteria verified (plan must-haves 65/65 verified, 4 of them as amended by user-accepted fixes)
|
||||
covered_files:
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-01-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-01-SUMMARY.md
|
||||
@@ -16,43 +16,77 @@ covered_files:
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-05-SUMMARY.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-06-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-06-SUMMARY.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-07-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-07-SUMMARY.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-08-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-08-SUMMARY.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-09-PLAN.md
|
||||
- .planning/phases/18.1-payment-gateway-subscriptions/18.1-09-SUMMARY.md
|
||||
- .planning/quick/261016-myi-fix-18-1-review-warnings/261016-myi-PLAN.md
|
||||
- .planning/quick/261016-myi-fix-18-1-review-warnings/261016-myi-SUMMARY.md
|
||||
- .planning/quick/261016-nqk-close-uat-gaps-g-18-1-7-and-g-18-1-4/261016-nqk-PLAN.md
|
||||
- .planning/quick/261016-nqk-close-uat-gaps-g-18-1-7-and-g-18-1-4/261016-nqk-SUMMARY.md
|
||||
- admin/src/components/form/fields/TaglistField.vue
|
||||
- admin/src/views/FormView.vue
|
||||
- admin/src/views/PreviewView.vue
|
||||
- admin/src/views/ReorderView.vue
|
||||
- modules/cabana/bulk_detached_test.go
|
||||
- modules/cabana/crud.go
|
||||
- modules/cabana/field_taglist.go
|
||||
- modules/cabana/form_schema.go
|
||||
- modules/cabana/list_schema.go
|
||||
- modules/cabana/phase181_records_test.go
|
||||
- modules/cabana/query.go
|
||||
- modules/cabana/relation_field.go
|
||||
- modules/cabana/reorder.go
|
||||
- modules/pact/capabilities.go
|
||||
- scripts/check-phase10.sh
|
||||
- scripts/check-phase18.1.sh
|
||||
covered_digest: "v2:sha256:3f6d8db93c3ed3d1a76a29d5111f67aa4333965ceacec7e0571c0c30035b0e70"
|
||||
covered_digest: "v2:sha256:4d5014a3f2a0a899ec2e3ad3896b5355c8fd446b212d578575869422cd4560e8"
|
||||
behavior_unverified: 0
|
||||
overrides_applied: 0
|
||||
coincidental_reliance_items:
|
||||
- truth: "SC-4: duplicate-event idempotency"
|
||||
reason: incidental-ordering
|
||||
harden: "TestHostSubscriptionFlow/duplicate_event proves two deliveries of one event fire once, but only for the same event id. Two different events about one subscription (created incomplete and updated active, or deleted before created) are applied in whatever order River runs them (default 4 workers per queue). D-21 accepts PHP's 'apply as they arrive'; if that stops being acceptable, declare an ordering rule (event.created watermark per subscription, or re-retrieve from Stripe in the job). See CR-01."
|
||||
human_verification:
|
||||
- test: "Decide on review findings CR-01 (out-of-order webhook snapshots) and CR-02 (checkout currency fallback to stripe_price_id) before the production cutover"
|
||||
expected: "Either accept them as PHP parity (D-21 and the frozen getStripePriceId fallback), recorded as overrides or a backlog item, or open a decimal gap-closure phase or quick task: CR-01 by retrieving the subscription from Stripe in the job (or an event.created watermark) and retrying ApplyDeleted on an unknown row; CR-02 by refusing a currency without its own entry in stripe_prices in api/subscription.go and in CreateCheckoutSession"
|
||||
why_human: "Both are faithful ports of PHP behaviour (verified against pgstripe jobs/ProcessStripeSubscriptionWebhook.php and SubscriptionPlan::getStripePriceId) and D-21 explicitly chose PHP's out-of-order semantics, so they do not fail the 'works as it does on PHP' goal. They are still money-relevant and easier to reach in Go (conga runs 4 workers per queue by default; the new customer API validates currency against local prices only). Accepting or fixing them is a product decision."
|
||||
- test: "Live Stripe test-mode checkout end to end: test-mode keys and the subscription webhook secret in pgstripe settings, `stripe listen --forward-to <host>/webhook/stripe/subscriptions`, run checkout from the layer playground (PgPlans -> PgSubscribeButton) with the user subscriber"
|
||||
expected: "Stripe Checkout opens on the plan's price in the shown currency; after payment the webhook answers 200 'Webhook received', the job stores the subscription as active, golem15.pgsub.subscription.activated fires once, GET /subscription answers has_access true; observe whether customer.subscription.created arrives as incomplete followed by updated active (the CR-01 race window)"
|
||||
why_human: "The Stripe fake cannot prove Stripe's hosted Checkout or real delivery order"
|
||||
- test: "Billing-portal cancellation (D-21 cancel_at question): open the portal through PgManageSubscriptionButton, cancel the subscription, inspect the delivered customer.subscription.updated"
|
||||
expected: "If Stripe sends cancel_at_period_end: true, the row gets cancel_at_period_end true and PgMySubscription shows the will-cancel notice. If it sends only cancel_at (newer API versions), the row keeps cancel_at_period_end false and the cancellation is invisible; that would need a follow-up mapping"
|
||||
why_human: "Depends on the Stripe account's API version and portal configuration"
|
||||
re_verification:
|
||||
previous_status: human_needed
|
||||
previous_score: "5/5 roadmap SCs; plan must-haves 64/65 verified, 1 present-behavior-unverified (WR-07 component wiring)"
|
||||
gaps_closed:
|
||||
- "Previous human item 1 (disposition of re-review WR-01..WR-07): UAT row 5 passed; all seven fixed in quick 261016-myi (pgsub 2c9e5ef, pgstripe 2e44508), each with a named test that ran green here"
|
||||
- "Previous human item 2 (live Stripe checkout): UAT row 6 passed (source=stripe logged, activated once, second subscribe 409)"
|
||||
- "Previous human item 3 (portal cancel_at question): UAT row 7 first failed (G-18.1-7), fixed in pgstripe a5b3314, retest passed"
|
||||
- "Previous human item 4 (browser pass, incl. WR-07 Back button): UAT row 4 passed after the G-18.1-4 fix (pgsub 6a3f2c4 d5579cd, framework bc5685e 013890c 17398fe)"
|
||||
- "WR-07 component wiring (previously PRESENT_BEHAVIOR_UNVERIFIED): Back from Checkout observed in a real browser in UAT row 4; layer code unchanged since (v0.2.1, 22368c5)"
|
||||
- "CR-01 sequence 3 coincidental reliance (Service.Cancel without the subscription lock): Cancel now runs under SubscriptionLockKey; TestCancelTakesSubscriptionLock passed here"
|
||||
- "G-18.1-8 (pgsub could not boot in a summer-built app): bootService goes through lagoon.OnDatabase; TestBootBeforeDatabase passed here; UAT row 8 passed"
|
||||
gaps_remaining: []
|
||||
regressions: []
|
||||
human_verified:
|
||||
- test: "Disposition of the re-review warnings (18.1-REVIEW.md, 2026-10-16; 18.1-REVIEW-DISPOSITION.md lists all open) before the Horoskopia cutover"
|
||||
uat_row: 5
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "Live Stripe test-mode checkout end to end: test-mode keys and the subscription webhook secret in pgstripe settings, `stripe listen --forward-to <host>/webhook/stripe/subscriptions`, checkout from the layer playground (PgPlans -> PgSubscribeButton) with the user subscriber"
|
||||
uat_row: 6
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "Billing-portal cancellation (D-21 cancel_at question): open the portal through PgManageSubscriptionButton, cancel the subscription, inspect the delivered customer.subscription.updated and the retrieved subscription"
|
||||
uat_row: 7
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "Browser pass through the admin screens and the layer components"
|
||||
expected: "Taglist chips (Enter, comma, remove, suggestions, read-only) on the plan form; flat drag reorder of plans writes sort_order; bulk sync_to_stripe and toolbar sync_all_to_stripe toasts with per-plan failures; Subscriptions list filters and the preview subscriber partial; the layer's plans table, subscribe redirect, my-subscription cancel through PgConfirm, portal redirect; press Back from Stripe Checkout and check whether the subscribe button is stuck on 'Subscribing...' (review WR-07, unfixed in app/utils/subscribe.ts)"
|
||||
why_human: "vitest and SSR specs do not drive a real browser or the back/forward cache"
|
||||
uat_row: 4
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
- test: "pgsub boots in an application built with `summer build`"
|
||||
uat_row: 8
|
||||
recorded_at: "2026-10-16T15:34:36.000Z"
|
||||
human_verification:
|
||||
- test: "Release go-ahead for the post-gate fixes: framework tag, plugin require bumps and tags, pushes"
|
||||
expected: "The user approves (or defers with a recorded reason) the release order from the quick-task summaries: push summercms.go master and tag v0.2.2 (carries pact.ListExtendRecords, pact.FormRecordTitle and meta.title); bump sm-paymentgatewaysubscriptions-plugin's require to v0.2.2, push main, tag (v0.1.1); bump sm-pgstripe-plugin's requires to v0.2.2 and the new pgsub tag, push main, tag (v0.1.1). After that a consumer pinned to the tags gets the G-18.1-8 boot fix, WR-01..WR-07 and G-18.1-7"
|
||||
why_human: "Project rule: nothing is pushed or tagged without the user's go-ahead. Today the released pgsub v0.1.0 lacks the G-18.1-8 fix, so an app built with `summer build` on the tagged plugin fails every command with 'database is not published' (the UAT 8 blocker). pgsub HEAD calls pact.ListExtendRecords and pact.FormRecordTitle, which no framework tag has; its go.mod requires summercms v0.2.1 and builds only through its replace ../summercms.go, which a consuming module ignores. Unpushed: framework 13 commits (3 code), pgsub 4, pgstripe 2"
|
||||
---
|
||||
|
||||
# Phase 18.1: Payment Gateway Subscriptions Verification Report
|
||||
|
||||
**Phase goal:** The generic core of `Golem15.PaymentGatewaySubscriptions` and pgstripe's subscription gateway and webhook run on SummerCMS. Horoskopia's subscription flow (plans, checkout, Stripe webhook, `golem15.pgsub.*` events, cancel and portal) works as it does on PHP.
|
||||
**Mode:** mvp (the ROADMAP goal is not in user-story form; verified against the goal text and its five success criteria)
|
||||
**Verified:** 2026-10-14T19:35:00Z
|
||||
**Status:** human_needed
|
||||
**Re-verification:** No, initial verification
|
||||
**Phase goal:** The generic core of `Golem15.PaymentGatewaySubscriptions` and pgstripe's subscription gateway and webhook run on SummerCMS. Horoskopia's subscription flow (plans, checkout, Stripe webhook, `golem15.pgsub.*` events, cancel and portal) works as it does on PHP. Exceptions are the user's recorded amendments: event ordering (D-21 amendment, 2026-10-14), the portal `cancel_at` mapping (D-21 amendment, 2026-10-16), and the re-review fixes the user accepted in UAT row 5 (`canceled` fires once on a deletion, `incomplete` no longer blocks checkout).
|
||||
**Mode:** mvp. The ROADMAP goal is not in user-story form, so this report checks the goal text and its five success criteria, as the earlier verifications did.
|
||||
**Verified:** 2026-10-16T15:45:06Z
|
||||
**Status:** human_needed (one item: the release go-ahead)
|
||||
**Re-verification:** Yes. The previous report (2026-10-16T14:45Z) had no `gaps:`. It went stale (covered_changed) after two quick tasks: 261016-myi (WR-01 to WR-07 and G-18.1-8) and 261016-nqk (G-18.1-7 and G-18.1-4). This run verifies both in full and regression-checks plans 01 to 09 through the full gate.
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
@@ -60,164 +94,168 @@ human_verification:
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|----------|
|
||||
| 1 | SC-1: Subscription and SubscriptionPlan port on a squashed, idempotent v2.4.0 baseline that adopts existing PHP databases, with polymorphic subscribers registered at build time and a built-in `user` subscribable | VERIFIED | `sm-paymentgatewaysubscriptions-plugin/updates/202610150001_create_golem15_pgsub_baseline.go`: three tables, every statement `IF NOT EXISTS`, PHP index and constraint names, eight-value status CHECK, RESTRICT FK; dormant tables absent. `202610150002_add_stripe_customer_id_to_users.go` is `ADD COLUMN IF NOT EXISTS` with a no-op rollback. `boot.go` bootRegistries walks `party.Activated` and collects `HasSubscribables` and `HasSubscriptionGateways`, failing on duplicates. `classes/user_subscribable.go` resolves soft-deleted users (Unscoped) and writes only `stripe_customer_id`. TestBaselineSchema, TestRegistry and TestUserSubscribable passed in this run's gate. |
|
||||
| 2 | SC-2: the `SubscriptionGateway` contract (including `syncPlanToGateway`) has a Stripe implementation; `POST /webhook/stripe/subscriptions` and its legacy alias verify the event, enqueue a job, process it idempotently and fire the six `golem15.pgsub.*` events | VERIFIED | `contracts/gateway.go` declares the 8-method Gateway including SyncPlanToGateway. `sm-pgstripe-plugin/subscriptions/gateway.go` and `sync_plan.go` implement it (usd/eur/pln loop, transfer_lookup_key, archive). `plugin.go` routes both `WebhookPath` and `LegacyWebhookPath` to one handler behind one 60/min bucket. `webhook.go` verifies the signature on the raw body capped at 1 MiB, cheap-checks the marker and enqueues only the five handled types. `job.go` claims the marker first in one transaction (`ClaimEvent`, ON CONFLICT DO NOTHING) and routes to Apply, ApplyDeleted and ApplyInvoice. `events/events.go` holds the six PHP names. The event diff in `classes/apply.go` matches PHP `fireStateChangeEvents` line for line. TestSubscriptionWebhookSmoke, TestSubscriptionJob, TestApplyEvents, TestClaimEventConcurrent, TestSyncPlan and TestSubscriptionGateway are in the gate. |
|
||||
| 3 | SC-3: SubscriptionPlans and Subscriptions admin screens, a Go service API, a headless customer API and subscription components in `vue-paymentgateway-layer` | VERIFIED | Admin: `controllers/subscriptionplans.go` (sync_to_stripe, sync_all_to_stripe through Service.SyncPlan), `models/subscriptionplan/fields.yaml` (`features: type: taglist, mode: array`), `config_reorder.yaml` (flat), `controllers/subscriptions.go` and the subscriber partial with the unknown-alias danger callout; three permission codes verbatim; navigation order 510. Framework support (`modules/cabana/field_taglist.go`, flat reorder in `reorder.go`) is in tag v0.1.26, including fix `0f09586`. Service API: `classes/service.go` (Latest, Active, Checkout, Portal, Cancel, SyncPlan, SyncSubscription, HasAccess, InGracePeriod and more). Customer API: `routes.go` `/_paymentgatewaysubscriptions/api/v1` with public `/plans` and four `RequireJWT` routes; redirect URLs only from config. Layer: `subscriptionsClient.ts` (five paths), `useSubscriptionsApi.ts` (runtimeConfig server and public bases), PgPlans, PgSubscribeButton (createSubscribeFlow), PgMySubscription, PgManageSubscriptionButton; package version 0.2.0, tag v0.2.0 on origin. Gate: admin smokes, TestCustomerAPISmoke and TestCustomerAPIAuthz passed; the layer reported 36 `subscriptions:` TAP tests passing, 105/105 total. |
|
||||
| 4 | SC-4: a host test proves checkout, activation, `invoice.paid`, `past_due` with its grace period, cancel and duplicate-event idempotency against a Stripe fake | VERIFIED | `sm-pgstripe-plugin/subscriptions/host_test.go` TestHostSubscriptionFlow has subtests checkout_activation, invoice_paid_basil, invoice_paid_pre_basil, past_due_grace (access inside the grace period, none after it), cancel_at_period_end, cancel_immediately, reactivation, duplicate_event (two deliveries, past_due fires once, one marker, third delivery answers 'Already processed'), app_tag, plan_sync, portal, sync_subscription and real_worker. All six event names are asserted. The run is in the gate's postgres stage. |
|
||||
| 5 | SC-5: the new code has unit tests, delivered in the phase's last plan | VERIFIED | Plan 06 commits: summercms.go `323b6d6` and `3cc4a5f` (taglist and flat reorder matrices, gate with self-test); pgsub `1da437f` (13 files, 2,469 lines: models, registry, apply events, baseline, service API, admin actions, customer API authz, lang parity); pgstripe `88f9125` (decode, job, sync_plan, gateway, php_map, subscription secrets); layer `30046f5`. `scripts/check-phase18.1.sh --self-test` passed in this run. |
|
||||
| 1 | SC-1: Subscription and SubscriptionPlan on a squashed, idempotent v2.4.0 baseline; subscribers registered at build time; built-in `user` subscribable | VERIFIED | TestBaselineSchema and TestUserSubscribable passed in this run's postgres stage. G-18.1-8: `boot.go` `bootService` now builds and publishes `*classes.Service` through `lagoon.OnDatabase`, so plugins can activate before a command opens the database (generated main.go order). TestBootBeforeDatabase passed here. UAT row 8 passed on a demo app built with `summer build`. The user resolver gained an optional `ResolveMany` (`contracts.SubscriberBatchResolver`). It still writes only `stripe_customer_id`. |
|
||||
| 2 | SC-2: Stripe implementation of the gateway incl. `syncPlanToGateway`; both webhook routes verify, enqueue, process idempotently, fire the six events | VERIFIED | `subscriptions/job.go` `applySubscription` is unchanged: marker check, then foreign-app claim, then `WithSessionLock(SubscriptionLockKey)`, then retrieve outside a transaction, then claim and Apply in one transaction. G-18.1-7: every mapping path goes through `subscriptionObject.args()`: the snapshot (`decode.go:222-226`), the fresh retrieve (`gateway.go:468-475`) and `Gateway.SyncSubscription` (`gateway.go:415-422`). `pendingCancel()` (`decode.go:162-167`) maps Stripe's flag, or `cancel_at` on an active or trialing subscription, to `cancel_at_period_end`. WR-02: `ApplyDeleted` fires `canceled` only when the row was not already canceled (`apply.go:241-243`). Passed here: TestFreshStateSmoke incl. `portal_cancel_at_only_then_uncancel` (reactivated exactly once) and `deleted_before_created` (canceled == 1); TestEventOrdering incl. `deleted_before_created` (canceled == 1, `ordering_test.go:152`); TestDecodeSubscription/a_cancel_at_date…; TestHostSubscriptionFlow; TestSubscriptionWebhookSmoke; TestSubscriptionJob. UAT rows 6 and 7 passed against real Stripe test mode. |
|
||||
| 3 | SC-3: admin screens, Go service API, headless customer API, layer components | VERIFIED | Admin: `models/subscription/columns.yaml` gains a `subscriber` column, filled per page by `subscriptionsAdminController.ListExtendRecords`. Type and id stay as invisible searchable columns. The preview title comes from `FormRecordTitle`, and the label element reads its `value` property (`assets/js/fields.js`). Framework: `pact.ListExtendRecords` and `pact.FormRecordTitle` (`capabilities.go:737-763`); the SPA uses `meta.title`; dist rebuilt and matching. Service API: `Cancel` takes `SubscriptionLockKey` (`service.go:471-499`). `CheckoutGuarded` refuses on `CheckoutBlocker`, cancels incomplete subscriptions and expires open sessions inside `SubscriberLockKey` (`service.go:351-437`). `SyncPlan` with a nil tx runs under `PlanLockKey` on a re-read plan (`service.go:693-712`). Customer API: 409 on `CheckoutBlocker`; 422 on `CheckoutPriceID == ""` and on `contracts.ErrPriceUnavailable` (`api/subscription.go:93-171`). Layer: unchanged at v0.2.1; 112/112 TAP. UAT row 4 passed in a browser, covering admin and layer, including Back from Checkout and the subscriber e-mail and title. |
|
||||
| 4 | SC-4: host test proves checkout, activation, invoice.paid, past_due grace, cancel, duplicate idempotency against a Stripe fake | VERIFIED | TestHostSubscriptionFlow passed in this run (postgres stage, pgstripe `./subscriptions`, 12 required tests). |
|
||||
| 5 | SC-5: unit tests delivered in the phase's last plan | VERIFIED | Plan 09 is the last plan, and the gate requires its names by stage and package. The quick tasks are outside the phase's plan sequence. Each carries named tests in its fix commits: pgsub TestCheckoutIncompleteAndOpenSessions, TestCancelTakesSubscriptionLock, TestSyncPlanSerialised, TestCheckoutPriceUnavailable, TestBootBeforeDatabase, TestLabelElementValueProperty; pgstripe TestReviewFixesGateway (4 subtests) and the cancel_at subtests; cabana TestListUnstoredColumn, TestListExtendRecords, TestFormRecordTitle. All passed when run by name here, with 0 skips. The gate does not pin these names (see Anti-Patterns, INFO). |
|
||||
|
||||
**Score:** 5/5 roadmap truths verified (0 present, behavior-unverified). All 41 plan must-have truths across plans 01 to 06 trace to the code above or to the D-ID table below.
|
||||
**Score:** 5/5 roadmap truths verified (0 present, behavior-unverified).
|
||||
|
||||
### Plan must-haves: changes since the previous report
|
||||
|
||||
The previous report verified plans 01 to 09 (65 truths). This run re-checked all of them through the full gate (below). The rows here are the truths whose status or wording changed.
|
||||
|
||||
| Plan | Truth | Status | Evidence |
|
||||
|------|-------|--------|----------|
|
||||
| 07 | CR-01 seq 3: an older queued updated after Service.Cancel fires no reactivated | VERIFIED | Was `coincidental-reliance`. `Service.Cancel` now calls the gateway and writes the flag inside `WithSessionLock(SubscriptionLockKey)`, so a job that has already retrieved Stripe's state finishes before the cancel starts. TestCancelTakesSubscriptionLock passed: Cancel blocks while another session holds the lock and makes no gateway call until the lock is released. TestFreshStateSmoke/stale_update_after_cancel_no_reactivated and TestEventOrdering/stale_cancel_flag_no_reactivated also passed |
|
||||
| 08 | WR-07: bfcache restore re-enables the three components; listener removed on unmount | VERIFIED (human UAT, row 4) | Was PRESENT_BEHAVIOR_UNVERIFIED. UAT row 4: "playground subscribe, Back from Checkout and 409 pass". The layer's last commit (22368c5, 13:35+02:00) predates the UAT record (15:34Z) |
|
||||
| 02 | ApplyDeleted sets canceled and ended_at and fires canceled unconditionally | VERIFIED (amended) | Changed by the user-accepted fix of review WR-02 (UAT row 5 note: "WR-02 now fires canceled once (deviation from PHP)"). It now fires only on the transition into canceled (`apply.go:241`). On the paths PHP could reach, the listener count is still one. The pgsub README states the deviation (README:36) |
|
||||
| 03 | D-21: PHP's cancel_at_period_end mapping kept; prices for usd, eur and pln | VERIFIED (amended) | D-21 amendment of 2026-10-16 in 18.1-CONTEXT.md:69: `cancel_at` on active or trialing maps to the flag. COVERAGE.md row "subscription cancel_at" is INTEGRATE. Sync currencies are now usd, eur and pln plus every supported currency the plan prices (`sync_plan.go:24-41`, WR-05), a superset of the original. TestReviewFixesGateway/WR-05 passed |
|
||||
| 04 | Subscriptions list with subscriber type, subscriber id, plan, status, period end… | VERIFIED (amended) | G-18.1-4: type and id are still columns, now invisible but searchable, and the visible `subscriber` column shows the e-mail. UAT row 4 retest passed ("subscriber e-mail column, title 'Subscription #1 (Pro)', Gateway/Status shown") |
|
||||
| 08 | WR-03: 409 for any status but canceled/incomplete_expired (incl. incomplete); session idempotency key per subscriber/plan/currency | VERIFIED (amended) | Changed by the user-accepted fixes of review WR-07 and WR-04 (UAT row 5). `incomplete` no longer blocks checkout (`models/subscription.go:133-135`); the guarded checkout cancels it at the gateway first. The session idempotency key was replaced by expiring the customer's open subscription sessions inside the subscriber lock (`gateway.go` `ExpireOpenCheckoutSessions`), which is stronger: at most one payable session. TestCheckoutIncompleteAndOpenSessions, TestCheckoutGuarded and TestReviewFixesGateway/WR-04 passed |
|
||||
| 08 | WR-01 residual (lost per-plan serialisation, review WR-03) | VERIFIED | `SyncPlan` with a nil tx holds `PlanLockKey` and re-reads the plan; `V1Products.Create` carries an idempotency key. TestSyncPlanSerialised (one product from two concurrent syncs) and TestReviewFixesGateway/WR-03 passed |
|
||||
| 08 | CR-02 amount residual (review WR-06) | VERIFIED | `checkPrice` (`gateway.go`) retrieves the price before the session and refuses a missing, inactive, other-currency or other-amount price with `ErrNoPriceForCurrency`, which wraps `contracts.ErrPriceUnavailable` (`gateway.go:33`). The API answers that with 422. TestCheckoutPriceUnavailable and TestReviewFixesGateway/WR-06 passed |
|
||||
| 09 | D-22 release | VERIFIED (for plan 09's scope) | The tags plan 09 recorded are still on origin. The quick-task commits that followed are unreleased: see the human item and the Anti-Patterns WARNING |
|
||||
|
||||
The amended rows follow the precedent set when D-21 was amended for ordering. A user decision recorded in UAT or CONTEXT changes the contract, and the code is checked against the amended contract. Two amendments are recorded only in UAT row 5 and the pgsub README, not in 18.1-CONTEXT.md § Amendments: WR-02 (canceled once) and WR-07 (incomplete no longer blocks). `apply.go:205` says the deviation is "recorded next to the D-21 amendment", but CONTEXT does not record it (INFO).
|
||||
|
||||
**Plan must-have score:** 65/65 verified (4 as amended, 1 by human UAT). 0 present-behavior-unverified.
|
||||
|
||||
### Requirements Coverage (phase-local IDs; REQUIREMENTS.md maps no IDs to 18.1)
|
||||
|
||||
| ID | Plans | Status | Evidence |
|
||||
|----|-------|--------|----------|
|
||||
| SC-1 | 02, 06 | SATISFIED | Truth 1 |
|
||||
| SC-2 | 02, 03, 06 | SATISFIED | Truth 2 |
|
||||
| SC-3 | 01, 02, 04, 05, 06 | SATISFIED | Truth 3 |
|
||||
| SC-4 | 03, 06 | SATISFIED | Truth 4 |
|
||||
| SC-5 | 06 | SATISFIED | Truth 5 |
|
||||
| D-01 | 02, 03, 06 | SATISFIED | Frozen status, gateway, table and event strings (`events/events.go`, `models.Statuses`, baseline CHECK); lead fees, metering, placements and analytics not ported (no such files) |
|
||||
| D-02 | 02, 04 | SATISFIED | Three permission codes verbatim incl. `manage_placements` with no screen (`admin_permissions.go`) |
|
||||
| D-03 | 02, 06 | SATISFIED | `models/feature_list.go` list plus legacy map decode; TestFeatureList |
|
||||
| D-04 | 02, 06 | SATISFIED | Idempotent baseline; TestBaselineSchema golden names |
|
||||
| D-05 | 02, 06 | SATISFIED | `GatewayStripe`, `GatewayGooglePlay`; `ErrGatewayNotInstalled` (`contracts/registry.go`) |
|
||||
| D-06 | 02 | SATISFIED | `go list -deps ./...` of pgsub names 0 stripe-go packages (run here) |
|
||||
| D-07 | 03 | SATISFIED | `golem15.pgstripesubscriptions` is its own plugin; `go list -deps .` of the pgstripe root names 0 pgsub packages (run here) |
|
||||
| D-08 | 02, 04, 06 | SATISFIED | Build-time registries in `boot.go`; unknown alias returns ErrUnknownSubscriberType; the admin partial shows a danger notice |
|
||||
| D-09 | 02, 06 | SATISFIED | users column migration plus a targeted UPDATE only (`user_subscribable.go:89`) |
|
||||
| D-10 | 03 | SATISFIED | Two routes, one handler, one 60/min IP bucket (`subscriptions/plugin.go`) |
|
||||
| D-11 | 03, 06 | SATISFIED | Webhook never writes the marker; the job claims first; TestSubscriptionJob STRIPE-006/007 |
|
||||
| D-12 | 03, 06 | SATISFIED | Five plain-text answers in PHP order (`webhook.go`); job args are PII-free (`WebhookJobArgs`); TestJobArgsMinimal |
|
||||
| D-13 | 02, 04, 05, 06 | SATISFIED | Service API and customer API; TestServiceAPI, TestCustomerAPIAuthz |
|
||||
| D-14 | 05 | SATISFIED | Layer client, composables, four Pg components, en/pl copy, no new dependency (`git diff v0.1.0 v0.2.0 -- package.json` changes only the version; lockfile unchanged) |
|
||||
| D-15 | 03, 06 | SATISFIED | `sync_plan.go` port of d00f850; TestSyncPlan |
|
||||
| D-16 | 02, 06 | SATISFIED | Dormant tables not created; JSONB baseline confirmed at the 02 T1 checkpoint |
|
||||
| D-17 | 01, 04, 06 | SATISFIED | Core taglist and flat reorder in cabana; the plan form uses them; framework tests in the gate's go and postgres stages |
|
||||
| D-18 | 03, 06 | SATISFIED | Additive secret columns and `SubscriptionWebhookSecret` (`client.go` diff is +19 lines, additive only); `--webhook-secret-target`; TestStripeSubscriptionSecrets |
|
||||
| D-19 | 03, 06 | SATISFIED | `metadata_app` config key, empty by default (`gateway.go:148`, `job.go` filter); no billing_owner_id |
|
||||
| D-20 | 03 | SATISFIED | Subpackage plugin registered in init() |
|
||||
| D-21 | 03, 04, 05, 06 | SATISFIED (manual item open) | PHP cancel mapping kept, out-of-order applied as they arrive, both payload shapes decoded (TestDecodeSubscription, TestDecodeInvoice); GET /subscription returns the latest with has_access and in_grace_period. The portal `cancel_at` question is a human item. |
|
||||
| D-22 | 01, 02, 04, 05, 06 | SATISFIED | All four repos pushed (`main...origin/main` clean for pgsub, pgstripe and the layer); tags v0.1.26 and v0.2.0 on origin; plugin go.mod files require v0.1.26 (`472b75b`, `baf3c55`). summercms.go is one commit ahead of origin: the planning-only review report `929cfd8`. |
|
||||
| ID | Plans / tasks | Status | Evidence |
|
||||
|----|---------------|--------|----------|
|
||||
| SC-1 to SC-5 | 01-09 | SATISFIED | Truths 1-5 |
|
||||
| D-01 to D-20, D-22 | 01-09 | SATISFIED | Decision coverage gate: 24/24 honored. D-06 re-run: `go list -deps ./...` in pgsub has 0 stripe-go packages |
|
||||
| D-21 | 03, 07, quick nqk | SATISFIED (amended twice) | Ordering (2026-10-14) and `cancel_at` (2026-10-16), both in CONTEXT § Amendments |
|
||||
| CR-01, CR-02 | 07-09 | SATISFIED | Unchanged; regression-checked by the gate |
|
||||
| Re-review WR-01 to WR-07 | quick 261016-myi | SATISFIED | 18.1-REVIEW-DISPOSITION.md marks all "fixed"; code read above; named tests passed here |
|
||||
| UAT G-18.1-4, G-18.1-7, G-18.1-8 | quick nqk, quick myi | SATISFIED | 18.1-UAT.md `## Gaps`: all `resolved`, with retest passes in rows 4, 7 and 8 |
|
||||
| First-review WR-05 (grace period from the advanced period end) | none | DEFERRED by user scope | `deferred-items.md`, unchanged |
|
||||
|
||||
No orphaned requirements: REQUIREMENTS.md has no rows for Phase 18.1.
|
||||
No orphaned requirements.
|
||||
|
||||
### Required Artifacts
|
||||
### Required Artifacts (changed since the previous report)
|
||||
|
||||
| Artifact | Status | Details |
|
||||
|----------|--------|---------|
|
||||
| `summercms.go/modules/cabana/field_taglist.go` | VERIFIED | compileTaglistKeys and checkTaglistValues wired from form_schema.go and crud.go; tests in the gate |
|
||||
| `summercms.go/modules/cabana/reorder.go` | VERIFIED | flat inference and save; `list_schema.go` keeps Flat after fix 0f09586 (in v0.1.26) |
|
||||
| `summercms.go/admin/src/components/form/fields/TaglistField.vue` | VERIFIED | registered in registry.ts; vitest 1401 passed |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/{plugin,boot}.go`, `contracts/*`, `classes/*`, `events/events.go`, `updates/*` | VERIFIED | substantive, wired through Boot and published services |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/controllers/*`, `routes.go`, `api/*` | VERIFIED | admin and customer API wired; smokes in the gate |
|
||||
| `sm-pgstripe-plugin/subscriptions/{plugin,gateway,sync_plan,webhook,decode,job}.go` | VERIFIED | gateway published via SubscriptionGateways(); job registered via Jobs() |
|
||||
| `sm-pgstripe-plugin/updates/202610150001_add_subscription_webhook_secrets.go` | VERIFIED | ADD COLUMN IF NOT EXISTS |
|
||||
| `vue-paymentgateway-layer/app/{utils,composables,components/Pg}/*` (subscriptions) | VERIFIED | no v-html in the new components (the only v-html is the pre-existing BankTransfer.vue) |
|
||||
| `summercms.go/scripts/check-phase18.1.sh` | VERIFIED | PHASE181_REQUIRE_POSTGRES, self-test, required names per repo |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/service.go` | VERIFIED | Cancel under the subscription lock; CheckoutBlocker; cancelIncomplete; expireOpenSessions; SyncPlan under PlanLockKey with a re-read |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/classes/apply.go` | VERIFIED | Transition-only canceled in ApplyDeleted; warnDuplicateOpen at Error |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/boot.go` | VERIFIED | `lagoon.OnDatabase`; grace_period_days still validated at Boot |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/contracts/gateway.go`, `contracts/subscriber.go` | VERIFIED | `CheckoutSessionExpirer`, `ErrPriceUnavailable`, `SubscriberBatchResolver` (optional interfaces; `Gateway` keeps its methods) |
|
||||
| `sm-paymentgatewaysubscriptions-plugin/controllers/subscriptions.go`, `models/subscription/columns.yaml`, `assets/js/fields.js` | VERIFIED | ListExtendRecords, FormRecordTitle, subscriber column, label `value` property |
|
||||
| `sm-pgstripe-plugin/subscriptions/decode.go` | VERIFIED | `CancelAt` decoded, `pendingCancel()` used by `args()` |
|
||||
| `sm-pgstripe-plugin/subscriptions/gateway.go`, `sync_plan.go` | VERIFIED | checkPrice, ExpireOpenCheckoutSessions, product idempotency key, planCurrencies |
|
||||
| `summercms.go/modules/pact/capabilities.go`, `modules/cabana/{list_schema,query,crud,relation_field}.go`, `admin/src/views/{FormView,PreviewView}.vue` | VERIFIED | Generic hooks; unstored columns compile unsortable; `meta.title`; OpenAPI and dist checks clean |
|
||||
|
||||
### Key Link Verification
|
||||
### Key Link Verification (new links)
|
||||
|
||||
| From | To | Via | Status |
|
||||
|------|----|-----|--------|
|
||||
| pgsub `boot.go` | `contracts/registry.go` | HasSubscriptionGateways and HasSubscribables collected from party.Activated | WIRED |
|
||||
| pgstripe `subscriptions/plugin.go` | pgsub registry | `SubscriptionGateways()` returns `NewGateway(p.app)` | WIRED |
|
||||
| `subscriptions/webhook.go` | `client.go` | `pgstripe.SubscriptionWebhookSecret` then `VerifySignature` | WIRED |
|
||||
| `subscriptions/webhook.go` | `subscriptions/job.go` | `conga.From(app).Enqueue(WebhookJobArgs)` | WIRED |
|
||||
| `subscriptions/job.go` | `classes/apply.go` | ClaimEvent, Apply, ApplyDeleted, ApplyInvoice in one lagoon.Transaction | WIRED |
|
||||
| `classes/apply.go` | `events/events.go` | `s.bus.Fire` inside tx | WIRED |
|
||||
| `controllers/subscriptionplans.go` | `classes/service.go` | `svc.SyncPlan` | WIRED |
|
||||
| `routes.go` | `api/subscription.go` | `pgapi.RequireJWT(...)` | WIRED |
|
||||
| `models/subscriptionplan/fields.yaml` | cabana taglist | `type: taglist`, `mode: array` | WIRED |
|
||||
| layer `SubscribeButton.vue` | `utils/subscribe.ts` | `createSubscribeFlow` | WIRED |
|
||||
| layer `subscriptionsClient.ts` | pgsub `routes.go` | `/plans`, `/subscription`, `/subscription/checkout`, `/portal`, `/cancel` | WIRED |
|
||||
| `scripts/check-phase18.1.sh` | host_test, baseline_test, layer tests | required names TestHostSubscriptionFlow, TestBaselineSchema, `subscriptions.test.ts` | WIRED |
|
||||
| `api/subscription.go` Checkout | `classes/service.go` | `d.svc.CheckoutBlocker` (409) then `CheckoutGuarded`; `errors.Is(err, contracts.ErrPriceUnavailable)` → 422 | WIRED |
|
||||
| `classes/service.go` CheckoutGuarded | pgstripe `Gateway.ExpireOpenCheckoutSessions` | `gw.(contracts.CheckoutSessionExpirer)` | WIRED (TestReviewFixesGateway/WR-04 through the fake) |
|
||||
| pgstripe `ErrNoPriceForCurrency` | `contracts.ErrPriceUnavailable` | `fmt.Errorf("...: %w", contracts.ErrPriceUnavailable)` (`gateway.go:33`) | WIRED |
|
||||
| `decode.go` `args()` | snapshot, fresh retrieve, legacy SyncSubscription | `pendingCancel()` | WIRED (all three paths share `args()`) |
|
||||
| `controllers/subscriptions.go` | framework cabana | `var _ pact.ListExtendRecords`, `var _ pact.FormRecordTitle` | WIRED (TestAdminSubscriptionsSmoke, TestListExtendRecords, TestFormRecordTitle) |
|
||||
| `classes/user_subscribable.go` | `controllers/subscriptions.go` | `contracts.SubscriberBatchResolver.ResolveMany` | WIRED (TestUserSubscribable) |
|
||||
| `boot.go` bootService | `lagoon.OnDatabase` | callback publishes `*classes.Service` | WIRED (TestBootBeforeDatabase, UAT 8) |
|
||||
|
||||
### Data-Flow Trace (Level 4)
|
||||
|
||||
| Artifact | Data | Source | Real data | Status |
|
||||
|----------|------|--------|-----------|--------|
|
||||
| GET /plans (`api/plans.go`) | active plans | `Service.ActivePlans` GORM query, is_active ordered by sort_order | yes | FLOWING |
|
||||
| GET /subscription | latest subscription and flags | `Service.Latest` query plus HasAccess and InGracePeriod | yes | FLOWING |
|
||||
| Webhook to row | subscription state | signed event to job args to `Apply` upsert | yes | FLOWING |
|
||||
| PgPlans and PgMySubscription | plans, subscription | usePlans and useSubscription through the client over useSubscriptionsApi | yes | FLOWING |
|
||||
| Webhook job → row | subscription state incl. pending cancel | `V1Subscriptions.Retrieve` → `decodeSubscription` → `args()` → `Apply` | yes (UAT 6, 7 live) | FLOWING |
|
||||
| POST /subscription/checkout | Stripe price | `CheckoutPriceID` → `V1Prices.Retrieve` check → session create | yes | FLOWING |
|
||||
| Admin subscriptions list | subscriber e-mail | `ResolveMany` (one query per alias) → `Subscription.Subscriber` (`gorm:"-"`) | yes (UAT 4 retest, demo API showed `customer@example.test`) | FLOWING |
|
||||
| Admin preview | `meta.title` | `FormRecordTitle` → plan name query | yes | FLOWING |
|
||||
|
||||
### Behavioral Spot-Checks and Probe Execution
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| pgsub links no Stripe SDK (D-06) | `go -C ../sm-paymentgatewaysubscriptions-plugin list -deps ./... \| grep -c stripe-go` | 0 | PASS |
|
||||
| pgstripe root links no pgsub (D-07) | `go -C ../sm-pgstripe-plugin list -deps . \| grep -c paymentgatewaysubscriptions` | 0 | PASS |
|
||||
| Full phase gate, Postgres required | `PHASE181_REQUIRE_POSTGRES=1 bash scripts/check-phase18.1.sh --all` | exit 0; PASS self-test, go, plugins, admin (vitest 1401), layers (105/105 TAP, 36 subscriptions), docs, postgres (cabana 3, pgsub 14, pgstripe root 4, subscriptions 7 incl. TestHostSubscriptionFlow); "Phase 18.1 gate passed" | PASS |
|
||||
| Tags published | `git ls-remote --tags origin v0.1.26` / layer `v0.2.0` | both present; fix 0f09586 is an ancestor of v0.1.26 | PASS |
|
||||
| Layer release additive | `git -C ../vue-paymentgateway-layer diff v0.1.0 v0.2.0 --stat -- package.json pnpm-lock.yaml` | package.json version line only | PASS |
|
||||
| Full phase gate, Postgres required | `PHASE181_REQUIRE_POSTGRES=1 bash scripts/check-phase18.1.sh --all` | EXIT=0. PASS self-test, go, plugins, admin (vitest 1431 in 93 files), layers (112/112, 43 `subscriptions:`), docs (docs:build clean, OpenAPI and admin dist match), postgres (cabana 4, pgsub 21, pgstripe root 4, subscriptions 12). "Phase 18.1 gate passed" | PASS |
|
||||
| Quick-task tests, pgsub (Docker) | `go test -count=1 -v -run '^(TestCheckoutIncompleteAndOpenSessions\|TestCancelTakesSubscriptionLock\|TestSyncPlanSerialised\|TestCheckoutPriceUnavailable\|TestBootBeforeDatabase\|TestLabelElementValueProperty\|TestUserSubscribable\|TestAdminSubscriptionsSmoke\|TestApplyEvents)$' .` | 9/9 PASS, 0 SKIP | PASS |
|
||||
| Quick-task tests, pgstripe (Docker) | `go test -count=1 -v -run '^(TestReviewFixesGateway\|TestDecodeSubscription\|TestFreshStateSmoke\|TestEventOrdering\|TestJobArgsMinimal)$' ./subscriptions` | all PASS incl. portal_cancel_at_only_then_uncancel, the 4 WR subtests, deleted_before_created; 0 SKIP | PASS |
|
||||
| Framework hooks (Docker) | `go test -count=1 -v -run '^(TestListUnstoredColumn\|TestListExtendRecords\|TestFormRecordTitle)$' ./modules/cabana` | 3/3 PASS | PASS |
|
||||
| Framework | `go vet ./...`; `go test -short ./...` | both exit 0, no failures | PASS |
|
||||
| Plugins vet | `go vet ./...` in pgsub and pgstripe | clean | PASS |
|
||||
| D-06 | `go list -deps ./...` in pgsub, count stripe-go | 0 | PASS |
|
||||
| Plugin build outside the workspace | `GOWORK=off GOPROXY=off go build ./...` in pgsub | builds only because pgsub's own go.mod has `replace .../summercms => ../summercms.go`; `go list -m` resolves summercms "v0.2.1" to the local checkout | INFO (see Anti-Patterns) |
|
||||
|
||||
The orchestrator also reports `scripts/check-phase18.sh --all` and `go test ./...` (40 packages) green in this run, so Phase 18 did not regress.
|
||||
### Test Quality Audit (new tests)
|
||||
|
||||
### Code Review Findings Weighed (18.1-REVIEW.md)
|
||||
| Test | Linked item | Skipped | Assertion level | Verdict |
|
||||
|------|-------------|---------|-----------------|---------|
|
||||
| TestCancelTakesSubscriptionLock | WR-01 | no | behavioral (blocks on held lock, no gateway call before release, flag stored) | OK |
|
||||
| TestCheckoutIncompleteAndOpenSessions | WR-04, WR-07 | no | behavioral | OK |
|
||||
| TestSyncPlanSerialised | WR-03 | no | behavioral (one product from two syncs) | OK |
|
||||
| TestCheckoutPriceUnavailable, TestReviewFixesGateway/WR-06 | WR-06 | no | value | OK |
|
||||
| ordering/fresh-state `canceled == 1` | WR-02 | no | value (event counts) | OK |
|
||||
| TestFreshStateSmoke/portal_cancel_at_only_then_uncancel | G-18.1-7 | no | behavioral (no event on cancel, reactivated once on un-cancel) | OK |
|
||||
| TestBootBeforeDatabase | G-18.1-8 | no | behavioral (activation before DB publish) | OK |
|
||||
| TestLabelElementValueProperty | G-18.1-4 | skips only without `node`; ran here | value | OK |
|
||||
|
||||
**CR-01 (stale webhook snapshots).** I compared `job.go` and `classes/apply.go` with PHP `pgstripe/jobs/ProcessStripeSubscriptionWebhook.php` at the port source. PHP also:
|
||||
- applies `data.object` as a snapshot with no `event.created` check (`firstOrNew` with no lock);
|
||||
- answers a deleted-before-created with a logged return and a recorded marker;
|
||||
- fails `subscription_plan_id` validation on an unknown price, which throws and retries.
|
||||
|
||||
The Go code reproduces each of these, adds a `FOR UPDATE` lock on the existing row and makes the claim-first marker transactional. Those are strict improvements. D-21 locks "out-of-order events behave as PHP does". The goal "works as it does on PHP" is therefore met, and CR-01 is not a blocker against it.
|
||||
|
||||
The race is still real. conga runs 4 workers per queue by default (`modules/conga/client.go:23`), so concurrent created and updated jobs for one subscription are plausible on every Checkout. It is recorded as a human decision item and in `coincidental_reliance_items`.
|
||||
|
||||
**CR-02 (currency fallback).** The PHP `SubscriptionPlan::getStripePriceId` falls back to `stripe_price_id` exactly as Go `StripePriceIDFor` does. The gateway's usd/eur/pln sync is the frozen d00f850 code. Horoskopia's own checkout (`plugins/golem15/horoscope/routes.php:641`) always passes `pln`, which plan sync prices, so Horoskopia's flow is unaffected.
|
||||
|
||||
The new customer API widens the exposure. Its `priced()` check validates local prices only, so a gbp, cad, aud, chf or jpy plan, or a `currency_prices` entry that was never synced, passes the 422 gate and is billed in USD. This is a WARNING for any other consumer and a human decision item.
|
||||
|
||||
**Warnings WR-01 to WR-07.** None defeats a success criterion:
|
||||
- WR-05 (grace from the advanced period end) is identical to PHP `isInGracePeriod`.
|
||||
- WR-02, WR-03 and WR-07 are in new Go and layer code, not PHP parity. WR-07 is confirmed unfixed: `subscribe.ts` sets `redirecting` and never resets it.
|
||||
- They are listed as anti-pattern warnings below for a follow-up.
|
||||
No disabled tests, no circular expected values. The Stripe fake's responses are hand-written, and UAT rows 6 and 7 back the mapping with real Stripe bodies.
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
| File | Line | Pattern | Severity | Impact |
|
||||
|------|------|---------|----------|--------|
|
||||
| sm-pgstripe-plugin/subscriptions/job.go | 119-147 | event snapshot applied with no ordering guard (CR-01, PHP parity per D-21) | WARNING | older event can overwrite newer state; human decision |
|
||||
| sm-paymentgatewaysubscriptions-plugin/api/subscription.go | 112-119, 140-149 | local-price currency check, gateway falls back to stripe_price_id (CR-02, fallback inherited) | WARNING | non-synced currency billed at the USD price via the new API |
|
||||
| sm-paymentgatewaysubscriptions-plugin/controllers/subscriptionplans.go | 130-178 | Stripe calls inside the bulk transaction; `_ = lagoon.Transaction` (WR-01) | WARNING | lost Stripe ids on rollback, duplicate products |
|
||||
| sm-paymentgatewaysubscriptions-plugin/classes/user_subscribable.go | 89 | unconditional customer id UPDATE, no idempotency key (WR-02) | WARNING | duplicate Stripe customers under parallel checkout |
|
||||
| sm-paymentgatewaysubscriptions-plugin/api/subscription.go | 87-93 | 409 guard only on active or trialing; not atomic (WR-03) | WARNING | possible double subscription |
|
||||
| vue-paymentgateway-layer/app/utils/subscribe.ts | 85 | `redirecting` never reset (WR-07) | WARNING | button stuck after Back from Stripe (bfcache) |
|
||||
| sm-paymentgatewaysubscriptions-plugin/go.work | 9 | names `../sm-summercmsio-app` (IN-08) | INFO | dev workspace path only; same convention as sm-pgstripe-plugin's go.work since Phase 18 |
|
||||
| sm-paymentgatewaysubscriptions-plugin/go.mod | 7, 149 | requires `summercms v0.2.1`, but HEAD calls `pact.ListExtendRecords` and `pact.FormRecordTitle`, which are in no framework tag; it builds only through its own `replace ../summercms.go`, which a consuming module ignores | WARNING | pgsub HEAD cannot be consumed by tag or pseudo-version until the framework is tagged and the require bumped. This is the human release item |
|
||||
| sm-paymentgatewaysubscriptions-plugin, tag v0.1.0 | n/a | released plugin lacks G-18.1-8 | WARNING | an app built with `summer build` on pgsub v0.1.0 fails every command ("database is not published"). Fixed on main (ab46d6f), unreleased |
|
||||
| scripts/check-phase18.1.sh | required names | the quick-task tests (above) are not in the gate's required lists; 18.1-VALIDATION.md has no rows for them | INFO | the gate would not notice if they were skipped or removed. They passed here by name |
|
||||
| .planning/phases/18.1-…/18.1-CONTEXT.md | § Amendments | WR-02 (canceled once) and WR-07 (incomplete no longer blocks) deviations not recorded; `apply.go:205` claims they are "recorded next to the D-21 amendment" | INFO | decision trail sits in UAT row 5 and the README only |
|
||||
| docs/backend/forms.md | FormRecordTitle paragraph | the example title "Subscription #12 (Pro)" | INFO | generic concept, no application named; the framework rule holds |
|
||||
|
||||
No unreferenced TBD, FIXME or XXX markers and no TODO, HACK or placeholder text in the phase's non-test files across the four repositories.
|
||||
No TBD, FIXME, XXX, TODO or HACK markers in the files these quick tasks changed: pgsub classes, api, models, contracts, controllers, boot and assets; pgstripe subscriptions gateway, decode, sync_plan and job; cabana list_schema, query, crud and relation_field; pact capabilities; FormView and PreviewView.
|
||||
|
||||
IN-01 to IN-05 of the re-review stay open as info. The disposition records them. IN-03 (502 instead of 422 on a gateway price refusal) is now closed in effect: the API maps `ErrPriceUnavailable` to 422.
|
||||
|
||||
### Prohibitions (judgment tier, autonomous LLM verdict, non-authoritative)
|
||||
|
||||
| Plan | Prohibition | Verdict |
|
||||
|------|-------------|---------|
|
||||
| 01 | framework code and fixtures name no consuming application or payment key | holds: the v0.1.25..v0.1.26 diff of `modules/` and `docs/` adds no stripe, subscription or app names |
|
||||
| 01 | existing field types and tree reorders byte-identical | holds per the gate (`TestReorderPostgres`, vitest 1401, check-admin-dist clean); flagged for human review |
|
||||
| 02, 04 | pgsub imports no stripe-go | holds (`go list -deps`: 0) |
|
||||
| 02, 03, 04 | no consuming-app names in code, comments, README or fixtures | holds for code, README and fixtures; the committed `go.work` names `sm-summercmsio-app` as a local path (INFO) |
|
||||
| 02 | user subscriber writes only `stripe_customer_id` | holds (`UPDATE users SET stripe_customer_id = ? WHERE id = ?` only) |
|
||||
| 03 | root pgstripe changes additive only | holds (`client.go` +19 lines; new migration ADD COLUMN IF NOT EXISTS) |
|
||||
| 03, 06 | no test calls the network | holds per the plans' BackendOverride fake; not independently audited line by line; flagged |
|
||||
| 04 | customer API takes no subscriber, subscription or redirect from the request | holds (`customer()` uses the JWT user; URLs from config; TestCustomerAPIAuthz) |
|
||||
| 05 | layer adds no npm dependency; no v-html in the new components | holds |
|
||||
| 06 | gate never green on a skip or missing Docker | holds (self-test passed; PHASE181_REQUIRE_POSTGRES=1 used) |
|
||||
| 01, 06 | nothing pushed or tagged without the human's reply | recorded as replies in VALIDATION.md (hold, create-and-push, push-all); not independently verifiable from git; flagged |
|
||||
| Source | Prohibition | Verdict |
|
||||
|--------|-------------|---------|
|
||||
| 02, 04, 07 | pgsub imports no stripe-go (D-06) | holds (0 deps, run here) |
|
||||
| 01, 08 | Framework code and docs name no consuming application; generic additions only | holds. The bc5685e and 013890c diffs name no app or provider. One docs example uses "Subscription #12 (Pro)" as a sample title (INFO) |
|
||||
| 07 | No Stripe call while a DB transaction is open in the job or SyncSubscription | holds by reading. The new Cancel and CheckoutGuarded Stripe calls also run under session locks with no transaction open |
|
||||
| 02 | Built-in user subscribable writes no users column other than stripe_customer_id | holds. `ResolveMany` only reads (TestUserSubscribable) |
|
||||
| 03, 06, 09 | No test calls the network; test-only secrets | holds per the fake (BackendOverride); the new subtests use the same fake. Not audited line by line. Flagged |
|
||||
| 06, 09 | Gate never green on a skip or missing Docker | holds: PHASE181_REQUIRE_POSTGRES=1, 0 skip lines |
|
||||
| 09, quick tasks | Nothing pushed or tagged without the user's reply | holds: sibling repos are ahead of origin (pgsub 4, pgstripe 2), and no new tags exist. Hence the human release item |
|
||||
| 08 | Another session's uncommitted edits never staged | the working tree still holds other sessions' STATE.md and state.json edits, untouched. Flagged |
|
||||
|
||||
### Decision Coverage
|
||||
|
||||
All trackable CONTEXT.md decisions are honored by shipped artifacts (24/24).
|
||||
|
||||
### Human Verification Required
|
||||
|
||||
1. **Decision on CR-01 and CR-02.** Accept them as PHP parity (record overrides or a backlog item) or open a gap-closure phase before the Horoskopia cutover.
|
||||
2. **Live Stripe test-mode checkout.** Run it end to end and confirm activation and a single `activated` event. Watch the created and updated delivery order.
|
||||
3. **Billing-portal cancellation.** Check whether Stripe sends `cancel_at_period_end` or `cancel_at` (D-21).
|
||||
4. **Browser pass.** Cover the admin taglist, flat reorder and sync toasts, and the layer components, including Back from Stripe Checkout (WR-07).
|
||||
UAT evidence (Step 8b): `18.1-UAT.md` is `complete`, recorded at 2026-10-16T15:34:36Z. `verification.uat-evidence` reports `changed_since_uat: []` for the covered framework files. The sibling repos' last commits are also older than the record: pgsub 17:22+02, pgstripe 17:07+02, layer 13:35+02. Their working trees are clean. Rows 4, 5, 6, 7 and 8 passed on that code and settle every human item of the previous report (see `human_verified`). Rows 2 and 3 are skipped duplicates of 6 and 7. No row needs a retest.
|
||||
|
||||
One new item remains:
|
||||
|
||||
1. **Release go-ahead.** Tag the framework v0.2.2, bump and tag pgsub, then pgstripe, and push all three. Until then the tagged pgsub v0.1.0 does not boot in a `summer build` app, and pgsub main cannot be required by a consumer. This needs the user's go-ahead under the project rule. If the user defers it, record the reason (for example "released together with the Horoskopia port").
|
||||
|
||||
### Gaps Summary
|
||||
|
||||
No success criterion failed. Every roadmap SC and every phase-local ID (SC-1 to SC-5, D-01 to D-22) has implementation evidence in the four repositories, and the phase gate ran green in this verification.
|
||||
No success criterion and no plan must-have failed. The two quick tasks did what they claim:
|
||||
- **WR-01:** Cancel is serialised with the webhook job.
|
||||
- **WR-02:** `canceled` fires once.
|
||||
- **WR-03:** plan syncs are serialised, and the product create is idempotent.
|
||||
- **WR-04:** open checkout sessions are expired.
|
||||
- **WR-05:** non-sync currencies stay sellable.
|
||||
- **WR-06:** a stale price is refused with 422.
|
||||
- **WR-07:** `incomplete` no longer locks a user out.
|
||||
- **G-18.1-7:** a portal `cancel_at` shows as will-cancel.
|
||||
- **G-18.1-4:** the admin shows the subscriber's e-mail and a record title.
|
||||
- **G-18.1-8:** pgsub boots in a summer-built app.
|
||||
|
||||
The two critical review findings reproduce the frozen PHP behaviour. D-21 explicitly chose that behaviour for event ordering, and Horoskopia's own checkout currency (pln) is unaffected. They therefore do not defeat "works as it does on PHP". They are money-relevant, and the Go runtime (4 concurrent workers) and the new customer API make them easier to reach. The status is `human_needed` rather than `passed` for two reasons: the product decision on CR-01 and CR-02, and the three manual items (live Stripe checkout, the portal `cancel_at` question, the browser pass). If the decision is to fix them, the review gives concrete fixes for CR-01 (re-retrieve in the job or an `event.created` watermark, retry ApplyDeleted on an unknown row) and CR-02 (refuse cross-currency fallback in the API and the gateway).
|
||||
Each fix has a named test that passed in this run, and the live UAT passed on the same code.
|
||||
|
||||
The status is `human_needed` only because the fixes are not released. The tagged plugins still carry the boot blocker, and pgsub main depends on framework hooks that no tag has. Releasing needs the user's go-ahead.
|
||||
|
||||
---
|
||||
|
||||
_Verified: 2026-10-14T19:35:00Z_
|
||||
_Verified: 2026-10-16T15:45:06Z_
|
||||
_Verifier: Claude (msd-verifier)_
|
||||
|
||||
@@ -9,21 +9,23 @@ 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 | success URL carries the session_id placeholder; subscription_data.metadata; an idempotency key per subscriber, customer, price, URLs and metadata in a 10-minute window, so parallel requests get one session (WR-03, plan 18.1-08); the price is the plan's own in the currency, never another currency'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 |
|
||||
| subscriptions.retrieve (syncSubscription / reconcile) | INTEGRATE | also the webhook job's fresh-state retrieval: created and updated events apply the subscription Stripe holds, not the event snapshot (CR-01, plan 18.1-07); a 404 falls back to the snapshot. `Service.SyncSubscription` uses it through `contracts.SubscriptionRetriever` and refreshes the plan (WR-06) |
|
||||
| subscriptions.retrieve (syncSubscription / reconcile) | INTEGRATE | also the webhook job's fresh-state read: created/updated apply Stripe's current subscription, 404 falls back to the snapshot (CR-01, 18.1-07); used by Service.SyncSubscription (WR-06) |
|
||||
| subscriptions.update cancel_at_period_end true (cancel at period end) | INTEGRATE | |
|
||||
| subscriptions.cancel (cancel immediately) | INTEGRATE | |
|
||||
| subscriptions.update cancel_at (flexible-billing cancel enums) | OPT-OUT | D-21 keeps PHP's cancel_at_period_end mapping; a manual Stripe test-mode check at UAT decides a follow-up |
|
||||
| subscription cancel_at (read from events and retrieval) | INTEGRATE | a cancel_at date on an active or trialing subscription maps to cancel_at_period_end true; cleared maps back (portal cancels report cancel_at only; D-21 amended 2026-10-16) |
|
||||
| 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 | |
|
||||
|
||||
@@ -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.
|
||||
@@ -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.
|
||||
@@ -0,0 +1,21 @@
|
||||
---
|
||||
quick_id: 261016-nqk
|
||||
slug: close-uat-gaps-g-18-1-7-and-g-18-1-4
|
||||
date: 2026-10-16
|
||||
gap_closure: true
|
||||
gap_ids: [G-18.1-7, G-18.1-4]
|
||||
repos: [summercms.go, sm-paymentgatewaysubscriptions-plugin, sm-pgstripe-plugin]
|
||||
source: .planning/phases/18.1-payment-gateway-subscriptions/18.1-UAT.md (## Gaps)
|
||||
---
|
||||
|
||||
# Quick 261016-nqk: close UAT gaps G-18.1-7 and G-18.1-4
|
||||
|
||||
No push, tag or release. Framework first (pgsub needs its new hooks), then the plugins; planning docs last.
|
||||
|
||||
## Tasks
|
||||
|
||||
1. **G-18.1-7 (pgstripe).** The decoded subscription maps `cancel_at_period_end` true for Stripe's flag or a `cancel_at` date while active or trialing; a cleared `cancel_at` maps back. One choke point (`subscriptionObject.args`) serves the snapshot, the fresh retrieval and `SyncSubscription`. Tests: both payload shapes, un-cancel, `cancel_at` in past_due/canceled; Postgres smoke of portal cancel then un-cancel (reactivated once). README, COVERAGE row, D-21 amendment.
|
||||
2. **G-18.1-4 (preview labels, pgsub).** The label element renders its `value` property (the SPA's plugin-element contract), not only the attribute. Node-run test.
|
||||
3. **G-18.1-4 (framework).** `pact.ListExtendRecords` (once per list page, before columns are read; unstored `gorm:"-"` columns compile unsortable and refuse sortable/searchable) and `pact.FormRecordTitle` (`meta.title` on show/create/update; the SPA heads form and preview with it). Roster fixture tests, SPA tests, docs, OpenAPI, admin dist.
|
||||
4. **G-18.1-4 (pgsub).** Subscriber column (e-mail, else name; `<alias> #<id>` fallback) through `ListExtendRecords` and the optional `contracts.SubscriberBatchResolver` (user resolver implements `ResolveMany`); subscriber type and id become invisible searchable columns; preview title "Subscription #<id> (<plan>)" (en/pl). Tests, README.
|
||||
5. Rebuild the UAT demo app and check the admin API.
|
||||
@@ -0,0 +1,37 @@
|
||||
---
|
||||
quick_id: 261016-nqk
|
||||
slug: close-uat-gaps-g-18-1-7-and-g-18-1-4
|
||||
status: complete
|
||||
date: 2026-10-16
|
||||
gap_closure: true
|
||||
gap_ids: [G-18.1-7, G-18.1-4]
|
||||
repos:
|
||||
sm-pgstripe-plugin: [a5b3314]
|
||||
sm-paymentgatewaysubscriptions-plugin: [6a3f2c4, d5579cd]
|
||||
summercms.go: [bc5685e, 013890c, 17398fe]
|
||||
---
|
||||
|
||||
# Quick 261016-nqk: UAT gaps G-18.1-7 and G-18.1-4
|
||||
|
||||
Nothing pushed, tagged or released.
|
||||
|
||||
| Gap | Fix | Commit |
|
||||
|-----|-----|--------|
|
||||
| G-18.1-7 | `cancel_at_period_end` = Stripe's flag, or a `cancel_at` date on an active or trialing subscription; un-cancel maps back and fires reactivated. D-21 amended; COVERAGE row now INTEGRATE | pgstripe a5b3314 |
|
||||
| G-18.1-4 (empty Gateway/Status on preview) | the label element renders its `value` property (the SPA sets the property, the element read only the attribute) | pgsub 6a3f2c4 |
|
||||
| G-18.1-4 (framework) | `pact.ListExtendRecords`, `pact.FormRecordTitle` (`meta.title`), unstored list columns unsortable/unsearchable; SPA uses `meta.title`; docs, OpenAPI, dist | summercms.go bc5685e, 013890c, 17398fe |
|
||||
| G-18.1-4 (subscriber, title) | list column `subscriber` (e-mail, else name, `<alias> #<id>` fallback) via the new optional `contracts.SubscriberBatchResolver`; type/id invisible but searchable; preview title "Subscription #<id> (<plan>)" / "Subskrypcja #<id> (<plan>)" | pgsub d5579cd |
|
||||
|
||||
The screenshot's "Cancel at Period End: No / Canceled At: —" was stale: the preview API for subscription 1 returns `cancel_at_period_end: true` and `canceled_at` (checked on the demo, before and after the rebuild).
|
||||
|
||||
## Verification
|
||||
|
||||
- summercms.go: `go vet ./...`, `go test ./...` (Docker), `summer docs:build --check`, `TestDocsTree`, admin vitest 1431/1431 and typecheck, `check-admin-openapi.sh`, `check-admin-dist.sh`.
|
||||
- pgsub and pgstripe: `go vet ./...`, `go test ./...` (Docker) green; the new subtests ran (not skipped).
|
||||
- Demo app rebuilt without overlays and restarted: list row `subscriber: customer@example.test`, preview `meta.title: "Subscription #1 (Pro)"`, `gateway: Stripe`, `stripe_status: Active`; the new admin bundle is served.
|
||||
|
||||
## Release notes
|
||||
|
||||
- pgsub now needs the framework's `pact.ListExtendRecords` and `pact.FormRecordTitle`: tag the framework (v0.2.2) first, bump pgsub's `require`, tag pgsub, then pgstripe (already needs unreleased pgsub from 261016-myi).
|
||||
- Search by subscriber e-mail is not offered: cabana searches stored columns and relations only, and the subscriber is polymorphic. Type and id stay searchable.
|
||||
- UAT 7 and the site-side cancel still need a live re-run in the demo.
|
||||
@@ -0,0 +1,235 @@
|
||||
---
|
||||
phase: quick-261016-s8x
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- modules/pact/capabilities.go
|
||||
- modules/pact/README.md
|
||||
- modules/cabana/navigation.go
|
||||
- modules/cabana/permissions_test.go
|
||||
- modules/cabana/README.md
|
||||
- docs/backend/users-and-permissions.md
|
||||
- docs/backend/admin-controllers.md
|
||||
- admin/src/styles/main.css
|
||||
- modules/boardwalk/dist/
|
||||
- ../sm-paymentgateway-plugin/admin_navigation.go
|
||||
- ../sm-paymentgateway-plugin/admin_navigation_test.go
|
||||
- ../sm-paymentgateway-plugin/controllers/quotes/_quotes_notice.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/quoteables/_quoteables_notice.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/shippings/_shippings_notice.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/orders/_orders_stats.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/payments/_payments_stats.htm
|
||||
- ../sm-paymentgateway-plugin/README.md
|
||||
autonomous: true
|
||||
requirements: [QUICK-261016-s8x]
|
||||
|
||||
estimate:
|
||||
tokens: 70000
|
||||
raw_tokens: 70000
|
||||
tasks: 3
|
||||
confidence: low
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- 'With quotes turned off in Settings -> Payment gateway, GET <prefix>/api/v1/navigation no longer lists Quotes and Quote items under "Payments & orders"; with orders turned off it lists neither Orders, Quotes, Quote items nor Shipping methods and the "Payments & orders" item opens Payments; with only shipping turned off just Shipping methods is gone. Payments is always listed. Turning a feature back on shows its items on the next navigation load (SPA boot or sign-in), with no restart.'
|
||||
- 'A screen whose feature is off, opened by its URL, still answers its list with the "Turned off" callout above it, exactly as before.'
|
||||
- 'In a header partial a callout followed by a stats strip has 12px of space below it, and a header partial with no stats renders no empty rounded strip: the five payment gateway header templates emit the summer-stats <dl> only when there is at least one stat. Field notices in forms (FormField.vue) and the order and payment hint partials look as before.'
|
||||
- 'pact.NavigationItem has a Visible func(ctx context.Context) bool field; nil keeps the entry always listed. cabana.Registry.Metadata runs it with the request context after the permission check, leaves out a main item (with its whole side menu) or a side item whose Visible answers false, and a main item whose Controller is the controller of a side item hidden this way links to its first visible openable side item (empty when there is none). Registry validation, the NavigationEntry JSON shape and the OpenAPI document are unchanged.'
|
||||
- 'go vet ./..., go test -short ./modules/pact ./modules/cabana, go test ./cmd/summer -run TestDocsTree, go run ./cmd/summer docs:build --check and scripts/check-admin-dist.sh pass in summercms.go; go vet ./... and go test -short ./... pass in sm-paymentgateway-plugin. modules/boardwalk/dist is rebuilt and committed. Nothing is tagged, pushed or deployed, no application submodule pointer is bumped, every commit is staged by explicit path and carries no co-author tag.'
|
||||
artifacts:
|
||||
- path: "modules/pact/capabilities.go"
|
||||
provides: "the Visible hook on a navigation entry"
|
||||
contains: "Visible func(ctx context.Context) bool"
|
||||
- path: "modules/cabana/navigation.go"
|
||||
provides: "Metadata skips hidden entries and repoints a parent whose controller is a hidden child's"
|
||||
contains: "Visible"
|
||||
- path: "modules/cabana/permissions_test.go"
|
||||
provides: "short unit test of the Visible rules next to TestNavigationDropsDeniedParentAndRepointsTarget"
|
||||
contains: "func TestNavigationHidesInvisibleItemsAndRepointsTarget"
|
||||
- path: "admin/src/styles/main.css"
|
||||
provides: "space below a callout that a sibling follows inside a partial tree"
|
||||
contains: ".summer-partial .summer-callout:not(:last-child)"
|
||||
- path: "../sm-paymentgateway-plugin/admin_navigation.go"
|
||||
provides: "Visible closures on orders, quotes, quoteables and shippings reading the settings per request"
|
||||
contains: "Visible:"
|
||||
- path: "../sm-paymentgateway-plugin/admin_navigation_test.go"
|
||||
provides: "short test of the visibility rules and of the no-app fallback"
|
||||
contains: "func TestNavigationVisibility"
|
||||
- path: "../sm-paymentgateway-plugin/controllers/quoteables/_quoteables_notice.htm"
|
||||
provides: "the stats strip only when there are stats (same change in the other four header templates)"
|
||||
contains: "{{- if .Stats -}}"
|
||||
- path: "../sm-paymentgateway-plugin/README.md"
|
||||
provides: "the navigation paragraph describes request-time visibility instead of always-listed items"
|
||||
contains: "Visible"
|
||||
key_links:
|
||||
- from: "../sm-paymentgateway-plugin/admin_navigation.go (Visible closures)"
|
||||
to: "modules/cabana/navigation.go (Metadata)"
|
||||
via: "cabana calls item.Visible(ctx) on every GET /navigation; the closure loads models.Settings through the booted app and answers the feature flag, true on any fallback"
|
||||
pattern: "Visible"
|
||||
- from: "modules/cabana/navigation.go (openableTarget)"
|
||||
to: "the set of side-item controllers hidden by Visible"
|
||||
via: "a parent whose Controller is in that set links to the first visible openable child, so Payments & orders opens Payments while orders are off"
|
||||
pattern: "openableTarget"
|
||||
- from: "the five header templates"
|
||||
to: "controllers statsView.Stats"
|
||||
via: "the <dl class=\"summer-stats\"> block is wrapped in an if on .Stats"
|
||||
pattern: "if .Stats"
|
||||
- from: "admin/src/styles/main.css"
|
||||
to: "PartialHost.vue wrapper div.summer-partial"
|
||||
via: "the rule is scoped to .summer-partial so FormField.vue's callout is untouched"
|
||||
pattern: "summer-partial .summer-callout"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Two admin fixes for the payment gateway plugin, reported from the "Quote items" screen while quotes are turned off:
|
||||
|
||||
1. The "Turned off" callout above a list has no space below it and an empty rounded strip (the stats `<dl>` with no stats) sits between it and the list card.
|
||||
2. Screens whose feature is off in Settings -> Payment gateway (Orders, Quotes, Quote items, Shipping methods) still appear in the admin side menu. They must disappear from the navigation; opening one by URL keeps showing the callout.
|
||||
|
||||
The framework has no request-time navigation visibility today (`pact.NavigationItem` is static and `cabana.Registry.Metadata` filters by permission only), so this adds one additive, non-breaking hook (`pact.NavigationItem.Visible`), uses it in the plugin, fixes the templates and adds the callout spacing in the admin CSS.
|
||||
|
||||
Purpose: the admin shows menu entries for features the shop has turned off, and the feature-off screen looks broken.
|
||||
Output: two commits in `summercms.go` (framework hook with README/docs and a unit test; CSS fix) plus a dist rebuild commit, and one commit in `../sm-paymentgateway-plugin` (navigation, templates, README, test). No tag, push, deploy or submodule pointer bump.
|
||||
|
||||
Hard limits: other Claude sessions work in `summercms.go`: stage by explicit path, never `git add -A`; do not touch `.planning/STATE.md`, `.planning/state.json`, `.msd/` or `.planning/phases/17-blog-companions/17-VERIFICATION.md`. No co-author tags. One logical change per commit. `sm-paymentgateway-plugin` is a core plugin: no breaking change to its exported API (every exported identifier listed in its README line 338 stays). Framework code and docs never name a consuming application.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@CLAUDE.md
|
||||
@modules/pact/capabilities.go
|
||||
@modules/cabana/navigation.go
|
||||
@modules/cabana/permissions_test.go
|
||||
@admin/src/styles/main.css
|
||||
@../sm-paymentgateway-plugin/admin_navigation.go
|
||||
@../sm-paymentgateway-plugin/admin.go
|
||||
@../sm-paymentgateway-plugin/controllers/orders_partials.go
|
||||
|
||||
## Planning-time observations (2026-10-16, live)
|
||||
|
||||
Edit scope rests on these facts; re-check the ones a task's precondition names before editing.
|
||||
|
||||
- `summercms.go` working tree: only `.planning/STATE.md`, `.planning/state.json` (modified), `.msd/` and `.planning/phases/17-blog-companions/17-VERIFICATION.md` (untracked), all other sessions' files. Nothing uncommitted under `modules/`, `admin/` or `docs/`. `../sm-paymentgateway-plugin` is clean on its own repo (`go.work` replaces `git.golem15.com/golem15/summercms` with `../summercms.go`; its `go.mod` still requires a tagged framework, so the plugin can only bump that requirement after a framework release that contains this hook).
|
||||
- Baselines pass today: `go vet ./modules/pact ./modules/cabana && go test -short ./modules/pact ./modules/cabana` in the framework (cabana short suite 0.9 s); `go vet ./... && go test -short ./...` in the plugin. `summer` is NOT on PATH: use `go run ./cmd/summer docs:build --check`. `admin/node_modules` is present (no `npm ci` needed). Docker is available but nothing here needs it.
|
||||
- `pact.NavigationItem` (`modules/pact/capabilities.go:541`): `Code, Label, Icon, Permissions, Order, Controller, SideMenu`. `pact` already imports `context`. `pact.SettingsItem` carries func fields tagged `json:"-"` (`NewModel`), the convention to follow for a func field.
|
||||
- `cabana.Registry.Metadata(ctx, principal, tr)` (`modules/cabana/navigation.go:39`): drops a parent the principal cannot open, filters children by `Allows`, then `view.Controller = r.openableTarget(principal, item, children)`; `openableTarget` (line 101) keeps `item.Controller` when empty or `canOpen`, else the first child the principal can open, else "". The items come from `r.navigation`, collected once in `registry.go:313-320` (`validateNavigationShape`) and validated in `validateNavigation` (line ~405-445): codes, duplicate children, known controllers, known permissions. Neither looks at any other field, so a func field needs no validation change. `GET /navigation` is served from `http.go:498` with the request context.
|
||||
- The SPA loads navigation once per boot and after sign-in (`admin/src/state/useNavigation.ts` `loadNavigation`, called from `main.ts:30` and `LoginView.vue:33`); `railEntries` omits a plugin whose side menu is empty. `NavigationEntry` (cabana) is the JSON shape and does not change, so swag/OpenAPI are untouched.
|
||||
- Existing test to model on: `TestNavigationDropsDeniedParentAndRepointsTarget` (`modules/cabana/permissions_test.go:70`) builds `&Registry{byID: ..., navigation: ...}` directly with the `navController` helper (same file, line 62) and calls `reg.Metadata(context.Background(), principal, nil)`.
|
||||
- Docs that describe navigation and must stay truthful (`docs:build --check` validates every `pkg.Identifier` and `pkg.Type.Field` in prose): `modules/pact/README.md` line 27 (Features bullet) and line 122 (API table row); `modules/cabana/README.md` line 58 (Features bullet "Backend navigation ... filtered per user by `cabana.Registry.Metadata`") and the routes table row `GET /navigation, GET /settings` (line ~83); `docs/backend/users-and-permissions.md` line 67 (the paragraph after the Navigation example: "...navigation and settings entries are filtered by the permissions they name... links to its first openable side-menu entry instead"); `docs/backend/admin-controllers.md` "Compilation at boot" paragraph (line ~159: "compiles every plugin's controllers, settings, navigation and permissions once... Nothing is parsed per request"). The Go examples in those pages are `src=` copies of `modules/cabana/example_controller_test.go`: do not edit them (prose only), so `docs:sync` is not needed. `docs/plugins/registration.md` line 84 only lists the interfaces and stays true.
|
||||
- Plugin navigation (`../sm-paymentgateway-plugin/admin_navigation.go`): `Navigation()` lists five side items always and picks the top-level `Controller` at boot via `ordersEnabledAtBoot()` (app nil, no `*gorm.DB`, settings table missing, load error: all answer true). `p.application()` (`admin.go:34`) returns the booted `*backpack.App` (nil before Register). `Navigation()` is called once by `cabana.Activate` at boot; a closure stored in the item runs later, at request time, when the app is set. Settings: `models.LoadSettings(ctx, db)` returns `*models.Settings` with `OrdersEnabled`, `ShippingsEnabled`, `UseQuotes` (`models/settings.go:34-36`), `TableName()` `golem15_paymentgateway_settings`.
|
||||
- Feature-off rules used by the callouts today (keep the navigation identical to them): orders off when `!OrdersEnabled` (`controllers/orders_partials.go:126`); quotes and quote items off when `!OrdersEnabled || !UseQuotes` (`controllers/quoteables.go:73 quotesNotice`); shippings off when `!OrdersEnabled || !ShippingsEnabled` (`controllers/shippings.go:86`). Payments is never off.
|
||||
- Header templates (`controllers/<name>/_<name>_notice.htm` for quotes, quoteables, shippings; `_orders_stats.htm`; `_payments_stats.htm`): each renders the callout inside `{{- with .Notice -}}` and then ALWAYS `<dl class="summer-stats">` ranging over `.Stats` (`statsView{Notice *noticeView; Stats []statView}`, `orders_partials.go:52`). An empty `<dl>` is the empty strip from the screenshot. No plugin test pins the `<dl>`.
|
||||
- CSS: `.summer-callout { margin: 0; ... }` (`admin/src/styles/main.css:459`), `.summer-stats { margin: 0; ... }` (line 349), `.summer-partial` rules at line 214. `PartialHost.vue:79` wraps every rendered partial in `<div class="summer-partial" data-partial="header|hint|...">`; `ListView.vue:444` is `<section class="flex flex-col gap-5">`, so the partial wrapper already has 20px to the list card and the only missing space is inside the partial, between the callout and the strip. `.summer-callout` is also used by `admin/src/components/form/FormField.vue:71` (a field notice between the comment and the error list, outside any `.summer-partial`) and by the plugin's `_order_hint.htm` and `_payment_hint.htm` (callout is the last child of the hint). A rule scoped to `.summer-partial` and to a callout that a sibling follows touches none of them.
|
||||
- Plugin tests are all `package paymentgateway_test`; Go allows an internal `package paymentgateway` test file next to them, which is what a pure unexported visibility function needs. The plugin's short admin tests live in `admin_short_test.go`.
|
||||
- Commit naming precedent for quick tasks in both repos: `feat(quick-261016-nqk): ...`, `fix(quick-...)`, `chore(quick-261016-nqk): rebuild embedded admin dist`. The plan and summary are committed by the quick workflow, not by these tasks.
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Framework hook pact.NavigationItem.Visible, honoured by cabana.Registry.Metadata, with test and docs</name>
|
||||
<precondition>In /media/nvme/dev/golem15/summercms.io/summercms/summercms.go `git status --short -- modules admin docs` prints nothing (no other session is editing pact, cabana, the SPA or docs).</precondition>
|
||||
<files>modules/pact/capabilities.go, modules/pact/README.md, modules/cabana/navigation.go, modules/cabana/permissions_test.go, modules/cabana/README.md, docs/backend/users-and-permissions.md, docs/backend/admin-controllers.md</files>
|
||||
<action>
|
||||
Repo: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go. One commit, `feat(quick-261016-s8x): pact.NavigationItem.Visible hides navigation entries at request time`, staged by the seven paths above.
|
||||
|
||||
pact (`modules/pact/capabilities.go`): add the field `Visible func(ctx context.Context) bool` with the tag `json:"-"` to `NavigationItem`, after `SideMenu`. Doc comment on the field: nil means always listed; when set, cabana calls it on every navigation request with the request context, after the permission check, and leaves the entry out when it answers false; a main item hidden this way takes its side menu with it; it must be cheap, must not panic and should answer true when it cannot decide (no database yet). Extend the type's comment accordingly. pact stays dependency-free (a func field only; `context` is already imported).
|
||||
|
||||
cabana (`modules/cabana/navigation.go`): in `Metadata`, after `Allows` passes for a main item, skip the item when `item.Visible != nil && !item.Visible(ctx)` (so a denied principal never runs plugin code). While walking `item.SideMenu`, after the child's `Allows` check, skip a child whose Visible answers false and record its `Controller` in a small set of hidden controllers (a local map keyed by controller ID, ignoring ""). Change the unexported `openableTarget` to take that set and to treat `item.Controller` as not usable when it is in the set, falling through to the existing loop over the (already visible and permitted) children, so a parent whose own controller is a hidden child's links to the first visible openable child and to "" when none is. Permission-denied children are not added to the set: the existing permission behaviour (parent keeps its controller when the principal can open it) is unchanged. Update the comments of `Metadata` and `openableTarget`. Do not touch `NavigationEntry`, registry validation, swag annotations or the SPA.
|
||||
|
||||
Test (`modules/cabana/permissions_test.go`, directly after `TestNavigationDropsDeniedParentAndRepointsTarget`, same `Registry{byID, navigation}` and `navController` pattern, superuser principal so permissions are not the factor): `TestNavigationHidesInvisibleItemsAndRepointsTarget` covering (a) a side item with Visible false is absent from `SideMenu` while a sibling with nil Visible stays; (b) a parent whose Controller is that hidden child's controller gets the first visible openable child as `Controller`; (c) a parent with Visible false is dropped although every child is visible; (d) a parent whose only child is hidden and whose Controller is that child's keeps an empty SideMenu and an empty Controller; (e) Visible receives the context passed to Metadata (a `context.WithValue` key read inside the func). Use neutral names (`acme.shop.*`).
|
||||
|
||||
Docs, same commit, prose only (the Go examples on these pages are `src=` copies and must not change): `modules/pact/README.md` line 27 bullet (add that a `pact.NavigationItem.Visible` func hides an entry at request time, nil = always listed) and line 122 row ("One backend navigation entry, with an optional side menu and an optional `Visible` func the admin runs on every navigation request."). `modules/cabana/README.md` line 58 bullet: add one sentence that an entry whose `Visible` func answers false for the request is left out like a denied one (a hidden main item with its side menu) and that a main item whose controller is a side item hidden this way links to its first visible openable side item; the `GET /navigation, GET /settings` row: "...the administrator may open and, for a navigation entry with a `Visible` func, that it answers true for the request". `docs/backend/users-and-permissions.md` line 67 paragraph: one sentence after "...links to its first openable side-menu entry instead." stating that a plugin can also hide an entry per request with `pact.NavigationItem.Visible` (for a screen whose feature is turned off in its settings) and that the same drop and relink rules apply. `docs/backend/admin-controllers.md` "Compilation at boot" paragraph: after "Nothing is parsed per request." add that the one run-time hook is `pact.NavigationItem.Visible`, which runs on every `GET /navigation` so a plugin can hide a screen whose feature is off; it must be cheap and answer true when unsure.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./modules/pact ./modules/cabana && go test -short ./modules/pact ./modules/cabana -count=1 && go test -short ./modules/cabana -run 'TestNavigationHidesInvisibleItemsAndRepointsTarget|TestNavigationDropsDeniedParentAndRepointsTarget' -count=1 -v && go test ./cmd/summer -run TestDocsTree -count=1 && go run ./cmd/summer docs:build --check && grep -q 'Visible func(ctx context.Context) bool' modules/pact/capabilities.go && grep -q 'Visible' modules/pact/README.md && grep -q 'Visible' modules/cabana/README.md && grep -q 'pact.NavigationItem.Visible' docs/backend/users-and-permissions.md && grep -q 'pact.NavigationItem.Visible' docs/backend/admin-controllers.md && HEAD_FILES="$(git show --name-only --format= HEAD)" && ! printf '%s\n' "$HEAD_FILES" | grep -q -E '^(admin/openapi|modules/cabana/admin_openapi\.go)' && test -z "$(git status --porcelain -- modules docs)"</automated>
|
||||
</verify>
|
||||
<done>The new cabana test and the whole pact and cabana short suites pass; the docs checker passes with the new identifiers; the OpenAPI inputs are untouched; the change is committed in summercms.go by explicit paths with the given message and no co-author tag.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Space below a header-partial callout (admin CSS) and the embedded dist rebuild</name>
|
||||
<files>admin/src/styles/main.css, modules/boardwalk/dist/</files>
|
||||
<action>
|
||||
Repo: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go. Two commits: `fix(quick-261016-s8x): space below a callout that a sibling follows in a partial` (main.css only) and then `chore(quick-261016-s8x): rebuild embedded admin dist` (`modules/boardwalk/dist` only), both staged by path.
|
||||
|
||||
In `admin/src/styles/main.css`, in the Callout block that starts at `.summer-callout` (line ~459, inside the same `@layer` block), add the rule `.summer-partial .summer-callout:not(:last-child) { margin-bottom: 12px; }` with a one-line comment that it spaces a callout from the stats strip (or any sibling) that follows it inside a rendered partial. Keep `.summer-callout { margin: 0 }` as is: the rule must not reach `FormField.vue`'s field notice (outside `.summer-partial`) nor change the order and payment hint partials, whose callout is the last child.
|
||||
|
||||
Rebuild the embedded SPA with `npm --prefix admin run build` (node_modules is present; run `npm --prefix admin ci` only if `admin/node_modules` is missing) and commit `modules/boardwalk/dist` as the second commit.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && grep -q '\.summer-partial \.summer-callout:not(:last-child)' admin/src/styles/main.css && grep -c '\.summer-callout {' admin/src/styles/main.css | grep -qx 1 && npm --prefix admin run build && scripts/check-admin-dist.sh && test -z "$(git status --porcelain -- admin modules/boardwalk)"</automated>
|
||||
</verify>
|
||||
<done>The scoped margin rule exists, the base `.summer-callout` rule is unchanged, `scripts/check-admin-dist.sh` reports that the committed dist matches a fresh build, and both commits exist with no co-author tag.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Plugin uses Visible for the feature-off screens, templates drop the empty stats strip, README and short test</name>
|
||||
<precondition>`grep -q 'Visible func(ctx context.Context) bool' /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/modules/pact/capabilities.go` succeeds (Task 1 committed; the plugin's go.work replace points at that checkout) and `git -C /media/nvme/dev/golem15/summercms.io/summercms/sm-paymentgateway-plugin status --short` prints nothing.</precondition>
|
||||
<files>../sm-paymentgateway-plugin/admin_navigation.go, ../sm-paymentgateway-plugin/admin_navigation_test.go, ../sm-paymentgateway-plugin/controllers/quotes/_quotes_notice.htm, ../sm-paymentgateway-plugin/controllers/quoteables/_quoteables_notice.htm, ../sm-paymentgateway-plugin/controllers/shippings/_shippings_notice.htm, ../sm-paymentgateway-plugin/controllers/orders/_orders_stats.htm, ../sm-paymentgateway-plugin/controllers/payments/_payments_stats.htm, ../sm-paymentgateway-plugin/README.md</files>
|
||||
<action>
|
||||
Repo: /media/nvme/dev/golem15/summercms.io/summercms/sm-paymentgateway-plugin (its own git repository). One commit there, `fix(quick-261016-s8x): hide feature-off screens from the navigation and drop the empty stats strip`, staged by the eight paths (relative to that repo). Do not bump any application's submodule pointer, do not change `go.mod`, do not push or tag.
|
||||
|
||||
`admin_navigation.go`: add a pure unexported function, e.g. `navVisibility(s models.Settings)` returning the three booleans orders, quotes, shippings, with exactly the callout rules: orders = `s.OrdersEnabled`; quotes = `s.OrdersEnabled && s.UseQuotes`; shippings = `s.OrdersEnabled && s.ShippingsEnabled`. Add a method on `*Plugin` that loads the settings for one navigation request, e.g. `navSettings(ctx) (models.Settings, bool)`, with the same fallbacks the boot-time helper has today (nil app, no `*gorm.DB` in `app.Lookup`, settings table missing, load error or nil: not loaded), a 5 s timeout derived from the request context, and no logging. Set `Visible` on the orders, quotes, quoteables and shippings side items to closures that call it and answer true when the settings could not be loaded, else the matching rule; payments gets no Visible. The closures capture `p`, so they see the app that `Register` sets even though `Navigation()` runs at boot. Make the top-level item's `Controller` simply `controllers.OrdersControllerID` (cabana relinks it to Payments while orders are hidden) and delete the boot-time orders check and its now-unused imports, keeping the registered-controller filter. Rewrite the `Navigation` doc comment: the four feature items are listed while their feature is on, read per request, a change shows on the next navigation load without a restart, a screen opened by URL while off still shows the callout.
|
||||
|
||||
`admin_navigation_test.go` (new, `package paymentgateway`, short, no Docker): `TestNavigationVisibility` with a table over `models.Settings{OrdersEnabled, UseQuotes, ShippingsEnabled}` combinations asserting the three rules (all on: all visible; orders off: all three hidden whatever the others; orders on and quotes off: quotes hidden, shippings visible; orders on and shippings off: shippings hidden, quotes visible), plus a check that `(&Plugin{}).Navigation()` yields one parent whose `Controller` is `controllers.OrdersControllerID`, whose `payments` side item has a nil Visible and whose other four side items have a non-nil Visible that answers true with `context.Background()` (the no-app fallback).
|
||||
|
||||
The five header templates: wrap the `<dl class="summer-stats">` block, from the `<dl` line through `</dl>`, in `{{- if .Stats -}}` ... `{{- end -}}` so an empty stats list renders nothing after the callout. Keep everything else byte-identical.
|
||||
|
||||
`README.md`: rewrite the navigation paragraph (line 122, "`Plugin.Navigation` adds ...") so it states that Orders, Quotes, Quote items and Shipping methods are listed only while their feature is on (`orders_enabled`; `orders_enabled` and `use_quotes`; `orders_enabled` and `shippings_enabled`), through a `Visible` func the admin runs on every navigation request, so a change shows on the next navigation load without a restart and every item is listed before the database is published or migrated; a screen opened by URL while its feature is off still shows the "Turned off" callout above its list; the top-level item opens Orders, or Payments while orders are off. Leave the partials table rows (lines 164-174) and the exported-API list (line 338) as they are; `Navigation` stays exported and its signature is unchanged.
|
||||
|
||||
In the SUMMARY, note that the plugin now needs a framework release containing `pact.NavigationItem.Visible` before its `go.mod` requirement is bumped (development works through `go.work`), and that no application pointer was bumped.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>cd /media/nvme/dev/golem15/summercms.io/summercms/sm-paymentgateway-plugin && go vet ./... && go test -short ./... -count=1 && go test -short . -run TestNavigationVisibility -count=1 -v && test "$(grep -l '{{- if .Stats -}}' controllers/quotes/_quotes_notice.htm controllers/quoteables/_quoteables_notice.htm controllers/shippings/_shippings_notice.htm controllers/orders/_orders_stats.htm controllers/payments/_payments_stats.htm | wc -l)" = 5 && grep -c 'Visible:' admin_navigation.go | grep -qx 4 && grep -q 'Visible' README.md && git diff --quiet HEAD -- go.mod go.sum && test -z "$(git status --porcelain)"</automated>
|
||||
</verify>
|
||||
<done>The plugin's vet and short suites pass with the new visibility test; the four feature items carry Visible and payments does not; all five templates emit the stats strip only when there are stats; the README describes request-time visibility; go.mod is unchanged; the commit exists in the plugin repo with no co-author tag.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| browser (SPA) -> GET /navigation | A navigation request now runs plugin code (the Visible funcs) per request, after authentication and the permission check. |
|
||||
| plugin Visible func -> database | The payment gateway reads its settings row on every navigation request through the booted app. |
|
||||
| admin CSS -> every rendered partial | A global stylesheet rule reaches every plugin's partial trees. |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-261016-s8x-01 | Information disclosure | cabana Metadata ordering | medium | mitigate | Visible runs only after `Allows` passes for the item (and for the parent), so a denied principal never triggers plugin code and a hidden entry leaks neither its label nor its controller; the new test and the existing permission test both stay green. |
|
||||
| T-261016-s8x-02 | Denial of service | plugin Visible on every GET /navigation | low | mitigate | The closure does one settings load with a 5 s timeout from the request context and answers true on any failure (no panic path, no retry); navigation is loaded once per SPA boot, not per screen. |
|
||||
| T-261016-s8x-03 | Elevation of privilege | hidden screen opened by URL | low | accept | Visibility is a menu concern: the controller's `RequiredPermissions` still guard every request and the screen keeps answering with the feature-off callout, exactly as before. |
|
||||
| T-261016-s8x-04 | Tampering | the margin rule reaching FormField.vue or the hint partials | low | mitigate | The rule is scoped to `.summer-partial` and to a callout that a sibling follows; FormField's notice is outside a partial and the hint callouts are last children; `check-admin-dist.sh` proves the committed dist is the one built from this CSS. |
|
||||
| T-261016-s8x-SC | Tampering | npm/Go dependency installs | low | accept | No new dependency in either repo; `npm --prefix admin run build` uses the installed node_modules from the committed lockfile; the plugin's go.mod is unchanged. No package-legitimacy checkpoint is needed. |
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
Framework (`summercms.go`): `go vet ./... && go test -short ./... -count=1 && go test ./cmd/summer -run TestDocsTree -count=1 && go run ./cmd/summer docs:build --check && scripts/check-admin-dist.sh`, then `test -z "$(git status --porcelain -- modules admin docs)"` (the other sessions' `.planning`/`.msd` files are expected to remain).
|
||||
|
||||
Plugin (`../sm-paymentgateway-plugin`): `go vet ./... && go test -short ./... -count=1 && test -z "$(git status --porcelain)"`.
|
||||
|
||||
Commit hygiene in both repos: `git log -5 --format=%B | grep -ci 'co-authored-by'` prints 0; every new commit message starts with `feat(quick-261016-s8x)`, `fix(quick-261016-s8x)` or `chore(quick-261016-s8x)`; no tag was created (`git tag --points-at HEAD` empty in both repos); no application repository was touched.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- With quotes off, `GET /navigation` for a superuser lists Orders, Payments and Shipping methods only under "Payments & orders"; with orders off it lists Payments only and the parent's `controller` is `golem15.paymentgateway.payments`; with everything on all five are listed and the parent opens Orders (proved by the pure-function test plus the cabana relink test; a live check against a running app is optional).
|
||||
- Opening a hidden screen by URL still returns its list with the "Turned off" callout.
|
||||
- A header partial with a callout and no stats renders the callout alone; with stats, the strip starts 12px below the callout.
|
||||
- `pact.NavigationItem.Visible` exists, is documented in the pact and cabana READMEs and the two docs pages, and the docs checker passes.
|
||||
- All gates in `<verification>` pass; the embedded dist is committed; nothing is pushed, tagged, deployed or pointer-bumped; no co-author tags.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/quick/261016-s8x-paymentgateway-banner-bottom-margin-and-/261016-s8x-SUMMARY.md` when done, listing the commits in both repositories and the framework-release note for the plugin's go.mod.
|
||||
</output>
|
||||
@@ -0,0 +1,166 @@
|
||||
---
|
||||
phase: quick-261016-s8x
|
||||
plan: 01
|
||||
subsystem: admin navigation, payment gateway admin screens
|
||||
tags: [cabana, pact, navigation, visibility, admin-css, paymentgateway]
|
||||
status: complete
|
||||
requirements: [QUICK-261016-s8x]
|
||||
|
||||
dependency_graph:
|
||||
requires:
|
||||
- cabana.Registry.Metadata permission filtering (WR-02)
|
||||
- PartialHost.vue wrapper div.summer-partial
|
||||
provides:
|
||||
- pact.NavigationItem.Visible (request-time navigation visibility hook)
|
||||
- cabana.Registry.Metadata honouring Visible and relinking a parent whose controller is a hidden child's
|
||||
- payment gateway side items hidden while their feature is off
|
||||
- header partials without an empty stats strip
|
||||
affects:
|
||||
- every plugin's GET /navigation response (only when a plugin sets Visible)
|
||||
- sm-paymentgateway-plugin go.mod (needs a framework release that contains the hook before its requirement is bumped)
|
||||
|
||||
tech_stack:
|
||||
added: []
|
||||
patterns:
|
||||
- func field tagged json:"-" on a pact registration struct (as pact.SettingsItem.NewModel)
|
||||
- plugin closures capture *Plugin so a boot-time Navigation() reads the booted app at request time
|
||||
- CSS rule scoped to .summer-partial so a global stylesheet change touches only rendered partials
|
||||
|
||||
key_files:
|
||||
created:
|
||||
- ../sm-paymentgateway-plugin/admin_navigation_test.go
|
||||
modified:
|
||||
- modules/pact/capabilities.go
|
||||
- modules/pact/README.md
|
||||
- modules/cabana/navigation.go
|
||||
- modules/cabana/permissions_test.go
|
||||
- modules/cabana/README.md
|
||||
- docs/backend/users-and-permissions.md
|
||||
- docs/backend/admin-controllers.md
|
||||
- admin/src/styles/main.css
|
||||
- modules/boardwalk/dist/ (rebuilt)
|
||||
- ../sm-paymentgateway-plugin/admin_navigation.go
|
||||
- ../sm-paymentgateway-plugin/controllers/quotes/_quotes_notice.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/quoteables/_quoteables_notice.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/shippings/_shippings_notice.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/orders/_orders_stats.htm
|
||||
- ../sm-paymentgateway-plugin/controllers/payments/_payments_stats.htm
|
||||
- ../sm-paymentgateway-plugin/README.md
|
||||
|
||||
decisions:
|
||||
- "Visible runs after Allows for the item (and after the parent's Allows for a side item), so a denied principal never runs plugin code and a hidden entry leaks nothing (T-261016-s8x-01)."
|
||||
- "Only Visible-hidden side items are recorded in the hidden set passed to openableTarget; permission-denied children keep the existing WR-02 behaviour (a parent keeps its own controller when the principal can open it)."
|
||||
- "The plugin's top-level item always declares Orders as its Controller; cabana relinks it to Payments while orders are hidden, so the boot-time ordersEnabledAtBoot check is gone and a settings change needs no restart."
|
||||
- "navSettings answers 'not loaded' (item listed) on every fallback: nil app, no *gorm.DB, settings table absent, load error; the load uses a 5 s timeout derived from the request context (T-261016-s8x-02)."
|
||||
- "The callout spacing is a scoped rule (.summer-partial .summer-callout:not(:last-child)) rather than a change to .summer-callout, so FormField.vue's field notice and the hint partials are untouched (T-261016-s8x-04)."
|
||||
|
||||
metrics:
|
||||
duration: ~25 min
|
||||
completed: 2026-10-16
|
||||
tasks: 3
|
||||
framework_commits: 3
|
||||
plugin_commits: 1
|
||||
|
||||
actuals:
|
||||
tokens: 10545
|
||||
tasks: 3
|
||||
commits: 3
|
||||
plan_head_before: 90b2d4e23bb9e70e59ea50fac66eef67377939c2
|
||||
plan_head_after: ab9f385db8afdf94769e400a9b121fff7db3b16d
|
||||
plugin_head_before: (clean main before 16484ba)
|
||||
plugin_head_after: 16484ba7f51be1ecdb6f7384a88bdf3fa6df2ef7
|
||||
---
|
||||
|
||||
# Quick 261016-s8x: Payment gateway banner bottom margin and feature-off screens hidden from the navigation Summary
|
||||
|
||||
Added `pact.NavigationItem.Visible`, a per-request visibility hook honoured by `cabana.Registry.Metadata` (drop a hidden main item with its side menu or a hidden side item, relink a parent whose controller is a hidden child's to its first visible openable child), used it in the payment gateway so Orders, Quotes, Quote items and Shipping methods disappear from the admin menu while their feature is off in Settings -> Payment gateway, made the five header partials emit the `summer-stats` strip only when there are stats, and gave a callout 12px of space below it when a sibling follows it inside a rendered partial.
|
||||
|
||||
## Commits
|
||||
|
||||
### summercms.go (framework, branch `master`)
|
||||
|
||||
| Hash | Message | Files |
|
||||
| --- | --- | --- |
|
||||
| `06e2e60` | `feat(quick-261016-s8x): pact.NavigationItem.Visible hides navigation entries at request time` | modules/pact/capabilities.go, modules/pact/README.md, modules/cabana/navigation.go, modules/cabana/permissions_test.go, modules/cabana/README.md, docs/backend/users-and-permissions.md, docs/backend/admin-controllers.md |
|
||||
| `2dd5fb3` | `fix(quick-261016-s8x): space below a callout that a sibling follows in a partial` | admin/src/styles/main.css |
|
||||
| `ab9f385` | `chore(quick-261016-s8x): rebuild embedded admin dist` | modules/boardwalk/dist/ (index.html, renamed index/editor JS chunks, new CSS asset) |
|
||||
|
||||
Measured: `git rev-list --count 90b2d4e..ab9f385` = 3.
|
||||
|
||||
### sm-paymentgateway-plugin (own repo, branch `main`)
|
||||
|
||||
| Hash | Message | Files |
|
||||
| --- | --- | --- |
|
||||
| `16484ba` | `fix(quick-261016-s8x): hide feature-off screens from the navigation and drop the empty stats strip` | admin_navigation.go, admin_navigation_test.go, controllers/quotes/_quotes_notice.htm, controllers/quoteables/_quoteables_notice.htm, controllers/shippings/_shippings_notice.htm, controllers/orders/_orders_stats.htm, controllers/payments/_payments_stats.htm, README.md |
|
||||
|
||||
No tag, push, deploy or application submodule pointer bump in either repo. No co-author trailer in any commit (`git log -5 --format=%B | grep -ci co-authored-by` = 0 in both).
|
||||
|
||||
## Framework release note for the plugin
|
||||
|
||||
The plugin now compiles only against a framework that has `pact.NavigationItem.Visible`. Development works through the plugin's `go.work` replace (`../summercms.go`); its `go.mod` was left unchanged (still requires the last tagged framework) and must be bumped only after a framework release containing commit `06e2e60` is tagged. No application pointer was bumped.
|
||||
|
||||
## What was built
|
||||
|
||||
### Task 1: framework hook (`06e2e60`)
|
||||
|
||||
- `pact.NavigationItem.Visible func(ctx context.Context) bool` (tag `json:"-"`), nil = always listed; field and type doc comments describe the contract (cheap, no panic, true when unsure, runs after the permission check, a hidden main item takes its side menu).
|
||||
- `cabana.Registry.Metadata`: skips a main item whose Visible answers false (after `Allows`); while walking the side menu skips a child whose Visible answers false after its `Allows` check and records its controller in a local `hidden` set; `openableTarget` now takes that set and treats `item.Controller` as unusable when it is in it, so the parent links to the first visible openable child, or "" when none.
|
||||
- `TestNavigationHidesInvisibleItemsAndRepointsTarget` covers: hidden side item absent while a nil-Visible sibling stays; parent repointed to the first visible openable child; parent with Visible false dropped although its child is visible; parent whose only child is hidden ends with empty SideMenu and empty Controller; Visible receives the context passed to Metadata.
|
||||
- Prose-only docs: pact README (Features bullet, API row), cabana README (Features bullet, `GET /navigation` route row), `docs/backend/users-and-permissions.md`, `docs/backend/admin-controllers.md`. The `src=` Go examples were not touched; `NavigationEntry`, registry validation, swag/OpenAPI untouched.
|
||||
|
||||
### Task 2: callout spacing (`2dd5fb3`, `ab9f385`)
|
||||
|
||||
- `.summer-partial .summer-callout:not(:last-child) { margin-bottom: 12px; }` next to the base `.summer-callout { margin: 0 }` rule (unchanged, still the only `.summer-callout {` rule).
|
||||
- `npm --prefix admin run build` and the rebuilt `modules/boardwalk/dist` committed; `scripts/check-admin-dist.sh`: "modules/boardwalk/dist matches a fresh build".
|
||||
|
||||
### Task 3: plugin (`16484ba`)
|
||||
|
||||
- `navVisibility(models.Settings) (orders, quotes, shippings bool)`: `OrdersEnabled`; `OrdersEnabled && UseQuotes`; `OrdersEnabled && ShippingsEnabled` (the callout rules).
|
||||
- `(*Plugin).navSettings(ctx) (models.Settings, bool)`: booted app -> `app.Lookup[*gorm.DB]` -> `HasTable` -> `models.LoadSettings`, 5 s timeout from the request context, no logging, false on every fallback.
|
||||
- `Visible` closures on orders, quotes, quoteables and shippings (true when settings cannot be loaded); payments has none; top-level `Controller` is `controllers.OrdersControllerID`; `ordersEnabledAtBoot` deleted; the registered-controller filter kept; `Navigation` doc comment rewritten.
|
||||
- The five header templates wrap the `<dl class="summer-stats">` block in `{{- if .Stats -}}` ... `{{- end -}}`; everything else byte-identical.
|
||||
- `admin_navigation_test.go` (`package paymentgateway`, short): `TestNavigationVisibility` with a six-case table over the settings flags and a shape check of `(&Plugin{}).Navigation()` (one parent opening Orders, payments nil Visible, the other four non-nil and true with `context.Background()`).
|
||||
- README navigation paragraph rewritten to request-time visibility; the partials table rows and the exported-API list are unchanged; `Navigation` stays exported with the same signature (no breaking change to this core plugin).
|
||||
|
||||
## Verification (real output)
|
||||
|
||||
Framework (`summercms.go`):
|
||||
|
||||
- `go vet ./... && go test -short ./... -count=1`: exit 0, every package `ok`.
|
||||
- `go test -short ./modules/cabana -run 'TestNavigationHidesInvisibleItemsAndRepointsTarget|TestNavigationDropsDeniedParentAndRepointsTarget' -v`: both PASS.
|
||||
- `go test ./cmd/summer -run TestDocsTree -count=1`: ok.
|
||||
- `go run ./cmd/summer docs:build --check`: `docs:build: no problems found`.
|
||||
- `scripts/check-admin-dist.sh`: `check-admin-dist: modules/boardwalk/dist matches a fresh build`.
|
||||
- `git show --name-only HEAD` of `06e2e60` touches neither `admin/openapi` nor `modules/cabana/admin_openapi.go`.
|
||||
- `git status --porcelain -- modules admin docs`: empty (the other sessions' `.planning/STATE.md`, `.planning/state.json`, `.msd/`, `17-VERIFICATION.md` remain untouched and unstaged).
|
||||
|
||||
Plugin (`sm-paymentgateway-plugin`):
|
||||
|
||||
- `go vet ./... && go test -short ./... -count=1`: exit 0, every package `ok`.
|
||||
- `go test -short . -run TestNavigationVisibility -v`: PASS with 6 subtests.
|
||||
- Extra, beyond the plan: `go test . -run TestOrdersAdminSmoke -count=1` (Docker/testcontainers Postgres, not short): ok in 5.5 s. Its `navigation` subtest fetches `GET /navigation` through the booted app with `orders_enabled`, `shippings_enabled` and `use_quotes` on and still sees Orders, Quotes and Quote items for an `access_orders` admin, so the `Visible` closures and `navSettings` work against a live database.
|
||||
- 5 templates contain `{{- if .Stats -}}`; `grep -c 'Visible:' admin_navigation.go` = 4; README mentions `Visible`; `git diff --quiet HEAD -- go.mod go.sum`: unchanged; `git status --porcelain`: empty; `git tag --points-at HEAD`: empty.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None - plan executed exactly as written. (The Docker-backed smoke run above is an extra check, not a change.)
|
||||
|
||||
## Deferred Issues
|
||||
|
||||
Pre-existing, out of scope, logged in `deferred-items.md` next to this file: `gofmt -l` reports `modules/cabana/account_tokens.go`, `account_tokens_test.go`, `token_postgres_smoke_test.go`, `token_routes_test.go`, `token_scopes_test.go` (framework) and `mail_smoke_test.go` (plugin) as unformatted. None was touched.
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None beyond the plan's register: the only new run-time surface is the `Visible` call on `GET /navigation`, which the plan's T-261016-s8x-01/02 cover (runs after the permission check; one bounded settings load that answers true on failure).
|
||||
|
||||
## State notes
|
||||
|
||||
`.planning/STATE.md` and `.planning/state.json` were not modified: both carry other sessions' uncommitted edits and the task constraints say to leave them alone. ROADMAP.md was not touched (quick task). Docs artifacts (this SUMMARY, PLAN.md, deferred-items.md) are left uncommitted for the orchestrator's docs commit.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
All listed files exist and commits 06e2e60, 2dd5fb3, ab9f385 (summercms.go) and 16484ba (sm-paymentgateway-plugin) are in history.
|
||||
@@ -0,0 +1,6 @@
|
||||
# Deferred items (quick 261016-s8x)
|
||||
|
||||
Out of scope, pre-existing, not touched by this task:
|
||||
|
||||
- `gofmt -l modules/cabana` lists `account_tokens.go`, `account_tokens_test.go`, `token_postgres_smoke_test.go`, `token_routes_test.go`, `token_scopes_test.go` as unformatted (framework).
|
||||
- `gofmt -l .` in sm-paymentgateway-plugin lists `mail_smoke_test.go` as unformatted.
|
||||
11
.planning/todos/pending/admin-action-audit-log.md
Normal file
11
.planning/todos/pending/admin-action-audit-log.md
Normal file
@@ -0,0 +1,11 @@
|
||||
---
|
||||
title: Audit-log admin action runs with the admin id (toolbar actions)
|
||||
date: 2026-10-16
|
||||
priority: low
|
||||
area: cabana / admin
|
||||
g15office: xkujbsv4
|
||||
---
|
||||
|
||||
cabana logs bulk action runs with `admin_id` (`modules/cabana/actions.go:166`), but a toolbar action such as pgsub's `sync_all_to_stripe` only logs failures (`runAction`, `actions.go:277-315`). A successful run leaves no record of which administrator triggered it; pgsub's per-plan "plan synced" line has no admin id.
|
||||
|
||||
Found by the Phase 18.1 security audit (T-18.1-22, accepted as AR-18.1-03 in `.planning/phases/18.1-payment-gateway-subscriptions/18.1-SECURITY.md`). Options the user named: a separate audit-log plugin, or extending cabana so every action run (toolbar, bulk, record) is logged or emitted as an event with the admin id, action name and controller.
|
||||
@@ -2360,6 +2360,10 @@
|
||||
"$ref": "#/components/schemas/cabana.FieldNotice"
|
||||
},
|
||||
"type": "array"
|
||||
},
|
||||
"title": {
|
||||
"description": "Title is the controller's record title (pact.FormRecordTitle), omitted\nwhen the controller names none.",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
5
admin/src/api/schema.d.ts
vendored
5
admin/src/api/schema.d.ts
vendored
@@ -6158,6 +6158,11 @@ export interface components {
|
||||
[key: string]: components["schemas"]["cabana.RelationOption"][];
|
||||
};
|
||||
notices?: components["schemas"]["cabana.FieldNotice"][];
|
||||
/**
|
||||
* @description Title is the controller's record title (pact.FormRecordTitle), omitted
|
||||
* when the controller names none.
|
||||
*/
|
||||
title?: string;
|
||||
};
|
||||
"cabana.RelationColumn": {
|
||||
key: string;
|
||||
|
||||
@@ -468,6 +468,11 @@
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
/* Spaces a callout from the stats strip (or any sibling) that follows it inside a rendered partial. */
|
||||
.summer-partial .summer-callout:not(:last-child) {
|
||||
margin-bottom: 12px;
|
||||
}
|
||||
|
||||
.summer-callout--warning {
|
||||
background: var(--c-sel);
|
||||
color: var(--c-text);
|
||||
|
||||
@@ -78,6 +78,8 @@ const recordId = mode === 'update' ? Number(route.params.id) : null
|
||||
const schema = ref<FormView | null>(null)
|
||||
const values = ref<AdminRecord>({})
|
||||
const labels = ref<RecordMeta['labels']>({})
|
||||
/** The controller's record title (meta.title, pact.FormRecordTitle); '' when it names none. */
|
||||
const recordTitle = ref('')
|
||||
const errors = ref<Record<string, string[]>>({})
|
||||
// The text of a save the server refused with 403 (UI-SPEC S6). It is a
|
||||
// banner, not a toast: it stays readable until the next save attempt.
|
||||
@@ -157,9 +159,11 @@ const panelFields = computed(() => {
|
||||
return fields.value.filter((field) => tabOf(field) === key)
|
||||
})
|
||||
|
||||
/** The record's display name: the first text-like field's value, in schema order. */
|
||||
const recordName = computed(() =>
|
||||
recordDisplayName(schema.value?.fields, values.value, schemaEnabledLocales(schema.value?.meta)),
|
||||
/** The record's display name: the controller's title, else the first text-like field's value, in schema order. */
|
||||
const recordName = computed(
|
||||
() =>
|
||||
recordTitle.value ||
|
||||
recordDisplayName(schema.value?.fields, values.value, schemaEnabledLocales(schema.value?.meta)),
|
||||
)
|
||||
|
||||
const title = computed(() => {
|
||||
@@ -217,6 +221,7 @@ function adopt(record: RecordEnvelope | undefined): void {
|
||||
}
|
||||
values.value = next
|
||||
labels.value = record.meta.labels ?? {}
|
||||
recordTitle.value = record.meta.title ?? ''
|
||||
}
|
||||
saved.value = snapshot(editablePayload(fields.value, values.value, mode))
|
||||
}
|
||||
|
||||
@@ -54,6 +54,8 @@ const recordPath = `${listPath}/${recordId}`
|
||||
const schema = ref<FormView | null>(null)
|
||||
const values = ref<AdminRecord>({})
|
||||
const labels = ref<RecordMeta['labels']>({})
|
||||
/** The controller's record title (meta.title, pact.FormRecordTitle); '' when it names none. */
|
||||
const recordTitle = ref('')
|
||||
const actions = ref<RecordAction[]>([])
|
||||
const loading = ref(true)
|
||||
const failed = ref(false)
|
||||
@@ -112,9 +114,11 @@ const panelFields = computed(() => {
|
||||
return fields.value.filter((field) => tabOf(field) === key)
|
||||
})
|
||||
|
||||
/** The record's display name: the first text-like field's value, in schema order. */
|
||||
const recordName = computed(() =>
|
||||
recordDisplayName(schema.value?.fields, values.value, schemaEnabledLocales(schema.value?.meta)),
|
||||
/** The record's display name: the controller's title, else the first text-like field's value, in schema order. */
|
||||
const recordName = computed(
|
||||
() =>
|
||||
recordTitle.value ||
|
||||
recordDisplayName(schema.value?.fields, values.value, schemaEnabledLocales(schema.value?.meta)),
|
||||
)
|
||||
|
||||
const subtitle = computed(() => message(schema.value?.messages.preview, undefined, { name: recordName.value }))
|
||||
@@ -146,6 +150,7 @@ async function loadRecord(): Promise<boolean> {
|
||||
}
|
||||
values.value = { ...result.data.data }
|
||||
labels.value = result.data.meta.labels ?? {}
|
||||
recordTitle.value = result.data.meta.title ?? ''
|
||||
actions.value = result.data.meta.actions ?? []
|
||||
return true
|
||||
} catch {
|
||||
|
||||
@@ -623,6 +623,14 @@ describe('password and preset on the form (UI-SPEC S7, D-19, D-27 G7)', () => {
|
||||
expect(editor(wrapper, 'slug').element.value).toBe('ada-lovelace')
|
||||
})
|
||||
|
||||
it("titles a record by the controller's meta.title when it sends one, and publishes it as the crumb", async () => {
|
||||
const record = clone(rosterRecordFixture)
|
||||
;(record.meta as { title?: string }).title = 'Person #1 (Ada)'
|
||||
const { wrapper } = await mountApp('/acme/roster/people/1', people({ [`GET ${PERSON}`]: { body: record } }))
|
||||
expect(wrapper.find('h1').text()).toBe('Person #1 (Ada)')
|
||||
expect(recordCrumb.value).toBe('Person #1 (Ada)')
|
||||
})
|
||||
|
||||
it('titles a record by the default locale of an mltext first field, and publishes it as the crumb', async () => {
|
||||
const record = clone(rosterRecordFixture)
|
||||
record.data.name = { en: 'Ada Lovelace', pl: 'Ada Lovelace PL' }
|
||||
|
||||
@@ -139,6 +139,19 @@ describe('preview view (UI-SPEC S3, D-11)', () => {
|
||||
expect(recordCrumb.value).toBe('Ada Lovelace')
|
||||
})
|
||||
|
||||
it("heads the record by the controller's meta.title when it sends one, before the first text field", async () => {
|
||||
const { wrapper } = await mountApp(
|
||||
PREVIEW,
|
||||
routes({
|
||||
[`GET ${RECORD}`]: recordWith((record) => {
|
||||
;(record.meta as { title?: string }).title = 'Person #1 (Ada)'
|
||||
}),
|
||||
}),
|
||||
)
|
||||
expect(wrapper.find('h1').text()).toBe('Person #1 (Ada)')
|
||||
expect(recordCrumb.value).toBe('Person #1 (Ada)')
|
||||
})
|
||||
|
||||
it('shows the muted dash for an empty value and the label of a relation, never its id', async () => {
|
||||
const { wrapper } = await mountApp(PREVIEW, routes({ [`GET ${RECORD}`]: recordWith((record) => (record.data.joined_ip = null)) }))
|
||||
expect(wrapper.find('[data-preview-field="joined_ip"] [data-empty]').text()).toBe('—')
|
||||
|
||||
@@ -158,7 +158,7 @@ The generated controller requires the permission `acme.blog.access_posts`, so de
|
||||
|
||||
## Compilation at boot
|
||||
|
||||
At start-up `cabana.Activate` compiles every plugin's controllers, settings, navigation and permissions once. Any schema mistake stops the start-up with an error that names the plugin, the controller and the file: an unknown YAML key, a `modelClass` that does not match `pact.AdminController.ModelName`, a list column the model does not have, an unknown field type. Nothing is parsed per request. The example below activates the admin for the plugin above:
|
||||
At start-up `cabana.Activate` compiles every plugin's controllers, settings, navigation and permissions once. Any schema mistake stops the start-up with an error that names the plugin, the controller and the file: an unknown YAML key, a `modelClass` that does not match `pact.AdminController.ModelName`, a list column the model does not have, an unknown field type. Nothing is parsed per request. The one run-time hook is `pact.NavigationItem.Visible`, which runs on every `GET /navigation` so a plugin can hide a screen whose feature is off; it must be cheap and answer true when unsure. The example below activates the admin for the plugin above:
|
||||
|
||||
```go src=modules/cabana/example_test.go#ExampleActivate
|
||||
cfg, err := compass.Open(compass.Options{Dir: "config", Env: "development", Environ: []string{}})
|
||||
|
||||
@@ -279,6 +279,7 @@ messages:
|
||||
- The screen shows every field whose `context` allows `preview`. A field without a `context` shows on every screen; `context: preview` shows a field only here. Values are rendered as text; `widget` and `relation-manager` fields are not shown.
|
||||
- A field with `context: preview` is never written. A save asks only for the `create` or the `update` context, so such a field in a request body is dropped like any field hidden on that form.
|
||||
- `messages.preview` is the screen's subtitle and `messages.edit` the label of its edit button; both default to framework texts (`cabana.FormMessages`).
|
||||
- The heading of the preview and of the update form is the record's name: the value of the form's first `text` (or multilingual text) field. A controller that implements `pact.FormRecordTitle` names the record itself, for example "Subscription #12 (Pro)": the show, create and update responses then carry its answer as `meta.title` (`cabana.RecordMeta`), which the SPA shows instead. The hook gets the loaded record and the request's database handle; an error fails the request.
|
||||
- `recordUrl` in `config_list.yaml` and the form's `create.redirect`, `update.redirectClose` and the other redirects may point at the screen as `<vendor>/<plugin>/<controller>/preview/:id`. On the update form of a record with a preview, the back arrow and Cancel return to the preview; after a delete the form goes to the list.
|
||||
- The footer holds the record actions the show response offers in `meta.actions`, then the edit button. After an action the record and the status hint are loaded again in place.
|
||||
|
||||
|
||||
@@ -138,6 +138,67 @@ The choices of a scope filter are asked from the controller first, when it imple
|
||||
|
||||
WinterCMS `conditions` SQL fragments are not supported; use a `column` or a scope the model implements. A scope filter whose name the model does not list in `FilterScopes` stops the start-up, so request text can never select another method.
|
||||
|
||||
## Computed columns
|
||||
|
||||
A column can show a value that is not stored on the model, such as the e-mail of a record's owner kept in another table. The model declares the field with a gorm `-` tag and a column name, and the controller implements `pact.ListExtendRecords`, which the framework calls once per list page, after the records are loaded and before their columns are read, with the page's records and the list's database handle. One query fills the field for the whole page, as WinterCMS's `listExtendRecords` does:
|
||||
|
||||
```go src=modules/cabana/example_extendrecords_test.go
|
||||
package cabana_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
// Ticket is a support ticket. OwnerEmail is not stored: the list's
|
||||
// owner_email column shows what ListExtendRecords puts there.
|
||||
type Ticket struct {
|
||||
ID uint `gorm:"column:id;primaryKey"`
|
||||
OwnerID uint `gorm:"column:owner_id"`
|
||||
OwnerEmail string `gorm:"-:all;column:owner_email"`
|
||||
}
|
||||
|
||||
// TicketsController is an admin controller with a computed column.
|
||||
type TicketsController struct{}
|
||||
|
||||
var _ pact.ListExtendRecords = TicketsController{}
|
||||
|
||||
// ListExtendRecords fills the owner e-mails of one list page with one
|
||||
// query. The framework calls it once per page, before the columns are read.
|
||||
func (TicketsController) ListExtendRecords(ctx context.Context, db *gorm.DB, records []any) error {
|
||||
ids := make([]uint, 0, len(records))
|
||||
for _, record := range records {
|
||||
ids = append(ids, record.(*Ticket).OwnerID)
|
||||
}
|
||||
var owners []struct {
|
||||
ID uint
|
||||
Email string
|
||||
}
|
||||
if err := db.WithContext(ctx).Table("acme_owners").Select("id, email").Where("id IN ?", ids).Scan(&owners).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
emails := make(map[uint]string, len(owners))
|
||||
for _, owner := range owners {
|
||||
emails[owner.ID] = owner.Email
|
||||
}
|
||||
for _, record := range records {
|
||||
ticket := record.(*Ticket)
|
||||
ticket.OwnerEmail = emails[ticket.OwnerID]
|
||||
}
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
```yaml
|
||||
columns:
|
||||
owner_email:
|
||||
label: acme.helpdesk::lang.tickets.owner_email
|
||||
```
|
||||
|
||||
Such a column is never sorted or searched: it compiles unsortable, and `sortable: true` or `searchable: true` on it stops the start-up. An error from the hook fails the list.
|
||||
|
||||
## Row state
|
||||
|
||||
A list can mark rows with a state, as a WinterCMS list does with `listInjectRowClass`: a deleted record, a blocked account, an inactive one. The controller implements `pact.ListRowStates`:
|
||||
|
||||
@@ -66,7 +66,7 @@ func (p *BlogPlugin) Navigation() []pact.NavigationItem {
|
||||
}
|
||||
```
|
||||
|
||||
A controller's `pact.AdminPermissioned.RequiredPermissions` are checked before any schema is served or query runs, and navigation and settings entries are filtered by the permissions they name, so an administrator sees only what they may open: a main menu item the administrator may not open is dropped even when one of its side-menu entries would pass, and an allowed item that links to a controller the administrator cannot open links to its first openable side-menu entry instead. `cabana.Allows` is the check and follows Winter's `hasAnyAccess` for JWT sessions: superusers pass, an administrator needs any one of the listed codes, and an empty requirement list allows any signed-in administrator. Wildcards match on both sides: a grant ending in `.*` covers every code with that prefix, and a required code such as `acme.blog.*` is met by any grant under `acme.blog.`. When the request carries a personal API token with a non-full scope, the scope is checked first (see [API tokens](#api-tokens)). The last lines of the activation example on [Admin controllers](admin-controllers.md) show it.
|
||||
A controller's `pact.AdminPermissioned.RequiredPermissions` are checked before any schema is served or query runs, and navigation and settings entries are filtered by the permissions they name, so an administrator sees only what they may open: a main menu item the administrator may not open is dropped even when one of its side-menu entries would pass, and an allowed item that links to a controller the administrator cannot open links to its first openable side-menu entry instead. A plugin can also hide an entry per request with `pact.NavigationItem.Visible` (for a screen whose feature is turned off in its settings); the same drop and relink rules apply. `cabana.Allows` is the check and follows Winter's `hasAnyAccess` for JWT sessions: superusers pass, an administrator needs any one of the listed codes, and an empty requirement list allows any signed-in administrator. Wildcards match on both sides: a grant ending in `.*` covers every code with that prefix, and a required code such as `acme.blog.*` is met by any grant under `acme.blog.`. When the request carries a personal API token with a non-full scope, the scope is checked first (see [API tokens](#api-tokens)). The last lines of the activation example on [Admin controllers](admin-controllers.md) show it.
|
||||
|
||||
An administrator's grants are the role's `permissions` merged with the administrator's own `backend_users.permissions`, the way WinterCMS merges them: the administrator's value for a code replaces the role's, and only `1` grants. A `-1` (or `0`) on the administrator therefore removes a permission the role grants, so rows copied from a WinterCMS database keep their denies. As in WinterCMS the merge compares codes exactly, so denying `acme.blog.access_posts` does not take it back from a role that grants `acme.blog.*`.
|
||||
|
||||
|
||||
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
1
modules/boardwalk/dist/assets/index-GIpW_WLy.css
vendored
Normal file
1
modules/boardwalk/dist/assets/index-GIpW_WLy.css
vendored
Normal file
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
4
modules/boardwalk/dist/index.html
vendored
4
modules/boardwalk/dist/index.html
vendored
@@ -7,8 +7,8 @@
|
||||
<meta name="summer-admin-base" content="__SUMMER_ADMIN_BASE__" />
|
||||
<link rel="icon" href="./favicon.ico" sizes="48x48" />
|
||||
<title>SummerCMS</title>
|
||||
<script type="module" crossorigin src="./assets/index-CxofrFdB.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="./assets/index-e1TjFTR3.css">
|
||||
<script type="module" crossorigin src="./assets/index-DzKf4ihI.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="./assets/index-GIpW_WLy.css">
|
||||
</head>
|
||||
<body>
|
||||
<div id="app"></div>
|
||||
|
||||
@@ -31,6 +31,8 @@ Use cases:
|
||||
- Toolbar actions: `toolbar.buttons` in `config_list.yaml` lists the built-in `create` and `delete` next to names the controller registers through `pact.HasAdminActions`. Registered actions share one namespace with widget actions, `create` and `delete` are reserved, and each toolbar action needs a label. The list schema's `toolbarActions` carries only the actions the requesting administrator may run, with localized labels; an unknown name fails boot.
|
||||
- Bulk actions: `bulkActions` in `config_list.yaml` lists names the controller registers through `pact.HasAdminBulkActions`; it needs `showCheckboxes: true`. Bulk actions have their own namespace (`create` and `delete` are reserved there too), and each needs a label. The posted ids are resolved and row-locked through `pact.ListExtendQuery` in one transaction and the action receives the loaded records, never ids: a selection that matches nothing answers `affected: 0` without running the action, and a partial match answers 409 and rolls back. The list schema's `bulkActions` carries the built-in `delete` and only the declared actions the requesting administrator may run, with localized `label` and `confirm`; an unknown or duplicate name fails boot. Each run is logged with the controller, action, administrator and affected count. A detached action (`pact.AdminBulkAction.Detached`) opens no transaction: its records are loaded through the same scope and rules without row locks, `TxFromContext` reports none, and `Run` opens its own short transactions, so no transaction or lock is held across an external call.
|
||||
- Invisible columns and filter choices: `invisible: true` in `columns.yaml` keeps a column searchable and sortable on the server while the schema flags it, the SPA does not render it and list rows do not carry it. The choices of a scope filter are served by the controller when it implements `pact.FilterOptions`, else by the model.
|
||||
- Computed columns: a controller implementing `pact.ListExtendRecords` is called once per list page, after the records are loaded and before their columns are read, so a column of a field gorm ignores (a `-` tag with a column name) shows the value the hook set. Such a column compiles unsortable; `sortable: true` or `searchable: true` on it fails the boot. A hook error fails the list with the generic 500.
|
||||
- Record title: a controller implementing `pact.FormRecordTitle` names a record; the show, create and update responses carry it as `meta.title` (`RecordMeta.Title`), and the admin heads the form and preview screens with it instead of the first text field's value.
|
||||
- Row state: a controller implementing `pact.ListRowStates` is called once per list page with the page's records and the list's database handle. The list response carries `meta.row_states`, keyed by row id, with values from the fixed set `deleted`, `negative`, `disabled` in that order; a value outside the set is dropped and logged, rows without a state are left out, and a controller without the hook sends no `row_states` key. The badge texts are the list messages `rowStateDeleted`, `rowStateNegative` and `rowStateDisabled`, defaulting to `backend::lang.messages.list.row_state_*`. A soft-deleted record that the controller's `pact.ListExtendQuery` and `pact.FormExtendQuery` include can be shown, updated (it stays soft-deleted), targeted by bulk and record actions and removed for good by the controller's `pact.FormAfterDelete`.
|
||||
- Record actions: `recordActions` in `config_form.yaml` lists names the controller registers through `pact.HasAdminRecordActions`, a third action namespace with the same reserved names. It needs the form's `preview` block: record actions are offered on the preview screen, and a form that declares them without one fails boot. The show response's `meta.actions` (`cabana.RecordAction` entries with localized `label` and `confirm`) carries only the declared actions the requesting administrator may run and whose `Applies` reports true for the record; the key is absent when none is offered, and create and update responses never carry it. The action route loads the record through `pact.FormExtendQuery` with a row lock in one transaction (one 404 for a missing and an out-of-scope id), checks `Applies` again (409 when it reports false) and then runs the action. An unknown or duplicate name, or an action without a label, fails boot. Each run is logged with the controller, action, administrator and record id.
|
||||
- Preview screen: a `preview` mapping in `config_form.yaml` (`preview: {}`, or with `headerPartial: <name>` for a status hint) gives the form a read-only record screen in the admin SPA. The form schema reports it as `preview` (`cabana.FormPreview`), fields with `context: preview` are shown only there and are never written by a save, `messages.preview` and `messages.edit` name the screen's subtitle and edit button, and `recordUrl` and the form redirects may point at it as `.../preview/:id`. An empty `preview:` key or an unknown key inside it fails boot.
|
||||
@@ -53,7 +55,7 @@ Use cases:
|
||||
- Refusals: a lifecycle hook, a relation hook or a bulk, record, toolbar or widget action that returns a `cabana.ForbiddenError` is answered 403 `forbidden` with the error's `Message` and `Details` (a field name to a list of messages), both translated in the request locale; an empty `Message` stays empty. The surrounding transaction is rolled back. Every other error that is not a `cabana.ValidationError` stays the opaque 500, logged on the server.
|
||||
- Danger record actions: `pact.AdminRecordAction.Danger` is served as `danger` on the record response's `cabana.RecordAction` (absent when false, so existing payloads are unchanged); the admin's confirm dialog then uses its danger variant.
|
||||
- Singleton settings screens declared with `pact.HasSettings`, read and saved by `cabana.SettingsService`. A `type: password` field on a settings form is a write-only secret on a `lagoon.Encrypted` column (any other column type fails boot naming the field): the GET omits it from `data` and reports `stored`, `env` or `empty` per field in `meta.secrets` (`cabana.SettingsMeta`; `env` when the column is empty and the model's `cabana.SecretFallbacks` config key resolves to a non-empty value); on PUT an absent key or an empty string keeps the stored value, JSON null clears it, a non-string is a 422 on the field, and the answer carries the new states, never a value. A `type: richeditor` on a settings form is sanitized on save exactly like a controller save, with the removals in `meta.notices`; `repeater` and `dependsOn` stay refused. Settings fields keep their `tab`. A `type: dropdown` whose `options` names a method takes its choices from the settings model, which must implement `pact.DropdownOptionsProvider` (boot fails naming the field otherwise); the settings schema serves them localized. A settings entry whose `Controller` field is set links to that admin controller instead: start-up fails when it also declares a model, form or model factory, or names an unknown controller; `GET /settings` lists it, with its `controller`, only to administrators who pass its permissions and may open the controller; and the singleton settings endpoints answer 404 for its code.
|
||||
- Backend navigation (`pact.HasNavigation`) and permissions (`pact.HasPermissions`), filtered per user by `cabana.Registry.Metadata`. An administrator's own `backend_users.permissions` are merged over the role's as in Winter (a `-1` denies a code the role grants). `cabana.Allows` implements the permission check with Winter's `hasAnyAccess` semantics for JWT sessions: superusers pass, a principal needs any one of the listed codes, and wildcards match on both sides (a grant ending in `.*` covers every code with that prefix, and a required code ending in `.*` is met by any grant under it). When the principal carries a non-full `bouncer.ScopeLimit` (a personal API token), the scope is checked first: one method-allowed scope must match every required code and the owner must hold a required code inside that scope; a superuser with a scoped token is limited to the scope, and an empty requirement list is refused unless the scope is full.
|
||||
- Backend navigation (`pact.HasNavigation`) and permissions (`pact.HasPermissions`), filtered per user by `cabana.Registry.Metadata`. An entry whose `pact.NavigationItem.Visible` func answers false for the request is left out like a denied one (a hidden main item with its side menu), and a main item whose controller is a side item hidden this way links to its first visible openable side item. An administrator's own `backend_users.permissions` are merged over the role's as in Winter (a `-1` denies a code the role grants). `cabana.Allows` implements the permission check with Winter's `hasAnyAccess` semantics for JWT sessions: superusers pass, a principal needs any one of the listed codes, and wildcards match on both sides (a grant ending in `.*` covers every code with that prefix, and a required code ending in `.*` is met by any grant under it). When the principal carries a non-full `bouncer.ScopeLimit` (a personal API token), the scope is checked first: one method-allowed scope must match every required code and the owner must hold a required code inside that scope; a superuser with a scoped token is limited to the scope, and an empty requirement list is refused unless the scope is full.
|
||||
- Admin authentication against WinterCMS's `backend_users` and `backend_user_roles` tables (`cabana.BackendUser`, `cabana.BackendUserRole`, `cabana.BackendUsers`): a composite `backend` guard (a guard another plugin already registered under that name fails `cabana.Activate`) that accepts an `Authorization: Bearer` personal API token (`cabana.APITokenPrefix` / `g15_…`) or, otherwise, the unchanged admin JWT path (Bearer JWT, then the admin cookie). Personal tokens never come from a cookie or the query string. Login throttling, token refresh and revocation, and the SPA cookie transport are unchanged. Cookie-authenticated requests that change state must carry `X-Requested-With: XMLHttpRequest`, which blocks cross-site request forgery.
|
||||
- Personal API tokens: administrators mint scoped tokens stored in `backend_api_tokens` (`cabana.BackendAPIToken`); only the sha256 hash and a four-character hint are kept. Framework scopes are `*` (full access as the owner on every method) and `admin:read` (as the owner on GET and HEAD); plugins add scopes through `pact.HasTokenScopes`. Interactive-only routes (account profile, avatar, token management, refresh and logout) refuse every personal token with `details.reason` `interactive_only`. Routes outside `cabana.Allows` (`GET /field-types`, `POST /markdown/preview`) are gated so only a full scope, or `admin:read` on GET, passes. Plugin routes resolve a backend principal with `cabana.ResolveBackend` (Bearer only; never the admin cookie).
|
||||
- My account: the signed-in administrator opens `/account` from the user menu and edits their own first name, last name, email and subtitle, and changes their password with the current one (PUT `/account`, body `cabana.AdminAccountRequest`). The row is always the token subject, never a request value, and any key outside the request type (login, role, superuser flag, permissions, activation) is a 422 on that key with nothing written. The subtitle is stored where WinterCMS keeps it, under `subtitle` in the jsonable `backend_users.metadata` column (`cabana.BackendUser.Metadata`), and is read with `cabana.BackendUser.Subtitle`, which treats NULL, `[]` and malformed metadata as an empty subtitle; saving it keeps every other metadata key. A password change ends every other session through `tokens_valid_after` and gives a cookie session a fresh cookie. The same screen uploads and removes the administrator's avatar (POST and DELETE `/account/avatar`): a public attachOne in `system_files` under WinterCMS's morph name (`cabana.BackendUserMorphType`, field `cabana.BackendUserAvatarField`; `cabana.BackendUser` implements `attach.Owner` and `attach.HasRelations`), so avatar rows copied from WinterCMS are picked up as they are. Only jpg, jpeg, png, gif and webp images that pass the image check, of at most `cabana.AvatarMaxBytes` (2 MiB), are stored, under a server-generated name. Plugins read the picture with `cabana.BackendUserAvatar`. The administrator's UI preferences (GET and PUT `/account/preferences`, `cabana.AdminPreferences`) live in the same metadata column under `admin_theme` and `admin_locale`; saving them keeps every other key.
|
||||
@@ -78,7 +80,7 @@ Guarded routes run surf's `locale.from-principal` after the backend guard: an in
|
||||
| GET `/account/tokens`, POST `/account/tokens` | List the caller's personal API tokens (newest first; revoked only with `?revoked=1`) and create one (`cabana.AdminAPITokenRequest`: `name`, `expires`, `scopes`). Create answers 201 with `cabana.AdminAPITokenCreated` (plaintext `token` once). Interactive session only. |
|
||||
| DELETE `/account/tokens/{id}` | Soft-revoke one of the caller's own active tokens (`cabana.AdminAPIToken`); 404 otherwise. Interactive session only. |
|
||||
| GET `/account/tokens/scopes` | Scopes the caller may assign (`cabana.AdminAPITokenScope`), including `*`, `admin:read` and overlapping plugin scopes. Interactive session only. |
|
||||
| GET `/navigation`, GET `/settings` | Navigation and settings entries the administrator may open; a settings entry's `controller` names the admin controller a link entry opens and is empty for a singleton form. |
|
||||
| GET `/navigation`, GET `/settings` | Navigation and settings entries the administrator may open and, for a navigation entry with a `Visible` func, that it answers true for the request; a settings entry's `controller` names the admin controller a link entry opens and is empty for a singleton form. |
|
||||
| GET `/field-types` | The plugin field types (`pact.HasFormFieldTypes`) sorted by name, each with its `element`, `translatable` flag and the versioned URLs of its `scripts` and `styles` (`cabana.FieldTypesResponse`); an empty list when no plugin registers one. The SPA loads them after sign-in. Needs any backend session. |
|
||||
| POST `/{vendor}/{plugin}/{controller}/{id}/fields/{field}/upload` | Store one multipart part named `file` through the `Upload` of the field's plugin field type and answer 201 `{url}` (`cabana.FieldUploadResponse`). `{id}` 0 is the create form and passes no record; any other id loads the record through the form scope (404 outside it). 403 when the controller does not declare the save the upload belongs to (create for `{id}` 0, which needs the list's `create` toolbar button, update otherwise), as the save routes answer. 404 for a field without a plugin type with `Upload` or whose context does not allow the operation, 413 over `http.body_limits.upload_bytes`, 422 on the field for a malformed body or a `pact.FormFieldError`. The body is read and checked whole before `Upload` runs, so a refused request stores nothing. |
|
||||
| GET `/settings/{code}/schema`, GET and PUT `/settings/{code}` | Settings form schema, values and update. |
|
||||
|
||||
@@ -611,6 +611,15 @@ func projectFullRecord(ctx context.Context, tx *gorm.DB, cc *CompiledController,
|
||||
if err := projectPermissionFields(withTx(ctx, tx), cc, model, data); err != nil {
|
||||
return RecordResult{}, err
|
||||
}
|
||||
if cc != nil {
|
||||
if hook, ok := cc.Controller.(pact.FormRecordTitle); ok && hook != nil {
|
||||
title, err := hook.FormRecordTitle(ctx, tx, model)
|
||||
if err != nil {
|
||||
return RecordResult{}, lifecycleFailure(cc, err)
|
||||
}
|
||||
meta.Title = title
|
||||
}
|
||||
}
|
||||
return RecordResult{Data: data, Meta: meta}, nil
|
||||
}
|
||||
|
||||
|
||||
46
modules/cabana/example_extendrecords_test.go
Normal file
46
modules/cabana/example_extendrecords_test.go
Normal file
@@ -0,0 +1,46 @@
|
||||
package cabana_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"git.golem15.com/golem15/summercms/modules/pact"
|
||||
"gorm.io/gorm"
|
||||
)
|
||||
|
||||
// Ticket is a support ticket. OwnerEmail is not stored: the list's
|
||||
// owner_email column shows what ListExtendRecords puts there.
|
||||
type Ticket struct {
|
||||
ID uint `gorm:"column:id;primaryKey"`
|
||||
OwnerID uint `gorm:"column:owner_id"`
|
||||
OwnerEmail string `gorm:"-:all;column:owner_email"`
|
||||
}
|
||||
|
||||
// TicketsController is an admin controller with a computed column.
|
||||
type TicketsController struct{}
|
||||
|
||||
var _ pact.ListExtendRecords = TicketsController{}
|
||||
|
||||
// ListExtendRecords fills the owner e-mails of one list page with one
|
||||
// query. The framework calls it once per page, before the columns are read.
|
||||
func (TicketsController) ListExtendRecords(ctx context.Context, db *gorm.DB, records []any) error {
|
||||
ids := make([]uint, 0, len(records))
|
||||
for _, record := range records {
|
||||
ids = append(ids, record.(*Ticket).OwnerID)
|
||||
}
|
||||
var owners []struct {
|
||||
ID uint
|
||||
Email string
|
||||
}
|
||||
if err := db.WithContext(ctx).Table("acme_owners").Select("id, email").Where("id IN ?", ids).Scan(&owners).Error; err != nil {
|
||||
return err
|
||||
}
|
||||
emails := make(map[uint]string, len(owners))
|
||||
for _, owner := range owners {
|
||||
emails[owner.ID] = owner.Email
|
||||
}
|
||||
for _, record := range records {
|
||||
ticket := record.(*Ticket)
|
||||
ticket.OwnerEmail = emails[ticket.OwnerID]
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -218,6 +218,7 @@ func compileColumns(pluginID string, ctl pact.AdminController, fsys fs.FS, colPa
|
||||
return nil, bootErr(pluginID, ctl.ID(), colPath, err)
|
||||
}
|
||||
modelCols, modelRels, checkModel := listModelContract(ctl)
|
||||
unstored := unstoredModelColumns(ctl)
|
||||
compiled := make([]ListColumn, 0, len(cols.Columns))
|
||||
seen := map[string]struct{}{}
|
||||
for _, item := range cols.Columns {
|
||||
@@ -280,6 +281,14 @@ func compileColumns(pluginID string, ctl pact.AdminController, fsys fs.FS, colPa
|
||||
if spec.Sortable != nil {
|
||||
sortable = *spec.Sortable
|
||||
}
|
||||
// A column of a field gorm ignores (filled by pact.ListExtendRecords)
|
||||
// has no database column: it is never sorted or searched.
|
||||
if _, ok := unstored[key]; ok && spec.Relation == "" {
|
||||
if sortable && spec.Sortable != nil || spec.Searchable {
|
||||
return nil, bootErr(pluginID, ctl.ID(), colPath, fmt.Errorf("column %s is not stored: it cannot be sortable or searchable", key))
|
||||
}
|
||||
sortable = false
|
||||
}
|
||||
compiled = append(compiled, ListColumn{
|
||||
Key: key,
|
||||
Label: spec.Label,
|
||||
@@ -538,6 +547,43 @@ func listModelContract(ctl pact.AdminController) (map[string]string, map[string]
|
||||
return cols, rels, true
|
||||
}
|
||||
|
||||
// unstoredModelColumns names the model fields gorm ignores (a "-" tag), by
|
||||
// their column tag or field name: values a controller fills through
|
||||
// pact.ListExtendRecords.
|
||||
func unstoredModelColumns(ctl pact.AdminController) map[string]struct{} {
|
||||
src, ok := ctl.(pact.AdminRecordSource)
|
||||
if !ok || src == nil {
|
||||
return nil
|
||||
}
|
||||
model := src.NewRecord()
|
||||
if model == nil {
|
||||
return nil
|
||||
}
|
||||
out := map[string]struct{}{}
|
||||
for _, mf := range modelFields(reflect.TypeOf(model)) {
|
||||
if !gormIgnored(mf.Field) {
|
||||
continue
|
||||
}
|
||||
name := gormColumn(mf.Field)
|
||||
if name == "" {
|
||||
name = mf.Field.Name
|
||||
}
|
||||
out[name] = struct{}{}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// gormIgnored reports a gorm "-" (or "-:all") tag.
|
||||
func gormIgnored(field reflect.StructField) bool {
|
||||
for _, part := range strings.Split(field.Tag.Get("gorm"), ";") {
|
||||
switch strings.TrimSpace(part) {
|
||||
case "-", "-:all":
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// listColumnKind is the Winter list type of a model field when columns.yaml
|
||||
// omits type: datetime for time.Time, date for lagoon.Date, time for
|
||||
// lagoon.TimeOfDay. Everything else stays text (the admin's default).
|
||||
|
||||
@@ -645,3 +645,33 @@ func filterCatalog(t *testing.T) *phrasebook.Catalog {
|
||||
}
|
||||
return cat
|
||||
}
|
||||
|
||||
// TestListUnstoredColumn: a column of a field gorm ignores (filled by
|
||||
// pact.ListExtendRecords) compiles unsortable, and declaring it sortable or
|
||||
// searchable fails the boot.
|
||||
func TestListUnstoredColumn(t *testing.T) {
|
||||
type row struct {
|
||||
ID uint `gorm:"column:id;primaryKey"`
|
||||
Name string `gorm:"column:name"`
|
||||
Badge string `gorm:"-:all;column:badge"`
|
||||
Note string `gorm:"-"`
|
||||
}
|
||||
ctl := filterController{schemaController: schemaController{model: "Widget"}, rec: &row{}}
|
||||
schema, err := CompileList("acme.demo", ctl, listFS(allColumnsConfig, "columns:\n name:\n label: Name\n badge:\n label: Badge\n Note:\n label: Note\n"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
sortable := map[string]bool{}
|
||||
for _, col := range schema.Columns {
|
||||
sortable[col.Key] = col.Sortable
|
||||
}
|
||||
if !sortable["name"] || sortable["badge"] || sortable["Note"] {
|
||||
t.Fatalf("sortable = %v, want only name", sortable)
|
||||
}
|
||||
for _, bad := range []string{"sortable: true", "searchable: true"} {
|
||||
_, err := CompileList("acme.demo", ctl, listFS(allColumnsConfig, "columns:\n badge:\n label: Badge\n "+bad+"\n"))
|
||||
if err == nil || !strings.Contains(err.Error(), "column badge is not stored") {
|
||||
t.Fatalf("%s: err = %v", bad, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,7 +35,11 @@ type SettingsEntry struct {
|
||||
}
|
||||
|
||||
// Metadata returns only entries the backend principal may open. Denied
|
||||
// entries are removed before any response value is constructed.
|
||||
// entries are removed before any response value is constructed. A navigation
|
||||
// entry with a pact.NavigationItem.Visible func is also left out when that
|
||||
// func answers false for ctx (a main item with its whole side menu, or one
|
||||
// side item); Visible runs only after the permission check has passed, so a
|
||||
// denied principal never runs plugin code.
|
||||
func (r *Registry) Metadata(ctx context.Context, principal *bouncer.Principal, tr *phrasebook.Translator) ([]NavigationEntry, []SettingsEntry) {
|
||||
navigation := make([]NavigationEntry, 0)
|
||||
settings := make([]SettingsEntry, 0)
|
||||
@@ -49,15 +53,29 @@ func (r *Registry) Metadata(ctx context.Context, principal *bouncer.Principal, t
|
||||
if !Allows(principal, item.Permissions) {
|
||||
continue
|
||||
}
|
||||
if item.Visible != nil && !item.Visible(ctx) {
|
||||
continue
|
||||
}
|
||||
children := make([]NavigationEntry, 0)
|
||||
// hidden collects the controllers of side items a Visible func hid
|
||||
// for this request, so the parent never links to one of them.
|
||||
// Permission-denied children are not recorded: the parent keeps its
|
||||
// own controller whenever the principal can open it.
|
||||
hidden := map[string]bool{}
|
||||
for _, child := range item.SideMenu {
|
||||
if !Allows(principal, child.Permissions) {
|
||||
continue
|
||||
}
|
||||
if child.Visible != nil && !child.Visible(ctx) {
|
||||
if child.Controller != "" {
|
||||
hidden[child.Controller] = true
|
||||
}
|
||||
continue
|
||||
}
|
||||
children = append(children, navigationView(ctx, tr, child, nil))
|
||||
}
|
||||
view := navigationView(ctx, tr, item, children)
|
||||
view.Controller = r.openableTarget(principal, item, children)
|
||||
view.Controller = r.openableTarget(principal, item, children, hidden)
|
||||
navigation = append(navigation, view)
|
||||
}
|
||||
sort.SliceStable(navigation, func(i, j int) bool {
|
||||
@@ -95,11 +113,13 @@ func (r *Registry) Metadata(ctx context.Context, principal *bouncer.Principal, t
|
||||
}
|
||||
|
||||
// openableTarget returns the controller a main item should link to. It is the
|
||||
// item's own controller unless the principal cannot open it, in which case it
|
||||
// is the first side-menu entry the principal can, so the menu never links to a
|
||||
// page that answers 403. It is empty when nothing is openable.
|
||||
func (r *Registry) openableTarget(principal *bouncer.Principal, item pact.NavigationItem, children []NavigationEntry) string {
|
||||
if item.Controller == "" || r.canOpen(principal, item.Controller) {
|
||||
// item's own controller unless the principal cannot open it or it is the
|
||||
// controller of a side item hidden for this request (in hidden), in which
|
||||
// case it is the first of the listed children (already visible and permitted)
|
||||
// the principal can open, so the menu never links to a page that answers 403
|
||||
// or to a screen whose feature is off. It is empty when nothing is openable.
|
||||
func (r *Registry) openableTarget(principal *bouncer.Principal, item pact.NavigationItem, children []NavigationEntry, hidden map[string]bool) string {
|
||||
if item.Controller == "" || (!hidden[item.Controller] && r.canOpen(principal, item.Controller)) {
|
||||
return item.Controller
|
||||
}
|
||||
for _, child := range children {
|
||||
|
||||
@@ -108,6 +108,74 @@ func TestNavigationDropsDeniedParentAndRepointsTarget(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
type navVisibleKey struct{}
|
||||
|
||||
// TestNavigationHidesInvisibleItemsAndRepointsTarget covers the
|
||||
// pact.NavigationItem.Visible rules: a side item whose Visible answers false
|
||||
// is left out while a sibling with no Visible stays, a parent whose own
|
||||
// controller is that hidden child's links to the first visible openable child
|
||||
// (or to nothing when none is), a parent whose Visible answers false is
|
||||
// dropped with its whole side menu, and Visible receives the context passed
|
||||
// to Metadata. The principal is a superuser so permissions are not a factor.
|
||||
func TestNavigationHidesInvisibleItemsAndRepointsTarget(t *testing.T) {
|
||||
hidden := func(context.Context) bool { return false }
|
||||
shown := func(context.Context) bool { return true }
|
||||
var seen []any
|
||||
fromCtx := func(ctx context.Context) bool {
|
||||
seen = append(seen, ctx.Value(navVisibleKey{}))
|
||||
return true
|
||||
}
|
||||
reg := &Registry{
|
||||
byID: map[string]*CompiledController{
|
||||
"acme.shop.orders": {Controller: navController{"acme.shop.orders", nil}},
|
||||
"acme.shop.payments": {Controller: navController{"acme.shop.payments", nil}},
|
||||
"acme.shop.quotes": {Controller: navController{"acme.shop.quotes", nil}},
|
||||
},
|
||||
navigation: []pact.NavigationItem{
|
||||
{
|
||||
Code: "shop", Label: "Shop", Controller: "acme.shop.orders", Order: 1,
|
||||
SideMenu: []pact.NavigationItem{
|
||||
{Code: "orders", Label: "Orders", Controller: "acme.shop.orders", Visible: hidden},
|
||||
{Code: "payments", Label: "Payments", Controller: "acme.shop.payments"},
|
||||
{Code: "quotes", Label: "Quotes", Controller: "acme.shop.quotes", Visible: fromCtx},
|
||||
},
|
||||
},
|
||||
{
|
||||
Code: "off", Label: "Off", Controller: "acme.shop.payments", Order: 2, Visible: hidden,
|
||||
SideMenu: []pact.NavigationItem{
|
||||
{Code: "payments", Label: "Payments", Controller: "acme.shop.payments", Visible: shown},
|
||||
},
|
||||
},
|
||||
{
|
||||
Code: "empty", Label: "Empty", Controller: "acme.shop.quotes", Order: 3,
|
||||
SideMenu: []pact.NavigationItem{
|
||||
{Code: "quotes", Label: "Quotes", Controller: "acme.shop.quotes", Visible: hidden},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
super := &bouncer.Principal{ID: 1, Backend: true, IsSuperuser: true}
|
||||
ctx := context.WithValue(context.Background(), navVisibleKey{}, "request")
|
||||
nav, _ := reg.Metadata(ctx, super, nil)
|
||||
if len(nav) != 2 || nav[0].Code != "shop" || nav[1].Code != "empty" {
|
||||
t.Fatalf("navigation = %#v, want shop and empty (the off parent must be dropped although its child is visible)", nav)
|
||||
}
|
||||
shop := nav[0]
|
||||
if len(shop.SideMenu) != 2 || shop.SideMenu[0].Code != "payments" || shop.SideMenu[1].Code != "quotes" {
|
||||
t.Fatalf("shop side menu = %#v, want payments and quotes (orders hidden)", shop.SideMenu)
|
||||
}
|
||||
if shop.Controller != "acme.shop.payments" {
|
||||
t.Fatalf("shop controller = %q, want the first visible openable child", shop.Controller)
|
||||
}
|
||||
empty := nav[1]
|
||||
if len(empty.SideMenu) != 0 || empty.Controller != "" {
|
||||
t.Fatalf("empty = %#v, want no side menu and no controller", empty)
|
||||
}
|
||||
if len(seen) != 1 || seen[0] != "request" {
|
||||
t.Fatalf("Visible saw context values %v, want the one passed to Metadata", seen)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRelationMutationsFollowToolbarButtons pins WR-05: link and unlink are
|
||||
// refused unless the relation's view panel declares them.
|
||||
func TestRelationMutationsFollowToolbarButtons(t *testing.T) {
|
||||
|
||||
@@ -56,6 +56,9 @@ type rosterPerson struct {
|
||||
// whose contract sets WritableForeignKey, writes it.
|
||||
OrganisationID *uint `gorm:"column:organisation_id"`
|
||||
DeletedAt gorm.DeletedAt `gorm:"column:deleted_at"`
|
||||
// Badge is not stored: ListExtendRecords fills it for the list's badge
|
||||
// column, once per page.
|
||||
Badge string `gorm:"-:all;column:badge" json:"-"`
|
||||
}
|
||||
|
||||
func (rosterPerson) TableName() string { return "roster_people" }
|
||||
@@ -123,6 +126,8 @@ type rosterSpy struct {
|
||||
record []pact.AdminRecordActionInput
|
||||
// states counts ListRowStates calls and keeps the size of each page.
|
||||
states []int
|
||||
// extended keeps the size of each page ListExtendRecords completed.
|
||||
extended []int
|
||||
// virtual keeps what each Form hook read from the context.
|
||||
virtual []rosterVirtual
|
||||
}
|
||||
@@ -156,6 +161,14 @@ func (s *rosterSpy) recordStates(n int) {
|
||||
s.states = append(s.states, n)
|
||||
}
|
||||
|
||||
func (s *rosterSpy) takeExtended() []int {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
out := s.extended
|
||||
s.extended = nil
|
||||
return out
|
||||
}
|
||||
|
||||
func (s *rosterSpy) takeStates() []int {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
@@ -218,6 +231,10 @@ const (
|
||||
rosterShort = "Short"
|
||||
// rosterStateErr: ListRowStates fails with a plain error.
|
||||
rosterStateErr = "StateErr"
|
||||
// rosterExtendErr: ListExtendRecords fails with a plain error.
|
||||
rosterExtendErr = "ExtendErr"
|
||||
// rosterTitleErr: FormRecordTitle fails with a plain error.
|
||||
rosterTitleErr = "TitleErr"
|
||||
// rosterAppliesErr: the activate record action's Applies fails.
|
||||
rosterAppliesErr = "AppliesErr"
|
||||
// rosterCrash: the archive bulk action fails with a plain error.
|
||||
@@ -413,6 +430,41 @@ func (c rosterController) ListRowStates(ctx context.Context, db *gorm.DB, record
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// ListExtendRecords fills the unstored badge of one list page in a single
|
||||
// pass: "<initial>#<id>", and "deleted" for a soft-deleted person.
|
||||
func (c rosterController) ListExtendRecords(ctx context.Context, db *gorm.DB, records []any) error {
|
||||
c.spy.mu.Lock()
|
||||
c.spy.extended = append(c.spy.extended, len(records))
|
||||
c.spy.mu.Unlock()
|
||||
if _, inTx := cabana.TxFromContext(ctx); inTx || db == nil {
|
||||
return fmt.Errorf("a list hook gets the list handle, not a transaction")
|
||||
}
|
||||
for _, record := range records {
|
||||
person := record.(*rosterPerson)
|
||||
if person.Name == rosterExtendErr {
|
||||
return fmt.Errorf("the badge table said hunter2")
|
||||
}
|
||||
person.Badge = fmt.Sprintf("%.1s#%d", person.Name, person.ID)
|
||||
if person.DeletedAt.Valid {
|
||||
person.Badge += " deleted"
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// FormRecordTitle names a person "Person #<id> (<name>)" on the form and
|
||||
// preview screens; a person named rosterTitleErr fails the request.
|
||||
func (rosterController) FormRecordTitle(_ context.Context, db *gorm.DB, record any) (string, error) {
|
||||
person := record.(*rosterPerson)
|
||||
if db == nil {
|
||||
return "", fmt.Errorf("no database handle")
|
||||
}
|
||||
if person.Name == rosterTitleErr {
|
||||
return "", fmt.Errorf("the title table said hunter2")
|
||||
}
|
||||
return fmt.Sprintf("Person #%d (%s)", person.ID, person.Name), nil
|
||||
}
|
||||
|
||||
// rosterStatus is the curated view model of the preview status hint: the
|
||||
// callout tone and the phrase keys of its title and text. It is empty when
|
||||
// no state applies, and the template then renders nothing.
|
||||
|
||||
63
modules/cabana/phase181_records_test.go
Normal file
63
modules/cabana/phase181_records_test.go
Normal file
@@ -0,0 +1,63 @@
|
||||
package cabana_test
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestListExtendRecords: the list hook completes each page once, before the
|
||||
// columns are read, so an unstored field's column shows the value it set; a
|
||||
// hook error fails the list with the generic 500 and no error text.
|
||||
func TestListExtendRecords(t *testing.T) {
|
||||
env, gdb := newRosterEnv(t)
|
||||
ada := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Ada"})
|
||||
gone := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Bob", DeletedAt: rosterDeleted()})
|
||||
body, raw := rosterList(t, env, "")
|
||||
badges := map[string]any{}
|
||||
for _, row := range body.Data {
|
||||
badges[fmt.Sprint(row["id"])] = row["badge"]
|
||||
}
|
||||
if badges[fmt.Sprint(ada)] != fmt.Sprintf("A#%d", ada) || badges[fmt.Sprint(gone)] != fmt.Sprintf("B#%d deleted", gone) {
|
||||
t.Fatalf("badges = %v\n%s", badges, raw)
|
||||
}
|
||||
if got := env.spy.takeExtended(); len(got) != 1 || got[0] != 2 {
|
||||
t.Fatalf("ListExtendRecords pages = %v, want one page of 2", got)
|
||||
}
|
||||
// The unstored column is not sortable.
|
||||
env.expect(t, http.StatusUnprocessableEntity, http.MethodGet, rosterPeople+"?sort=badge", "", "bearer")
|
||||
|
||||
logs := captureLog(t)
|
||||
rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: rosterExtendErr})
|
||||
rec := env.expect(t, http.StatusInternalServerError, http.MethodGet, rosterPeople, "", "bearer")
|
||||
if got := rosterError(t, rec); got.Code != "error" || strings.Contains(rec.Body.String(), "hunter2") {
|
||||
t.Fatalf("500 body = %s", rec.Body.String())
|
||||
}
|
||||
if failed := logs.matching("cabana: list extend records failed"); len(failed) != 1 || !strings.Contains(failed[0], "controller=acme.roster.people") {
|
||||
t.Fatalf("server log = %q", failed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFormRecordTitle: the show response carries the controller's record
|
||||
// title as meta.title; a hook error fails the request without its text.
|
||||
func TestFormRecordTitle(t *testing.T) {
|
||||
env, gdb := newRosterEnv(t)
|
||||
ada := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: "Ada"})
|
||||
rec := env.expect(t, http.StatusOK, http.MethodGet, fmt.Sprintf("%s/%d", rosterPeople, ada), "", "bearer")
|
||||
var body struct {
|
||||
Meta map[string]json.RawMessage `json:"meta"`
|
||||
}
|
||||
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if got := string(body.Meta["title"]); got != fmt.Sprintf("%q", fmt.Sprintf("Person #%d (Ada)", ada)) {
|
||||
t.Fatalf("meta.title = %s\n%s", got, rec.Body.String())
|
||||
}
|
||||
bad := rosterInsert(t, gdb, rosterPerson{Tenant: "acme", Name: rosterTitleErr})
|
||||
rec = env.expect(t, http.StatusInternalServerError, http.MethodGet, fmt.Sprintf("%s/%d", rosterPeople, bad), "", "bearer")
|
||||
if strings.Contains(rec.Body.String(), "hunter2") {
|
||||
t.Fatalf("500 body = %s", rec.Body.String())
|
||||
}
|
||||
}
|
||||
@@ -148,11 +148,18 @@ func ExecuteList(ctx context.Context, db *gorm.DB, cc *CompiledController, in Li
|
||||
return nil, err
|
||||
}
|
||||
values := slice.Elem()
|
||||
data := make([]map[string]any, 0, values.Len())
|
||||
records := make([]any, 0, values.Len())
|
||||
for i := 0; i < values.Len(); i++ {
|
||||
record := values.Index(i).Addr().Interface()
|
||||
records = append(records, record)
|
||||
records = append(records, values.Index(i).Addr().Interface())
|
||||
}
|
||||
if hook, ok := cc.Controller.(pact.ListExtendRecords); ok && hook != nil && len(records) > 0 {
|
||||
if err := hook.ListExtendRecords(ctx, db.WithContext(ctx), records); err != nil {
|
||||
slog.Error("cabana: list extend records failed", "controller", controllerID(cc), "error", err)
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
data := make([]map[string]any, 0, len(records))
|
||||
for _, record := range records {
|
||||
data = append(data, projectRowCtx(ctx, record, cc.Controller, cc.List.Columns))
|
||||
}
|
||||
states, err := listRowStates(ctx, db, cc, records)
|
||||
|
||||
@@ -94,6 +94,9 @@ type RelationOption struct {
|
||||
// create and update responses carry them, and the key is omitted when no type
|
||||
// reported anything.
|
||||
type RecordMeta struct {
|
||||
// Title is the controller's record title (pact.FormRecordTitle), omitted
|
||||
// when the controller names none.
|
||||
Title string `json:"title,omitempty"`
|
||||
Labels map[string][]RelationOption `json:"labels"`
|
||||
Actions []RecordAction `json:"actions,omitempty"`
|
||||
Notices []FieldNotice `json:"notices,omitempty"`
|
||||
|
||||
@@ -25,6 +25,7 @@ people:
|
||||
inactive_title: This person is not active
|
||||
inactive_text: Activate the person to let them sign in.
|
||||
slug: Slug
|
||||
badge: Badge
|
||||
password: Password
|
||||
password_confirmation: Repeat the password
|
||||
notify: Send a welcome message
|
||||
|
||||
@@ -25,6 +25,7 @@ people:
|
||||
inactive_title: Ta osoba jest nieaktywna
|
||||
inactive_text: Aktywuj osobę, aby mogła się zalogować.
|
||||
slug: Slug
|
||||
badge: Odznaka
|
||||
password: Hasło
|
||||
password_confirmation: Powtórz hasło
|
||||
notify: Wyślij wiadomość powitalną
|
||||
|
||||
@@ -8,3 +8,5 @@ columns:
|
||||
invisible: true
|
||||
slug:
|
||||
label: acme.roster::lang.people.slug
|
||||
badge:
|
||||
label: acme.roster::lang.people.badge
|
||||
|
||||
@@ -24,12 +24,12 @@ Use cases:
|
||||
|
||||
- Plugin capability interfaces: `pact.HasRoutes`, `pact.HasConfig`, `pact.HasMigrations`, `pact.HasCommands`, `pact.HasModels`, `pact.HasJobs`, `pact.HasSchedule`, `pact.HasLang`, `pact.HasLangOverrides` and `pact.HasMailTemplates`.
|
||||
- HTTP contracts: the `pact.Router` group builder (implemented by surf), the `pact.Middleware` type, and named, parameterized (`name:param`) and house-envelope middleware through `pact.HasMiddleware`, `pact.HasMiddlewareFactories` and `pact.HasHouseMiddleware`.
|
||||
- Backend registration data: `pact.Permission`, `pact.NavigationItem` and `pact.SettingsItem`, exposed through `pact.HasPermissions`, `pact.HasNavigation` and `pact.HasSettings`. A settings entry is either a singleton form or a link that opens an admin controller from the Settings page.
|
||||
- Backend registration data: `pact.Permission`, `pact.NavigationItem` and `pact.SettingsItem`, exposed through `pact.HasPermissions`, `pact.HasNavigation` and `pact.HasSettings`. A settings entry is either a singleton form or a link that opens an admin controller from the Settings page. A `pact.NavigationItem.Visible` func hides a navigation entry at request time (nil = always listed).
|
||||
- Personal API token scopes: `pact.TokenScope` and `pact.HasTokenScopes`, so a plugin declares named scopes (permissions and HTTP methods) that administrators can assign when creating a personal API token.
|
||||
- Admin controller contracts: `pact.AdminController`, `pact.HasAdminControllers`, `pact.AdminAssets` (embedded Winter-shaped admin YAML), `pact.AdminPermissioned` and `pact.AdminRecordSource`.
|
||||
- Admin extension contracts, so a plugin extends the compiled admin SPA without a Node build: `pact.AdminClientAssets` (per-controller JS and CSS from the plugin's embedded `assets/` tree, Winter's `addJs`/`addCss`), `pact.HasAdminActions` with `pact.AdminAction`, `pact.AdminActionInput` and `pact.AdminActionResult` (named toolbar and widget actions whose routes, CSRF check, permissions and record scoping the framework owns), `pact.HasAdminBulkActions` with `pact.AdminBulkAction`, `pact.AdminBulkActionInput` and `pact.AdminBulkActionResult` (named actions on the rows selected in a list, which receive records the framework loaded through the list scope, never ids; a `Detached` action runs outside the framework transaction), `pact.HasAdminRecordActions` with `pact.AdminRecordAction`, `pact.AdminRecordActionInput` and `pact.AdminRecordActionResult` (named actions on one record, each with an `Applies` rule for the record's state), and `pact.AdminPartialData` (the curated view model a partial template renders).
|
||||
- Plugin field types: `pact.HasFormFieldTypes` returns `pact.FormFieldType` entries, admin form field types any controller's `fields.yaml` uses by a short alias (`type: notes`). A type names its custom element, its scripts and styles, its own option keys and its Go funcs: `Decode` turns a submitted `pact.FormFieldValue` into a `pact.FormFieldDecoded` (the value to store plus optional `pact.FormFieldNotice` messages for the administrator, such as what a sanitizer removed) or refuses it with a `pact.FormFieldError`; `Load` serves the stored value to the element, `ListValue` formats a list column, and `Upload` stores a file from a `pact.FormFieldUpload` and answers a `pact.FormFieldUploadResult`. The admin framework owns the routes, permissions, record scope, locale storage of a translatable type and the localization of notices.
|
||||
- Optional admin hooks a controller or model can implement: list and form query scoping (`pact.ListExtendQuery`, `pact.FormExtendQuery`), list row states (`pact.ListRowStates` with the fixed `pact.RowState` set `pact.RowStateDeleted`, `pact.RowStateNegative` and `pact.RowStateDisabled`), create, update and delete hooks (`pact.FormBeforeCreate`, `pact.FormAfterUpdate`, `pact.FormBeforeDelete` and their siblings), a hook after a reorder save (`pact.ReorderAfterSave`), form-only fields that reach those hooks without being model columns (`pact.FormVirtualFields`), validation rules per operation for admin saves (`pact.FormRules`), relation hooks (`pact.RelationExtendManageQuery`, `pact.RelationExtendOptionsQuery`, `pact.RelationBeforeLink`), relation child hooks around creating, updating and deleting a related record (`pact.RelationBeforeCreate`, `pact.RelationAfterCreate`, `pact.RelationBeforeUpdate`, `pact.RelationAfterUpdate`, `pact.RelationBeforeDelete`, `pact.RelationAfterDelete`), filter scopes (`pact.FilterScope`, `pact.FilterOptions`) and dropdown options (`pact.DropdownOptionsProvider`).
|
||||
- Optional admin hooks a controller or model can implement: list and form query scoping (`pact.ListExtendQuery`, `pact.FormExtendQuery`), list row states (`pact.ListRowStates` with the fixed `pact.RowState` set `pact.RowStateDeleted`, `pact.RowStateNegative` and `pact.RowStateDisabled`), values of unstored list columns filled once per page (`pact.ListExtendRecords`), the record title of the form and preview screens (`pact.FormRecordTitle`), create, update and delete hooks (`pact.FormBeforeCreate`, `pact.FormAfterUpdate`, `pact.FormBeforeDelete` and their siblings), a hook after a reorder save (`pact.ReorderAfterSave`), form-only fields that reach those hooks without being model columns (`pact.FormVirtualFields`), validation rules per operation for admin saves (`pact.FormRules`), relation hooks (`pact.RelationExtendManageQuery`, `pact.RelationExtendOptionsQuery`, `pact.RelationBeforeLink`), relation child hooks around creating, updating and deleting a related record (`pact.RelationBeforeCreate`, `pact.RelationAfterCreate`, `pact.RelationBeforeUpdate`, `pact.RelationAfterUpdate`, `pact.RelationBeforeDelete`, `pact.RelationAfterDelete`), filter scopes (`pact.FilterScope`, `pact.FilterOptions`) and dropdown options (`pact.DropdownOptionsProvider`).
|
||||
- Dependent dropdown options: `pact.DependentOptionsProvider` answers a `pact.DependentOptionSet` (whether the sibling's value takes a choice at all, and the choices) for a dropdown with `dependsOn`, from the submitted values of its siblings and with a context, so the choices can come from the database; the admin framework calls it for the options route and again on every save. `pact.Option.Depth` indents a choice taken from a tree.
|
||||
- Menu item types: `pact.HasMenuItemTypes` returns `pact.MenuItemType` values (key, label, default URL pattern, `PatternFrom` for a list type, the `References` an item can point at as a `pact.MenuItemReference` tree, and `Resolve`), the typed successor of WinterCMS's `pages.menuitem.*` events. `Resolve` receives a `pact.MenuItemResolve` with the pattern already layered and answers a `pact.ResolvedMenuItem` with its alternates as an ordered `[]pact.MenuItemAlternate`. [signpost](../signpost/README.md) collects them at boot.
|
||||
- Singleton admin controllers: `pact.AdminSingleton` (`SingletonRecord`, first or create) makes a controller edit exactly one record, opened directly instead of from a list.
|
||||
@@ -119,7 +119,7 @@ func (p *Plugin) Schedule() []pact.ScheduledCommand {
|
||||
| `pact.HasPermissions` | Declares backend permissions. |
|
||||
| `pact.TokenScope` | One personal API token scope: name, label, description, permission patterns and HTTP methods. |
|
||||
| `pact.HasTokenScopes` | Declares personal API token scopes compiled at admin boot. |
|
||||
| `pact.NavigationItem` | One backend navigation entry, with an optional side menu. |
|
||||
| `pact.NavigationItem` | One backend navigation entry, with an optional side menu and an optional `Visible` func the admin runs on every navigation request. |
|
||||
| `pact.SettingsItem` | One settings screen entry: a singleton settings form (`Model`, `Form`, `NewModel`), or a link to an admin controller when `Controller` names its ID, as a WinterCMS `'url' => Backend::url(...)` entry. |
|
||||
| `pact.AdminController` | Admin controller identity: ID, model name and YAML config directory. |
|
||||
| `pact.AdminAssets` | Embedded tree of the plugin's admin YAML. |
|
||||
@@ -148,6 +148,8 @@ func (p *Plugin) Schedule() []pact.ScheduledCommand {
|
||||
| `pact.FormFieldUploadResult` | `Upload`'s answer: the URL of the stored file. |
|
||||
| `pact.FormFieldError` | A value or upload a field type refuses: messages shown on the field (validation failed). |
|
||||
| `pact.ListRowStates` | Optional controller hook called once per list page; returns the states of the page's records, index-aligned. |
|
||||
| `pact.ListExtendRecords` | Optional controller hook called once per list page, before the columns are read; fills unstored fields of the page's records. |
|
||||
| `pact.FormRecordTitle` | Optional controller hook naming a record on its form and preview screens; sent as the record envelope's `meta.title`. |
|
||||
| `pact.RowState` | One state of a list row: `pact.RowStateDeleted`, `pact.RowStateNegative` or `pact.RowStateDisabled`. |
|
||||
| `pact.FormVirtualFields` | Optional controller list of form fields that are not model columns for the form (a password and its confirmation, for example): never bound, filled or returned; their submitted values reach the Form hooks through the admin framework's context accessor. |
|
||||
| `pact.FormRules` | Optional controller hook returning the validation rules of an admin save for `create` or `update`; the set replaces the model's `Rules()` for those saves and may name virtual fields. |
|
||||
|
||||
@@ -537,7 +537,10 @@ type HasTokenScopes interface {
|
||||
}
|
||||
|
||||
// NavigationItem is one registerNavigation() entry. Controller is the admin
|
||||
// controller ID; the SPA derives its route from that ID.
|
||||
// controller ID; the SPA derives its route from that ID. Every field but
|
||||
// Visible is static and compiled once at boot; Visible is the one hook the
|
||||
// admin runs per navigation request, so a plugin can hide a screen whose
|
||||
// feature is turned off in its settings without a restart.
|
||||
type NavigationItem struct {
|
||||
Code string
|
||||
Label string
|
||||
@@ -546,6 +549,14 @@ type NavigationItem struct {
|
||||
Order int
|
||||
Controller string
|
||||
SideMenu []NavigationItem
|
||||
// Visible decides, per request, whether the entry is listed. Nil means
|
||||
// always listed. When set, the admin (cabana) calls it on every
|
||||
// navigation request with the request context, after the permission
|
||||
// check has passed, and leaves the entry out when it answers false; a
|
||||
// main item hidden this way takes its whole side menu with it. The func
|
||||
// must be cheap, must not panic, and should answer true when it cannot
|
||||
// decide (no database yet, a load error).
|
||||
Visible func(ctx context.Context) bool `json:"-"`
|
||||
}
|
||||
|
||||
// HasNavigation is implemented by plugins that declare backend navigation.
|
||||
@@ -734,6 +745,17 @@ type ListRowStates interface {
|
||||
ListRowStates(ctx context.Context, db *gorm.DB, records []any) ([][]RowState, error)
|
||||
}
|
||||
|
||||
// ListExtendRecords optionally completes the records of one admin list page
|
||||
// before their columns are read (WinterCMS's listExtendRecords). The framework
|
||||
// calls it once per page with the page's records (pointers, in page order) and
|
||||
// the list's database handle, so a controller can fill non-persisted fields
|
||||
// its columns show with one batched query instead of one per row. A column of
|
||||
// such a field names it by its gorm column tag and sets sortable: false. An
|
||||
// error fails the list.
|
||||
type ListExtendRecords interface {
|
||||
ListExtendRecords(ctx context.Context, db *gorm.DB, records []any) error
|
||||
}
|
||||
|
||||
// ListRelationColumnMapper maps a source-schema relation column onto the
|
||||
// physical column exposed by the related Go model. It supports legacy admin
|
||||
// schemas whose public field names no longer match the database schema.
|
||||
@@ -741,6 +763,16 @@ type ListRelationColumnMapper interface {
|
||||
ListRelationColumn(relation, column string) (string, bool)
|
||||
}
|
||||
|
||||
// FormRecordTitle optionally names a record on its form and preview screens.
|
||||
// The framework calls it whenever it sends a record (the show, create and
|
||||
// update responses) with the loaded record and the request's database handle.
|
||||
// A non-empty answer is sent as the record envelope's meta.title, which the
|
||||
// admin shows instead of the value of the form's first text field. An error
|
||||
// fails the request.
|
||||
type FormRecordTitle interface {
|
||||
FormRecordTitle(ctx context.Context, db *gorm.DB, record any) (string, error)
|
||||
}
|
||||
|
||||
// FormExtendQuery optionally narrows admin form record lookup.
|
||||
type FormExtendQuery interface {
|
||||
FormExtendQuery(ctx context.Context, db *gorm.DB) *gorm.DB
|
||||
|
||||
Reference in New Issue
Block a user