JITSU-123: blocking-modal presentation for billing banners#1419
JITSU-123: blocking-modal presentation for billing banners#1419absorbb wants to merge 11 commits into
Conversation
…-123) Banner payloads gain kind: "banner" | "modal". Modals render the same parametrized payload (severity theme, icon, title, badge, body, action) in an antd Modal with a non-closable mask; Jitsu admins can dismiss regardless of closeable (session-only). Modals never render on the pages needed to fix billing (settings/domains/billing — the old doNotBlockIfUsageExceeded set). Replaces BillingBlockingDialog entirely: the past-due case is now a server-driven modal from billing/settings (jitsu-cloud-billing#27), and the console-computed quota-exceeded modal is retired — it locked the UI at >100% while delivery runs to 105%, conflicting with JITSU-88 semantics (and its mask swallowed clicks on the banners underneath). Removes the doNotBlockIfUsageExceeded plumbing from the layout and its four pages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![jitsu-code-review[bot]](https://avatars.githubusercontent.com/in/3391383?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the billing modal migration and related page-layout updates in this PR. I focused on behavior/security regressions introduced by replacing BillingBlockingDialog with server-driven modal banners and removing per-page blocking props. I left two inline comments with concrete questions about potential blocking-edge cases.
… billing-page banners, throttle line, modal design - verifyAccessWithRole rejects editEntities/deleteEntities mutations (403) for workspaces carrying the `readonly` feature flag (billing enforcement, e.g. unpaid invoices). Reads stay open; manageUsers stays role-gated only, so inviting the teammate who holds the credit card remains possible; Jitsu admins and service accounts bypass. - Banner contract gains `extra` (widget zone, carries the quota progress bar); the billing page nests all non-modal banners under Events Usage via BannerCard's new `compact` mode (no extra, no action, no dismiss) instead of showing them in the top strip. - Partial throttle alongside server banners no longer replaces them — its one fact is appended as a line to the first banner's body; a standalone throttle banner appears only when there is nothing to attach to. - BannerModal redesigned to the approved mock: 600px, top accent bar, card-language header (icon tile + title + badge), indented body and full-width action with centered subtitle — replacing the legacy 1000px/text-4xl proportions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The active-throttle and projection banners — and the priority rules between them and server banners — now come fully composed from the billing server (composeWorkspaceBanners). BillingBanners drops useEventsUsage and all client banner construction; it only renders, filters by onBillingPage, and dismisses. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the billing banner/modal refactor and readonly-access enforcement across the touched console files. I left two inline notes for potential correctness regressions: modal dismissibility via keyboard and compact billing-page banners bypassing dismissal behavior.
…ted banners The throttling-warning, usage-exceeded-upgrade and projection alerts in BillingManager duplicated the server banners now nested under Events Usage — remove them (and the orphaned copy.tsx). The past-due alert stays (it carries the direct Stripe payment link, which compact nested banners deliberately omit) and past-due surfaces are excluded from the nesting to avoid the same duplication. Overage-fee alerts stay — paid plans get no banners. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the billing banner/modal refactor and readonly API enforcement across the changed console files.
I found one additional correctness concern to double-check: readonly enforcement currently appears scoped to verifyAccessWithRole(..., editEntities|deleteEntities), while some mutating endpoints still gate writes with verifyAccess only.
…admin bypass, section-route permission, fullscreen modals, nested filter
- BannerModal always renders an action (falls back to "Go to billing")
so a non-closeable modal can never strand a non-admin without a CTA,
and gates keyboard (Esc) dismissal with the same condition as the ✕.
- Jitsu admins bypass the readonly guard even when they hold an
explicit workspaceAccess row.
- POST /api/workspace/[id]/[section] (workspaceOptions writes) moves
from verifyAccess to verifyAccessWithRole("editEntities") so it
honors roles and the readonly flag.
- Fullscreen pages mount BillingBanners in modalsOnly mode — blocking
modals are no longer skipped there (parity with the old dialog).
- The Events Usage nesting shows only banners the server moved off the
top strip (onBillingPage === false), preventing double display of any
future default-visible banner.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…past-due exclusion Applies the filter that a failed command chain silently dropped from an earlier commit: nest only banners the server moved off the top strip (onBillingPage === false), never modals, never past-due surfaces (the dedicated unpaid-invoices alert carries the payment link). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the billing modal/banner refactor plus readonly API enforcement in this PR. The modal fallback CTA, Esc gating, fullscreen modal mount, and workspace-options POST role/readonly check all look correct. I left one inline follow-up question about a potential billing-page banner duplication edge case.
There was a problem hiding this comment.
Reviewed the billing banner/modal refactor, settings-page layout changes, and the new readonly enforcement in verifyAccessWithRole plus the updated workspace section writer. Overall the direction looks good. I found one remaining readonly-enforcement gap (noted inline): there is still a mutating route gated only by verifyAccess, so readonly workspaces can still mutate backend state through that path.
POST /api/[workspaceId]/profile-builder/state mutates
ProfileBuilderState2.fullRebuildInfo but used plain verifyAccess —
switch to verifyAccessWithRole("editEntities") so it honors roles and
the billing readonly flag.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the diff from f1f843f to 654ca9c, focusing on billing banner/modal behavior, readonly enforcement, and the updated write-route access checks.
I verified the moved blocking-modal flow in , fullscreen mounting in , and the readonly gate plus the two route migrations to role-based checks.
No new actionable bugs, security issues, or correctness regressions stood out in this change set.
There was a problem hiding this comment.
Reviewed the diff from f1f843f to 654ca9c, focusing on billing banner/modal behavior, readonly enforcement, and the updated write-route access checks.
I verified the moved blocking-modal flow in BillingBanners, fullscreen mounting in WorkspacePageLayout, and the verifyAccessWithRole readonly gate plus the two route migrations to role-based checks.
No new actionable bugs, security issues, or correctness regressions stood out in this change set.
/settings/domains was in the modal-exempt list only as a carry-over of the legacy doNotBlockIfUsageExceeded set — nothing there is needed to fix billing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the changes in f1f843fc22ee72ddbe62da19c6bf57e4ecd663d4..9441b3763382b3f1d4ea086c299d7dbc3625ef61, focusing on billing banner/modal rendering and readonly-access enforcement updates in the touched API routes.
I did not find new actionable bugs, security issues, or correctness regressions in this revision.
…ODAL_EXEMPT_PAGES Per review: billing-modal exemption is now declared by the page itself via a WorkspacePageLayout property instead of a hardcoded path list in BillingBanners — settings, billing and billing/details set it, so "which pages stay reachable with unpaid invoices" is visible right in the page files. Applies to fullscreen mounts too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oarse-flag alert The nested list excluded past-due ids in favor of the legacy "unpaid invoices" alert — but that alert fires on the old coarse subscriptionStatus.pastDue, so the new state-driven warning showed nowhere on the billing page. The legacy alert is removed (the plan section keeps its Stripe-portal links) and the nesting now includes every banner moved off the top strip; modal payloads (past-due escalated) render as compact cards here since the page suppresses blocking modals but must still inform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed the billing banner/modal refactor, WorkspacePageLayout wiring, and readonly API enforcement changes in this PR. I found one potential regression to double-check: the Domains settings page appears to have lost its billing-modal exemption, which can make that page inaccessible for past-due workspaces.
2 participants
JITSU-123 console slice (items 1, 2-partial, 4, 5 + design decisions). Banner payloads gain
kind: "banner" | "modal"andextra:closeable(session-only). Never renders on billing-fix pages.verifyAccessWithRolerejectseditEntities/deleteEntitiesfor workspaces carrying thereadonlyfeature flag; reads open,manageUsersstays role-gated (inviting the card-holding teammate stays possible), Jitsu admins/service accounts bypass.extra/action/dismiss) under Events Usage instead of the top strip.JITSU-123🤖 Generated with Claude Code