fix: clarify sandbox subscription success states#249
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@perkcord preview |
Manual Vercel PreviewTriggered from PR comment for branch Use |
Greptile SummaryThis PR centralises sandbox detection for the member subscribe flow so that both the checkout (pay) and post-payment (celebrate) pages use the same detection utilities, and surfaces additive "this was a test transaction" notices at the appropriate points in each flow.\n\nKey changes:\n- Confidence Score: 5/5Safe to merge — all three previously raised concerns are resolved and the new logic is well-covered by unit, integration, and visual tests. All prior review concerns (production-default for missing AUTHORIZE_NET_ENV, leaking provider fields on non-sandbox grants, unreachable branch in shouldShowSandboxCelebrateNote) have been addressed. The remaining comment is a P2 style nit about a redundant call to resolveCheckoutSandboxContexts inside helpers.ts — non-blocking and trivially cheap at runtime. The logic across checkout and celebrate paths is internally consistent and the test suite covers the key edge cases. apps/web/app/subscribe/pay/helpers.ts — minor double computation worth tidying, no correctness impact. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Pay Page
HP[helpers.ts resolvePaymentMethods] --> SC[resolveCheckoutSandboxContexts\nper-provider contexts]
HP --> SCX[resolveCheckoutSandboxContext\nsingle global context]
SC --> CCX{sandboxContext\nnon-null?}
CCX -- Yes - single method all-sandbox --> GB[Global sandbox banner]
CCX -- No --> PMN[Per-method inline notices]
end
subgraph Status API
ST[status.ts resolveSubscribeStatus] --> BSI[buildSandboxInfo\ngrant source + env vars]
BSI --> SR[StatusResponse\nsandbox field]
end
subgraph Celebrate Page
SR --> DPH[derivePhase]
DPH -- sandbox + pending/in_progress/failed roleSync --> WP[phase = warning]
DPH -- sandbox + completed roleSync --> SP[phase = success]
WP --> SHN[shouldShowSandboxCelebrateNote]
SP --> SHN
SHN -- true --> SN[sandbox-success-note rendered]
end
Prompt To Fix All With AIThis is a comment left during a code review.
Path: apps/web/app/subscribe/pay/helpers.ts
Line: 114-143
Comment:
**`resolveCheckoutSandboxContexts` computed twice**
`resolveCheckoutSandboxContexts` is invoked directly to produce `sandboxContexts`, and then invoked a second time inside `resolveCheckoutSandboxContext` (which calls it internally). Both calls receive the identical argument object, so the same work is done twice.
You could either expose a two-value variant that returns both artefacts at once, or derive `sandboxContext` from the already-computed `sandboxContexts`:
```ts
const sandboxContexts = resolveCheckoutSandboxContexts({
showAuthorizeNet,
showStripe,
showNmi,
authorizeNetEnv: authorizeNetState.env,
stripeEnvironment,
nmiSandbox: false,
});
const visibleSandboxContexts = Object.values(sandboxContexts).filter(
(c): c is SandboxContext => c !== null,
);
const visibleMethodCount =
Number(showAuthorizeNet) + Number(showStripe) + Number(showNmi);
const sandboxContext =
visibleMethodCount === 1 &&
visibleSandboxContexts.length === 1 &&
visibleSandboxContexts.length === visibleMethodCount
? visibleSandboxContexts[0]
: null;
```
This is a minor efficiency concern and not a functional bug, but it avoids duplicating the resolution logic.
How can I resolve this? If you propose a fix, please make it concise.Reviews (8): Last reviewed commit: "test: refresh celebrate visual baselines" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 401e23e2b0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Snapshot Preview7 changed snapshot file(s) in this PR. Storybook snapshotsNo changed snapshots. Playwright visual snapshots (7)
|
There was a problem hiding this comment.
Pull request overview
Centralizes sandbox detection/formatting for the subscribe flow and updates pay/celebrate UI to show sandbox “success with warning” messaging consistently.
Changes:
- Added
subscribeSandboxhelpers to resolve sandbox context across checkout and grant/celebrate flows. - Updated subscribe status API + celebrate/pay UIs to surface sandbox notices alongside role-sync warnings.
- Added/extended unit + visual coverage for sandbox success-with-warning states.
Reviewed changes
Copilot reviewed 12 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/lib/subscribeSandbox.ts | New centralized sandbox context resolution + formatting utilities |
| apps/web/lib/subscribeSandbox.test.ts | Unit tests for sandbox context resolution and formatting |
| apps/web/lib/api/subscribe/status.ts | Adds sandbox info to subscribe status response |
| apps/web/lib/api/subscribe/status.test.ts | Adds tests around sandbox info construction |
| apps/web/e2e/visual.spec.ts | Visual coverage for celebrate sandbox success note |
| apps/web/app/subscribe/pay/helpers.ts | Resolves sandbox contexts for checkout methods in pay flow |
| apps/web/app/subscribe/pay/CombinedCheckout.tsx | Renders sandbox notices globally and per payment method |
| apps/web/app/subscribe/pay/CombinedCheckout.test.tsx | Unit tests for sandbox notice rendering behavior |
| apps/web/app/subscribe/celebrate/sandboxSummary.ts | New helpers to decide and format sandbox celebrate copy |
| apps/web/app/subscribe/celebrate/sandboxSummary.test.ts | Unit tests for celebrate sandbox note logic |
| apps/web/app/subscribe/celebrate/access-status-helpers.ts | Adds sandbox-aware phase/message semantics |
| apps/web/app/subscribe/celebrate/AccessStatus.tsx | Displays sandbox success note for success/warning states |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Remove the literal HTML entity from the celebrate retry copy and lock the global checkout sandbox notice to a single rendered banner. Refresh the Linux pay-page snapshot to match the intended single-banner layout.
Summary
Testing
Closes #243