From badd6cb9ca1a67ccc4d50be8433f59414c86ce2a Mon Sep 17 00:00:00 2001 From: Connor Yoh Date: Tue, 11 Aug 2026 15:47:10 +0100 Subject: [PATCH] Ask with the dialog, and gate where it cannot be walked around Review feedback. Three behavioural changes and a copy pass. Pages look as they always did. Sources and Pipelines keep their own empty states; the ask is the dialog raised when someone tries to do something, not a lock screen in place of the feature. That removed the only caller of LinkGate, and of LinkAccountPrompt, so both are gone rather than left as dead code. Selecting Usage while unconnected raises the dialog instead of rendering a prompt page. A page whose only content is "you cannot see this page" is a worse version of the dialog that would follow it, and leaving the real page behind means dismissing the dialog lands somewhere useful. The gate moves off the buttons. Guarding click handlers was whack-a-mole and it had already been walked around four ways: the Documents review queue reached both the pipeline builder and the source create flow, Home's processor flow deep-linked into the policy wizard, and the pipelines empty state offered to connect a source. The two deep links bypassed the guards entirely, because they set state from an effect rather than through the handler. Now the builder is gated at the route, so the list, the review queue, the Connect flow's own next steps and a typed URL are all covered by one guard, and the ?new= and ?setup= effects carry their own. Those effects write back to the URL, so they hold the callback in a ref rather than depending on its identity: a caller that rebuilds it each render would loop, which is exactly what a view test caught by exhausting the heap. Copy: the subheading goes, six rows become three, pipelines and policies and sources and audit sit under the Processor that owns them rather than competing with it, credits move off the top so the screen does not open as a price list, and the folder-watching line is gone. --- .../public/locales/en-US/translation.toml | 20 +--- frontend/editor/src/portal/ViewRouter.tsx | 16 ++- .../account-link/ConnectGuardedRoute.tsx | 37 ++++++ .../account-link/LinkAccountModal.test.tsx | 2 +- .../account-link/LinkGate.stories.tsx | 84 -------------- .../components/account-link/LinkGate.tsx | 65 ----------- .../connect/ConnectBenefitsSlide.tsx | 89 +++++--------- .../billing/LinkAccountPrompt.stories.tsx | 14 --- .../components/billing/LinkAccountPrompt.tsx | 36 ------ .../billing/PortalBillingGate.test.tsx | 37 +++--- .../components/billing/PortalBillingGate.tsx | 34 +++--- .../src/portal/views/Pipelines.gated.test.tsx | 70 ++++++----- .../editor/src/portal/views/Pipelines.tsx | 49 ++++---- .../src/portal/views/Sources.gated.test.tsx | 109 ++++++++++++++++++ frontend/editor/src/portal/views/Sources.tsx | 49 ++++---- 15 files changed, 326 insertions(+), 385 deletions(-) create mode 100644 frontend/editor/src/portal/components/account-link/ConnectGuardedRoute.tsx delete mode 100644 frontend/editor/src/portal/components/account-link/LinkGate.stories.tsx delete mode 100644 frontend/editor/src/portal/components/account-link/LinkGate.tsx delete mode 100644 frontend/editor/src/portal/components/billing/LinkAccountPrompt.stories.tsx delete mode 100644 frontend/editor/src/portal/components/billing/LinkAccountPrompt.tsx create mode 100644 frontend/editor/src/portal/views/Sources.gated.test.tsx diff --git a/frontend/editor/public/locales/en-US/translation.toml b/frontend/editor/public/locales/en-US/translation.toml index 77217a663b..6dcdf7837a 100644 --- a/frontend/editor/public/locales/en-US/translation.toml +++ b/frontend/editor/public/locales/en-US/translation.toml @@ -6332,18 +6332,11 @@ step = "Step {{current}} of {{total}}" [portal.accountLink.connect.benefits] creditsDetail = "500 free on a new account" creditsLabel = "Credits" -lede = "Connecting unlocks the platform features around the editor. Manual PDF editing stays free, connected or not." -pipelinesDetail = "Chain tools and run them unattended" -pipelinesLabel = "Pipelines" -policiesDetail = "Rules that run on every file" -policiesLabel = "Policies" -processorDetail = "Watch folders and act on files" +processorDetail = "Pipelines, policies, sources and audit" processorLabel = "Processor" teamsDetail = "Free for 5 users and under" teamsLabel = "Teams" title = "Connect this server to a Stirling account" -usageDetail = "Pay only for what you run" -usageLabel = "Usage" [portal.accountLink.connect.done] addPolicy = "Add a policy" @@ -6361,12 +6354,6 @@ lede = "Your credits and team live in your Stirling account. This server connect reauthLede = "Your session expired. Sign back in to your Stirling account. This server stays connected." title = "Sign in to Stirling" -[portal.accountLink.gate] -action = "Connect account" -description = "Connect your Stirling account to use this. Manual PDF tools keep working either way." -title = "This needs a connected account" -titleFeature = "{{feature}} need a connected account" - [portal.accountLink.instances] active = "Active" revoke = "Revoke" @@ -6544,11 +6531,6 @@ title = "Invoice history" viewAriaLabel = "View invoice {{number}} in Stripe" viewLink = "View ↗" -[portal.billing.linkPrompt] -cta = "Connect account" -description = "Manual PDF editing is always free, connected or not. Connecting adds teams, the processor, pipelines and policies, and a new Stirling account starts with 500 free credits for automation, AI and the API." -title = "Connect your Stirling account" - [portal.billing.paymentMethod] billedMonthly = "Billed monthly" cardEnding = "{{brand}} ending {{last4}}" diff --git a/frontend/editor/src/portal/ViewRouter.tsx b/frontend/editor/src/portal/ViewRouter.tsx index 8e77c20830..c7b87f7388 100644 --- a/frontend/editor/src/portal/ViewRouter.tsx +++ b/frontend/editor/src/portal/ViewRouter.tsx @@ -11,6 +11,7 @@ import { Policies } from "@portal/views/Policies"; import { EditorAdmin } from "@portal/views/EditorAdmin"; import { Infrastructure } from "@portal/views/Infrastructure"; import { PortalBillingGate } from "@portal/components/billing/PortalBillingGate"; +import { ConnectGuardedRoute } from "@portal/components/account-link/ConnectGuardedRoute"; import { VIEW_PATHS, toPortalPath } from "@portal/contexts/ViewContext"; // Lazy so the generated docs manifest (bundled JSON) lands in its own chunk. @@ -32,13 +33,24 @@ export function ViewRouter() { } /> } /> } /> + {/* Building and editing need a linked account. Gated at the route so every way in is + covered: the list, the Documents review queue, the Connect flow's next steps, and a + typed URL. */} } + element={ + + + + } /> } + element={ + + + + } /> } /> {/* Source create/edit is a modal on the list now; old deep links land there. */} diff --git a/frontend/editor/src/portal/components/account-link/ConnectGuardedRoute.tsx b/frontend/editor/src/portal/components/account-link/ConnectGuardedRoute.tsx new file mode 100644 index 0000000000..2928dce372 --- /dev/null +++ b/frontend/editor/src/portal/components/account-link/ConnectGuardedRoute.tsx @@ -0,0 +1,37 @@ +import { useEffect, type ReactNode } from "react"; +import { Navigate } from "react-router-dom"; +import { useConnectGate } from "@portal/hooks/useConnectGate"; + +interface Props { + /** The gated page. */ + children: ReactNode; + /** Where to send someone who cannot open it yet, e.g. the list this builder belongs to. */ + fallback: string; +} + +/** + * Route-level connect gate: arriving at a page that needs a linked account asks for the connection + * and returns you to where you came from. + * + *

