master
47 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Fraser
|
48b9e66904 | Use oxlint (#82015) | ||
|
Mahatthana (Kelvin) Nomsawadi
|
d53a909a92 |
Revert "Integration branch for Embedding settings reorganization (#80336)" (#82570)
* Revert "Integration branch for Embedding settings reorganization (#80336)"
This reverts commit
|
||
|
Fraser
|
e677b6b267 | Use affected-test plans to narrow Loki visual tests (#82050) | ||
|
Mahatthana (Kelvin) Nomsawadi
|
b9d217cbb4 |
Integration branch for Embedding settings reorganization (#80336)
* refactor: rename the embedding hub checklist to setup guide (#79514)
* refactor: rename the embedding hub checklist to setup guide
"Embedding hub" originally meant the whole embedding section; what
shipped under that name in Sept 2025 was only the onboarding checklist,
which users already see as "Setup guide" -- it is the nav label and the
route. Renaming the code to match frees the name for the section the
embedding settings reorganization is building, and stops "embedding hub"
meaning two things.
Directory: frontend/src/metabase/embedding/embedding-hub -> setup-guide,
and the admin wrapper alongside it. Symbols: EmbeddingHub -> SetupGuide,
EmbeddingHubXrayPickerModal -> SetupGuideXrayPickerModal,
EmbeddingHubAdminSettingsPage -> SetupGuideAdminSettingsPage.
Deliberately unrenamed: useGetEmbeddingHubSteps, EmbeddingHubStepId and
the rest of the checklist payload types. Those name the
/api/ee/embedding-hub/checklist contract, and the backend keeps that name
on purpose -- renaming three persisted booleans buys nothing user-visible
and costs a migration.
Frontend-only. No persisted state, no API contract, no route change.
* refactor: finish the setup guide rename in tests and backend
EMB-2208 also covers the spec file names and the two backend comments.
The e2e spec and the SDK embed-setup unit spec move to setup-guide names
and their describe wording follows.
The backend keeps embedding-hub in the setting names, the API path and
the namespace, so both files get a note saying the names track the
Setup guide checklist rather than the embedding hub section.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: rename setup guide UI types and hooks
The UI model for the checklist still carried the old word:
EmbeddingHubStep, EmbeddingHubStepId, EmbeddingHubAction,
EmbeddingHubModalToTrigger, the two step hooks, and the home page
component. None of these name the API contract. They are declared in
setup-guide/types, independent of api/embedding-hub.ts, and
EmbeddingHubStepId lists seven step ids where the API's own
CheckListApiStep lists twelve.
api/embedding-hub.ts and everything in it keeps the old name, as does
the embedding-hub-checklist cache tag: those do name
/api/ee/embedding-hub/checklist.
Comments follow. The two above the embedding-hub-*-embed-snippet-created
settings now say the setting names stay while the thing they track is
the Setup guide.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: reword two missed setup guide mentions
An it() title in the SDK embed-setup spec and a comment above the
checklist keys in the API both still said "embedding hub" where they
meant the Setup guide.
The docstrings that name the API itself are left alone, as is the
:embedding-hub token feature.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: rename the checklist API client to setup guide
The client module named the endpoint rather than the contract. Nothing
in it is frozen except the URL, which is a literal string: the RTK cache
tag lives only in the frontend store, and the exported types and hook are
ours. So api/embedding-hub.ts becomes api/setup-guide.ts, its types and
query hook follow, and the embedding-hub-checklist tag becomes
setup-guide-checklist.
The surviving url: "/api/ee/embedding-hub/checklist" gets a comment
saying it is the one thing that has to keep the name.
The backend is untouched apart from comments. Its namespace keeps the old
name, so the symbols inside it stay consistent with it -- the seam is
between the two modules, not inside either.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* docs: name the setup guide in two comments
Shortening these to "the checklist" dropped which checklist it is, to buy
consistency with the surrounding names. Losing the information is the
worse trade.
In collection.ts there is no longer a mismatch to buy off anyway, since
the tag beside it is now setup-guide-checklist. In api.clj the comment
does disagree with its own namespace, and that is fine: the namespace
docstring already explains the seam.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: rename the checklist schema and builder
Both are private to the namespace and referenced nowhere else, so
nothing about them is frozen. The frontend type for the same payload is
already SetupGuideChecklistResponse, and two names for one response
across the seam is worse than a symbol disagreeing with the namespace
around it -- that disagreement is explained by the ns docstring.
The namespace, the route, the settings and the token feature keep the
old name, as does the endpoint docstring, which is published in the API
docs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: say Setup guide in the two setting descriptions
They said "for tracking in the embedding hub" directly under a comment
saying these track the Setup guide and not the hub section, so the file
contradicted itself and a reader could not tell which line was stale.
Nothing renders these descriptions -- no admin settings widget reads
them, only the settings API payload carries them -- so they are internal
documentation, and the same accuracy argument applies as to any comment.
The setting names still stay.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: say Setup guide in the endpoint docstring
It described the response as the embedding hub checklist while the schema
right above it is SetupGuideChecklistResponse. The path in the route
docstring and the namespace still name the hub, because those are bound
to the URL.
This string is published in resources/openapi/openapi.json and
docs/api.json. Add the openapi-self-healing label to the PR and CI
regenerates the spec; docs/api.json has no per-PR check and picks it up
on its next regeneration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* refactor: lowercase setup guide in prose
The codebase capitalises "Setup guide" only where it is the literal UI
label, as in EmbeddingNav, and writes it lowercase in running prose --
"Go to setup guide", "Return to the setup guide to continue". The
backend comments and descriptions added here did the opposite, and
api.clj disagreed with itself between one docstring and the next.
Sentence-initial mentions keep their capital, since that is ordinary
sentence case rather than the name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* style: format the setup guide route imports
oxfmt --check failed fe-lint on the integration branch; the two
setup-guide lazy imports were wrapped in the pre-oxfmt style.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BGNfTR7qNNvEQczJQDomoi
* feat: add the embedding hub shell and its Get started tab (#80346)
* feat: add the embedding hub shell and its Get started tab
The hub is a top-level section at /embedding, built on AreaLayout like
Monitor and Data Studio rather than on the admin sidebar, with its own nav
and an app-switcher entry. /admin/embedding stays exactly as it is; removing
it is EMB-1526 and comes last.
Get started replaces the vertical stepper with a grid of numbered cards in
two sections plus a useful-links row. Each Fine-tune step is locked by the
feature it needs rather than by one stand-in for "Pro", since an instance can
license SSO without modular embedding. A feature-locked card names no
prerequisite: "Set up SSO to unlock" is a promise the step is one action
away, which is only true above the paywall.
One tab today. The remaining six arrive with their own issues, each adding a
route, an export and a nav entry.
Two shared changes ride in front of it, each its own concern:
* `BaseLink` resolves an explicit `..` against the current route, which the
hub needs because it mounts admin components under a second base path.
* `AreaContent` grows a `fullWidth` prop, mirroring what the admin settings
layout already does, for the tabs that are editors rather than settings.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QHCSv6oQRLUUGo34V1eEDp
* docs: correct the embedding hub module tier note
The comment said the hub could drop to feature tier once its components
were lifted out of admin. It cannot: the Permissions and Tenancy tabs
mount admin permissions and admin People a second time on purpose, and
admin keeps its own copy of both, so the hub imports admin-tier code
permanently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* fix: finish the setup guide rename in the Get started page
The rename landed on the base branch, so it never reached files that
only exist here: the Get started spec still imported EmbeddingHubChecklist
from metabase/api/embedding-hub, which no longer resolves, and renaming
useGetEmbeddingHubSteps moved it past useSetupGuideModals alphabetically
and broke the sorted import blocks.
Both edits were made during a rebase, where pre-commit hooks do not run,
which is why lint-staged did not catch either one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NnWkk2XpAZurnyELCZFtSE
* feat: let AreaTab render an action as well as a link
The embedding hub's "New embed" control opens a modal rather than
navigating, so it was a Mantine Button with its own height and padding
and did not collapse with the navbar. It now goes through AreaTab, which
makes it the same size as every other rail item by construction.
Omitting `to` renders an UnstyledButton instead of a link. The two forms
are spelled out separately because their polymorphic component types do
not unify.
* refactor: make the UpsellBanner body text optional
The embedding hub's banner is a title and a button with no body copy.
Rendering an empty Text left a stray paragraph, and the large variant
top-aligned the row, so the gem and the button sat above the single
title line.
Without children the body is skipped and the row centres. Every existing
caller passes children, so none of them change.
* fix: return to the setup guide the user actually came from
The guide has three hosts, and every route out of it was hardcoded to
the admin one, so a user who started in the embedding hub was deposited
in admin settings on the way back.
The wizards now navigate relatively, resolving to whichever host mounted
them. The connect-a-database round trip leaves the app entirely, so it
carries the origin instead: the return param holds the path rather than
a bare "true", and the modal navigates there. Links already in flight
carry the old literal and fall back to admin as before.
The home-page stepper returns home; the admin guide keeps the default.
* feat: serve the setup guide checklist without a licence, and add the hub's two steps
The checklist sat behind the :embedding premium gate while the hub that
renders it is admin-only, so an unlicensed admin reached the page and
got a 402. Every step then read as incomplete -- including ones the
instance had genuinely done, which is worse than showing nothing. The
gate is gone; the endpoint checks superuser itself, which the gate had
been providing as a side effect.
The hub's own two steps join the response. Completion belongs to the
instance rather than to whichever host is asking, so both hosts read the
same source: configure-ai moves off a pair of frontend settings reads,
and create-custom-theme becomes answerable at all.
Telling a seeded theme from a user's needs a marker: the names are
stored already translated, and seeding is lazy so the ids are not
ordered. Existing rows are deliberately left false rather than
backfilled -- themes are recent, so the instances affected are few and
already embedding-active, and no retroactive rule is reliable.
* refactor: give the hub its own path, step list and unlicensed layout
Get started was the index route, so /embedding was a prefix of every
future tab's path and the wizards were siblings rather than children.
Both forced special cases in tab selection. It now lives at
/embedding/get-started with the wizards nested under it and /embedding
redirecting there, so tab selection is a plain segment match.
The hub also owns its steps outright rather than borrowing actions from
the shared hook. It already owned the copy, and the half-and-half
arrangement is what forced the action lookup and the omitted-step
handling; both are gone, and so is the URL parameter the hook only had
so the hub could pass its own wizard paths.
Without modular embedding every Fine-tune step is locked and AI is the
one advanced step still reachable, so it moves up into the first section
and the grid fixes two columns. Numbering follows position in the list
rather than arithmetic across three groups, which is what let the order
vary at all.
Also drop the helpers, the fullWidth chain and the SetupGuide prop this
branch introduced but nothing calls yet -- each arrives with the tab that
first needs it -- and order each file from its entry point down.
* refactor: address review findings on the hub shell
Restore the two setup-guide e2e tests the branch had deleted; nothing in
it justified their removal.
Drop the AvailableInEmbedding guard from the hub and Data Studio: it sat
inside a guard built from a selector that already returns false in an
iframe, so no state rendered differently. Add the hub selector spec its
Data Studio and Monitor siblings already had.
Take getIsEmbeddingHubApp as a tenth input selector rather than repeating
the path check inline; the instantiation-depth limit that comment warned
about no longer applies.
Order BaseLink's relative check last so the change is a pure append, and
say what actually goes wrong rather than describing it abstractly.
* fix: keep the checklist from 400ing without a database
has-user-added-database? ended in a when-let, so an instance with
neither a user database nor a sample database got nil rather than
false. The response schema is closed and requires a boolean, so the
whole checklist 400d. Every existing test called (mt/id) first, which
creates a database and hid it.
has-configured-ai? gets the same treatment: it ands two settings, and
a nil from either would fail the schema the same way.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzD6eCBAWFddH31oQg9woe
* test: cover unlicensed checklist access and the hub's two steps
The 402-without-:embedding assertion described the gate this branch
removed, so it was red. It is replaced by the behaviour that gate
change was for: a 200 with real completion on an unlicensed instance,
paid steps reporting false rather than erroring, and non-admins
rejected now that check-superuser carries the access control the
premium gate used to provide as a side effect.
Adds coverage for create-custom-theme (a seeded theme does not count)
and configure-ai (credentials and embedded Metabot, both halves).
Leaves data-isolation-strategy-test without an (mt/id) call - that is
what caught the nil add-data, so it is the regression guard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzD6eCBAWFddH31oQg9woe
* test: add e2e coverage for the hub's Get started tab
The tab shipped on unit tests alone. These five cover what needs a
real browser or a real backend: the redirect from /embedding, both
step orderings, the wizard's relative back link, feature locking, and
completion arriving from the checklist endpoint unlicensed.
Its own filename rather than sharing the security tab's spec - the two
cover different surfaces, and separate files mean no conflict when the
stack restacks.
Both blocks are @EE. The unlicensed case is an EE build with no token,
not an OSS build: the checklist is served from /api/ee, so on OSS the
route would not exist and every step would read incomplete for a
reason this page is not responsible for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzD6eCBAWFddH31oQg9woe
* chore: record the embedding-hub module's new dependencies
Regenerated by ./bin/mage fix-modules-config. Both entries come from
the two checklist steps this branch adds: metabot for the AI settings
has-configured-ai? reads, and :model/EmbeddingTheme for the is_default
lookup behind has-created-custom-theme?.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzD6eCBAWFddH31oQg9woe
* fix: account for embedding_theme.is_default in the serdes spec
serialization-complete-spec-test requires every column on a serialized
model to be declared, and is_default was new. It goes in :skip rather
than :copy: the flag records whether this instance seeded the row, not
anything about the theme itself, so it does not travel. An imported
theme lands as false - the admin's own - which is what importing one
is, and it ticks the create-custom-theme checklist step accordingly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JzD6eCBAWFddH31oQg9woe
* refactor: serve the setup guide checklist from OSS
A community build never mounts /api/ee/*, so the checklist endpoint did
not exist there at all -- the four steps its UI actually shows without a
licence (connect a database, create a dashboard, get an embed snippet,
configure AI) had no backend to report them.
Move the namespace to metabase.embedding-hub.api and the route to
/api/embedding-hub/checklist. Two things had to change to load without
enterprise code:
- The JWT and SAML settings ship only with enterprise code, so a missing
require would fail at load. has-configured-sso? becomes a defenterprise
whose OSS body returns false; the real implementation stays in
metabase-enterprise.embedding-hub.api behind :feature :embedding, the
same token the route was gated on before.
- :model/Tenant, :model/ConnectionImpersonation and :model/DatabaseRouter
resolve to enterprise namespaces a community build does not ship.
Guarding each query with the :embedding feature short-circuits before
Toucan resolves the model, the way has-configured-sandboxes? already
did with :sandboxes.
Every key stays in the response and keeps its type, so the frontend needs
only the new URL. Enterprise behaviour is unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F5HK8wzZwmAsF9PkgFMfLu
* docs: spell out "embedding hub" in setup guide comments
A bare "hub" reads ambiguously next to the setup guide, which the same
files also talk about.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F5HK8wzZwmAsF9PkgFMfLu
* refactor: keep the moved checklist code closer to its original
The move added a few things it did not need to: boolean wraps around
expressions the :type :boolean settings already returned as booleans, a
nested cond where a flat one had been, and a named helper where the file
already inlines its feature checks. Undo all of it, so the diff against
the original shows only what moving the namespace forced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F5HK8wzZwmAsF9PkgFMfLu
* refactor: address review feedback on the embedding hub
Frontend: object args for useSetupGuideModals, {isLocked, reason} for
locked steps, edition-aware embed snippet copy, no complete badge on
locked cards, margins instead of nested stacks, and trimmed comments.
Backend: gate enterprise-only models on ee-available? rather than the
:embedding feature, and move the checklist api test to OSS.
* refactor: keep the original order in useGetCurrentApp
The hub lives at /embedding, so it never overlaps /admin -- moving the
admin branch to the end changed nothing at runtime and only widened the
diff. Append the hub case instead.
* refactor: rename the upsell utm constants
Renames UPSELL_CAMPAIGN/UPSELL_LOCATION to UTM_CAMPAIGN/UTM_CONTENT and
points SetupGuide at the "setup-guide" campaign.
Also restores the `boolean` wrapper on has-user-added-database?, since the
trailing when-let yields nil on an instance with no sample database and the
checklist response schema requires a boolean.
* fix: unbreak the embedding hub module boundary
The OSS embedding-hub api namespace now requires metabase.config.core, so
the module needs `config` in its :uses set — regenerated with
`./bin/mage fix-modules-config`.
Also reformats admin/routes.tsx to satisfy oxfmt.
* test: drop unit tests the e2e spec already covers
The Get started page's e2e spec walks the real step order, the upsell banner
and a checklist round-trip against a live backend, so the unit tests asserting
the same things were restating a weaker version of it. What is left is what
e2e cannot reach: mixed token shapes, checklist states that need a mocked
response, and the locked-card click gate.
routes.unit.spec.tsx stubbed the guard, the layout, the page and the whole
setup-guide module, which left it asserting that React Router matches a path.
Rewritten to read the route tree as data instead -- no mocks, no rendering,
and it still covers sso-setup, which no e2e visits.
Also drops two tests that asserted a code path the page does not have: it
builds its Fine-tune steps from a local list and never calls
useGetSetupGuideSteps, so no card can be omitted.
* test: drop the hub root redirect test
One line of route config that the Get started e2e spec already asserts
against a real browser. Removing it takes the Navigate lookup, the type cast
and the ReactNode plumbing with it, leaving the path list -- which is the part
nothing else covers.
* refactor: give the New embed action its own nav component
New embed is not a tab -- it renders in the nav's lower slot, outside the
tab list, is never selected and navigates nowhere. Reusing AreaTab for it
meant making `to` optional and branching the render, which put a link/button
split into a component seven other tabs depend on.
AreaNavButton is the button form as its own sibling, so AreaTab goes back
untouched.
* test: name only what the AI checklist test arranges
The block sets the provider key and nothing else -- embedded Metabot being
on is the default, not part of the arrangement. The block below it is what
pins that flag's contribution.
* style: match the New embed button to the design
It carries its own fill at rest rather than lighting up only on hover the
way a tab does, and its label and icon take the brand colour. Centred in
both the expanded and collapsed states.
* style: build the New embed button on the design system Button
The Figma spec is the design system Button -- subtle variant, MD, an `add`
leftSection -- so use it rather than hand-rolling the chrome. The tinted
resting fill comes from Mantine's `light` variant; our own `subtle` renders
transparent, which is a divergence from the Figma token worth raising with
whoever owns the design system.
* test: drop the AreaNavButton spec
It pinned that the component renders a button rather than a link, which was
a real decision while the chrome was hand-rolled. Building on the design
system Button makes it structurally true, so the spec now covers Mantine.
The layout spec already covers the only branch left, the collapsed label.
* test: sharpen the Get started page assertions
The per-feature test framed licensing one feature as a customer shape; it is
only how the gating is shown, since Pro carries everything bar the cloud-only
features. The prerequisite comment now says why an unlicensed step names none:
it is locked regardless. The AI modal is asserted by its title rather than by
there being any dialog at all.
* test: walk both setup wizards to the end
Each wizard now has a test that seeds its steps, reaches the last one and
takes the action that closes it out, asserting the hub is what it lands on.
They end differently: SSO navigates through the anchor itself, permissions
through a navigate("..") handler.
Also corrects the header comment. Both blocks are @EE because the steps are
enterprise-only, not because of where the checklist endpoint used to live --
it moved to OSS, so the old reason no longer holds.
* feat: name the area in the sidenav
The design gives the embedding hub a title beside its logo. AreaLayout
had no notion of one, so it takes an optional title and passes it
through; Monitor and Data Studio are unchanged.
It hides with the navbar, like the header controls do, so a collapsed
rail stays icon-only.
* docs: promise only the basics on the OSS first-embed blurb
AI is not part of a simple embed on OSS, so the pro line oversells what
the steps below it cover.
* perf: load the embedding hub in its own chunk
The hub pages were reachable from the eagerly imported route tree, so
they shipped in the main bundle for every user, embedding or not. Name
each page in an import() and register it with lazy, as monitor and
transforms routes do.
Resolve every loader in the route spec: an import() path and its export
are no longer type-checked, and a typo would first show as a blank page.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* test: name the hub spec for the hub, not one tab
Every tab's browser test belongs in one spec, one describe each, so the
file name cannot go stale as tabs land. Its header pointed at a path that
does not exist; the older onboarding checklist is in
onboarding/embedding-homepage.cy.spec.ts.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* fix: import the user selectors from metabase/current-user
getUserIsAdmin and useUserKeyValue moved into the current-user module
on master, so the old paths no longer resolve and the frontend build
fails outright.
* test: mock the LLM endpoints the AI card opens
The AI card opens AIProviderConfigurationModal, which fetches
/api/llm/providers and /api/llm/provider-types. Neither was mocked, so
"opens the provider modal while unconfigured" failed on the unmocked-route
guard -- deterministically, on every branch carrying this spec.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* style: sort the LLM mock imports
sort-imports orders members by codepoint, so ProviderTypes precedes
Providers. Caught by fe-lint, not by the pre-commit hook.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* fix: restore connect-database card copy on the get started page
The card had inherited the "Create a dashboard" card's description
from a copy-paste. Restore the wording used by the old setup guide
checklist while the design catches up.
* refactor: replace NumberedCard render closures with a HubCard union
The section arrays only needed to carry which card to render and its
data; wrapping each in a (position) => ReactNode closure just to defer
numbering added a layer with no benefit over switching on a plain
union at render time.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat: add the embedding hub's Security tab (#80347)
* feat: add the embedding hub's Security tab
The method toggles become rows in one card rather than a card each, in the
design's order: Modular embedding, Modular embedding SDK, Guest embeds,
Full-app embedding. Below the paywall the card lists guest embeds alone --
the others need `embedding_simple`, whose runtime only registers when the
token carries it, so their toggles would be switches that turn nothing on.
The card's heading appears only when it lists more than one method.
SameSite is paid-only for the same reason: it governs the session cookie and
guest embeds never set one, so on a guest-only instance it offers a decision
that cannot affect anything the admin owns.
Published guest embeds are listed when any exist, whatever the toggle says.
That needs the two listing endpoints to answer with embedding switched off,
so they drop `check-embedding-enabled` and keep only their settings
permission check. Publishing and every serving path stay gated -- an admin
who has just turned guest embeds off is exactly the one who needs to audit
what is still live.
Admin's own Security page and its siblings keep their current exports
alongside the new ones; EMB-1526 removes that section and them with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QHCSv6oQRLUUGo34V1eEDp
* fix: keep the CORS description literal for i18n extraction
babel-plugin-ttag extracts translatable strings at build time, so a jt
template's static text has to be literal in the source. Hoisting the
sentence into a variable and interpolating it made the extractor
hard-fail, which broke verify-i18n-files on this branch and on every
branch stacked above it.
Branch the whole jt expression instead, so each arm carries its own
literal sentence and the HoverCard stays the only interpolation. The
edition-aware copy is unchanged, and the resulting msgid matches what the
base branch already emits, so existing translations still apply.
* refactor: use the shared UpsellBanner on the Security page
Follows the Get started page off the hub's fork of UpsellBanner and onto
the shared component, wired the same way the embedding settings page
already wires it.
* test: scroll the published guest embeds card into view
The hub clips its content and scrolls it internally, so on a CI-sized
viewport the card sits below the fold. Cypress treats an element clipped
by a non-scrollable `overflow: hidden` ancestor as hidden, so findByText
resolved the heading but `be.visible` failed.
Both assertions were identical, so they move into one helper.
* feat: present modular, SDK and guest embeds as one switch
Separate switches add complexity without adding security, since whoever
can embed also controls them, and guest is a property of modular rather
than a method beside it. Full-app keeps its own switch.
The merge is presentational: the switch reads on when any of the three
settings is on and writes all three at once, which is what the backend
flag will do once EMB-2257 replaces them with one. A setting pinned to an
env var cannot be written, so one pinned setting locks the row rather
than letting a partial write recreate the mixed state.
* test: cover the Security page's conditional cards
The spec only exercised the published-embeds card. SameSite and the full-app
origins list are conditional too, and their conditions are the ones a reader
is most likely to get wrong.
* feat: rewrite the embedding legalese modal copy
One switch now covers modular embedding, the SDK and guest embeds, so
the modal no longer needs a per-setting headline. The list becomes one
paragraph, and the actions read Cancel / Agree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* feat: name the method each Security setting belongs to
One page now holds settings from several embedding methods, so the
secret key and the authorized origins say which method they apply to.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* feat: reword the SameSite cookie description
Drop the method list, split the copy into two paragraphs, and render it
in the secondary colour the other rows inherit from SettingHeader --
the nested Text was resetting the colour to primary.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* feat: reword the embedding methods rows
The paid row leads with what modular embedding and the SDK let you
build. OSS has one method, so its row says Enable embedding rather
than naming guest embeds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* test: match the renamed authorized origins title
The widget title now names the method, so the exact-text query no
longer matched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* docs: source the merged switch rationale
The reason was real but read as an assertion. Name where it came from:
Alessio's call in the Embedding Settings Reorganization thread, carried
by EMB-2257.
Trim the OSS paragraph to its first sentence. Which methods the token
feature gates is visible in the code right below it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* perf: load the hub's Security tab in its own chunk
Follows the lazy route registration the Get started tab now uses, so the
Security page does not pull the admin settings widgets into the main
bundle. Resolve every loader in the spec: an import() path and its export
are no longer type-checked.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* refactor: drop the EmbeddingSecurityWidgets wrapper
It had one consumer. The admin Security page never used it -- it renders
the two widgets itself -- so the extraction shared nothing, and living in
that page's file made the two look like one component renamed. The hub
page composes the widgets directly instead.
Restores the admin page's spec, which had been repointed at the wrapper
and so left the shipped page uncovered. The widget behaviour it asserted
is already covered by the hub page's own spec.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* refactor: compose the methods card the way peer settings do
Pass the heading to SettingsSection instead of rendering a Title inside
it, and drop the Stack the section already renders. Label the row's
switch, which every other embedding toggle does.
Say where the merged-switch decision came from rather than asserting it,
and mark mergedSettingKeys as temporary at both ends -- EMB-2257 replaces
the fan-out with one setting and deletes the prop.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* refactor: title the published embeds card as its source page does
SettingTitle, matching the admin Guest embeds page this card came from,
which also gives the section its anchor id.
Drops embeddingHubAppearance -- no appearance route exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* fix: write every merged setting from the terms modal
The modal intercepts the first enable, and it wrote only the setting it
was handed. So the very first time an admin turned the merged switch on
-- the path every new instance takes, since the terms start unseen --
enable-embedding-sdk and enable-embedding-static stayed off while the
switch read on.
The existing test missed it by pinning show-simple-embed-terms to false,
which routes around the modal. Covered now at that seam, and end to end
through a real click and a fresh load.
The browser test has to start from embedding off, which the default
snapshot does not: it enables sdk and static, and the merged switch reads
on when any of the three is on. Publishing a dashboard is itself gated on
guest embeds, so the sibling test enables them before it publishes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* fix: check embedding and embedding_simple features independently
Modular embedding and full-app embedding are separate token
features. Gating full-app on embedding_simple hid it for accounts
that only have the embedding feature.
* test: cover embedding_simple and embedding features independently
The methods card now checks the two token features separately, so
the test setup needs to mock embedding alongside embedding_simple
and cover the full-app-only case the merge previously collapsed.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat: consolidate the three embedding method settings into one flag (#80348)
* feat: back the merged embedding switch with one setting
Modular embedding, the SDK for React and guest embeds become one
setting, `enable-embedding-modular`; full-app keeps its own. The
previous commit merged them in the UI only, writing all three at once,
which left the gates reading something the switch did not say.
There is no migration. Env beats the app db when a setting resolves, and
a Liquibase changeset cannot see env, so a migration would mishandle
env-pinned instances -- and writing env-derived values into the db would
freeze them, breaking the operator's env var as the live control.
Instead the getter falls back: until the new setting is set it reads as
the OR of the three it replaces, taken through normal resolution so
their env vars count. Nothing is rewritten, so nothing is lost going
back.
The fallback keys off the raw value's *source*, not its value, because
an unset boolean reads as its `false` default. Without that, "an admin
turned it off" and "nobody set it" are the same state, and turning
embedding off bounces back on from a legacy value.
Analytics use `simple_event` rather than a new `embed_share` version:
its event field is a closed enum, and Snowplow is on its way out in
favour of Metaplow, so versioning it is throwaway work.
The three old settings are deprecated rather than removed. Retiring
their env vars is a breaking change, documented separately.
* chore: mark the replaced embedding settings deprecated in 0.65.0
The embedding hub is expected to ship in 65. Tracked in the Notion
release-projects entry; if it slips, these three move with it.
* fix: report embedding telemetry from the setting that is now written
Nothing writes the three replaced settings after the cutover, so on any
instance using the merged toggle they stay false forever while embedding
is on. Left alone, every field derived from them would report embedding
as disabled and adoption would look like it collapsed.
Both call sites now read the setting that is actually written. The field
names stay as they are so existing reports keep resolving, which does
mean three fields that used to differ now always agree -- there is no
distinction left to report.
internal-stats reads through `setting/get` rather than
`get-value-of-type`, which bypasses custom getters and would have missed
the fallback to the replaced settings. Its output feeds metering-stats,
so this one is a billing signal, not only analytics.
* test: point the embedding tests at the merged setting
The three settings the modular flag replaces still exist, so these kept
passing through the fallback getter rather than through the gate they mean
to exercise.
The migration test keeps the legacy keys: it asserts on rows a historical
changeset wrote, not on current behaviour.
* refactor: read the merged embedding setting everywhere
Frontend and e2e still keyed off the three settings the modular flag replaces.
Repointing them is what makes the flag the single source of truth rather than
one more value alongside them.
The legacy keys stay in the settings types: the backend still defines them.
The legalese modal loses its setting argument. It now writes the merged flag
and clears both terms settings -- either one left on would re-open it. Turning
guest embeds on now goes through the modal, which it did not before; that
follows from guest being folded into modular.
* fix: keep the embedding terms modal paid-only
Consolidation pointed the OSS row at the same setting the modal guards,
so an OSS admin enabling guest embeds would have been asked to accept
terms that do not apply: the copy is about modular embedding and the SDK,
and about shared accounts as unfair use of a paid seat, while guest
embeds serve anonymous viewers over a signed JWT and OSS has no seats.
Gate the terms path on the embedding_simple token feature. Paid still
sees the modal on off -> on, OSS never does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ErtCFepL9GnTTBE3QwSmiH
* test: start the embed modal card test from embedding off
The merged setting reads as the OR of the settings it replaces, which
the pro snapshot leaves on, so the card the test looks for never
rendered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* fix: stop reading the deprecated embedding settings
Marking the three replaced settings deprecated made every remaining
call site a lint failure. some-embedding-enabled? reads the merged
setting, which answers the same until an admin writes it. The env-var
sync tests exercise the deprecated settings on purpose, so they keep
them behind an ignore, and the budget records the ten new ones.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JVkbXYuZi14Cpnv22AGGJu
* test: drive the guest embed tests from the static flag
These exercise the /api/embed endpoints, which guest embedding gates.
Pointing them at the merged enable-embedding-modular made them assert
against a flag wider than the behaviour under test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* fix: collapse the merged embedding flag in the e2e setup
Renaming the three settings to one left each call site written twice.
In embed-parameters the two calls disagreed -- simple true, static false
became modular true then modular false -- so the whole spec ran with
embedding off.
The two auth-switch tests now share one setup by design, since one flag
governs both options. Their titles said otherwise.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* feat: consolidate the two embedding terms settings into one flag
The merged toggle governs modular embedding and the SDK, so the terms
behind it are one gate, not two. The switch was OR-ing the pair inline
while the wizard and theme editor still wrote only the simple one, which
re-opened the legalese modal over terms the admin had accepted.
show-modular-embed-terms reads as that same OR until an admin answers it,
so an upgrade cannot drop an acceptance, and every accept path now writes
the one setting.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* docs: explain the merged embedding stats read and e2e setup
Note why the stats namespace reads through setting/get (to run the
fallback getter), and what the merged e2e specs start their embedding
state from.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* test: give the embedding settings specs one enable option
The helper carried isEmbeddingSdkEnabled and isEmbeddingSimpleEnabled,
OR-ing them into the single setting that now backs the switch. They also
doubled as the embedding_sdk and embedding_simple token features, which
stay separate, so the two uses are split rather than merged: one
isEmbeddingEnabled for the setting, token features passed on their own.
Restores envSettingKeys over a boolean, since the card still houses a
second row whose setting can be pinned to an env var, and drops terms
fixtures from specs that cannot reach the modal.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* test: cover the merged legalese behaviour
One flag means a state that could not happen before: embedding on with
the terms never accepted. An OSS admin turns guest embeds on, sees no
legalese because none is shown below the paywall, and upgrades.
The admin toggle prompts on off to on only, so it stays quiet there by
design; the embed flow asks for the terms instead. Both halves are
covered, along with the other states the flow section renders.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* test: drive the embed setup helper from one enable flag
The helper OR-ed simpleEmbeddingEnabled and guestEmbeddingEnabled into
the single setting the switch now backs -- two options that cannot be set
independently once there is one flag. They collapse into one
modularEmbeddingEnabled, which drives the setting and the token feature.
The two guest AppearanceStep tests kept modular enabled via the simple
option anyway, so dropping the guest option leaves their behaviour (dim
state follows the static terms) unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011eWbw5TiosJfLk9uLwKLgH
* test: point the embed API tests at the merged flag
check-embedding-enabled now gates on enable-embedding-modular, but this
namespace still set enable-embedding-static at four sites, so the whole
embed API suite 400'd with "Embedding is not enabled." in every driver
job. Its siblings were all repointed; this file was missed.
The merged getter's legacy fallback does not cover it: it only fires
when get-raw-value-source is :default, and that checks the db value
first, so any earlier test writing the merged flag kills the fallback
for the rest of the JVM.
* fix: add modular embedding to the embed_share enum
The merged setter emits modular_embedding_enabled/_disabled, which was
not in the embed_share iglu enum. The comment called that acceptable
because Snowplow is being retired, but public-sharing-embed-flow's
afterEach asserts zero bad rows, so it failed CI.
Adds schema 1-0-3 with the two values, the same way simple_embedding_*
was added in 1-0-2, and bumps the version the tracker sends. 1-0-3 also
drops the stale self.version 1-0-1 inherited from 1-0-2.
* test: add the merged terms setting to the serdes baseline
show-modular-embed-terms is :export? true, so it appears in a fresh
export but not in the checked-in baseline, failing
baseline-comparison-test on every app-db and the EE backend suite.
enable-embedding-modular needs no entry -- it is :export? false.
* fix: track the modular embedding toggle as a simple_event
embed_share's `event` is a closed enum, so modular_embedding_* needed a
new schema version. Cutting 1-0-3 was a worse trade than it looked:
schema->version holds one entry per event type and the four legacy
toggles all fire embed_share, so pointing that entry at an unpublished
1-0-3 makes the URI unresolvable and drops every embed_share event in
prod, not just the modular ones.
simple_event takes a free-form event string, needs no schema at all, and
track-event! already fans out to Metaplow, which is where embedding
analytics are headed anyway. This is what EMB-2257 specified. The context
the enum-typed event carried moves into event_detail, the way the SDK
bundle already sends it.
Reverts the 1-0-3 schema added in a5cc6db545d.
* refactor: name the legalese gate for what it decides
The flag reads as show/do-not-show the legalese, so the toggle guard and
the fallback test labels say that, and the labels spell out that
unaccepted is true and accepted is false.
* test: enable preview embed tests through the merged flag
The local helper turned on enable-embedding-static, which only reaches
the endpoint guard through the legacy fallback. An earlier test in the
namespace leaves enable-embedding-modular explicitly set -- with-temporary-setting-values
captures through the getter and restores through the setter -- so the
fallback is gone by the time these run and they 400.
* chore: budget the vector-less kondo ignores this branch adds
The embedding settings sync tests read deprecated settings on purpose,
and use the vector-less ignore form at ten new call sites. Master's
ratchet schema has no :all entry, so seed one at the actual count.
* test: justify the vector-less kondo ignores in the settings tests
Master's kondo-ratchet-test now requires an explanatory comment on any
ignore whose linter is outside :comment-exempt, and :all is not in that
set. Each of the ten sites says why it reads a deprecated setting.
* test: use the map form for the deprecated-var ignores
The ten ignores in the settings tests were the vector-less
#_:clj-kondo/ignore form, which suppresses every linter on the next
form. Naming the rule means the suppression says what it suppresses.
:all drops out of the ratchets entirely (that budget was all this
branch's) and :deprecated-var goes 86 -> 96. Numbers regenerated on
this branch with ./bin/mage fix-kondo-ratchets --seed :deprecated-var.
* test: accept the merged terms in the get-code spec
show-modular-embed-terms falls back to the legacy sdk and simple
settings until an admin accepts it, so clearing only the simple one
left the terms still owed and the Agree CTA still reachable.
* refactor: drop the replaced embedding settings from the types
enable-embedding-sdk, -simple and -static and the two legacy terms
settings have no frontend reader left now that everything goes through
enable-embedding-modular and show-modular-embed-terms.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: point the merged-in EMB-2309 test at the modular flag
Master's new test used simpleEmbeddingEnabled, which the three-way
embedding flag consolidation renamed to modularEmbeddingEnabled. The
merge took both sides cleanly, so nothing conflicted.
* feat: add the embedding hub's Authentication tab (#79509)
* feat: add the embedding hub's Authentication tab
A JWT-only view of the authentication settings. Admin's authentication page
does not change -- this is a second view onto the same settings, so the admin
JWT form is mounted as-is rather than forked, which brings user provisioning
along with it.
Below it, a banner links out to the other SSO methods. After the admin
embedding section goes, this is the hub's only route to SAML, OIDC, Google
and LDAP, so the link is functional rather than decoration.
Below the paywall the tab shows an upsell. `UpsellSSO` could not be reused:
it is the narrow sidebar card from admin authentication and hardcodes a
242px max width, so it renders squished as a page-level upsell. The new one
follows the shape the shipped embedding upsells already use, which is also
why `UpsellCardContent` gains an `image-panel` variant -- a single bordered,
tinted panel, for artwork that carries its own framing or none.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QHCSv6oQRLUUGo34V1eEDp
* fix: reword the upsell trial copy
Design's wording for the shared upsell card: "Get a 14 day trial of this
and other Pro features." The theme upsell spec asserted the old string,
so its trial-copy test could not pass and its negative assertion had
quietly stopped matching anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* feat: give the Authentication tab its three states
The tab showed the JWT form unconditionally. Per the design and
Alessio's answers it now has three: the form once JWT is configured, a
card pointing at admin when only SAML is, and an empty state offering
to set JWT up when neither is.
Only JWT and SAML count as configured SSO, matching has-configured-sso?
on the backend. They are the only methods an embed can drive --
/auth/sso resolves to one or the other, and the SDK ships a jwt and a
saml auth path and nothing else. OIDC, Google and LDAP sign people into
Metabase itself and stay linked out.
Configure JWT goes to the guide's SSO wizard rather than dropping the
admin into a bare form: it is the flow Get started already uses and it
covers the endpoint and the test too. It is mounted under this tab, so
the wizard's back link names Authentication and finishing it returns
here.
The page also waits on the settings query the JWT form gates on, so the
banner no longer renders beside the form's spinner, and the banner is
dropped from the SAML state where the card already links to the same
page.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* refactor: show the plain JWT form when nothing is configured
The tab offered an empty state that sent the admin into the setup
guide's stepped SSO wizard, mounted under this tab. Alessio's call:
show the standard JWT page here and leave the stepped flow to the setup
guide, which already owns it -- he is also redesigning the JWT page, so
a bespoke entry point here would be short-lived.
The SAML-only state is unchanged: it still shows its card and its link
to admin, because JWT cannot be configured from this tab in that case.
Removes the empty-state card, the nested authentication/sso-setup route,
its URL helper, and the back-link label prop that only existed to name
this tab from inside the wizard.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* fix: drop the JWT form's own heading in the hub
The hub's Authentication page already renders a page title, and mounting
the admin JWT form inside it added a second one -- two h1s stacked, the
lower reading just "JWT", which the design does not have.
The form now takes an optional `title`, defaulting to the same "JWT" it
always used, so the admin route at settingsRoutes.tsx renders unchanged.
The hub passes `null`; SettingsPageWrapper already skips the heading box
when the title is falsy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* refactor: give the hub its own upsells module
data-studio and monitor each keep their upsell pages in an `upsells/`
directory with a BaseUpsellPage that assembles DottedBackground,
LineDecorator and UpsellCardContent. The hub had no such module, so its
SSO upsell hand-assembled the same four imports from admin/upsells.
Mirrors that structure: BaseUpsellPage holds the assembly, and
AuthenticationUpsellPage is left with the token gate and the copy. The
card is aligned to the page heading the hub renders above it, where the
other two centre theirs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* refactor: trim the authentication page's comments
Cuts the JWT/SAML rationale down to the fact that matters -- embedding
supports only those two -- and the loading note to why a second
subscriber is not a second request.
Drops `?? false` on the `jwt-configured` read. It is optional in
metabase-types/api/settings.ts where its SAML peer is not, but the only
use site treats undefined and false alike, and nowhere else in the
codebase coalesces it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* refactor: drop the image-panel upsell variant
The variant existed so the SSO illustration could sit in a single
bordered panel rather than the nested double frame `image-card` draws.
That is a change to a component every upsell in the product renders, for
one card, so it waits until design confirms the frame is wrong.
The SSO upsell falls back to `image-card` in the meantime.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* chore: drop the unused embeddingHubAppearance url
Nothing calls it and `/embedding/appearance` has no route. It belongs to
the Appearance tab, which lands with EMB-1532.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* refactor: stop parameterising the SSO upsell's source
One call site, one value. The data-studio upsell pages hardcode their
campaign and location the same way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* test: cover the Authentication tab end to end
Three cases the unit tests cannot reach: that the mounted admin JWT form
actually writes its settings from the hub, that a SAML-only instance gets
the Admin card instead, and that an unlicensed one gets the upsell with
nothing configurable under it.
The heading assertion is here rather than in a unit test because the
duplicate title only shows once the real form renders inside the page.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* test: tighten the Authentication tab's e2e assertions
Matches the JWT field by its full label rather than a loose regex. The
asterisk is part of it: Mantine renders the required marker inside the
label element, and getByLabelText matches the label's textContent, which
includes aria-hidden nodes.
Also asserts the upsell's own copy on an unlicensed instance, rather than
only that the form is absent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdJ7UHzej4V16G2W6AP3hQ
* fix: lazy-load the JWT form in the hub through the route slot
#80268 (merged from master) moved SettingsSAMLForm/JWTForm/OIDCForm from
eager PLUGIN_AUTH_PROVIDERS components to lazy route slots, dropping the
eager JWT assignment this page's direct render relied on. Loading the same
route slot with React.lazy keeps the hub on the auth-jwt chunk #80268 split
off instead of re-adding an eager import.
* fix: add a background wash to the SSO upsell illustration
Matches the illustration background the Localization and Tenants
upsells use, so the four embedding hub upsell tabs read consistently.
* test: trim duplicate copy assertions from the Authentication e2e spec
Card copy and the SAML-configured link's href are already unit-tested
next to the component. What is left is what needs a real browser --
confirming the upsell (or the SAML card) renders in place of the JWT
form.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: enable modular embedding in new param-values test
Master added card-param-values-native-card-without-parameters-test
enabling only enable-embedding-static. This branch consolidated the
three embedding method settings into one flag, so that setting alone
no longer enables the static embed endpoint and the request 400s with
"Embedding is not enabled". Use the file's own
with-embedding-enabled-and-new-secret-key! macro, as every other test
here does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amK37aKSDk7sgJsT3AVJQ
* fix: carry the design scale renames onto the stack
GDGT-2486 (#80481) renamed the spacing and radius scales in place,
mechanically remapping every call site that already existed on master.
These files were authored on the stack before that landed, so their
tokens are stale -- still-valid names that now resolve to different
sizes.
Spacing xs->xxs, md->lg, lg->xl, xl->xxl; radius sm->xs, in both the
component props and the CSS module custom properties. Spacing sm is
unchanged at 8px.
Verified against the mapping #80481 applied across its own 1339 files.
CI cannot catch this, so it is a deliberate pass rather than a fix for
a failing check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
* feat: add the embedding hub's Permissions tab (#79510)
* feat: add the embedding hub's Permissions tab
The admin permissions editor, mounted a second time under the hub. Admin
permissions does not change, and the tab shows whatever the admin editor
shows -- five tabs when tenants is on -- rather than a narrowed copy, so
there is no forked component to keep in sync.
Making it mountable elsewhere needs a base path the URL builders can read.
That is a module-level value rather than a React context, because
`getDatabaseFocusPermissionsUrl` and `getGroupFocusPermissionsUrl` are
called from redux selectors and thunks, which have no component to read a
context from. Only one editor renders at a time, and the write happens
during render, so the first child render already builds correct URLs; admin
wraps its own routes too, so the value is restored whenever admin's editor
mounts after the hub's.
Three navigations inside the editor built their target from a literal
/admin/permissions rather than that base path, so picking a collection or
switching between the databases and groups views threw a hub user into
admin. They go through the base path now, like every other permission URL.
The tab carries the hub's page title, which is load-bearing: without it the
editor's tab row starts at the top of the area, where "Permissions help"
sits under the app switcher.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QHCSv6oQRLUUGo34V1eEDp
* fix: restore the permissions base path from admin's own routes
The hub's Permissions tab set a module-level base path during render and
relied on its unmount effect alone to put admin's back. Navigating from the
tab to /admin/permissions runs admin's render before that passive cleanup,
so admin's breadcrumb selectors could build hub links on their first pass.
Admin now wraps its permissions routes in `PermissionsBasePath` as well, so
the default is set during admin's own render. That is what the tab's commit
message already claimed.
Also drops the tab's explicit `<Outlet />`: `PermissionsBasePath` defaults
its children to one, and adds the spec the component never had -- the
set-during-render contract is what the admin wrapper rests on.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* fix: make the embedding hub's Permissions tab full-width
The 800px content cap and AreaContent's own padding were both still
applying, so the permissions editor never reached the design's full
width. Adds a fullWidth escape hatch to AreaContent and wires it
through for the permissions route only.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFKUvQLEMmyQ43m5VPtLMa
* fix: make the permissions editor's selection color configurable
Admin's purple (navbar-admin*, protected from whitelabeling) was
hardcoded into AdminTreeNode's selected row and EntityNameLink,
leaking into the embedding hub's blue-branded mount of the same
editor. Both now read from a context, defaulting to admin's purple;
the hub overrides it via PermissionsBasePath's new selectionColors
prop.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFKUvQLEMmyQ43m5VPtLMa
* fix: derive every permissions accent color from one value
Three separate tokens (background_surface-brand-strong/-subtle, plus
navbar-admin's own triplet) made the override fragile, and missed the
"You've made changes" bar entirely -- it stayed on accent7 regardless.
Collapses the context to a single ColorName; every consumer (tree
selection, entity link, edit bar) derives its own shade from it via
color()/alpha() instead of picking a second unrelated token. EditBar's
admin boolean becomes an accentColor prop so permissions can plug into
the same lever.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFKUvQLEMmyQ43m5VPtLMa
* fix: move the permissions accent color to CSS Modules, off fabricated tokens
The prior fix introduced custom CSS variables (--permissions-tree-node-*)
that read as new design-system colors, and extended two emotion files
that are deprecated in favor of CSS Modules. Replaces both:
- The context now carries a plain isHub boolean (utils/is-hub.tsx)
instead of a color value, mirroring base-path.ts's own module-singleton
shape where that pattern is actually needed (redux selectors/thunks
with no component tree) -- isHub's consumers are all components, so it
stays a React context.
- AdminTreeNode moves out of FilterableTree.styled.tsx into a plain
component + AdminTreeNode.module.css, selecting between colors the
design system already owns (navbar-admin/accent7 vs core-brand/brand)
rather than a fabricated bridge variable. Selectors use `li.<class>` --
a bare class ties in specificity with TreeNodeRoot's own emotion rule
and the winner would depend on injection order.
- EntityNameLink and EditBar keep their original, byte-identical emotion
styling for admin; the hub override is a plain inline style / an
existing `admin` boolean, never a new token.
- EditBar itself is migrated off emotion entirely (EditBar.module.css),
since it was being touched anyway and the pattern was already
deprecated.
- EmbeddingHubLayout's full-width routing moves from an inline pathname
check into a `fullWidth` field on each tab's config, so adding another
full-width tab is a one-line change.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFKUvQLEMmyQ43m5VPtLMa
* test: cover navigation through every level of the hub's permissions editor
Clicks through Data (Groups and Databases views), Collections, and
Application, drilling one level into each, then asserts the URL and nav
never leave the embedding hub.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFKUvQLEMmyQ43m5VPtLMa
* refactor: rename permissions is-hub to isEmbeddingHub and rework tree colors
Full name makes the flag's purpose explicit at call sites. Selection
colors move off ad-hoc emotion overrides onto AdminTreeNode.module.css,
using the data-css-specificity-hack convention and core-white instead
of mantine's color token.
* fix: route tenant collection navigation through the hub's base path
Both pages hardcoded navigate("/admin/permissions/...") in their
item-select handler, so selecting a shared or tenant collection while
inside the embedding hub kicked the user out into full admin. Use
getPermissionsBasePath() like every other permissions sub-page, and
restore the e2e test's item-click coverage for both tabs.
* test: assert permissions editor path by prefix, not substring
cy.url().should("include", ...) matches anywhere in the URL. Anchor on
pathname with a leading-^ regex instead, which also makes the paired
not.include("/admin/permissions") checks redundant -- an anchored
/embedding/permissions match already rules that out.
* refactor: simplify permissions color override and drop redundant comment
Use the raw CSS var string directly in PermissionsTable's inline style
instead of the color() helper -- no compile-time benefit was needed here
that outweighs the extra import. Also trims a comment on
PermissionsBasePath's isEmbeddingHub prop that only restated its name.
* refactor: derive the hub color switch from the base path, drop the context
isEmbeddingHub tracked its own state alongside base-path.ts's, always set
together by the same caller -- a second variable with no way to drift
independently. Collapse it into a plain derived check instead of a
context/provider pair.
Also drops PermissionsBasePath's now-redundant admin wrapper: reaching
admin permissions with the hub's base path still set requires a hard
navigation (typed URL, or this app's back button), which reinitializes
the module and needs no explicit reset. Only the hub's own mount/unmount
still needs to touch the singleton.
* test: cover the new table-under-database permissions route
getAdminPermissionsRoutes() gained a route for tables directly under a
database, without a schema segment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8T4ZvTbdRbF85B98maGNh
* fix: carry the design scale renames onto the stack
GDGT-2486 (#80481) renamed the spacing scale in place, mechanically
remapping every call site that already existed on master. This page was
authored on the stack before that landed, so its token is stale -- a
still-valid name that now resolves to a different size.
Spacing md->lg.
Verified against the mapping #80481 applied across its own 1339 files.
CI cannot catch this, so it is a deliberate pass rather than a fix for
a failing check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
* fix: align permissions chrome with the hub app switcher
The hub places its app switcher 3.5rem from the right edge and its page
title 2.5rem from the left, but the permissions toolbar's help button
ended 1.75rem from the right and the save banner sat at 2rem on both
sides, so neither lined up on the hub's full-width permissions page.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NoNBWFMfL68mPmofjNq66x
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat: add the embedding hub's Tenancy tab (#79511)
* feat: restyle the tenants upsell for the embedding hub
New copy, a new illustration and a left-aligned card, from the embedding
hub's design. `UpsellTenants` is shared: admin People renders the same card
above its people list, so this changes that surface too. That is intended --
the two should not diverge -- and it is split out here rather than buried in
the Tenancy tab commit, which is what actually needs it.
The old artwork had this card as its only reference, so it goes with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* feat: add the embedding hub's Tenancy tab
A second view of the tenant surfaces. Tenants live under admin People and
stay there, so the route fragment is mounted a second time rather than
copied. What that costs is not the routes -- it is the roughly thirty
hardcoded /admin/people/tenants strings inside them, which now come from a
`useTenantUrls` context that admin and the hub each provide.
A context rather than the module-level value the Permissions tab uses: these
builders are only ever called from components, while
`getGroupFocusPermissionsUrl` is also called from redux selectors and thunks
that have no component to read a context from.
The tab renders conditionally rather than reusing `createTenantsRouteGuard`:
that guard redirects to /admin/people when tenants are licensed but
`use-tenants` is still off, which is exactly the state a hub user reaches by
clicking a tab that is always visible. So: no licence shows the upsell,
licensed-but-off shows the enable card, both on mounts the fragment.
The enable card opens the user-strategy modal in place rather than routing to
it, because that modal route hangs off the tenants listing, which this branch
of the tab does not render -- the URL would match and nothing would mount.
Two width interactions look like the hub's fault and are not: the listing
centres itself with an auto inline margin, which beats align-items on a flex
item and collapses it, so it gets a child with an explicit width; and its
empty state capped at 700px, narrower than every sibling tab, so the cap is
now the hub's column width.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* fix: remove tenant/user URL helpers left dead by createTenantUrls
useTenantUrls and useUserUrls replaced every caller of these; nothing
references them any more.
* fix: fix tenancy page UTM campaign, dead route fallback, and a typo
The docs link used a different UTM campaign than the rest of the hub
(embedding_hub vs embedding-hub), so its clicks were not attributed
with the others. The routes.tsx fallback for PLUGIN_TENANTS.tenantsRoutes
could never render, since EmbeddingHubTenancyPage only mounts an Outlet
once tenants are licensed and enabled. Also fixes a missing "you" in the
enable-tenancy copy, and tags the new unlicensed e2e block @EE to match
its siblings in the file.
* refactor: trim comments to their one-line why
No behavior change; shortens comments that had drifted into restating
the code or arguing a design decision instead of stating the reason.
* fix: add breathing room around the tenants upsell diagram
The diagram frame filled the full width of its bordered box with no
margin, unlike the design. Shrinks the frame ~15% and recenters it
within the same canvas so the artwork carries its own padding, the
same way the shape geometry already did vertically.
* fix: revert tenancy oss e2e block to match its true siblings
An earlier commit tagged this block @EE to match the file's older
unlicensed-SSO convention, but the tab's actual siblings (appearance,
localization, nav) all use a plain "oss" describe with no tag. Matches
those instead.
* test: enable tenancy through the UI instead of the API in e2e
The test only asserted the tenant tabs appear once use-tenants is
true, setting it via H.updateSetting. Drives the enable card and the
Pick a user strategy modal instead, so at least one path through the
actual UI is exercised.
* refactor: rebuild tenant URLs as a module singleton, not a context
createTenantUrls returned a whole object of URL builders bound to a
basePath threaded through React context, so tracing what a given call
produces meant following the provider tree. Every real consumer of
these URLs is a component, same as the permissions editor's, and that
one already solves this with a module-level base path plus plain
exported functions (admin/permissions/utils/base-path.ts). Matches
that shape here: setTenantsBasePath/getTenantsBasePath/
resetTenantsBasePath, called imperatively by whichever host mounts
the tenant routes.
This also removes the two describe/hook layers (TenantUrlsProvider,
useUserUrls) that existed only to have somewhere to call a hook from.
* refactor: keep tenant/user URL functions at their original names
newTenant, editTenant, editUser, deactivateUser, etc. now read the
shared base path directly instead of being deleted and rebuilt behind
a hook -- same signatures every other call site already expects.
* refactor: revert tenant-aware components to plain function calls
These only became components or gained a hook call because a hook
needed a component to run in. Reading the new singleton is a plain
import, so UnsubscribeUserMenuItem goes back to a bare element, and
ReactivateUserButton/TenantsListingEmptyState/
TenantsSummaryOnboardingStep/EditUserStrategySettingsButton go back
to their original arrow-function bodies.
EmbeddingHubTenancyPage sets the base path itself now, the same way
EmbeddingHubPermissionsBasePath does for the permissions editor.
* test: assert on real content after enabling tenancy in e2e
Replaces the toast assertion (flaky) with the tenants listing's own
empty-state copy, and adds a check on the OSS upsell's description
alongside its title, so the test confirms the actual tenants-specific
content rendered, not just any upsell.
* refactor: consolidate tenant URL builders
Reuse the permissions editor's existing setPermissionsBasePath/
getPermissionsBasePath instead of a second, parallel permissionsPath
field on the tenant URL module. Use namespace imports (Urls/TenantUrls)
consistently instead of importing individual functions, matching the
convention already used throughout the urls files. Inline the two
single-use tenantGroupUrl/tenantPeopleUrl helpers at their call sites
now that getTenantsBasePath is exported directly. Preserve the
declaration order of the pre-existing tenant URL functions and the
Urls import alias rather than reshuffling them.
* fix: keep tenants upsell centered in admin
The embedding hub's restyle of UpsellTenants (left-aligned, no
horizontal padding) was applied unconditionally, so admin People's
existing render of the same card lost its centered layout. Add an
align prop, defaulting to admin's original centered layout, and have
the hub opt into the left-aligned one.
* refactor: rename tenant-urls to base-path, matching its sibling
Drops the tautological spec — the round-trip it tested has no
equivalent for the permissions editor's base-path.ts, and the real
behavior is covered by EmbeddingHubTenancyPage's own spec and e2e.
Aligns file name, accessor order, and comment placement to that
sibling, and un-exports ADMIN_TENANTS_BASE_PATH now nothing outside
the module reads it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* refactor: use named imports for getTenantsBasePath
The Urls/TenantUrls namespace alias is reserved for actual urls.ts
files; common/tenants isn't one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* fix: rename UpsellTenants's location prop to source
Avoids confusion with router location; matches
UpsellEmbeddingTheme/UpsellBigCard/UpsellPill's existing convention
of taking a source prop and mapping it to the tracking location
internally.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* refactor: simplify EmbeddingHubTenancyPage from self-review
Reuses SettingsPageWrapper instead of a hand-rolled Stack/Title shell,
matching the Authentication and Security tabs. Renames the local
`root` to `tenancyBasePath`, marks the docs-link icon aria-hidden so
its accessible name is just "Documentation", and simplifies the
route-guard comment. Also asserts on the upsell's description text,
not just its title.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* fix: match tab boundaries in TenancyTabs, not raw string prefixes
pathname.startsWith(tab.to) has no segment boundary, so a tenant
whose id starts with "groups" or "people" (:tenantId is a free-form
path param) would light up the wrong tab. Reuses EmbeddingHubLayout's
isUnder, which already solves this for the hub's own top-level tabs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* fix: extend timeout for the post-apply tenants-listing assertion
Enabling tenancy through the modal chains two network round trips
before the empty state can render: the use-tenants setting update's
own refetch, then the tenants list query once the listing mounts.
That's tight against Cypress's 4s default under CI load — reproduced
twice in a row in CI, not flaky-random.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* fix: match the tenants-listing empty state text with a regex
The prior timeout bump was the wrong diagnosis: the empty state's
copy is interpolated with three inline anchors (jt`...`), so no
single DOM node's text content ever equals the plain string exactly
-- Cypress's default string matcher requires a whole-node match, not
a substring. The CI screenshot showed the text rendered correctly;
the assertion just couldn't find it. A regex matches the substring
instead, which is why it kept failing even at 10s.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146A1MwgsMWTE4xdRyfdLpY
* fix: align the Tenancy tab's title-to-card gap with its siblings
Dropped the gap="xl" override on SettingsPageWrapper so it uses the
same default gap as the other embedding hub tabs, and added the same
background wash the other upsell illustrations use.
* test: trim duplicate copy assertions from the Tenancy oss e2e test
Card copy is already unit-tested next to the component. What is left is
what needs a real browser -- confirming the upsell renders in place of
the real tenants surfaces.
* fix: keep the reset-password link under the tenants path
UserSuccessModal is mounted both by admin People and by the tenants
routes the hub's Tenancy tab renders, so the hardcoded /admin/people
link sent a hub user into admin. Urls.resetPassword already branches
on isInternalUser via getTenantsBasePath.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014H8Ma3D2dsLvaz4PkpsxTS
* test: count the new table-under-database permissions loader
* fix: carry the design scale renames onto the stack
GDGT-2486 (#80481) renamed the spacing scale in place and dropped the
shadow scale's md key, mechanically remapping every call site that
already existed on master. This page was authored on the stack before
that landed, so its tokens are stale.
Spacing md->lg, lg->xl, xl->xxl, and shadow md->sm -- the shadow scale
has no md key any more, so that card was rendering no shadow at all.
Verified against the mapping #80481 applied across its own 1339 files.
CI cannot catch this, so it is a deliberate pass rather than a fix for
a failing check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat: add the embedding hub's Appearance tab (#79512)
* feat: add the embedding hub's Appearance tab
The embedding theme editor plus the appearance settings that show up inside
an embed, on one tab -- there is no separate Themes tab, so the standalone
nav item goes away and its upsell moves here.
Both listing and editor now take their base path from the host so their links
resolve under whichever section mounted them. The listing's own "Themes"
heading is behind `showHeading`, which admin keeps and the hub turns off:
the hub page is titled "Appearance" and puts the grid under a "Themes"
section of its own, and two headings would stack.
The two halves are technically gated on different features -- themes on
`embedding_simple`, the appearance settings on `whitelabel` -- but they
arrive together in practice, so the tab is gated as a whole rather than
building a partial state nobody will see.
`EmbeddedAppearanceSettings` is a deliberate subset: loading message, the
Metabase links toggle and the two empty-state illustrations. Logo, favicon,
login and landing illustrations, instance colours, fonts and application
name stay in admin, since they are not what an embed shows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* docs: trim self-review comments in the Appearance tab
Cut the multi-paragraph JSDoc/inline comments down to their factual
core, and drop soft pointers to "the design" in favor of stating the
actual reason.
* fix: lowercase "full-app embedding" in the Appearance banner copy
The phrase sits mid-sentence, not as a heading, so it shouldn't be
capitalized like the Full-app embedding settings card title.
* fix: nest the theme editor route under appearance/theme
New and edit theme URLs were /embedding/appearance/new and
/embedding/appearance/:themeId, colliding with the tab's own path
segment convention. Nest them under a theme segment instead, and make
the editor full-width like the rest of its side-by-side layout instead
of capped to the tab's default 800px.
* fix: match the full-app appearance banner to the design spec
Was background-info with a border; the Figma spec calls for
background-brand with no border.
* fix: link the Get started theme card to the hub's Appearance tab
The Appearance tab now exists, so the card no longer needs to fall
back to the admin theme listing.
* fix: update routes spec for the appearance/theme route nesting
The spec still expected embedding/appearance/:themeId from before the
theme editor route moved under embedding/appearance/theme/:themeId.
* fix: use SettingsPageWrapper for the Appearance tab's title
Was a bespoke Stack + Title with a hardcoded 2.5rem gap, which put the
tab out of step with the other embedding hub tabs' title-to-card
spacing.
* test: count the new table-under-database permissions loader
* fix: load the hub theme editor lazily, behind its token gate
The theme editor route mounted its page eagerly via `element=` with a
static import, instead of the `lazy=` pattern every sibling route
uses. That pulled the EAJS runtime into the main bundle rather than a
lazy chunk, and skipped the `embedding_simple` token gate the
Appearance page applies before showing the editor, so a deep link to
/embedding/appearance/theme/:themeId opened it on plans that cannot
use it.
Add EmbeddingHubThemeEditorPage as a lazy-loaded wrapper that checks
the gate and redirects to the Appearance tab when it's absent, and
route :themeId through it via `lazy=` like the rest of the hub.
Co-Authored-By: Claude Sonnet 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* fix: carry the design scale renames onto the stack
GDGT-2486 (#80481) renamed the spacing scale in place, mechanically
remapping every call site that already existed on master. This page was
authored on the stack before that landed, so its tokens are stale --
still-valid names that now resolve to different sizes.
Spacing xs->xxs, md->lg.
Verified against the mapping #80481 applied across its own 1339 files.
CI cannot catch this, so it is a deliberate pass rather than a fix for
a failing check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
* fix: full-bleed the embedding hub's upsell pages
Review on #79513 flagged the Localization upsell's dotted background as
cropped. Root cause: BaseUpsellPage rendered inside the hub's 800px
capped column, so DottedBackground had no room to fade before its edge,
and LineDecorator's oversized bleed lines got clipped mid-fade.
Fixes it by making a gated tab's whole content area full-width (same
mechanism Permissions already uses) and moving BaseUpsellPage's title
inside the dotted field, so the field owns the whole tab instead of
sitting under a separately-rendered heading. Title and card now share
one fit-content, centered box so their edges always line up, with an
explicit 160px gap between them matching the design.
Authentication, Tenancy and Appearance each get their own hub-local
upsell page through BaseUpsellPage now, instead of reusing admin's
UpsellTenants/UpsellEmbeddingTheme -- those stay untouched, since they
still serve admin surfaces directly and get removed once EMB-1526 drops
the admin embedding routes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015WHaGA2f7AsXGCdQmBzsHw
* fix: redirect the bare appearance/theme path back to Appearance
Visiting /embedding/appearance/theme with no theme ID matched the
theme route segment but had nothing to render, leaving a blank page
(flagged in review on #79512). Adds an index redirect back to the
Appearance tab, matching the pattern the hub root already uses.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015WHaGA2f7AsXGCdQmBzsHw
* fix: flow the full-app appearance banner's link inline
The Admin link sat in its own nowrap Group next to the sentence, so on
narrow screens the sentence wrapped but the link got pushed onto its
own line at the far edge instead of flowing with the text (flagged in
review on #79512). Renders the whole banner as one Text with the link
inline, so it wraps as a normal sentence.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015WHaGA2f7AsXGCdQmBzsHw
* test: cover the hub's theme basePath and the editor guard
Three gaps on the Appearance tab. Clicking an existing theme card was
only proven for the New theme button, so a regression sending card
clicks to the admin route would have passed. The theme editor's token
guard had no test at any level, and its route table redirects the bare
`theme` path regardless of plan, so the deep link needs a themeId to
exercise the guard at all. The branding section asserted only the
loading message, not the two illustration widgets beside it.
Also renames the "oss" block to "unlicensed": it runs on the EE jar
with no token, which is the unlicensed state, not an OSS build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZe6AQywXThKuntLQtLt1B
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix: use the embedding db helpers in the snowplow payload
The master merge kept two t2/count calls in the modular-embedding event
while dropping the toucan2 require, so the namespace failed to compile.
* test: use the merged embedding flag in the collection browser spec
#80881 landed on master using enable-embedding-sdk, which this branch
folds into enable-embedding-modular.
* feat: add the embedding hub's Localization tab (#79513)
* feat: add the embedding hub's Localization tab
Content translation renders only from the admin embedding section today --
it is not on the admin Localization page -- so this is the one tab that is a
genuine move rather than a second view. The instance-wide localization
settings stay in admin and are not surfaced. Admin's copy goes away with the
rest of the admin embedding section, in EMB-1526.
Below the paywall it shows an upsell, which keeps the hub's tab set the same
seven on every edition. Its copy and illustration are placeholders: no Figma
frame exists yet, so the artwork is a generated placeholder captioned as one
rather than another upsell's, which would make a design that does not exist
look settled. Both are drop-in replacements.
The two specs that need the finished nav -- seven tabs in design order, and
the gem on each paid tab -- land here, because this is the first branch where
all seven exist. They get a nav describe of their own rather than riding
along in a tab's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* fix: correct stale comments on the Localization tab
The page comment claimed admin already loses content translation, but
this PR only adds the hub tab -- the admin route is removed separately
by EMB-1526. The upsell comment pointed at a gitignored plan file that
no reader of the merged code can ever open.
* refactor: reuse BaseUpsellPage for the Localization upsell
Was a hand-duplicated copy of the same DottedBackground + LineDecorator
+ UpsellCardContent shape AuthenticationUpsellPage already builds via
BaseUpsellPage, which is also why its title-to-card gap didn't match
its sibling tabs. Also finalizes the description copy, which was a
placeholder.
* fix: replace the Localization upsell's placeholder illustration
Real artwork in place of the generated placeholder box.
* fix: correct backwards duplicate/link wording in Localization's doc comment
The comment described 'duplicates the admin embedding section's UI'
and 'linking to it' the wrong way round: rendering the same settings
component in two places is reuse, and what the other tabs do instead
is genuinely link out to admin.
* test: move nav-completeness and copy assertions from e2e to unit specs
Two e2e tests --reaches all seven tabs from the nav, shows every tab
with a gem on the paid ones-- duplicated coverage routes.unit.spec.tsx
and EmbeddingHubLayout.unit.spec.tsx already had (route existence,
page resolution, nav link order) or could cheaply get (the gem
indicator, now covered by a new EmbeddingHubLayout test). Dropped both
and moved the Localization tab's body copy assertion into its own
unit spec, alongside the title check it already had.
* test: cover the licensed Localization tab in e2e
The tab branches on the content_translation token feature, but only the
upsell half was covered. Add the pro case asserting the dictionary
configuration mounts, and rename the sibling block from "oss" to
"unlicensed" — it runs on the EE jar with no token, which is the
unlicensed state, not an OSS build. Matches the naming the get started
and authentication blocks already use in this file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZe6AQywXThKuntLQtLt1B
* fix: keep the permissions base path across hub tabs
Both the Permissions tab and the Tenancy tab declare the hub as the
permissions editor's URL host, and both reset that on unmount. React
renders the incoming tree before cleaning up the outgoing one, so
navigating between them reset a value the other had just claimed, and
the tenants listing's permissions link fell back to the admin route.
Comparing the stored value could not fix it: both hosts store the same
string. Track which host claimed last with a useId, and let only that
one release.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
* style: wrap the permissions base path setter signature
oxfmt requires it under the line limit.
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* feat: update the JWT signing key flow to the approved design (#81068)
* feat: add the embedding hub's Localization tab
Content translation renders only from the admin embedding section today --
it is not on the admin Localization page -- so this is the one tab that is a
genuine move rather than a second view. The instance-wide localization
settings stay in admin and are not surfaced. Admin's copy goes away with the
rest of the admin embedding section, in EMB-1526.
Below the paywall it shows an upsell, which keeps the hub's tab set the same
seven on every edition. Its copy and illustration are placeholders: no Figma
frame exists yet, so the artwork is a generated placeholder captioned as one
rather than another upsell's, which would make a design that does not exist
look settled. Both are drop-in replacements.
The two specs that need the finished nav -- seven tabs in design order, and
the gem on each paid tab -- land here, because this is the first branch where
all seven exist. They get a nav describe of their own rather than riding
along in a tab's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* fix: correct stale comments on the Localization tab
The page comment claimed admin already loses content translation, but
this PR only adds the hub tab -- the admin route is removed separately
by EMB-1526. The upsell comment pointed at a gitignored plan file that
no reader of the merged code can ever open.
* refactor: reuse BaseUpsellPage for the Localization upsell
Was a hand-duplicated copy of the same DottedBackground + LineDecorator
+ UpsellCardContent shape AuthenticationUpsellPage already builds via
BaseUpsellPage, which is also why its title-to-card gap didn't match
its sibling tabs. Also finalizes the description copy, which was a
placeholder.
* fix: replace the Localization upsell's placeholder illustration
Real artwork in place of the generated placeholder box.
* fix: correct backwards duplicate/link wording in Localization's doc comment
The comment described 'duplicates the admin embedding section's UI'
and 'linking to it' the wrong way round: rendering the same settings
component in two places is reuse, and what the other tabs do instead
is genuinely link out to admin.
* test: move nav-completeness and copy assertions from e2e to unit specs
Two e2e tests --reaches all seven tabs from the nav, shows every tab
with a gem on the paid ones-- duplicated coverage routes.unit.spec.tsx
and EmbeddingHubLayout.unit.spec.tsx already had (route existence,
page resolution, nav link order) or could cheaply get (the gem
indicator, now covered by a new EmbeddingHubLayout test). Dropped both
and moved the Localization tab's body copy assertion into its own
unit spec, alongside the title check it already had.
* test: cover the licensed Localization tab in e2e
The tab branches on the content_translation token feature, but only the
upsell half was covered. Add the pro case asserting the dictionary
configuration mounts, and rename the sibling block from "oss" to
"unlicensed" — it runs on the EE jar with no token, which is the
unlicensed state, not an OSS build. Matches the naming the get started
and authentication blocks already use in this file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DZe6AQywXThKuntLQtLt1B
* fix: keep the permissions base path across hub tabs
Both the Permissions tab and the Tenancy tab declare the hub as the
permissions editor's URL host, and both reset that on unmount. React
renders the incoming tree before cleaning up the outgoing one, so
navigating between them reset a value the other had just claimed, and
the tenants listing's permissions link fell back to the admin route.
Comparing the stored value could not fix it: both hosts store the same
string. Track which host claimed last with a useId, and let only that
one release.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
* style: wrap the permissions base path setter signature
oxfmt requires it under the line limit.
* feat: update the JWT signing key flow to the approved design
Replace the single setup modal with the approved flow: a read-only
generated key shown in "Create a secret key", and a confirm step before
regenerating an existing key.
- read-only key box on one line, icon-only copy button
- "Regenerate key" is now a secondary button
- regenerating asks "Delete key and generate a new one?" first, then
shows the new key in "Store your new key"
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSPdZEZk6NgAeB9C21YWDA
* test: cover the JWT secret key create and regenerate flows
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSPdZEZk6NgAeB9C21YWDA
* fix: always show a freshly generated JWT signing key
Reading the token from the RTK Query cache surfaced the key generated
the last time the modal was open, so a user could copy or save a key
that was about to be replaced. Hold it in local state instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CSPdZEZk6NgAeB9C21YWDA
* refactor: reuse ConfirmModal and CopyTextField props in FormSecretKey modals
RegenerateKeyConfirmModal duplicated ConfirmModal's title/message/
confirm/cancel layout instead of using it, and SecretKeyModal hand-rolled
the CopyButton/Tooltip/ActionIcon markup that getCopyTextFieldProps
already provides. Also aligns SecretKeyModal's body spacing, text color,
and cancel button styling with ConfirmModal's conventions.
* fix: prevent dismissing FormSecretKey modals via X, outside click, or escape
The generated key can't be shown again, and the regenerate confirmation
is destructive, so both must be dismissed through an explicit button
rather than accidentally closed.
* fix: don't autofocus the secret key modal's readonly input
Mantine's modal focus trap was landing on the readonly key field by
default, which triggers its focus border color, not any override on
the field itself. Autofocus the confirm button instead, and exclude
the input from focus entirely since it's disabled until the token
loads.
* style: drop border/padding overrides on the secret key input
Mantine's own Input[data-variant="default"] rule already sets a
neutral border color at the same specificity, so the custom border
was a coin-flip fight against the design system rather than a real
override; the default padding already accounts for the copy button's
rightSection too. Keep the CSS specificity hack for the properties
that do need overriding (radius, background, monospace font).
* test: use full-string queries for the new secret key assertions
Only the lines this diff added; existing regex queries elsewhere in
the file are left as-is.
* refactor: rename SecretKeyModal.tsx back to SetupKeyModal.tsx
Keeps the reviewer-facing diff on this branch to a plain modification
instead of a delete+add across an unrelated PR. The component itself
stays named SecretKeyModal.
* refactor: name the has-secret-key check instead of checking value directly
value is a raw secret-key string; using its truthiness inline four
times reads as cryptic. hasSecretKey names what's actually being
checked.
* test: assert the copy button writes the token to the clipboard
The old test only checked the button rendered with no visible label,
which didn't verify copying actually works.
* fix: stop naming an embed type in the shared enable CTA
Enabling embedding via SSO or Guest now flips the same toggle
(enable-embedding-modular), so wording like "enable modular
embedding"/"enable guest embeds" implied a type-scoped action that
no longer exists. Reworded the CTA sentences and toasts to generic
"embedding"; the two terms flags and their legalese stay separate
since SSO and Guest/AGPL are gated to different editions and never
shown to the same admin.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdRnBCJB84JkuZwLrdhsJp
* test: follow the new secret key modal flow in the JWT spec
The set-up modal's confirm button is now "Create", and regenerating
goes through a confirmation modal ("Delete key and generate a new
one?") before the store-new-key modal, so the spec's single "Done"
click no longer matched either step.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8T4ZvTbdRbF85B98maGNh
* test: expect the generic wording in the guest enable CTA
146acfbb6ea reworded the shared CTA to "enable embedding" when the
simple-embed feature is unavailable, which is the case for the guest
card in both the EE-without-token and OSS/Starter specs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8T4ZvTbdRbF85B98maGNh
* test: use the renamed Create button in the hub's JWT spec
The hub's copy of the admin JWT form goes through the same secret key
modal, whose confirm button this branch renamed from Done to Create.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8T4ZvTbdRbF85B98maGNh
* fix: import useMetadataToasts from its new home
Master moved the hook out of metabase/metadata/hooks into
metabase/common/hooks; this file was the last caller left behind.
* fix: close the secret key modal when generation fails
The modal turns off its close button, escape and click-outside so an unsaved
key cannot be dismissed by accident. On a failed `random_token` request no key
ever arrives, so the confirm button stays disabled and the modal has no exit at
all. Closing from the catch keeps the guard for the case it was written for.
Reported by @lorem--ipsum on #81068.
* fix: carry the design scale renames onto the stack
GDGT-2486 (#80481) renamed the spacing and radius scales in place,
mechanically remapping every call site that already existed on master.
This modal was rewritten on the stack before that landed, so the tokens
on its new markup are stale -- still-valid names that now resolve to
different sizes.
Spacing md->lg and lg->xl on the modal, radius md->sm on the secret key
field's CSS module.
Verified against the mapping #80481 applied across its own 1339 files.
CI cannot catch this, so it is a deliberate pass rather than a fix for
a failing check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
* fix: keep the guest CTA a whole sentence per edition
Interpolating a translated noun into a translated sentence leaves
translators with "To continue, enable %s and agree to…" and no way to
inflect the surrounding words. Give each edition its own complete
sentence instead.
The "agree to the usage conditions" branch loses its edition split
altogether: show-static-embed-terms is force-false on Pro, so
termsAccepted is always true there and the branch never matches.
Updates the three negative assertions in the OSS/starter spec that
still named the old copy, and which no longer guarded anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
* refactor: leave the modular section comment alone
The file's only change was a reworded doc comment, with no code under
it -- and the new wording invented "SSO-specific terms", which do not
exist: the SSO radio gates on show-modular-embed-terms, the Guest radio
on show-static-embed-terms.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
* fix: let the keyboard reach the generated secret key
tabIndex={-1} took the field out of the tab order, so the key could not
be read or selected without a mouse. It was not what kept autofocus off
the input: useFocusTrap returns the [data-autofocus] element first, and
that is already the confirm button.
Records why the modal has no escape hatches, and what a hung request
costs, next to the props that cause it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
* test: cover that the secret key modals resist Escape
All three modals disable the close button, outside clicks and Escape so
a shown key cannot be lost, and nothing asserted it -- the PR asked
reviewers to check it by hand instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
* style: drop redundant rules on the secret key modals
ConfirmModal already renders its confirm button feedback-negative, and
flex-wrap: nowrap is the default for the flex container the rule itself
declares. Trims the comment above the specificity hack to what the
rules under it actually do.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdtGmjnS93RHcNfu5mMUA7
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Remove the embedding admin settings and relocate its components (#81018)
* feat: delete the admin embedding route section, redirect to the hub
Deletes the /admin/embedding route subtree now that the embedding hub
replaces it, and adds redirects from every old path to its /embedding
equivalent so no bookmarked or linked URL breaks. Removes the
"Embedding" entry from the admin sidebar/paths list, and the three
route-chrome files that back the deleted section directly
(AdminEmbeddingApp, EmbeddingNav, the admin-side setup-guide page) --
none of them are imported anywhere else.
Sweeps every hardcoded /admin/embedding/... reference this forces:
the SSO wizard's troubleshooting copy, the setup-guide default return
paths, the related-settings links, and the entire e2e suite (15 spec
files). Two e2e specs that only asserted on the deleted admin sidebar
are removed; their edition-specific content checks (OSS/starter
upsell + CORS visibility) are ported into the hub's own security e2e
spec, updated to match its actual (redesigned) upsell banner. Adds
dedicated e2e coverage for the redirect map itself, including the two
backward-compat chained-redirect cases.
Moves the version-pinning card to admin General, the one piece with
no hub-tab home -- and fixes a dropped `embedding_simple` gate in the
process: the original render path required embedding_simple AND
embedding_sdk AND is-hosted?, and the first port of this card only
carried the latter two.
Part 1 of 2 for this PR -- part 2 relocates the components this
section leaves stranded under admin/, once this part isolates the
routing diff.
* refactor: relocate stranded embedding admin components
Moves the components left homeless by commit 1's admin/embedding route
deletion: the theme editor/listing cluster to embedding/themes/, and
the hub-only settings widgets to embedding/settings/.
Along the way, moving the settings widgets surfaced that shared/embedding
cannot import feature/admin (shared is the architectural foundation and
never imports up). SettingHeader, SettingsSection, and AdminSettingInput
had to come out of admin/ too, into a new shared settings-components/
module, so the widgets are self-sufficient. All ~114 consumers across
admin/, enterprise/, and transforms/ were repointed to the new module.
The rest of the old EmbeddingSettings/ cluster (EmbeddingSettings.tsx,
GuestEmbedsSettings, the EmbeddingSecuritySettings wrapper,
EmbeddingSdkSettings, NewEmbedButton, SharedCombinedEmbeddingSettings)
had zero remaining consumers once commit 1 landed, so it's deleted
rather than left as dead weight.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* fix: carry the theme editor's app-tier boundary override to its new path
ThemeEditor already had its own app-tier module-boundaries.mjs entry,
predating this PR: it previews the live embed through the app-tier
embedding-iframe-sdk runtime, which has real module-load side effects
(custom element registration, a window.metabaseConfig watcher) and
can't be split into a shared-tier reusable piece.
The previous commit moved the folder but left that entry's pattern
pointing at the old admin/embedding path, so it silently stopped
matching and the files fell through to the general shared/embedding
classification -- caught by a boundaries/element-types lint failure.
Updates the pattern to follow the move, no suppression needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EMyCTcwtcQsGggkCL9QmGp
* fix: update EmbeddingHubTenancyPage's SettingsSection import path
SettingsSection moved to metabase/settings-components in this branch's
relocation commit; this import was missed.
* fix: correct theme-editor redirect target to include the /theme segment
/admin/embedding/themes/:themeId redirected to /embedding/appearance/:themeId,
but the theme editor is actually mounted at /embedding/appearance/theme/:themeId
(routes.tsx nests it under a "theme" path segment). The old target matched no
route.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QdRnBCJB84JkuZwLrdhsJp
* fix: update EmbeddingHubAppearancePage's SettingsSection import path
The move of SettingsSection to metabase/settings-components missed this
call site, so the Appearance tab failed to resolve the module at runtime.
* refactor: drop the unmounted InteractiveEmbeddingSettingsCard
Removing the embedding admin settings page deleted the only render site
for PLUGIN_ADMIN_SETTINGS.InteractiveEmbeddingSettingsCard, leaving the
slot written but never read. The origins widget it wrapped is still
mounted, by the hub's Security tab.
* fix: return setup wizards to the guide host they came from
The permissions and SSO wizards left the guide with a relative "..",
which resolves against the route, not the host, so a user who entered
from the embedding home page landed in the hub. Carry the host in the
existing returnToEmbeddingSetupGuide param instead, so it also survives
a reload mid-wizard.
* fix: remount a redirect whose target is itself a redirect
Without a key, React reuses the RedirectRoute instance across a
sibling redirect chain, so the first target stays frozen in state and
/admin/embedding/modular never reaches /embedding/security.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8T4ZvTbdRbF85B98maGNh
* fix: repoint DatabaseCachingEditor at the relocated SettingsSection
Master added this importer after the module move, so it still pointed
at metabase/admin/components/SettingsSection, which this branch
deletes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W8T4ZvTbdRbF85B98maGNh
* fix: redirect retired embedding paths to the hub
Six legacy redirects still pointed at /admin/embedding and
/admin/embedding/guest, which this branch turns into redirects
themselves. Chained that way the second RedirectRoute reused the
first's instance and froze on the first target, so they were made to
work by keying the shared redirect() helper -- changing behaviour for
every redirect in the app to serve six call sites in one file.
Point the six straight at /embedding/security instead and restore
router/redirect.tsx and its spec. No redirect in the tree targets
another redirect now, and the helper's own spec passes unkeyed.
Cover the whole table rather than a spec per page: admin/routes.unit.spec.tsx
gains the 14 path/target pairs read off the real route tree, plus the
no-chaining invariant. The four legacy paths that had no e2e case at all
join the redirects spec's table, which now covers all 14.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* fix: default the theme editor base path to the hub
The editor's basePath still fell back to /admin/embedding/themes, a
route this branch deletes, while its ThemeListing sibling had already
been moved onto the hub path. Only the hub mounts the editor and it
passes basePath explicitly, so nothing was broken -- but cancel and
delete would have navigated through a redirect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* docs: trim the module boundary comments
The theme-editor note grew from five lines to ten while restating the
move it documents; the settings-components and embedding-hub ones
listed ticket history the code does not need. Keep the reason each
carve-out exists and drop the narrative around it.
Every line touched here is a comment this branch itself added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* refactor: drop the -setup suffix from the wizard paths
get-started/permissions-setup and get-started/sso-setup stutter under a
parent segment that already says setup. Rename them to
get-started/permissions and get-started/sso.
Safe to rename rather than redirect: the hub reaches master only when
embedding-hub-main merges, and that PR is still open, so no released
Metabase has ever served these URLs.
Sharing a final segment with the /embedding/permissions tab is fine --
different parent -- and EmbeddingHubLayout's "does not light up
Permissions on the permissions wizard" test covers exactly that.
The two hub routes come from an earlier PR in this stack; they are
touched here because this branch's redirects point at them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* test: restore the OSS and starter nav checks, fix the upsell
The deleted embedding-admin-settings-{oss,starter} specs each checked
which links their edition's sidebar offered. That sidebar is gone; the
hub shows every tab to every edition and marks the unlicensed ones with
a gem, so assert that instead, once per edition as before.
The OSS upsell test asserted a button and so could never pass. OSS
leaves PLUGIN_ADMIN_SETTINGS.useUpsellFlow returning no trigger, which
sends UpsellCta down its url branch to an ExternalLink; paid builds wire
the trigger up and get an UnstyledButton. Assert the link.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* test: assert against the hub layout, not admin
The embedding pages now render inside the embedding hub, so the specs
look for embedding-hub-main and the "Embedding hub" nav instead of
admin-layout-content and admin-layout-sidebar.
* test: visit the content localization page
The content translation settings moved off /embedding/security onto
their own /embedding/localization page.
* test: follow the theme editor to its hub route
The editor moved from /admin/embedding/themes/:id to
/embedding/appearance/theme/:id, and it renders full width rather than
inside the admin sidebar layout.
* test: start New embed from the hub nav
The button now lives in the hub nav rather than in the SDK and guest
embed setting cards, so the specs share one clickNewEmbedButton helper.
The guest-only entry point is gone with those cards, and with it the
assertion that it forces guest auth.
* test: drop the modular embedding settings spec
It asserted on the Tenants link inside the settings card, which the
embedding admin settings removal took with it.
* fix: drop the modular embedding gate on version pinning
Version pinning is about the SDK package, so the SDK feature and a
hosted instance are the only conditions it needs.
* docs: say why theme-editor is app tier
Name the import chain that forces it, and why settings-components is
separate from the data-only settings module.
* docs: correct the no-literal-metabase-strings reasons
The strings are exempt because only admins see them, not because they
are legal copy.
* docs: note where the theme upsell coverage went
The component no longer gates itself, so its upsell tests live in
theme-upsell.cy.spec.ts.
* fix: import AdminSettingInput from its relocated path
Master added the fast-mode input to this section after the relocation
commit was written, so the new import still pointed at the old widgets
directory this branch empties.
* fix: redirect the setup guide straight to the hub
/admin/embedding/setup-guide pointed at /embedding, which is itself a
redirect to /embedding/get-started. Every other retired path lands on a
real page, so send this one there directly too.
* refactor: require basePath on the theme editor and listing
Both defaulted basePath for a second host that no longer exists, and the
move to the hub left each default pointing at the other one's path: the
editor would have returned to /embedding/appearance/theme and the listing
would have created themes at /embedding/appearance/new, neither of which
routes anywhere. The hub always passes basePath, so drop the defaults.
* test: assert every hub tab from one configuration
The per-edition nav checks only asserted the four gated tabs, so a gem
appearing on an ungated one went unnoticed. List all seven with their
expected gem state instead, shared by the OSS and Starter blocks.
* test: cut the admin redirect spec to two browser cases
admin/routes.unit.spec.tsx already asserts the whole retired-path table
off the route tree. Keep only what reading the tree cannot show -- that a
redirect fires in a browser and carries its params -- instead of a restore
per path.
* docs: state where the New embed button lives now
The comment contrasted the hub nav against settings cards that no longer
exist, which goes stale as soon as nobody remembers them.
* fix: point the theme editor page at the relocated module
This branch moves the theme editor components from
metabase/admin/embedding/components/ThemeEditor to
metabase/embedding/themes/components/ThemeEditor. The hub's lazy
theme editor page still imported the old path, which the rebase
onto this branch left broken since git has no way to track an
import string across a rename.
Co-Authored-By: Claude Sonnet 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* fix: drop the stray unused ThemeEditor import in routes.tsx
The ThemeEditor relocation commit's routes.tsx hunk updated a
top-level `import { EmbeddingThemeEditorApp } from ...` line to its
new path. That line had already been deleted when the theme editor
route switched to a `lazy=` loader, so rebasing the relocation
commit onto that fix reintroduced it as an unused import, since git
applied the patch by fuzzy context match rather than conflicting.
Co-Authored-By: Claude Sonnet 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YT26JiAJ25NK11W8PJLymT
* fix: align embedding e2e specs with the hub pages
Repointing these specs from the admin embedding pages to the hub was a
URL and test-id swap, so assertions that the hub renders differently
were left behind:
- The Get started card reads "Configure data permissions and tenants";
only the permissions page kept "and enable tenants".
- The Security page's upsell passes an onClick, so UpsellCta renders a
button rather than an anchor and the upgrade URL is not in the DOM.
- Published guest embeds render only when embeds exist, so unpublishing
removes the section instead of showing empty states.
- The tenants links are gone: tenancy off opens the user-strategy modal
in place and tenancy on renders a tab to the hub's own tenancy page,
both covered by EmbeddingHubTenancyPage.unit.spec.tsx.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amK37aKSDk7sgJsT3AVJQ
* fix: match the hub's checklist card DOM in setup-guide e2e
The migration to the hub kept the admin card's DOM contract. A hub card
is an anchor, a button or a div depending on whether the step links,
opens a modal or is locked, so closest("button") only matched a third of
them; its done badge is a check icon labelled "Step N complete" with no
"Done" text; a locked card carries aria-disabled and a hover tooltip
rather than a lock icon; and the SSO card is titled "Set up SSO".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amK37aKSDk7sgJsT3AVJQ
* fix: drop the stale settings wait in content translation e2e
The dictionary upload helper waited on GET /api/setting after visiting
the localization page. The admin page fetched it on load and the hub's
Localization tab does not, so the wait timed out and took the five
upload-form tests with it. The findByTestId assertion that follows
already retries.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016amK37aKSDk7sgJsT3AVJQ
* test: assert the OSS upsell renders a link, not a button
The hub's Security page takes its click handler from
PLUGIN_ADMIN_SETTINGS.useUpsellFlow, whose OSS stub returns
triggerUpsellFlow: undefined. UpsellBanner then falls back to
useUpgradeAction, which also returns onClick: undefined off a hosted
instance, so UpsellCta matches its url branch and renders an anchor.
An OSS bundle can therefore never render this CTA as a button. The
assertion passed locally only because build-hot bundles the enterprise
plugins regardless of MB_EDITION; CI's oss-subset job builds the real
OSS bundle and got a link.
Restores the href assertion the migration dropped, with the hub's
campaign and location.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
* test: await the throttled recents before asserting the embed
The spec aliases the throttled /api/activity/recents response as
@getRecents but never waits on it. That worked while visitNewEmbedPage
clicked a card on /admin/embedding, which only rendered once that page
had loaded its settings -- several seconds in which the throttled
response made progress.
The hub's "New embed" button is in the nav and is clickable as soon as
the layout mounts, so the wizard now opens immediately and the response
is still in flight when the assertions run. CI shows neither dashboard
title present, not the example dashboard, so this is the wizard still
resolving rather than a fallback.
Awaiting the alias restores what the test name already promises.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFXLWvx9oSucyyp5Ezu3Hv
* fix: order the embedding imports as import/order expects
eslint import/order wants metabase/embedding/themes before
metabase/embedding-hub/upsells. Fixes the fe-lint CI failure.
* fix: restore the Box import on the JWT form
The master merge's import resolution dropped Box while the group-schema
block still renders it.
* fix: repoint DomainsSettingsPage at the relocated modules
Master added this page against the pre-move paths for SettingsSection,
SettingHeader and isSettingSetFromEnvVar.
* test: restore the csp-img and custom-viz mock settings
setup() indexes both off generalSettings; master's copy of the mock
dropped them.
* style: order the relocated settings-components imports
import/order after repointing them at the new module.
* fix: drop the settings left behind on the general page
csp-img-enabled and custom-viz-enabled render on the domains page now,
so their reads here were dead, along with the imports they needed.
* style: drop the blank line eslint left in the import block
* refactor: query the hub content by its main landmark
The hub's content area is the main content of the page, so mark it up as
one instead of reaching for it by test id. Drops embedding-hub-main in
favour of component="main" and repoints all 45 call sites.
Kept on the outer Box rather than AreaContent, which the monitor layout
also renders -- tagging it there would give that page a main landmark
too. MonitorContent still uses monitor-main on its equivalent Box.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0198kPp8K99BRJ15z73m5MY4
* test: assert the checklist step number explicitly
The label carries the step's position, so matching /^Step \d+ complete$/
proved only that some badge was there. Every one of these sites runs
under a pro token, so the numbers come from the licensed order.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0198kPp8K99BRJ15z73m5MY4
* Revert "refactor: query the hub content by its main landmark"
This reverts commit
|
||
|
Romeo Van Snick
|
a22958cee3 |
Track page load times on every master merge (#80978)
* Add a harness for measuring bundle load time * Report the second load and make the benchmark safe to run in CI * Run the load benchmark across network and CPU conditions * Sign the load benchmark in to a real instance * Track page load times on every master merge * Backfill load times for recent master commits * Rewrite the load-time backfill in TypeScript with a tested core Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Launch the benchmark jar the way we ship it, and lint the .ts scripts * Measure the whole load, not just DOMContentLoaded * Do not let the performance marks gate a reading * Drop the backfill workflow The goal is a global trend, which the per-merge job builds on its own after a few days. Backfilling only bought a readable chart sooner, at the cost of a second workflow and the largest script in the PR. The commit date stamp goes with it. It existed so a backfilled row could claim its commit's date rather than today's, and the per-merge job never set it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Let a PR run the load benchmark against its own uberjar A `workflow_run` workflow always runs the copy on the default branch, so a change here cannot be tried out before it merges. The `workflow_call` trigger lets CI hand the workflow a commit and reuse the uberjar it already built. A called run measures and reports; it does not write rows to Stats. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01171Lkmug15JzrcN2aMBLMk * Report each state from one load, not a median per reading Every reading was medianed on its own, so a row described a load that never happened. It could also put the readings out of order and print a page-ready that precedes the TTFB beside it. `measure.js` now picks the run whose DOMContentLoaded is the median of its series and reports that run whole. The second visit happens once per browser profile, so it stays the run itself, and its page-ready now comes from that same load rather than from a median that included the series' first, uncached run. `Cold spread %` still comes from all the cold runs. A spread is a property of the series rather than of one load. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01171Lkmug15JzrcN2aMBLMk * Read the largest paint with an observer, and cut the timeout The first CI run reported a zero largest paint in all four conditions, and would have done so forever. These entries never reach the performance timeline, so `getEntriesByType` cannot see them. Only a `PerformanceObserver` can, and it has to exist before the paint. Paint entries are buffered, which is why first-contentful-paint needed none of this. The browser keeps revising the value until the user interacts, and the reading is taken once the page reports ready. It is a lower bound rather than the final figure, which is what makes it comparable between commits. That same run took 4.4 minutes, of which the measurement was 164 seconds. The timeout drops from 25 minutes to 10. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01171Lkmug15JzrcN2aMBLMk * Move the bench to frontend/test and write it in TypeScript --------- Co-authored-by: Fraser <frasermcintosh1@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
Fraser
|
ecde907aff | Add definePluginSlot and a reset registry to metabase/plugins (#81926) | ||
|
Romeo Van Snick
|
e4cc4a61ff |
Reduce the frontend to one memoize helper, and stop top-level caches leaking (#81866)
* Add a jest project for heap retention specs Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Scope the parsed x value cache to one getXValues pass Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Assert heap retention on the settled pass, not the first touch Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Key the log timestamp cache on the log object Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Key the schema viewer flow graph on the response object The memo key serialized every table and field of the schema, so each distinct schema held a megabyte-scale string forever. RTK Query returns a stable reference per cache entry, so a WeakMap keeps the identity that useGraphSync depends on without retaining the schema. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Key pie.rows on the dataset instead of a JSON copy of it The memo key serialized every row and column, so each dataset the user viewed held roughly its own size in cache key for the life of the tab. A WeakMap on the dataset with a short settings subkey keeps the same hit rate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Ban new uses of underscore's memoize The 17 remaining call sites are allowlisted. The list may only shrink. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Narrow the memoize rule to module scope A cache built inside a function, a component render, a class field or a constructor is released with whatever owns it. Only a module scope cache lives for the life of the tab, so the rule now flags just those. This drops the allowlist from 17 files to 5, and each remaining entry carries the reason its set of keys stays small. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Remove the memoize allowlist Three of the five module scope caches are gone: - getScrollBarSize takes no arguments, so it is a lazy value, not a cache. - getScheduleExplanation returns a string that no caller compares by identity. - cronToBuilderValue moves to the one caller that needs a stable identity. Schedule takes its value as a useMemo dependency, so StrategyForm now holds that memo itself. A user can write any number of cron strings, so a module cache for them was unbounded. The remaining two carry an eslint-disable with the reason their set of keys stays small. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Follow renderer_utils into the viz-core module Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Cover metabase/utils/memoize in the module scope rule That memoize roots its WeakMap on the function it wraps, so it only releases once that function does. At module scope the function never does, and the nested Maps hold every argument ever passed. No call site does this today, but the rule could not have caught one. The rule now covers both sources, so it drops "underscore" from its name. memoizeClass stays allowed: it keys on the instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop re-reselect from the dashboard selectors createCachedSelector keyed getQuestionByCard and getDashcardParameterMappingOptions on the card id, a primitive, in a FlatObjectCache. That cache is a plain object with no eviction, so a session kept one Question per card id it ever saw, and each Question pins the whole Metadata snapshot it was built with. reselect v5 defaults to weakMapMemoize, which keys on the card and the metadata objects themselves. Entries are released when their card leaves the store or the metadata is replaced, and identity still holds for every card in the working set, which is what connect and useSelector compare on. Measured over 300 cards, with the states dropped: 6.2 MB retained before, 0.0 MB after. Per entry, 21462 B before and 51 B after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Drop re-reselect from the lockfile Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Back metabase/utils/memoize with reselect's weakMapMemoize The hand-rolled implementation had two problems. It allocated two arrays on every call, including cache hits: 225 ms against 20 ms for weakMapMemoize over two million hits. And its WeakMap was keyed on the wrapped function rather than the wrapper, so a cache built around a module-level helper outlived the wrapper that owned it. A plain Map would have been safer. weakMapMemoize matches on the same terms, identity for objects and value for primitives, so no call site changes. It also weak-keys object arguments, which the old one held strongly. definition-cache passed weakMapMemoize as the memoize and argsMemoize options. reselect v5 already defaults to both, so those are dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Route all memoization through metabase/utils/memoize underscore's memoize is gone from the app. Its 16 call sites now use the house helper, which is reselect's weakMapMemoize. Three of them were in metabase-lib, which cannot import metabase/utils, and all three took no arguments, so they became plain lazy fields instead. Two rules keep it that way: - no-direct-memoize-import bans memoize from underscore and weakMapMemoize or lruMemoize from reselect, in any scope. Only metabase/utils/memoize may reach for an implementation. - no-module-level-memoize keeps its job, now that the house helper is the only one left to police. measureTextWidthSafely took a style object that was rebuilt on every render, so identity keying would never have hit. Its third argument is now the font family string, which is the only field any caller ever set. useMemoizedCallback returns the callable shape rather than casting to T, since the memoized function also carries clearCache and resultsCount. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Build memoizeClass on the house memoize It kept a module-level WeakMap and walked a nested Map trie keyed on the instance, the method, the argument count and then each argument, assembling that path into an array on every call. Keying weakMapMemoize on the instance and then the arguments gives the same per-instance caching. One cache per decorated method replaces the shared trie, so the method no longer needs to be part of the key, and neither does the argument count. The file is down to 61 lines from 128. A leak spec covers the property this rests on: an instance's results are released with the instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Measure memoizeClass retention under load 2,000 instances per pass, 20 calls each, across a method with no arguments and one taking a primitive and an object. Nothing is retained once the instances are dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Delete memoizeClass It decorated a prototype method by rewriting the property descriptor, and put the memoization 600 lines away from the method it applied to. Its one consumer memoized a method whose body uses no `this`, so the same caching comes from assigning a memoized arrow to a class field. The field is created per instance, so the cache is released with the instance, and the memoization is now visible at the definition. metabase/utils/memoize is 14 lines, from 128. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Run the heap specs from their own jest config They were a project inside jest.config.js, and a bare `jest` runs every project, which is what CI does. So the sharded unit run picked them up, without --expose-gc, and they failed as designed. They now live in jest.memory.conf.js, the way the timezone specs live in their own config. The shared base moves to jest.base.conf.js so neither file has to duplicate it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Reduce the heap specs to one collection assertion each Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh * Follow the store mocks into test support Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012b67ubbXJVMuQrpuEksbeh --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Romeo Van Snick
|
5db23f4182 |
Move the test-side store mocks into test support (#81499)
* Move the test-side store mocks into test support * Drop the moved store mocks from the side-effect registry Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HNvsg9cPuNQ6BHiDwa1bif --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
Stas Gavrylov
|
5885d30ad0 |
GDGT-2486 Add elevation, radius, and spacing scale variables (#80481)
* GDGT-2486 Add elevation, radius, and spacing scale variables
* GDGT-2486 Remap scale tokens in files merged from master
* Address review
Review fixes for the GDGT-2486 scale migration:
- Revert double-shifted tokens from the merge-remap pass: it re-applied
the old-to-new mapping to 22 files already remapped by the first pass,
shifting spacing values one extra step (e.g. xl 24px -> xxl 32px).
- Fix codemod escapes where old key names kept their name but changed px:
Modal default padding (lg->xl, 24px), MultiSelect radius (sm->xs, 6px),
MetricSearchInput radius-sm, ThemeEditor PreviewPanel radius-md,
ColumnInput theme.spacing.xs->xxs, ChartTypeSettings Space h props,
TableRowForCacheableItem icon placeholder w (md->lg, 16px),
ResizeHandle/ResizableBoxHandle grip h (xs->xxs, 4px).
- Pin Group/Stack default gap to "lg" (16px): Mantine's default is
"md", which is 12px in the new scale (was 16px in the old scale).
- Allowlist the runtime-defined --mantine-spacing-xxs/-xxl and
--mantine-radius-xxs CSS variables in find-never-defined-css-variables
(fixes fe-lint CI).
- Add theme.unit.spec.ts covering the spacing/radius/shadow scales,
dark-mode shadow set, whitelabel independence, and a guard asserting
every scale-based component default resolves to a defined key.
- Derive Card/Paper story argTypes options from the real scale.
- Augment MantineThemeSizesOverride so the custom scale tokens appear in
editor autocomplete for p/m/gap/radius/shadow props.
- Update the emotion-migrate and metabase-ui-component-from-figma skill
token tables to the new scale (spacing/radius values, shadow tokens,
snap tables).
* Update Loki Snapshots
* Address review: fix h/w style props missed by the scale codemod
The codemod did not remap height/width style props, so every bare h=/w=
scale-key value silently changed px with the new scale. Remap each to the
px-preserving key (xs 4px->xxs, md 16px->lg, lg 24px->xl, xl 32px->xxl):
- PaneHeader breadcrumb row h xl->xxl (32px, restores data studio
pane header height)
- BrowseDatabases logo boxes h/w xl->xxl (32px)
- DatabaseConnectionStringField rows h lg->xl (24px)
- DatabaseEngineList logo Center h/w lg->xl (24px)
- Metabot chat chevron flexes h/w md->lg (16px)
- MetricsFilterPill h lg->xl (24px)
- DatasetNotebook divider h xs->xxs (4px)
- DataReference NodeList icon w md->lg (16px)
- transforms-python ResizableBoxHandle grip h xs->xxs (4px)
Audited: no remaining bare h/w/maw/mah/miw/mih old-key props whose px
changed (verified line-identical against master for every occurrence).
* Address review: fix bdrs and responsive-object size tokens missed by the codemod
Two more escape classes from the scale codemod, found by a full sweep of
every touched-token form (string style props, responsive objects, CSS
vars, theme accessors) with each line compared against master:
- bdrs style prop (border-radius): remapped px-preservingly at 69 sites
in 58 files (xs 4px->xxs, sm 6px->xs, md 8px->sm). The codemod remapped
radius= but not its style-prop alias bdrs.
- Responsive object values (e.g. p={{ base: "md", sm: "xl" }}): remapped
the token VALUES at 9 sites in 6 files (base/sm/md breakpoint keys are
untouched; only their spacing values shifted: md 16->lg, xl 32->xxl,
xs 4->xxs).
Sweep result after fixes: zero remaining line-identical old-key token
references to spacing xs/md/lg/xl, radius xs/sm/md, or shadow xs/md in
any form. Intentional survivors unchanged: radius/shadow/bdrs "lg"
(disclosed broken-key-now-16px class), spacing/radius sm where the
px value did not change.
Verified: tsc, eslint (changed files), metabase/ui jest suite.
* Address review: fix remaining token escape forms (object props, braces props, style objects)
Final codemod escape classes from the exhaustive token sweep, all
remapped px-preservingly:
- Object-literal / default-parameter prop values (15 sites / 8 files):
ItemList picker-item default containerProps pb xs->xxs (4px, the
reported gap), FormFooter pt lg->xl, SlackSetup pt lg->xl,
EmbeddingSecretKeyWidget mb xs->xxs, CreateDashboardForm labelProps
mb xs->xxs x2, databases styles mb lg->xl, NotebookDataPicker
menuDropdownProps mt xl->xxl, Banner story py md->lg
- Braces-wrapped string props: CloudSMTPConnectionCard p lg->xl (24px)
and gap md->lg (16px), SettingsLdapForm mt xs->xxs,
DashboardLeaveConfirmationModal mb lg->xl
- style-object CSS properties: DatasetNotebook borderRadius xs->xxs
- Ternary token values inside responsive objects: SummarizeStep gap
md->lg (16px)
Sweep now returns zero real misses across every checked form: string
style props (incl. h/w/bdrs), braces props, responsive objects with
ternaries, object-literal/default-param props, style-object CSS props,
CSS var() references, theme.* accessors. Remaining grep hits are
untouched scales (lh, fz, size, breakpoints) or px-identical sm values.
* Address review: preserve Mantine spacing defaults exposed by Loki
* Update Loki Snapshots
* Address review: fix remaining scale regressions and visual guards
* Address review: make Loki update capture new references
* Update Loki Snapshots
* Address review: keep only targeted Loki references
* Address review: assign ticket component elevations
* Update Loki Snapshots
* Address review: tighten MonthPicker header spacing
* Address review: refine elevation visual stories
* Update Loki Snapshots
* Address review: keep renamed Menu Loki story selected
* Update Loki Snapshots
* Address review: preserve inherited Mantine defaults
* Address review: allow stock Mantine shadow tokens
* Address review: replace token audit test with ESLint rule
* Address review: drop unrelated whitespace-only dev/src changes
* Address review: use rem() in radius scale and rem docs
* Update Loki Snapshots
* GDGT-2486: Address PR comments
* GDGT-2486: Adjust negative tokens
* GDGT-2486: Address review findings
* Update Loki Snapshots
---------
Co-authored-by: Metabase Automation <github-automation@metabase.com>
|
||
|
Uladzimir Havenchyk
|
dff2fd94ce |
chore: replace lodash.orderby with inline function (#81160)
* chore: replace lodash.orderby with inline function * drop deps * fix eslint |
||
|
Andrei Chernykh
|
ad48fed046 | Remove the direct reselect usage and dependency (#81034) | ||
|
Fraser
|
4071663479 | Tidy module side-effects lint rule (#80734) | ||
|
Fraser
|
d2c5d44b63 | Add a lint rule that keeps side-effect-free modules side-effect free (#80047) | ||
|
Andrei Chernykh
|
68746a6d43 |
Clean up leftover JS-era config, deps, and docs after the TypeScript migration (#80032)
* Clean up leftover JS-era config, deps, and docs after the TypeScript migration * Address review: keep .jsx in resolve.extensions, drop .js from loader tests, includes-based e2e validator filter * Keep @babel/node for a separate deps cleanup round * Keep js/jsx in rspack loader tests |
||
|
Fraser
|
e81d02d15d | Give dayjs a facade module (#79963) | ||
|
Romeo Van Snick
|
aa1d39ebc4 | Move SDK storybook themes into embedding-sdk-shared (#79796) | ||
|
Andrei Chernykh
|
b0826c98f3 |
Convert app.js and app-main.js entry points to TypeScript (#79724)
* Convert app.js and app-main.js entry points to TypeScript * Throw instead of logging when the root element is missing |
||
|
Fraser
|
a6b5e09200 | Count dynamic imports as module boundary edges (#79693) | ||
|
Egor Grushin
|
4efc94ee84 | Lint against unchained ttag c() calls (#79502) | ||
|
Fraser
|
27a7dc50e6 | Enforce module public APIs (#79292) | ||
|
Fraser
|
1591e74409 | Fix module stats counting (#78184) | ||
|
Timofey Kachalov
|
806e4b756c |
Data Apps Beta (#74840)
* initial proof-of-concept for data apps POC 2 POC 3 POC 4 Revert "POC 4" This reverts commit |
||
|
Romeo Van Snick
|
cf56c33cdd |
Delete react-router-redux and redux-auth-wrapper (#77590)
* Delete react-router-redux and redux-auth-wrapper * Dedupe redux to a single v5 copy Removing the @types packages dropped a transitive redux@3 pin that kept a typeless redux at the top level and masked a redux@4-vs-RTK-v5 type conflict. Force a single redux@5 so types stay consistent. * Migrate remaining react-router-redux references Drop the now-dead eslint import bans for the deleted package, remove the obsolete redux-action cast in auth logout, and reword stale comments. Keep the metabase/router provenance docs that name the package as their reference point. |
||
|
Kamil Mielnik
|
bd2df5059b | GDGT-2770 RFC 147. Require justification for type casts (#77168) | ||
|
Romeo Van Snick
|
836e12b526 |
Re-own react-router-redux behind the metabase/router facade (#77142)
* Re-own react-router-redux mechanisms behind the metabase/router facade * Wire store, app and store types onto the re-owned router facade * Re-point react-router-redux call sites onto metabase/router * Update router tests and harness for the re-owned facade * Ban react-router-redux imports outside metabase/router * Model routing state's null location contract instead of casting * Flatten CallHistoryMethodAction into per-method history action types * Match history actions with ts-pattern in routerMiddleware * Preserve explicit null location in createMockRoutingState |
||
|
Alexander Polyankin
|
22725a83b9 | Extract a StaticVizRenderer protocol for static-viz rendering (#77315) | ||
|
Romeo Van Snick
|
56879decb0 |
Migrate direct react-router usage onto the metabase/router facade + eslint ban (#77068)
* Facade re-exports full react-router surface via a single leaf seam * Re-point all react-router imports onto the metabase/router facade * Ban react-router imports outside the metabase/router facade (eslint) * Migrate InviteToViewModal Link import onto metabase/router facade |
||
|
Fraser
|
aae7c76e84 | Integrate e2e coverage into module graph (#74621) | ||
|
Stas Gavrylov
|
aa0fe0bdd6 |
GDGT-2676/2677: Migrate Dependency Diagnostics & Admin Tools UI source code under Monitor space (#76416)
* Add monitor dir in metabase-enterprise * GDGT-2676: Migrate Dependency Diagnostics under Monitor space * GDGT-2677: Migrate Admin -> Tools under Monitor space * GDGT-2676: Fix paths * GDGT-2676: Create separate high-level directories for migrated features --------- Co-authored-by: Kamil Mielnik <kamil@kamilmielnik.com> |
||
|
Fraser
|
24ca0fe913 | Error for files that aren't in modules and enforce remaining modules (#76126) | ||
|
Fraser
|
8460bb5470 | Fix unit test violations (#75919) | ||
|
Jakub Chodorowicz
|
feaaafb8e8 | Gdgt 2390 add sources for the tgz test plugins (#73819) | ||
|
Phoomparin Mano
|
b3869c0b89 |
Add MCP Apps with React SDK (#71001)
* Convert metabase/utils files to TypeScript, part 2 (#72679) * Convert metabase/utils spec files to TypeScript, part 2 * Remove some type castings * Convert memoize.unit.spec.js > ts * fix(sdk,embed-js): Refactor query handling to show aggregations/breakout for the last non-empty stage (#72219) * Merge `metabase-lib/v1/utils/card` and `metabase/utils/card` into `metabase/common/utils/card` (#72609) * Rename isTransientId to isTransientCardId for clarity * Move metabase-lib/v1/utils/card into metabase/utils/card * WIP * Fix card comparison types * Move metabase/utils/card to metabase/common/utils/card * Fix lint * Add ts expect error for unknown card usage * Fix import order * Allow passing other values to isTransientId Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix spacing issue Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Colocate isTransientCardId with Question to fix import loop * Be more lenient as to what card can be * Remove dead code branch * Fix formatting * Fix applyParameters helper --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Refactor breakout handling to utilize custom query handlers (#73093) * feat: MCP Apps implementation for visualizing queries Adds MCP Apps endpoint for visualizing ad-hoc queries via the Embedding SDK. Backend: - MCP JSON-RPC endpoint at /api/mcp (SSE + HTTP streaming) - Session management with TTL sweep and scope-based auth via OAuth bearer tokens - resources/list + resources/read handlers serving embed-mcp.html - Short-lived embedding sessions created per resources/read, cleaned up on MCP teardown - visualize_query tool returning structuredContent.query (base64 MBQL) - EE tools/manifest generated from agent API endpoint metadata - Embedded OAuth/OIDC provider for MCP authentication (moved to OSS) - DCR default-scope injection for MCP clients that omit scope during registration - skip-frame-ancestors CSP option for SDK embed route Frontend: - app-embed-mcp.tsx entrypoint + McpUiAppRoute component - Uses embedding-sdk-package (MetabaseProvider + InteractiveQuestion) as a plain SDK consumer — no internal SDK coupling - MCP postMessage protocol: ui/initialize, host-context, tool-input, tool-result - CSS variable resolution for MCP hosts (VSCode sends var(...) references) - Safe area insets + dark/light theme fallbacks - embed-mcp.html Mustache template with stable filename (no content hash) - sessionToken auth config type added to SDK - Removes metabase-adhoc custom element and AdHocQuestion public component Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> add mark session initialized add agent:visualize as an extra OAuth tool scope fix instance url in embed-mcp template use component provider instead of metabase provider fix component provider implementation avoid layout shift via fixed height add automatic sizing hide editor button in MCP Apps view add simple loading indicator revert sessionToken changes as we now inject it directly remove useless CSS variables injection remove useless sessionToken in template comment the colors refactor: extract color transformer add tests for color transformer fix chunks remove the unneeded connect domains revert session-for-user secure the agent:visualize scope remove unneeded eslint exception fix CSS variables lint declare that mcp uses session module add withEditorButton to schema Inline site-url and avoid double call Restore detailed comment for MCP app HMR exclusion Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> skip sdk checks for mcp apps skip EE auth flow for MCP Apps revert prettier changes support vscode and cursor colors Declarative UI tool registration (#71568) address review: remove enableEmbeddingSettingKey rename useMcpTheme to buildMcpAppsTheme (not a hook) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> skip spec files in CSS variable script, remove stale whitelist entries Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> add linear task id on analytics header migrate useMcpApp to @modelcontextprotocol/ext-apps Replaces hand-rolled JSON-RPC postMessage implementation with the official ext-apps library. The lib handles protocol versioning, ResizeObserver auto-resize, and CSS variable/font application. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> add tsconfig paths for @modelcontextprotocol/ext-apps subpaths moduleResolution:node doesn't support package.json exports field, so we need explicit path aliases for the /react subpath. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> use ambient module declaration for @modelcontextprotocol/ext-apps subpaths Replaces tsconfig paths aliases with a declaration file in frontend/src/types/, consistent with how other third-party module types are handled in the project. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> cleanup types in useMcpApp fix theme switching by using imperative style application The useHostStyleVariables and useHostFonts hooks each overwrite onhostcontextchanged, so only the last one registered would fire. Switch to imperative applyDocumentTheme/applyHostStyleVariables/ applyHostFonts calls within a single handler to fix live updates. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> bump @modelcontextprotocol/ext-apps to 1.3.2 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> use HtmlWebpackPlugin for MCP app template instead of manual cache busting The MCP app HTML was previously a hand-written Mustache template with hardcoded script/link references and manual ?v= cache busting. This required several rspack config hacks: stable filenames (no content hash), runtime: false, chunk exclusion from splitChunks, and a CSS filename override. Now the MCP app uses HtmlWebpackPlugin like all other entry points. The template (mcp_apps_template.html) contains only MCP-specific markup and Mustache vars, while HtmlWebpackPlugin injects script/link tags with content hashes at build time. A Mustache variable in the HtmlWebpackPlugin publicPath ({{{instanceUrlRaw}}}/app/dist/) is emitted literally at build time and resolved by Stencil at runtime, working around the srcdoc iframe's about:srcdoc origin and CSP base-uri restriction. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Stateless MCP sessions (#71819) * remove incorrect rebase changes * Expand `construct_query` MCP tool description The tool previously shipped with a ~430-char description and a 12KB auto-generated JSON schema. That's enough for the LLM to see what's valid structurally, but not enough to know which idioms are canonical — so agents routinely produced non-canonical shapes (field-options in a third slot, bare aggregated sort refs, no `append-stage` before filtering on aggregations). Extract the description into a `construct-query-tool-description` def that covers source types, every top-level operation, the nested reference forms (including `with-temporal-bucket` and `with-binning`), filter/aggregation/temporal operator names, and four worked examples (top-N, metric with monthly bucket, filter-on-aggregated with `append-stage`, named expression). Stops short of the full agent_api reference; aims for enough structure to write valid programs without guessing. * Add workflow and pitfalls sections to `construct_query` description Expand the `construct_query` MCP tool description with the guidance agents consistently get wrong: the workflow (fetch entity detail → construct → execute/visualize), an `offset` example where `append-stage` must NOT be added, and a rules section sourced from the capability catalog's `prompt-notes` covering stage boundaries, ref/shape constraints (aggregation-ref for sorting, no third-slot field options, bare `case` fallback, JSON objects only where helpers explicitly call for them), when to prefer direct related-field refs over explicit joins, and metric/date caveats. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trigger new charts through some drill-through actions (#73278) Trigger new charts through drills and show question title adjust interactivity patterns for mcp apps add prototype for query name + context add sorting as stay drill update stay drills add prototype for bars add prototype for time range picker prototype: default to table and do not trigger if no breakouts/aggregations make prompt more assertive inject encoded query as structured content prototype: custom question title store pending cards and use render_drill_through tool call experiment: question titles remove duplicated query bar logic refactor: move mcp question title out always show the question title for drills extract a query stage friendly utility cleanup the query description proof-of-concept refactor: use the extracted query explorer bar cleanup: remove mcp debugging hook add table visualization to query explorer bar do not show query bar if there is ONLY tables add extra top padding style adjustments cleanup unused mcp-dev endpoints use PATCH /api/mcp/ui/drills endpoint use POST instead of PATCH type fixes stub out McpQueryBar with placeholder box for split PR Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> prototype for stored queries add mcp-session-id to request move mcpSessionId to POST body revert execute_query related changes update liquibase migration add tests for drills layout adjustments Address review feedback on MCP query-handle store Schema redesign of mcp_ui_query_handle (renamed mcp_query_handle): unified UUID PK, indexed mcp_session_id, nullable core_session_id with cascading FK to core_session. Defines :model/McpQueryHandle deriving :hook/created-at-timestamped?. Lifecycle changes: - store-handle! returns a fresh UUID per call; reads no longer consume (supports retries and multiple in-flight drills) - delete! reaps both the core_session and remaining handles by mcp_session_id; FK cascade handles the rest - The /ui/drills bypass branch in mcp.api/handler is gone; the iframe callback now lives in metabase.mcp.callback-api at /api/embed-mcp/drills, reads Mcp-Session-Id from the header and validates UUID + ownership Tool dispatch: - *current-session-id* dynamic var removed; session-id threaded explicitly through mcp.tools/call-tool - render_drill_through now takes a {handle uuid} arg; the iframe embeds the handle in the message so the agent passes it through Resource registry no longer conjes tools onto a per-URI vector — keyed by tool name so REPL reloads stay idempotent. Frontend handler now awaits the handle, embeds it in app.sendMessage, and falls back to defaultNavigate when MCP plumbing is unavailable or the POST fails. Drop dead :uri on registered MCP UI tools The tool map was being assoc'd with :uri uri, but no caller reads it — list-tools select-keys'es to [:name :description :inputSchema :_meta] and drops it before sending to MCP clients, and call-tool only touches :scope and :response-fn. The URI is already on the registry key and is exposed to the frontend via :_meta.ui.resourceUri. Index UI tools by name; one resource per iframe The previous shape registered a second :render-drill-through resource that served the exact same iframe — same render-fn, same HTML output. Two URIs serving identical bytes don't represent two resources. Collapse back to one resource (the embed-mcp.html iframe) and key tools by name instead of by URI. Tool names are unique, multiple tools can target the same iframe via :_meta.ui.resourceUri, and REPL re-registration is still idempotent because both registry maps overwrite by their key. Register McpQueryHandle and refresh kondo modules config Wire the new model into copy/serdes-aware places and refresh the kondo modules config now that mcp.callback-api is a public API in the mcp module. reduce visualization height exclude McpQueryHandle models from serialization remove unnecessary Promise.resolve fallback make drill-thru type stronger use "Show me the result" instead of "Show me the drill-through result" harden the prompt for cursor auto model address kondo lint for Toucan2 table-name defmethods update schema file to fix type errors * Show MCP Apps question title without time granularity (#73397) add minimal question title without time granularity extract DescribeQueryStageOptions type add unit test for stripping temporal buckets fix: preserve non-temporal breakout suffixes when stripping temporal buckets refine mcp question title generation * Add QueryExplorerBar pure UI component for MCP Apps (#73373) add query bar component revert McpUiAppRoute changes (moved to next branch) * Apply the query explorer bar to MCP Apps (#73272) add McpQueryBar with chart type, date filter, and granularity controls Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> update query bar explore button must open link to question address code formatting reviews * Store construct_query result server-side to prevent LLM hallucination (#73210) Address review feedback on MCP query-handle store Schema redesign of mcp_ui_query_handle (renamed mcp_query_handle): unified UUID PK, indexed mcp_session_id, nullable core_session_id with cascading FK to core_session. Defines :model/McpQueryHandle deriving :hook/created-at-timestamped?. Lifecycle changes: - store-handle! returns a fresh UUID per call; reads no longer consume (supports retries and multiple in-flight drills) - delete! reaps both the core_session and remaining handles by mcp_session_id; FK cascade handles the rest - The /ui/drills bypass branch in mcp.api/handler is gone; the iframe callback now lives in metabase.mcp.callback-api at /api/embed-mcp/drills, reads Mcp-Session-Id from the header and validates UUID + ownership Tool dispatch: - *current-session-id* dynamic var removed; session-id threaded explicitly through mcp.tools/call-tool - render_drill_through now takes a {handle uuid} arg; the iframe embeds the handle in the message so the agent passes it through Resource registry no longer conjes tools onto a per-URI vector — keyed by tool name so REPL reloads stay idempotent. Frontend handler now awaits the handle, embeds it in app.sendMessage, and falls back to defaultNavigate when MCP plumbing is unavailable or the POST fails. use stored query results to avoid hallucination Address review feedback on construct_query handle store Update construct_query tool description to reflect the new return shape ({"query_handle": "<uuid>"}) so the LLM knows to pass the handle on to execute_query / visualize_query rather than expect a raw base64 string. Update execute_query test to use :query_handle now that construct_query returns it instead of :query. (Bottom-PR amendments folded into the rebased commit during conflict resolution: drop dynamic var, switch to :model/McpQueryHandle + read-handle unified store, log a warning when the agent passes a UUID in :query instead of :query_handle.) Fix input schema breaking some mcp clients Polish visualize_query tool description and error messages - Drop trailing space from the tool description - Reword schema comment from "exactly one of" to "at least one of" (the runtime contract was always at-least-one, not exactly-one) - Distinguish "neither argument provided" from "handle not found" so missing-input clients get a clearer error - Add tests covering the four response branches * Show authentication and CORS errors for MCP Apps (#73633) show authentication and CORS errors update auth error copy to be more actionable render in ComponentProvider boundary move into renderContent * fix: show query bar even when only one data point remains (#73628) show query bar when one data point remains * Remove x and y labels from ad-hoc charts for MCP Apps (#73663) hide x and y axis labels * Send MCP Apps client header from MCP Apps (#73709) update metabase client to mcp-apps * Open drills in Metabase for Claude and tweak tool description (#74087) * do not perform external drills in claude * adjust tool call description * Thumbs up / thumbs down feedback buttons for MCP Apps (#73344) * Implement feedback buttons feedback add explore button for opening in metabase always show explore on the right add thumbs up/down button increase feedback gap rebase prototype branch feedback for metabot add context for feedback * scope the mcp session handle * adjust feedback button gap * add loading state for submit button * add unit tests for feedback submission * throw 400 error if feedback fails to submit to harbormaster * move feedback tests to callback endpoint * add docstrings for mcp feedback types * allow submitting multiple feedback * add docstring for prompt * code cleanup * prompt is optional in construct_query for agent api * add docstring for why prompts has to be stored * remove active view references * add timeout to /api/v2/metabot/feedback harbormaster submission * address kondo linter issues * Handle MCP feedback submission failures * prevent race condition on feedback submission * limit how big construct_query prompt can be * split tool argument type into input and result * re-use the type for feedback submission * cleanup MCP callback tests --------- Co-authored-by: Anderson <ands.alves.nunes@gmail.com> Co-authored-by: Timofey Kachalov <timofei@metabase.com> Co-authored-by: Romeo Van Snick <romeo@romeovansnick.be> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Truter <crisptrutski@users.noreply.github.com> Co-authored-by: Mihael Konjevic <konjevic@gmail.com> |
||
|
Kamil Mielnik
|
7819b3c779 | Custom visualizations (#70479) | ||
|
Romeo Van Snick
|
7ff626e966 | Clean up url utils (#73167) | ||
|
Mahatthana (Kelvin) Nomsawadi
|
673ee61589 |
feat(sdk-package, sdk-bundle): Introduce useMetabot hook (#73075)
* Expose query prop on StaticQuestion and InteractiveQuestion (#72713) * Expose query prop on StaticQuestion and InteractiveQuestion (EMB-1585) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add tests for query prop on StaticQuestion and InteractiveQuestion (EMB-1585) - Convert card.unit.spec.js → card.unit.spec.ts with typed getCard helper - Add deserializeCardFromQuery unit tests (prefix stripping, dataset_query wrapping) - Add InteractiveQuestion query prop render test and schema rejection test - Add StaticQuestion query prop render test and schema rejection test - Fix IQ/SQ schemas to accept query as an alternative to questionId Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * don't intercept props unnecessarily * Remove unnecessary casting * Keep `query` prop internal-only (EMB-1585) Per team discussion, `query` should not be part of the public SDK API. It stays as an internal channel for the upcoming `useMetabot` hook (Layer 2 / EMB-1554). - `SdkQuestionEntityPublicProps`: drop the `query` branch - `SdkQuestionEntityInternalProps`: new internal type that adds `query` - `InteractiveQuestion` / `StaticQuestion`: cast public export to the narrow props; expose `_InteractiveQuestionInternal` / `_StaticQuestionInternal` for tests and the Metabot hook - Yup schemas: type against the internal shape so runtime still accepts `query` - Unit tests: switch `query` specs to the internal component * Clean up PR * Fix unit tests * cleaner API * Remove embarassments * Consolidate tests --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * Expose currentChart from useMetabot (#72714) * Expose currentChart from useMetabot (EMB-1561) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add useMetabot currentChart unit tests (EMB-1561) - New use-metabot.unit.spec.tsx with StaticQuestion/InteractiveQuestion mocked - Tests: currentChart is null before navigate_to, becomes a component after it fires - Tests: routes to StaticQuestion (no drills) or InteractiveQuestion (drills=true) - Tests: currentChart updates when a second navigate_to fires Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Clean up * Defer chart message plumbing to EMB-1562 * Clean up pr * Add missing tests * Allow CurrentChart to be null because users need to check it * Revise public API, drop irrelevant types. * drop more public API unrelated to the SDK * Fix merge conflict --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * Expose chart in messages (#72383) * Add Component to chart messages in useMetabot (EMB-1562) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add MetabotChatHistory filter tests and messages[n].Component tests (EMB-1562) - New MetabotChatHistory.unit.spec.tsx: chart messages filtered from message list, non-chart messages render normally - Extend use-metabot.unit.spec.tsx: messages[n].Component renders StaticQuestion or InteractiveQuestion based on drills prop; Component reference is stable (cached) after a second chart message arrives Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add clarity to code * Clean up * Rename messages[n].Component to messages[n].Chart (EMB-1562) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: document untested passthrough props * test: mock only internal hook * docs: add JSDoc to isProcessing * feat: warn on navigate_to overwrite * test: document why we keep as any --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * Add `useMetabot` storybook stories (#71996) * Add Component to chart messages in useMetabot (EMB-1562) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add MetabotChatHistory filter tests and messages[n].Component tests (EMB-1562) - New MetabotChatHistory.unit.spec.tsx: chart messages filtered from message list, non-chart messages render normally - Extend use-metabot.unit.spec.tsx: messages[n].Component renders StaticQuestion or InteractiveQuestion based on drills prop; Component reference is stable (cached) after a second chart message arrives Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add clarity to code * Clean up * Rename messages[n].Component to messages[n].Chart (EMB-1562) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: document untested passthrough props * test: mock only internal hook * docs: add JSDoc to isProcessing * feat: warn on navigate_to overwrite * test: document why we keep as any * Add useMetabot Storybook demos (EMB-1524) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test more useMetabot properties * cleanup --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * expose `useMetabot` in the package + add E2E (#73047) * WIP first pass on the useMetabot before verification * Prepare for debugging * Debug code * add more tests Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Reconcile with master merge - Rename authProviderUri -> jwtProviderUri (master renamed auth-config field). - Filter locked MetabotErrorMessage variant from SDK public type. - Rename emitted chart-message field Component -> Chart to match public type. - Replace require() with jest.requireActual() in integration spec (lint rule). - Widen test stubs for new SdkStoreState surface. - Drop unused act wrap + silence exhaustive-deps on mock useLoadSdkBundle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * expose proper types * Fix E2E * Remove debug files * Fix implementation from merging master * Fix formatting * Address review * Fix useMetabot hook not working from merging master * Make global state name consistent * remove unused export * Make type consistent * Make tests more readable * Make code more readable * Polish E2E test * Fix useMetabot not working under StrictMode --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Fix moved module paths on master * Move useMetabot code to EE --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
Romeo Van Snick
|
cf67b911e6 |
Clean up utils/redux and utils/entities (#72832)
* Remove star import * Remove any * Code style * Move updateMetadata action to metabase/redux/metadata * Use _.indexBy instead of custom resourceListToMap # Conflicts: # frontend/src/metabase/utils/redux/utils.ts * Move redux action utils into metabase/redux/utils * Move redux utils into metabase/redux # Conflicts: # enterprise/frontend/src/metabase-enterprise/metabot/components/MetabotAdmin/MetabaseAIProviderSetup.tsx # frontend/src/metabase/setup/components/LanguageSelector/LanguageSelector.tsx # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/redux/utils.ts * Move withAction to metabase/redux/utils # Conflicts: # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/redux/utils.ts * Move handleEntities into metabase/utils/entities # Conflicts: # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/entities.unit.spec.ts * Move withNormalize into metabase/utils/entities # Conflicts: # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/redux/utils.ts * Move withCachedDataAndRequestState to metabase/utils/entities # Conflicts: # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/redux/utils.ts * Use sdk api in sandbox components and remove unused selectors * Remove withRouter hoc * Move withRequestState to metabase/utils/entities # Conflicts: # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/redux/utils.ts * Remove fetchData wrapper in fetchRevision as loading state is not used * Move fetchData and updateData into metabase/utils/entities # Conflicts: # frontend/src/metabase/utils/entities.js # frontend/src/metabase/utils/entities.unit.spec.ts # frontend/src/metabase/utils/redux/utils.ts # frontend/src/metabase/utils/redux/utils.unit.spec.ts * Defer rtk instantiation in entities to break import loop # Conflicts: # frontend/src/metabase/utils/entities.js * Fix imports * Fix import * Move entities helpers to metabase/entities * Handle loading and error states from useListUserAttributesQuery * Fix withAction imports * Import from internal modul * Import from utils directly to avoid import loop * Fix imports * Fix types for gtap endpoint helpers * Coalesce empty string to null |
||
|
Fraser
|
c417b95a44 | Selectively enforce module boundaries (#72436) | ||
|
Romeo Van Snick
|
773692eaf3 |
Move metabase/lib to metabase/utils and move domain-related utils to other modules (#71467)
* Rename metabase/lib to metabase/utils * Move metabase/lib to metabase/utils * Move getCrumbs to getCollectionBreadcrumbs in common/utils * Fold compat into react-compat * Remove composeEventHandlers as it is too simple * Move fields types to metabase/common/utils/fields * Always go through getCspNonce * Move jwt helpers into jwt file * Remove unused scroll helpers * Use helper function instead of constant * Remove unneeded removeAllChildren helper * Move data url helper to separate file * Move lib tests inline with modules * Remove unused clipPathReference * Remove unused getMainElement helper * Move canDownloadResults to metabase/common * Merge metabase/utils/engine and metabase/databases/utils/engine * Clean up fetchWithTimeout * Move formatting into formatting folder * Split up formatting tests into formatting folder * Allow null for date_style * Remove need for eslint comments * Remove unused i18n-debug package * Move iframe utils into metabase/utils/iframe * Remove unneeded getObjectValues helepr * Move sortObject into metabase/utils/object * Import delay from the delay module * Rename utils/security to utils/password * Remove unused foreignKeyCountsByOriginTable helper * Move fields helpers to metabase/common/utils * Move entities helpers to metabase/entities * Move passwordComplexityDescription out of Settings * Remove unused string utils * Move tests to have same name as module under test * Merge time-dayjs tests * Move timelines to metabase/common/utils * Use crypto.randomUUID * Replace copy() with builtin structuredClone * Remove single-use isEmpty helper * Remove unused propertiesEqual helper * Move version helpers to metabase/utils/version * Move numberToWord into passwords file where it is used * Remove simple _.isEqual wrapper * Remove isEEBuild wrapper * Fix new lib imports after merge * Fix missing import * Fix whitespace linter exclusions * Remove utils/utils imports * Fix import * Fix noop imports * Fix more aliases * Fix formatting * Fix lint * Allow utils/promise in embedding tests * Fix imprts after merge * Fix lint * Remove delay confusion * Fix lint * Revert to clone implementation * Add fallback for crypto.randomUUID * Fix lint * Revert jspdf * Do not coerce nonce to empty string * Fix types * Fix lint |
||
|
Copilot
|
12a66b2abe |
Add ESLint rule to restrict analytics imports to analytics files (#66780)
* Add ESLint rule to restrict analytics imports to analytics files Co-authored-by: iethree <30528226+iethree@users.noreply.github.com> # Conflicts: # .eslintrc.js # frontend/lint/eslint-plugin-metabase/rules/no-analytics-import-outside-analytics-files.js * Fix test regex patterns to match full error message Co-authored-by: iethree <30528226+iethree@users.noreply.github.com> * formatting * add lint rule * fix use-table-crud * fix violations * remove dead exports * type fixes --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: iethree <30528226+iethree@users.noreply.github.com> Co-authored-by: Ryan Laurie <iethree@gmail.com> |
||
|
Phoomparin Mano
|
811e4466b2 |
Update in-app JWT onboarding to use snippets instead of iframes (#70475)
* share the same snippets for onboarding flow * use sdk-docs-snippets namespace * sync snippets * fix rspack config * rspack config: use path * rspack: simplify resolve aliases * remove node_modules from react refresh * add copy button * cleanup codemirror styling * future proof: allow importing tsx files as well * fix eslint pattern * chore: move eslint ignore * test to simplify aliases * add context to ttag strings --------- Co-authored-by: Nicolò Pretto <info@npretto.com> |
||
|
Fraser
|
7e5435f6f1 | Dev 1662 move libcolors into UI (#70754) | ||
|
Maksym Yakubych
|
bc7ffe094f | feat: fix this query feature not using inline sql edits (#70172) | ||
|
Nicolò Pretto
|
6901847ed2 |
remove noisy logs from sdk component tests (#70416)
* should fix clj files imported from sdk component tests * reduce noise when not finding the sdk package * remove mantine from tests that don't need it * eslint rule |
||
|
Uladzimir Havenchyk
|
727664ccc6 | chore: cleanup moment leftovers (#69733) | ||
|
Mahatthana (Kelvin) Nomsawadi
|
d3f97ef81c |
Fix broken Metabase icon in SDK banner when using EE with license (#69215)
* Make eslint rule easier to read * Prevent future misuse of the Metabase logo in the SDK * Fix wrong Metabase icon on the SDK when using EE build with whitelabel feature * fix eslint error message |
||
|
Fraser
|
4842e16f7c | Upgrade to eslint v9 (#68772) |