At the route rather than on the buttons, because guarding click handlers is whack-a-mole. The + * pipeline builder alone is reachable from its own list, from the Documents review queue, from the + * Connect flow's own next-steps, and from anyone typing the URL. A guard on each of those is a + * guard we have to remember every time a new link is added; a guard on the route is one that cannot + * be walked around. + * + *

It redirects rather than rendering a locked page: the ask is a dialog, so the page behind it + * should be the one the admin already knows. + */ +export function ConnectGuardedRoute({ children, fallback }: Props) { + const { gated, loading, connect } = useConnectGate(); + + useEffect(() => { + if (gated) connect(); + }, [gated, connect]); + + // Hold while the capability is unknown. Redirecting first would bounce a linked admin off a page + // they are entitled to, and the answer is cached after the first check. + if (loading) return null; + if (gated) return ; + return <>{children}; +} diff --git a/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx b/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx index dfe465db13..e8a902b30c 100644 --- a/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx +++ b/frontend/editor/src/portal/components/account-link/LinkAccountModal.test.tsx @@ -24,7 +24,7 @@ import { LinkAccountModal } from "@portal/components/account-link/LinkAccountMod type Props = Parameters[0]; -const BENEFITS = /Connecting unlocks the platform features/; +const BENEFITS = /Pipelines, policies, sources and audit/; const SIGN_IN = /This server connects once/; const REAUTH = /Your session expired/; const DONE = /now runs against your Stirling account/; diff --git a/frontend/editor/src/portal/components/account-link/LinkGate.stories.tsx b/frontend/editor/src/portal/components/account-link/LinkGate.stories.tsx deleted file mode 100644 index 3c8205f7b9..0000000000 --- a/frontend/editor/src/portal/components/account-link/LinkGate.stories.tsx +++ /dev/null @@ -1,84 +0,0 @@ -import { useState, type ReactNode } from "react"; -import type { Meta, StoryObj } from "@storybook/react-vite"; -import { http, HttpResponse } from "msw"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { LinkProvider, type LinkState } from "@portal/contexts/LinkContext"; -import { LinkGate } from "@portal/components/account-link/LinkGate"; - -const Feature = () => ( -

The feature itself, rendered once the account is connected.

-); - -/** - * A fresh query client per story. The preview shares one across all stories, so the app-config - * answer from whichever story ran first would otherwise be served from cache here and these four - * would stop showing what they claim to. - */ -function Isolated({ - state, - children, -}: { - state: LinkState; - children: ReactNode; -}) { - const [client] = useState( - () => new QueryClient({ defaultOptions: { queries: { retry: false } } }), - ); - return ( - - {children} - - ); -} - -const setup = (accountLinkAvailable: boolean, state: LinkState) => ({ - parameters: { - msw: { - handlers: [ - http.get("/api/v1/config/app-config", () => - HttpResponse.json({ accountLinkAvailable }), - ), - ], - }, - }, - decorators: [ - (Story: () => React.JSX.Element) => ( - - - - ), - ], -}); - -const meta: Meta = { - title: "Portal/AccountLink/LinkGate", - component: LinkGate, - parameters: { layout: "padded" }, - args: { feature: "Pipelines", children: }, -}; -export default meta; -type Story = StoryObj; - -/** - * The gate. It replaces the feature rather than sitting beside it, which is what makes it convert: - * an admin who clicked "New pipeline" has already declared intent. - * - * No credit claim here. On the modal the free grant is an inducement; on a gate it reads as a price - * of entry, and it is not ours to promise anyway. - */ -export const Gated: Story = setup(true, "unlinked"); - -/** Bare, for a caller that already supplies its own card. */ -export const GatedBare: Story = { - ...setup(true, "unlinked"), - args: { bare: true }, -}; - -/** Connected, so the feature renders untouched. */ -export const Linked: Story = setup(true, "linked-free"); - -/** - * Linking unavailable on this instance, which is the default everywhere the feature flag is off. - * The feature must still render: gating a server that CANNOT link would lock it with no way out. - */ -export const LinkingUnavailable: Story = setup(false, "unlinked"); diff --git a/frontend/editor/src/portal/components/account-link/LinkGate.tsx b/frontend/editor/src/portal/components/account-link/LinkGate.tsx deleted file mode 100644 index 24d0033f09..0000000000 --- a/frontend/editor/src/portal/components/account-link/LinkGate.tsx +++ /dev/null @@ -1,65 +0,0 @@ -import type { ReactNode } from "react"; -import { useTranslation } from "react-i18next"; -import { Button, Card, EmptyState } from "@app/ui"; -import { useConnectGate } from "@portal/hooks/useConnectGate"; - -interface Props { - /** The gated feature, rendered once the instance is linked. */ - children: ReactNode; - /** Feature name for the lock copy, e.g. "Pipelines". */ - feature?: string; - /** Render the empty state bare, for a caller that already supplies a card. */ - bare?: boolean; -} - -/** - * Blocks a feature that needs a linked Stirling account, replacing it with the reason and the - * remedy. - * - *

This replaces its children rather than sitting beside them: a blocked feature converts far - * better than a banner next to a working one, and it is the strongest driver in the connect flow. - * Scope it to creating and editing and leave viewing alone, so an upgrade never takes away - * something that already runs. - * - *

No credit claim here. On the modal the free grant is an inducement; on a gate it reads as a - * price of entry, and it is not ours to promise anyway (the grant is seeded per team at team - * creation, not by linking). The gate names the feature and the remedy; the modal makes the case. - * - *

Renders children untouched when the instance is linked, and also when linking is unavailable - * on this instance: a server with the feature flag off cannot link, so gating it would lock the - * feature with no way out. - */ -export function LinkGate({ children, feature, bare = false }: Props) { - const { t } = useTranslation(); - const { gated, loading, connect } = useConnectGate(); - - // Hold rather than flash: painting the gate before the capability lands would show a lock to - // someone who is about to turn out to be linked. - if (loading || !gated) return <>{children}; - - const body = ( - - {t("portal.accountLink.gate.action", "Connect account")} - - } - /> - ); - - return bare ? body : {body}; -} diff --git a/frontend/editor/src/portal/components/account-link/connect/ConnectBenefitsSlide.tsx b/frontend/editor/src/portal/components/account-link/connect/ConnectBenefitsSlide.tsx index 08de59d97b..ec6d3fbb25 100644 --- a/frontend/editor/src/portal/components/account-link/connect/ConnectBenefitsSlide.tsx +++ b/frontend/editor/src/portal/components/account-link/connect/ConnectBenefitsSlide.tsx @@ -2,27 +2,29 @@ import { useTranslation } from "react-i18next"; import "@portal/components/account-link/connect/connect.css"; /** - * Step 1 of the Connect flow: what linking a Stirling account actually gets you. + * Step 1 of the Connect flow: what connecting a Stirling account gets you. * *

The step this flow was missing. Before it, the only thing an admin ever saw was a login form - * subtitled "the account this server should bill against", which frames linking as the moment they - * start paying rather than what they gain. + * subtitled "the account this server should bill against", which frames connecting as the moment + * they start paying rather than what they gain. * - *

Two copy constraints are load-bearing. The free grant is seeded per team at team creation and - * is NOT granted by linking, so the allowance is stated as a property of a new account rather than - * a reward for connecting. Teams are free at five users and under, matching the free tier limit the - * server already reports, because an unqualified "free" breaks on the sixth invite. + *

Names what you get rather than selling it. The Processor owns pipelines, policies, sources and + * audit, so it is one row naming its parts rather than four competing ones. Credits come last: put + * them first and the whole screen reads as a price list. */ export function ConnectBenefitsSlide() { const { t } = useTranslation(); - const benefits: { key: string; label: string; detail: string }[] = [ + const unlocks: { key: string; label: string; detail: string }[] = [ { - key: "credits", - label: t("portal.accountLink.connect.benefits.creditsLabel", "Credits"), + key: "processor", + label: t( + "portal.accountLink.connect.benefits.processorLabel", + "Processor", + ), detail: t( - "portal.accountLink.connect.benefits.creditsDetail", - "500 free on a new account", + "portal.accountLink.connect.benefits.processorDetail", + "Pipelines, policies, sources and audit", ), }, { @@ -34,62 +36,23 @@ export function ConnectBenefitsSlide() { ), }, { - key: "processor", - label: t( - "portal.accountLink.connect.benefits.processorLabel", - "Processor", - ), + key: "credits", + label: t("portal.accountLink.connect.benefits.creditsLabel", "Credits"), detail: t( - "portal.accountLink.connect.benefits.processorDetail", - "Watch folders and act on files", - ), - }, - { - key: "pipelines", - label: t( - "portal.accountLink.connect.benefits.pipelinesLabel", - "Pipelines", - ), - detail: t( - "portal.accountLink.connect.benefits.pipelinesDetail", - "Chain tools and run them unattended", - ), - }, - { - key: "policies", - label: t("portal.accountLink.connect.benefits.policiesLabel", "Policies"), - detail: t( - "portal.accountLink.connect.benefits.policiesDetail", - "Rules that run on every file", - ), - }, - { - key: "usage", - label: t("portal.accountLink.connect.benefits.usageLabel", "Usage"), - detail: t( - "portal.accountLink.connect.benefits.usageDetail", - "Pay only for what you run", + "portal.accountLink.connect.benefits.creditsDetail", + "500 free on a new account", ), }, ]; return ( - <> -

- {t( - "portal.accountLink.connect.benefits.lede", - "Connecting unlocks the platform features around the editor. Manual PDF editing stays free, connected or not.", - )} -

- -
- {benefits.map((benefit) => ( -
-
{benefit.label}
-
{benefit.detail}
-
- ))} -
- +
+ {unlocks.map((unlock) => ( +
+
{unlock.label}
+
{unlock.detail}
+
+ ))} +
); } diff --git a/frontend/editor/src/portal/components/billing/LinkAccountPrompt.stories.tsx b/frontend/editor/src/portal/components/billing/LinkAccountPrompt.stories.tsx deleted file mode 100644 index 20a9ab2e80..0000000000 --- a/frontend/editor/src/portal/components/billing/LinkAccountPrompt.stories.tsx +++ /dev/null @@ -1,14 +0,0 @@ -import type { Meta, StoryObj } from "@storybook/react-vite"; -import { LinkAccountPrompt } from "@portal/components/billing/LinkAccountPrompt"; -import "@portal/components/billing/billing.css"; - -const meta: Meta = { - title: "Portal/Billing/LinkAccountPrompt", - component: LinkAccountPrompt, - parameters: { layout: "padded" }, -}; -export default meta; -type Story = StoryObj; - -/** Unlinked billing page — CTA opens the login modal (UIProvider from the preview decorator). */ -export const Default: Story = {}; diff --git a/frontend/editor/src/portal/components/billing/LinkAccountPrompt.tsx b/frontend/editor/src/portal/components/billing/LinkAccountPrompt.tsx deleted file mode 100644 index e8c0cc675b..0000000000 --- a/frontend/editor/src/portal/components/billing/LinkAccountPrompt.tsx +++ /dev/null @@ -1,36 +0,0 @@ -import { useTranslation } from "react-i18next"; -import { Button, Card, EmptyState } from "@app/ui"; -import { useUI } from "@portal/contexts/UIContext"; - -/** - * Unlinked state on the billing page. The CTA opens the Connect flow directly, so the admin gets - * the full case for connecting rather than a bare login box. - * - *

The copy states the free grant as a property of a new account, not as a reward for connecting: - * the allowance is seeded per team at team creation, so an existing account that has spent it gains - * nothing by linking. - */ -export function LinkAccountPrompt() { - const { t } = useTranslation(); - const { openLinkModal } = useUI(); - return ( - - openLinkModal()}> - {t("portal.billing.linkPrompt.cta", "Connect account")} - - } - /> - - ); -} diff --git a/frontend/editor/src/portal/components/billing/PortalBillingGate.test.tsx b/frontend/editor/src/portal/components/billing/PortalBillingGate.test.tsx index f15d8ade51..096a17c4d7 100644 --- a/frontend/editor/src/portal/components/billing/PortalBillingGate.test.tsx +++ b/frontend/editor/src/portal/components/billing/PortalBillingGate.test.tsx @@ -1,17 +1,23 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { render, screen } from "@testing-library/react"; -const linkState = { isLinked: false }; +/** + * Billing asks for the connection with the dialog, not by replacing the page. The page must render + * either way: a prompt page whose only content is "you cannot see this page" is a worse version of + * the dialog that follows it, and dismissing the dialog has to land somewhere real. + */ +const gate = { gated: false, loading: false, available: true }; +const connect = vi.fn(); + +vi.mock("@portal/hooks/useConnectGate", () => ({ + useConnectGate: () => ({ ...gate, connect, guard: (f: unknown) => f }), +})); vi.mock("@portal/contexts/LinkContext", () => ({ - useLink: () => linkState, useApplyLinkFacts: () => vi.fn(), })); vi.mock("@portal/contexts/UIContext", () => ({ useUI: () => ({ openLinkModal: vi.fn() }), })); -vi.mock("@portal/components/billing/LinkAccountPrompt", () => ({ - LinkAccountPrompt: () =>

, -})); vi.mock("@portal/views/Usage", () => ({ Usage: () =>
, })); @@ -20,20 +26,25 @@ import { PortalBillingGate } from "@portal/components/billing/PortalBillingGate" describe("PortalBillingGate — self-hosted", () => { beforeEach(() => { - linkState.isLinked = false; + connect.mockReset(); + gate.gated = false; }); - it("shows the link prompt when unlinked (billing gated on link)", () => { - linkState.isLinked = false; + it("asks for the connection on arrival when unconnected", () => { + gate.gated = true; render(); - expect(screen.getByTestId("link-prompt")).toBeInTheDocument(); - expect(screen.queryByTestId("usage")).not.toBeInTheDocument(); + expect(connect).toHaveBeenCalledTimes(1); }); - it("renders the Usage page once linked", () => { - linkState.isLinked = true; + it("still renders the page behind the ask", () => { + gate.gated = true; render(); expect(screen.getByTestId("usage")).toBeInTheDocument(); - expect(screen.queryByTestId("link-prompt")).not.toBeInTheDocument(); + }); + + it("asks for nothing once connected", () => { + render(); + expect(connect).not.toHaveBeenCalled(); + expect(screen.getByTestId("usage")).toBeInTheDocument(); }); }); diff --git a/frontend/editor/src/portal/components/billing/PortalBillingGate.tsx b/frontend/editor/src/portal/components/billing/PortalBillingGate.tsx index e448cfb93a..0c80652c7c 100644 --- a/frontend/editor/src/portal/components/billing/PortalBillingGate.tsx +++ b/frontend/editor/src/portal/components/billing/PortalBillingGate.tsx @@ -1,25 +1,34 @@ -import { useCallback } from "react"; -import { useApplyLinkFacts, useLink } from "@portal/contexts/LinkContext"; +import { useCallback, useEffect } from "react"; +import { useApplyLinkFacts } from "@portal/contexts/LinkContext"; import { useUI } from "@portal/contexts/UIContext"; -import { LinkAccountPrompt } from "@portal/components/billing/LinkAccountPrompt"; +import { useConnectGate } from "@portal/hooks/useConnectGate"; import { Usage } from "@portal/views/Usage"; import type { Wallet } from "@portal/api/billing"; /** - * Billing access gate — the seam the SaaS build overrides. + * Billing access gate: the seam the SaaS build overrides. * - *

Self-hosted (this base): billing only makes sense once the instance has - * linked its SaaS account, so gate on link state — unlinked shows the link prompt; - * linked renders the (flavor-agnostic) Usage page and maps its callbacks onto the - * link/tier dimension: the wallet's subscription status refines the plan/tier - * badge, and a lapsed SaaS session re-opens the account-link re-auth. This keeps - * the "link" concept entirely out of the Usage page. The SaaS build shadows this - * with a passthrough — there is no linking there. + *

Self-hosted (this base): billing only means anything once the instance has connected its + * Stirling account, so arriving here unconnected asks for the connection. It asks with the dialog + * rather than replacing the page with a prompt: a page whose only content is "you cannot see this + * page" is a worse version of the dialog that would follow it anyway. + * + *

The Usage page still renders behind. Its own reads fail without a link and it already handles + * that, so there is nothing to protect it from, and leaving it in place means dismissing the dialog + * lands you somewhere real. + * + *

It also maps the page's callbacks onto the link dimension: the wallet's subscription status + * refines the plan badge, and a lapsed SaaS session re-opens the re-auth. That keeps the "link" + * concept entirely out of the Usage page. The SaaS build shadows this with a passthrough. */ export function PortalBillingGate() { - const { isLinked } = useLink(); const applyLinkFacts = useApplyLinkFacts(); const { openLinkModal } = useUI(); + const { gated, connect } = useConnectGate(); + + useEffect(() => { + if (gated) connect(); + }, [gated, connect]); const onWalletLoaded = useCallback( (w: Wallet) => applyLinkFacts(true, w.status === "subscribed"), @@ -27,6 +36,5 @@ export function PortalBillingGate() { ); const onReauth = useCallback(() => openLinkModal("reauth"), [openLinkModal]); - if (!isLinked) return ; return ; } diff --git a/frontend/editor/src/portal/views/Pipelines.gated.test.tsx b/frontend/editor/src/portal/views/Pipelines.gated.test.tsx index 80e866f37c..52aa2f23dd 100644 --- a/frontend/editor/src/portal/views/Pipelines.gated.test.tsx +++ b/frontend/editor/src/portal/views/Pipelines.gated.test.tsx @@ -4,11 +4,15 @@ import { MemoryRouter, Route, Routes } from "react-router-dom"; import { PortalViewProviders } from "@portal/test/TestQueryProvider"; /** - * The gate as a user meets it. Pipelines stands in for the five gated views: they all take the - * same hook, so what is worth pinning here is the behaviour rather than the wiring. + * How the gate behaves for someone who has not connected an account. * - * The decision this encodes is that gating covers creating and editing but never viewing, so an - * upgrade cannot take away a pipeline that already runs. + * The page must look exactly as it always does. The ask is a dialog raised when they try to do + * something, not a lock screen in place of the feature: an admin who has pipelines still needs to + * see them, and one who has none should still see the empty state that explains what they are. + * + * The route guard is the important half. Guarding click handlers is whack-a-mole, and the builder + * is reachable from its own list, the Documents review queue, the Connect flow's next steps, and a + * typed URL. These pin the route, so a new link added later cannot walk around it. */ const { connect } = vi.hoisted(() => ({ connect: vi.fn() })); @@ -38,6 +42,7 @@ vi.mock("@portal/api/pipelines", () => ({ })); import { Pipelines } from "@portal/views/Pipelines"; +import { ConnectGuardedRoute } from "@portal/components/account-link/ConnectGuardedRoute"; const PIPELINE = { id: "plc-1", @@ -51,19 +56,19 @@ const PIPELINE = { owner: "security@acme.com", }; -function renderView() { +function renderAt(initial: string) { return render( - + } /> builder new

} - /> - pipeline page
} + element={ + +
builder
+ + } /> @@ -71,41 +76,50 @@ function renderView() { ); } -describe("Pipelines view when the account is not connected", () => { +describe("Pipelines when the account is not connected", () => { beforeEach(() => { connect.mockReset(); fetchPipelines.mockReset(); }); - it("replaces the empty state with the connect gate", async () => { + it("leaves the empty state exactly as it is", async () => { fetchPipelines.mockResolvedValue({ kpis: [], pipelines: [] }); - renderView(); + renderAt("/processor/pipelines"); expect( - await screen.findByText("portal.accountLink.gate.titleFeature"), + await screen.findByText("portal.pipelines.empty.title"), ).toBeInTheDocument(); - expect(screen.queryByText("portal.pipelines.empty.title")).toBeNull(); - }); - - it("asks to connect instead of opening the builder", async () => { - fetchPipelines.mockResolvedValue({ kpis: [], pipelines: [] }); - renderView(); - await screen.findByText("portal.accountLink.gate.titleFeature"); - fireEvent.click(screen.getByText("portal.pipelines.actions.newPipeline")); - expect(connect).toHaveBeenCalled(); - expect(screen.queryByText("builder new")).toBeNull(); }); it("still lists pipelines that already exist", async () => { fetchPipelines.mockResolvedValue({ kpis: [], pipelines: [PIPELINE] }); - renderView(); + renderAt("/processor/pipelines"); expect(await screen.findByText("Redact claims")).toBeInTheDocument(); }); + it("asks to connect instead of opening the builder", async () => { + fetchPipelines.mockResolvedValue({ kpis: [], pipelines: [] }); + renderAt("/processor/pipelines"); + await screen.findByText("portal.pipelines.empty.title"); + fireEvent.click(screen.getByText("portal.pipelines.actions.newPipeline")); + expect(connect).toHaveBeenCalled(); + expect(screen.queryByText("builder")).toBeNull(); + }); + it("asks to connect instead of opening an existing pipeline", async () => { fetchPipelines.mockResolvedValue({ kpis: [], pipelines: [PIPELINE] }); - renderView(); + renderAt("/processor/pipelines"); fireEvent.click(await screen.findByText("Redact claims")); expect(connect).toHaveBeenCalled(); - expect(screen.queryByText("pipeline page")).toBeNull(); + }); + + it("turns away a direct arrival at the builder, however it was reached", async () => { + fetchPipelines.mockResolvedValue({ kpis: [], pipelines: [] }); + renderAt("/processor/pipelines/new"); + // Bounced to the list, which is the page it would have come from. + expect( + await screen.findByText("portal.pipelines.empty.title"), + ).toBeInTheDocument(); + expect(screen.queryByText("builder")).toBeNull(); + expect(connect).toHaveBeenCalled(); }); }); diff --git a/frontend/editor/src/portal/views/Pipelines.tsx b/frontend/editor/src/portal/views/Pipelines.tsx index dde525dc12..25140cebc1 100644 --- a/frontend/editor/src/portal/views/Pipelines.tsx +++ b/frontend/editor/src/portal/views/Pipelines.tsx @@ -9,7 +9,6 @@ import { VIEW_PATHS, toPortalPath } from "@portal/contexts/ViewContext"; import { PipelinesIcon } from "@portal/components/icons"; import { KpiStrip } from "@portal/components/pipelines/KpiStrip"; import { PipelinesTable } from "@portal/components/pipelines/PipelinesTable"; -import { LinkGate } from "@portal/components/account-link/LinkGate"; import { useConnectGate } from "@portal/hooks/useConnectGate"; import "@portal/views/Pipelines.css"; @@ -33,8 +32,10 @@ export function Pipelines() { const openCreate = guard(() => navigate(`${toPortalPath(VIEW_PATHS.pipelines)}/new`), ); - const connectSource = () => - navigate(`${toPortalPath(VIEW_PATHS.sources)}/new`); + // Guarded in its own right so the ask happens here rather than after a pointless hop to Sources. + const connectSource = guard(() => + navigate(`${toPortalPath(VIEW_PATHS.sources)}/new`), + ); // A row opens that pipeline's own page (view / edit / run / delete live there). const openPipeline = guard((pipeline: PipelineView) => navigate(`${toPortalPath(VIEW_PATHS.pipelines)}/${pipeline.id}`), @@ -70,28 +71,26 @@ export function Pipelines() { )} {showEmpty && ( - - } - title={t("portal.pipelines.empty.title")} - description={t("portal.pipelines.empty.description")} - actions={ - <> - - - - } - /> - + } + title={t("portal.pipelines.empty.title")} + description={t("portal.pipelines.empty.description")} + actions={ + <> + + + + } + /> )} {!isLoading && pipelines.length > 0 && ( diff --git a/frontend/editor/src/portal/views/Sources.gated.test.tsx b/frontend/editor/src/portal/views/Sources.gated.test.tsx new file mode 100644 index 0000000000..0ba25392ae --- /dev/null +++ b/frontend/editor/src/portal/views/Sources.gated.test.tsx @@ -0,0 +1,109 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; +import { PortalViewProviders } from "@portal/test/TestQueryProvider"; + +/** + * The deep link into the create flow, which is the hole the first attempt at this left open. + * + * `?new=1` opens the modal from an effect rather than through the click handler, so guarding + * openCreate did nothing for it. Both the Documents review queue and the pipelines empty state + * arrive here that way, and each was a way past the gate. + */ +const { connect } = vi.hoisted(() => ({ connect: vi.fn() })); +const gate = { gated: true }; + +vi.mock("@portal/hooks/useConnectGate", () => ({ + useConnectGate: () => ({ + gated: gate.gated, + loading: false, + available: true, + connect, + guard: + (action: (...args: A) => void) => + (...args: A) => { + if (gate.gated) connect(); + else action(...args); + }, + }), +})); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + i18n: { changeLanguage: vi.fn() }, + }), +})); + +const fetchSources = vi.fn(); +vi.mock("@portal/api/sources", () => ({ + fetchSources: () => fetchSources(), + fetchSource: vi.fn(), + createSource: vi.fn(), + deleteSource: vi.fn(), + isFolderAccessDeniedError: () => false, +})); +vi.mock("@portal/api/integrations", () => ({ + fetchIntegrations: () => Promise.resolve([]), + fetchIntegrationCapabilities: () => Promise.resolve({ customApi: false }), + fetchS3Connections: () => Promise.resolve([]), + deleteIntegration: vi.fn(), +})); + +import { Sources } from "@portal/views/Sources"; + +const EDITOR_ROW = { + id: "editor", + name: "Editor", + type: "editor", + status: "active", + referenceCount: 0, + referencingPolicies: [], + config: [], + docsTotal: 0, + docs24h: 0, + docs30d: 0, +}; + +const renderAt = (initial: string) => + render( + + + + } /> + + + , + ); + +describe("Sources deep link when the account is not connected", () => { + beforeEach(() => { + connect.mockReset(); + gate.gated = true; + fetchSources.mockReset(); + fetchSources.mockResolvedValue({ kpis: [], sources: [EDITOR_ROW] }); + }); + + it("asks to connect instead of opening the create modal", async () => { + renderAt("/processor/sources?new=1"); + expect( + await screen.findByText("portal.sources.empty.title"), + ).toBeInTheDocument(); + expect(connect).toHaveBeenCalled(); + expect(screen.queryByText("portal.sources.builder.save")).toBeNull(); + }); + + it("leaves the page looking exactly as it always does", async () => { + renderAt("/processor/sources"); + expect( + await screen.findByText("portal.sources.empty.title"), + ).toBeInTheDocument(); + }); + + it("still honours the deep link once connected", async () => { + gate.gated = false; + renderAt("/processor/sources?new=1"); + await screen.findByText("portal.sources.empty.title"); + expect(connect).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/editor/src/portal/views/Sources.tsx b/frontend/editor/src/portal/views/Sources.tsx index 3bd38c868e..56ccb23bd8 100644 --- a/frontend/editor/src/portal/views/Sources.tsx +++ b/frontend/editor/src/portal/views/Sources.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { Navigate, useSearchParams } from "react-router-dom"; import AddRoundedIcon from "@mui/icons-material/AddRounded"; @@ -11,14 +11,13 @@ import { VIEW_PATHS, toPortalPath } from "@portal/contexts/ViewContext"; import { KpiStrip } from "@portal/components/sources/KpiStrip"; import { SourcesTable } from "@portal/components/sources/SourcesTable"; import { SourceModal } from "@portal/components/sources/SourceModal"; -import { LinkGate } from "@portal/components/account-link/LinkGate"; import { useConnectGate } from "@portal/hooks/useConnectGate"; import "@portal/views/Sources.css"; export function Sources() { const { t } = useTranslation(); const [searchParams, setSearchParams] = useSearchParams(); - const { guard } = useConnectGate(); + const { guard, gated, connect } = useConnectGate(); const state = useSources(); const { data, loading } = state; @@ -31,13 +30,23 @@ export function Sources() { sourceId: string | null; }>({ open: false, sourceId: null }); + // Held in a ref so the effect below does not re-run on its identity. The effect writes back to + // the URL, so a callback that changes each render would loop: strip the param, re-render, run + // again. + const connectRef = useRef(connect); + connectRef.current = connect; + + // Deep link into the create flow. It sets the modal directly, so it needs the gate in its own + // right: guarding openCreate would leave ?new=1 as a way past it (the Documents review queue and + // the pipelines empty state both arrive here that way). useEffect(() => { if (searchParams.get("new") !== "1") return; - setModal({ open: true, sourceId: null }); + if (gated) connectRef.current(); + else setModal({ open: true, sourceId: null }); const next = new URLSearchParams(searchParams); next.delete("new"); setSearchParams(next, { replace: true }); - }, [searchParams, setSearchParams]); + }, [searchParams, setSearchParams, gated]); const sources = data?.sources ?? []; @@ -85,23 +94,19 @@ export function Sources() { )} {showEmpty && ( - - } - title={t("portal.sources.empty.title")} - description={t("portal.sources.empty.description")} - actions={ - - } - /> - + } + title={t("portal.sources.empty.title")} + description={t("portal.sources.empty.description")} + actions={ + + } + /> )} {!isLoading && sources.length > 0 && (