From 7c6a2d5d8f3981f7e8b9c809ba33c0f3b590c41b Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Tue, 25 Aug 2026 15:55:06 -0500 Subject: [PATCH 01/17] fix(athena): honor ui.athena.defaultWorkgroup for ?bucket=-scoped console MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace re-home dropped BucketPreferences from the query console, so useWorkgroup's preferences param went dead and customers with ui.athena.defaultWorkgroup landed on the alphabetically-first workgroup. Every legacy /b/:bucket/queries URL redirects with ?bucket= set — resolve that bucket's preferences and thread ui.athena back into the model, exactly the scope the preference applied to before the re-home. Co-Authored-By: Claude --- .../containers/Queries/Athena/Athena.spec.tsx | 93 +++++++++++++++++++ .../app/containers/Queries/Athena/Athena.tsx | 38 +++++++- .../containers/Queries/Athena/model/state.tsx | 11 ++- 3 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 catalog/app/containers/Queries/Athena/Athena.spec.tsx diff --git a/catalog/app/containers/Queries/Athena/Athena.spec.tsx b/catalog/app/containers/Queries/Athena/Athena.spec.tsx new file mode 100644 index 00000000000..b4b1c418968 --- /dev/null +++ b/catalog/app/containers/Queries/Athena/Athena.spec.tsx @@ -0,0 +1,93 @@ +import * as React from 'react' +import { MemoryRouter } from 'react-router-dom' +import { render, cleanup, screen } from '@testing-library/react' +import { describe, it, expect, vi, afterEach } from 'vitest' + +import Athena from './Athena' + +vi.mock('constants/config', () => ({ default: {} })) + +// The wiring under test is Wrapper → Model.Provider(preferences). Everything +// below the Provider is irrelevant here, so the probe renders a marker instead +// of children — AthenaContainer and its AWS-backed hooks never mount. +const providerProps = vi.fn() +vi.mock('./model', async () => { + const utils = await vi.importActual('./model/utils') + return { + ...utils, + Provider: (props: { preferences?: unknown }) => { + providerProps(props) + return
+ }, + use: () => { + throw new Error('not reachable: the Provider probe renders no children') + }, + } +}) + +const prefsResult = vi.fn<() => unknown>() +vi.mock('utils/BucketPreferences', async () => { + const actual = await vi.importActual('utils/BucketPreferences') + return { + ...actual, + Provider: ({ bucket, children }: { bucket: string; children: React.ReactNode }) => ( +
+ {children} +
+ ), + use: () => ({ prefs: prefsResult() }), + } +}) + +function renderAthena(entry: string) { + return render( + + + , + ) +} + +describe('containers/Queries/Athena/Athena', () => { + afterEach(() => { + cleanup() + vi.clearAllMocks() + }) + + describe('Wrapper preferences wiring', () => { + it('passes no preferences without a bucket in scope', () => { + // The bare global console has no preference document to consult. + renderAthena('/queries/athena') + expect(screen.queryByTestId('prefs-provider')).toBeNull() + expect(providerProps).toHaveBeenCalledTimes(1) + expect(providerProps.mock.calls[0][0].preferences).toBeUndefined() + }) + + it('threads ui.athena from the ?bucket= scope into the model', async () => { + // The regression this pins: every legacy /b/:bucket/queries URL redirects + // here with ?bucket= set, and ui.athena.defaultWorkgroup must keep + // applying for those — it silently stopped when the console went global. + const { Result } = await vi.importActual( + 'utils/BucketPreferences', + ) + const athena = { defaultWorkgroup: 'analytics-prod' } + prefsResult.mockReturnValue(Result.Ok({ ui: { athena } } as never)) + renderAthena('/queries/athena?bucket=my-bucket') + expect(screen.getByTestId('prefs-provider').dataset.bucket).toBe('my-bucket') + expect(providerProps).toHaveBeenCalledTimes(1) + expect(providerProps.mock.calls[0][0].preferences).toEqual(athena) + }) + + it('holds rendering until the scoped preferences resolve', async () => { + // Rendering the console before prefs settle would seed the workgroup from + // localStorage/first-in-list and then not correct it — Init must not + // reach the model at all. + const { Result } = await vi.importActual( + 'utils/BucketPreferences', + ) + prefsResult.mockReturnValue(Result.Init()) + renderAthena('/queries/athena?bucket=my-bucket') + expect(providerProps).not.toHaveBeenCalled() + expect(screen.queryByTestId('model-provider')).toBeNull() + }) + }) +}) diff --git a/catalog/app/containers/Queries/Athena/Athena.tsx b/catalog/app/containers/Queries/Athena/Athena.tsx index 57ba833193b..4fbbc648fd6 100644 --- a/catalog/app/containers/Queries/Athena/Athena.tsx +++ b/catalog/app/containers/Queries/Athena/Athena.tsx @@ -5,7 +5,9 @@ import * as RRDom from 'react-router-dom' import * as M from '@material-ui/core' import Code from 'components/Code' +import Placeholder from 'components/Placeholder' import Skeleton from 'components/Skeleton' +import * as BucketPreferences from 'utils/BucketPreferences' import * as CatalogSettings from 'utils/CatalogSettings' import * as NamedRoutes from 'utils/NamedRoutes' @@ -414,10 +416,40 @@ function AthenaContainer() { ) } +// `ui.athena.defaultWorkgroup` is a per-bucket preference, and the console is +// workspace-global — so it applies exactly when a bucket is in scope via +// `?bucket=` (every legacy `/b/:bucket/queries/...` URL redirects here with it +// set). Without a bucket there is no preference document to consult. +function ScopedWrapper() { + const { prefs } = BucketPreferences.use() + return BucketPreferences.Result.match( + { + Ok: ({ ui }) => ( + + + + ), + _: () => , + }, + prefs, + ) +} + export default function Wrapper() { + const location = RRDom.useLocation() + const bucket = React.useMemo( + () => new URLSearchParams(location.search).get('bucket'), + [location.search], + ) + if (!bucket) + return ( + + + + ) return ( - - - + + + ) } diff --git a/catalog/app/containers/Queries/Athena/model/state.tsx b/catalog/app/containers/Queries/Athena/model/state.tsx index c46200f64b7..29aff2967c5 100644 --- a/catalog/app/containers/Queries/Athena/model/state.tsx +++ b/catalog/app/containers/Queries/Athena/model/state.tsx @@ -2,6 +2,7 @@ import invariant from 'invariant' import * as React from 'react' import * as RRDom from 'react-router-dom' +import type * as BucketPreferences from 'utils/BucketPreferences' import * as NamedRoutes from 'utils/NamedRoutes' import * as requests from './requests' @@ -62,10 +63,16 @@ export interface State { export const Ctx = React.createContext(null) interface ProviderProps { + /** + * `ui.athena` from the `?bucket=` scope's preferences. The console is + * workspace-global, so this only arrives when a bucket is in scope — which is + * exactly when `defaultWorkgroup` was ever honored before the re-home. + */ + preferences?: BucketPreferences.AthenaPreferences children: React.ReactNode } -export function Provider({ children }: ProviderProps) { +export function Provider({ preferences, children }: ProviderProps) { const { urls } = NamedRoutes.use() const location = RRDom.useLocation() @@ -77,7 +84,7 @@ export function Provider({ children }: ProviderProps) { const execution = requests.useWaitForQueryExecution(queryExecutionId) const workgroups = requests.useWorkgroups() - const workgroup = requests.useWorkgroup(workgroups, workgroupId) + const workgroup = requests.useWorkgroup(workgroups, workgroupId, preferences) const queries = requests.useQueries(workgroup.data) const query = requests.useQuery(queries.data, execution) const queryBody = requests.useQueryBody(query.value, query.setValue, execution) From cff8067cc5c2ce14ce0f6fd05d6571aed2bfdae5 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Tue, 25 Aug 2026 15:58:49 -0500 Subject: [PATCH 02/17] fix(search): stable ordering-control count, accessible Order-by name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes to the metadata-ordering controls: - The server-side filter path passed the post-filter facet count as totalAvailable, so typing in "Find metadata" on a truncated-facet stack made the Order-by control vanish once matches dropped below the offer threshold. Pass the pre-filter count, matching the other three paths and the documented invariant. - The Order-by select's aria-labelledby sat in inputProps, which MUI spreads onto the aria-hidden native input — the focusable node had no accessible name. Moved to SelectDisplayProps. Spec renders the block and asserts both selects name their focusable node; the Order-by case fails without the fix. Co-Authored-By: Claude --- .../Search/Layout/PackageFilters.spec.tsx | 43 +++++++++++++++++++ .../Search/Layout/PackageFilters.tsx | 8 +++- catalog/app/containers/Search/model.ts | 5 ++- 3 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 catalog/app/containers/Search/Layout/PackageFilters.spec.tsx diff --git a/catalog/app/containers/Search/Layout/PackageFilters.spec.tsx b/catalog/app/containers/Search/Layout/PackageFilters.spec.tsx new file mode 100644 index 00000000000..8ff225b3018 --- /dev/null +++ b/catalog/app/containers/Search/Layout/PackageFilters.spec.tsx @@ -0,0 +1,43 @@ +import * as React from 'react' +import { render, cleanup, screen } from '@testing-library/react' +import { describe, it, expect, vi, afterEach } from 'vitest' + +import * as KTree from 'utils/KeyedTree' + +import * as SearchUIModel from '../model' + +import { AvailablePackagesMetaFilters } from './PackageFilters' + +vi.mock('constants/config', () => ({ default: { registryUrl: '' } })) + +const EMPTY_TREE: SearchUIModel.FacetTree = KTree.Tree([]) + +function renderFilters() { + return render( + , + ) +} + +describe('containers/Search/Layout/PackageFilters', () => { + afterEach(cleanup) + + describe('the ordering control', () => { + // MUI v4's Select spreads `inputProps` onto the aria-hidden native input, + // not the focusable display node -- so an aria-labelledby placed there + // names nothing, and the control reads as a bare "button" to a screen + // reader. Same trap QuerySelect documents. + it('names its focusable node', () => { + renderFilters() + expect(screen.getByRole('button', { name: /Sort by/ })).toBeDefined() + }) + }) +}) diff --git a/catalog/app/containers/Search/Layout/PackageFilters.tsx b/catalog/app/containers/Search/Layout/PackageFilters.tsx index d7f884c93d6..38de3fcfaa1 100644 --- a/catalog/app/containers/Search/Layout/PackageFilters.tsx +++ b/catalog/app/containers/Search/Layout/PackageFilters.tsx @@ -189,7 +189,9 @@ interface AvailablePackagesMetaFiltersProps { fetching: boolean } -function AvailablePackagesMetaFilters({ +// Exported for testing: the ordering control's accessible name lives in how +// props land on MUI's Select internals, which only a render can assert. +export function AvailablePackagesMetaFilters({ className, filtering, facets, @@ -246,7 +248,9 @@ function AvailablePackagesMetaFilters({ disabled={fetching} extents={FACET_ORDERING_VALUES} getOptionLabel={(value) => FACET_ORDERING_LABELS[value]} - inputProps={{ 'aria-labelledby': 'meta-order-label' }} + // On the display node, not `inputProps`: MUI spreads `inputProps` + // onto the aria-hidden native input, where a label names nothing. + SelectDisplayProps={{ 'aria-labelledby': 'meta-order-label' }} onChange={(value) => ordering.set(SearchUIModel.parseFacetOrdering(value, ordering.value)) } diff --git a/catalog/app/containers/Search/model.ts b/catalog/app/containers/Search/model.ts index 4e3bc520e35..5605557e4eb 100644 --- a/catalog/app/containers/Search/model.ts +++ b/catalog/app/containers/Search/model.ts @@ -1299,7 +1299,10 @@ function AvailablePackagesMetaFiltersServerFilterQuery({ return React.createElement(AvailablePackagesMetaFiltersGroup, { state, children, - totalAvailable: available.length, + // `initial`, not `available`: `available` is the server-filtered result, and + // the pre-filter count is what keeps `ordering.offered` stable while the + // user types — otherwise the Order-by control vanishes as matches narrow. + totalAvailable: initial.length, }) } From 22e77428b7aaabf63a7e9cce62377226fe9cfe2f Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Tue, 25 Aug 2026 16:02:33 -0500 Subject: [PATCH 03/17] fix(queries): name the QuerySelect field, blank it on load error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The InputLabel had no id and the Select no labelId, so the focusable role=button node had no accessible name — a screen reader announced "Custom, button" with no hint it is the query selector. Athena also passes value={null} for the error state, so the field read "Custom" (asserting a hand-written query is loaded) beside a red helper saying the load failed. Render blank when error is set. Snapshots gain the label id; useId is Math.random-based, so the spec pins it deterministically now that an id renders unconditionally. Co-Authored-By: Claude --- .../containers/Queries/QuerySelect.spec.tsx | 54 ++++++++++++++++++- .../app/containers/Queries/QuerySelect.tsx | 11 +++- .../__snapshots__/QuerySelect.spec.tsx.snap | 4 ++ 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/catalog/app/containers/Queries/QuerySelect.spec.tsx b/catalog/app/containers/Queries/QuerySelect.spec.tsx index ca37e123a6c..2ec926f4a42 100644 --- a/catalog/app/containers/Queries/QuerySelect.spec.tsx +++ b/catalog/app/containers/Queries/QuerySelect.spec.tsx @@ -1,11 +1,19 @@ import * as React from 'react' import { render, cleanup } from '@testing-library/react' -import { describe, it, expect, afterEach } from 'vitest' +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest' import noop from 'utils/noop' import QuerySelect from './QuerySelect' +// The label id now renders unconditionally, and the real useId is +// Math.random-based -- snapshots need it deterministic. +const ids = vi.hoisted(() => ({ n: 0 })) +vi.mock('utils/useId', () => ({ default: () => `test-id-${(ids.n += 1)}` })) +beforeEach(() => { + ids.n = 0 +}) + describe('containers/Queries/QuerySelect', () => { it('should render', () => { const { container } = render( @@ -68,4 +76,48 @@ describe('containers/Queries/QuerySelect', () => { ).toBeNull() }) }) + + describe('the accessible name', () => { + afterEach(cleanup) + + it('names the focusable node after the label', () => { + // The label must reach the role="button" display div through + // labelId/aria-labelledby -- InputLabel next to a Select names nothing by + // itself, which reads as "Custom, button" to a screen reader. + const { getByRole } = render( + , + ) + expect(getByRole('button', { name: /Select a query/ })).toBeDefined() + }) + }) + + describe('the display value under error', () => { + afterEach(cleanup) + + it('does not claim "Custom" when the load failed', () => { + // Athena passes value=null for the error state too. "Custom" asserts a + // hand-written query is loaded, directly beside a helper saying the load + // failed -- the field must stay blank instead. + const { container } = render( + , + ) + expect(container.querySelector('[role="button"]')?.textContent).not.toContain( + 'Custom', + ) + }) + + it('still reads "Custom" for a genuine no-selection state', () => { + const { container } = render( + , + ) + expect(container.querySelector('[role="button"]')?.textContent).toContain('Custom') + }) + }) }) diff --git a/catalog/app/containers/Queries/QuerySelect.tsx b/catalog/app/containers/Queries/QuerySelect.tsx index 9a2caed0aae..516ce656f67 100644 --- a/catalog/app/containers/Queries/QuerySelect.tsx +++ b/catalog/app/containers/Queries/QuerySelect.tsx @@ -35,6 +35,7 @@ export default function QuerySelect({ value, }: QuerySelectProps) { const helperId = useId() + const labelId = useId() const handleChange = React.useCallback( (event) => { if (event.target.value === LOAD_MORE && onLoadMore) { @@ -56,8 +57,11 @@ export default function QuerySelect({ error={error} fullWidth > - {label} + {label} ({ // line-height and renders 5px taller than a plain-text Select beside it, // leaving the two underlines misaligned. Same trap `Workgroups` avoids by // using bare text in its rows. - renderValue={() => value?.name ?? 'Custom'} + // Blank under error: callers null the value on a failed load, and + // "Custom" would assert a hand-written query is loaded right beside a + // helper saying the load failed. + renderValue={() => value?.name ?? (error ? '' : 'Custom')} // Not `aria-describedby` on the Select: that lands on the hidden native // input. The focusable node is the `role="button"` display div, which is // only reachable through `SelectDisplayProps`. diff --git a/catalog/app/containers/Queries/__snapshots__/QuerySelect.spec.tsx.snap b/catalog/app/containers/Queries/__snapshots__/QuerySelect.spec.tsx.snap index 411532d1eed..a075f240b44 100644 --- a/catalog/app/containers/Queries/__snapshots__/QuerySelect.spec.tsx.snap +++ b/catalog/app/containers/Queries/__snapshots__/QuerySelect.spec.tsx.snap @@ -7,6 +7,7 @@ exports[`containers/Queries/QuerySelect > should render 1`] = ` @@ -16,6 +17,7 @@ exports[`containers/Queries/QuerySelect > should render 1`] = `
@@ -48,6 +50,7 @@ exports[`containers/Queries/QuerySelect > should render with selected value 1`] @@ -56,6 +59,7 @@ exports[`containers/Queries/QuerySelect > should render with selected value 1`] >
Date: Tue, 25 Aug 2026 16:47:18 -0500 Subject: [PATCH 04/17] fix(admin): explain the disabled Enabled switch, gate roles on isService locally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The disabled branch of EditableSwitch dropped the tooltip entirely, so the service-user and this-is-you cases rendered as an identical dead control with no cause. Wrap the disabled switch in a cause-specific tooltip (a disabled element fires no events, so it needs a live wrapper span). Also gate the roles dialog on isService directly instead of trusting the registry to couple isRoleAssignmentDisabled to it — a registry that reports isService alone would otherwise offer a Save it then refuses. Co-Authored-By: Claude --- .../Admin/UsersAndRoles/Users.spec.tsx | 60 +++++++++++++++++++ .../containers/Admin/UsersAndRoles/Users.tsx | 38 ++++++++++-- 2 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx new file mode 100644 index 00000000000..db584348200 --- /dev/null +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx @@ -0,0 +1,60 @@ +import * as React from 'react' +import { render, cleanup, fireEvent, screen } from '@testing-library/react' +import { describe, it, expect, vi, afterEach } from 'vitest' + +import { EditableSwitch } from './Users' + +vi.mock('constants/config', () => ({ default: {} })) + +describe('containers/Admin/UsersAndRoles/Users', () => { + describe('EditableSwitch', () => { + afterEach(cleanup) + + it('explains why it is disabled', async () => { + // A dead control with no cause is indistinguishable from a rendering + // bug -- and two causes (self, service user) share this column. + const { container } = render( + , + ) + expect(container.querySelector('input')?.disabled).toBe(true) + fireEvent.mouseOver(container.querySelector('span')!) + expect( + await screen.findByText('This service user is managed by the stack'), + ).toBeDefined() + }) + + it('keeps the hint on the enabled control', async () => { + const { container } = render( + , + ) + // MUI seeds the tooltip as a `title` before opening the popper; hover the + // node that actually carries it. + const titled = container.querySelector('[title]')! + expect(titled.getAttribute('title')).toBe("Deactivated users can't sign in") + fireEvent.mouseOver(titled) + expect(await screen.findByText("Deactivated users can't sign in")).toBeDefined() + }) + + it('renders a plain disabled switch when no reason is given', () => { + const { container } = render( + , + ) + expect(container.querySelector('input')?.disabled).toBe(true) + }) + }) +}) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx index c716fb2fc1f..5a7d7444d93 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx @@ -680,10 +680,14 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { name="roles" validate={RoleSelect.validate}> {(props) => ( + // `|| isService` locally, not via the registry's coupling of the + // two flags: a registry that reports isService without + // isRoleAssignmentDisabled would otherwise offer a Save the + // registry then refuses. @@ -775,21 +779,38 @@ const useEditableStyles = M.makeStyles((t) => ({ interface EditableSwitchProps { disabled?: boolean + /** Why the control is disabled — a disabled switch with no cause is + * indistinguishable from a rendering bug, and two different causes (self, + * service user) sit in the same column. */ + disabledReason?: NonNullable checked: boolean onChange: (v: boolean) => void hint: NonNullable } -function EditableSwitch({ +// Exported for testing: the disabled branch must keep explaining itself, and +// only a render proves the tooltip survives the disabled element's dead events. +export function EditableSwitch({ disabled = false, + disabledReason, checked, onChange, hint, }: EditableSwitchProps) { const classes = useEditableStyles() - return disabled ? ( - - ) : ( + if (disabled) { + const sw = ( + + ) + if (!disabledReason) return sw + return ( + + {/* a disabled element fires no events, so the tooltip needs a live wrapper */} + {sw} + + ) + } + return ( {({ change, busy, value }) => ( @@ -879,6 +900,13 @@ const columns: Table.Column[] = [ setActive(u.name, active)} /> From 9dfc25d74db740bde9e0c8918f550453f6a0bf54 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Tue, 25 Aug 2026 16:52:48 -0500 Subject: [PATCH 05/17] fix(data-products): load the fixture adapter lazily hooks.ts statically imported fixtureAdapter, which imports the 33KB fixture corpus. The volumes landing calls useProducts, so every flag-off customer downloaded and evaluated demo product records ("Clinical Cohort 2024", "acme_cohort_2024") on first load of /, readable in devtools. Load the adapter through a dynamic import so webpack splits it out; the flag-off resource keys resolve to [] without consulting it, so the chunk is never fetched. useAdapter keeps its synchronous contract by suspending on the same load. Verified against a production build: fixture strings appear in exactly one async chunk, absent from app/runtime/vendor entries and not preloaded by index.html. Co-Authored-By: Claude --- catalog/app/model/DataProducts/hooks.ts | 55 ++++++++++++++++++------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/catalog/app/model/DataProducts/hooks.ts b/catalog/app/model/DataProducts/hooks.ts index 59340ca4268..b3f4092a833 100644 --- a/catalog/app/model/DataProducts/hooks.ts +++ b/catalog/app/model/DataProducts/hooks.ts @@ -20,7 +20,6 @@ import * as Cache from 'utils/ResourceCache' import type { ContentsResult, DataProductAdapter, EntryBodyResult } from './adapter' import { supportsBrowsing, supportsFetching } from './adapter' -import { fixtureAdapter } from './fixtureAdapter' import type { AccessRequest } from './requests' import type { Connection } from './connections' import type { DataProduct } from './types' @@ -28,16 +27,28 @@ import type { DataProduct } from './types' /** * Which adapter the hooks read from. * - * A module-level constant rather than a React context, deliberately: there is + * A module-level singleton rather than a React context, deliberately: there is * exactly one adapter per deployment, chosen by what the registry can serve, and * nothing in the UI ever wants two at once. A context would invite per-subtree * overriding -- which sounds flexible and in practice means two screens * disagreeing about what exists. * + * Loaded lazily, not statically: this module is imported by the volumes landing + * (through `useProducts`), which every flag-off customer renders -- and a static + * import here drags the whole fixture corpus into that chunk. The dynamic import + * keeps the adapter (and everything behind it) in its own chunk, fetched only + * when a resource below actually consults it, which the flag-off keys never do. + * * When a GraphQL-backed adapter lands this becomes a build- or config-time * choice here, and no container changes. That is the whole point of the port. */ -const adapter: DataProductAdapter = fixtureAdapter +let adapterPromise: Promise | null = null +function loadAdapter(): Promise { + if (!adapterPromise) { + adapterPromise = import('./fixtureAdapter').then((m) => m.fixtureAdapter) + } + return adapterPromise +} // The cache keys on `input`; these resources take none beyond the ids below, so // `key` is explicit rather than relying on `R.identity` over an object. @@ -55,7 +66,7 @@ const adapter: DataProductAdapter = fixtureAdapter const ProductsResource = Cache.createResource({ name: 'DataProducts.list', fetch: ({ enabled }: { enabled: boolean }) => - enabled ? adapter.listProducts() : Promise.resolve([]), + enabled ? loadAdapter().then((a) => a.listProducts()) : Promise.resolve([]), // @ts-expect-error key: ({ enabled }: { enabled: boolean }) => enabled, }) @@ -63,21 +74,22 @@ const ProductsResource = Cache.createResource({ const ConnectionsResource = Cache.createResource({ name: 'DataProducts.connections', fetch: ({ enabled }: { enabled: boolean }) => - enabled ? adapter.listConnections() : Promise.resolve([]), + enabled ? loadAdapter().then((a) => a.listConnections()) : Promise.resolve([]), // @ts-expect-error key: ({ enabled }: { enabled: boolean }) => enabled, }) const ProductResource = Cache.createResource({ name: 'DataProducts.product', - fetch: ({ id }: { id: string }) => adapter.getProduct(id), + fetch: ({ id }: { id: string }) => loadAdapter().then((a) => a.getProduct(id)), // @ts-expect-error key: ({ id }: { id: string }) => id, }) const RequestsResource = Cache.createResource({ name: 'DataProducts.requests', - fetch: ({ productId }: { productId: string }) => adapter.listRequests(productId), + fetch: ({ productId }: { productId: string }) => + loadAdapter().then((a) => a.listRequests(productId)), // @ts-expect-error key: ({ productId }: { productId: string }) => productId, }) @@ -94,9 +106,11 @@ const ContentsResource = Cache.createResource({ // enumerate contents at all. NOT_FOUND is the honest answer -- we have no way // to look, so we did not find anything -- and it keeps the UI on one code // path rather than branching on adapter shape at every call site. - supportsBrowsing(adapter) - ? adapter.listContents(productId, member) - : Promise.resolve({ ok: false, reason: 'NOT_FOUND' }), + loadAdapter().then((a) => + supportsBrowsing(a) + ? a.listContents(productId, member) + : ({ ok: false, reason: 'NOT_FOUND' } as ContentsResult), + ), // @ts-expect-error key: ({ productId, member }: { productId: string; member: string }) => `${productId}::${member}`, @@ -110,9 +124,11 @@ const EntryBodyResource = Cache.createResource({ // Same reasoning as contents: an adapter that cannot fetch is a real shape, // not a broken one, and NOT_FOUND keeps the UI on one path rather than // branching on adapter capability at the call site. - supportsFetching(adapter) - ? adapter.fetchEntry(productId, member, logicalKey) - : Promise.resolve({ ok: false, reason: 'NOT_FOUND' }), + loadAdapter().then((a) => + supportsFetching(a) + ? a.fetchEntry(productId, member, logicalKey) + : ({ ok: false, reason: 'NOT_FOUND' } as EntryBodyResult), + ), // @ts-expect-error key: ({ productId, member, logicalKey }: EntryInput) => `${productId}::${member}::${logicalKey}`, @@ -198,13 +214,24 @@ export function useContents(productId: string, member: string): ContentsResult { ) as ContentsResult } +const AdapterResource = Cache.createResource({ + name: 'DataProducts.adapter', + fetch: () => loadAdapter(), + // @ts-expect-error + key: () => 'adapter', +}) + /** * The adapter itself, for the one thing hooks cannot express: asking whether a * write path exists. * * Exposed so a container can call `supportsRequests(useAdapter())` and disable * its submit affordance honestly, instead of hardcoding "no adapter yet". + * + * Suspends, like the rest: the adapter is loaded on demand so it stays out of + * the chunks a flag-off deployment downloads. Only call it where the product + * screens already suspend -- never to decide whether to show them. */ export function useAdapter(): DataProductAdapter { - return adapter + return Cache.useData(AdapterResource, {}, { suspend: true }) as DataProductAdapter } From 5c8babe89cf97a2f68b523022a7d43743af6a36b Mon Sep 17 00:00:00 2001 From: Sergey Fedoseev Date: Wed, 26 Aug 2026 18:04:12 +0500 Subject: [PATCH 06/17] docs(admin): say what the isService gate actually does The comment claimed the local `|| isService` stops the dialog offering a Save the registry then refuses. It doesn't: the dialog title and actions still read isRoleAssignmentDisabled alone, so only the selector is gated. Co-Authored-By: Claude Opus 5 --- catalog/app/containers/Admin/UsersAndRoles/Users.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx index 5a7d7444d93..f4c487ccaac 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx @@ -680,10 +680,10 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { name="roles" validate={RoleSelect.validate}> {(props) => ( - // `|| isService` locally, not via the registry's coupling of the - // two flags: a registry that reports isService without - // isRoleAssignmentDisabled would otherwise offer a Save the - // registry then refuses. + // The registry couples these: isService implies + // isRoleAssignmentDisabled. `|| isService` keeps the selector + // read-only even if that stops holding — and only the selector: + // the title and actions still read isRoleAssignmentDisabled. Date: Wed, 26 Aug 2026 18:04:25 +0500 Subject: [PATCH 07/17] fix(admin): explain the disabled Admin switch too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Enabled column now says why its switch is dead; the Admin column beside it kept the unexplained one. Same treatment, with the three causes the catalog can actually tell apart: yourself, a service user, an SSO-managed account. The tests render the column's own getDisplay rather than EditableSwitch in isolation — a reason passed at one call site and not its sibling is exactly what this fixes, so only the call site proves it. Watched red against the unfixed source. Co-Authored-By: Claude Opus 5 --- .../Admin/UsersAndRoles/Users.spec.tsx | 42 ++++++++++++++++++- .../containers/Admin/UsersAndRoles/Users.tsx | 13 +++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx index db584348200..4cffd590723 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx @@ -2,7 +2,7 @@ import * as React from 'react' import { render, cleanup, fireEvent, screen } from '@testing-library/react' import { describe, it, expect, vi, afterEach } from 'vitest' -import { EditableSwitch } from './Users' +import { EditableSwitch, columns } from './Users' vi.mock('constants/config', () => ({ default: {} })) @@ -57,4 +57,44 @@ describe('containers/Admin/UsersAndRoles/Users', () => { expect(container.querySelector('input')?.disabled).toBe(true) }) }) + + describe('the Admin column switch', () => { + afterEach(cleanup) + + const column = columns.find((c) => c.id === 'isAdmin')! + + function renderSwitch(user: object, isSelf = false) { + return render( + <> + {column.getDisplay!(undefined, user as never, { isSelf, openDialog: vi.fn() })} + , + ) + } + + it.each([ + [ + 'a service user', + { isAdminAssignmentDisabled: true, isService: true }, + false, + 'This service user is managed by the stack', + ], + [ + 'an SSO-managed user', + { isAdminAssignmentDisabled: true, isService: false }, + false, + 'Admin capabilities for this user are managed by the SSO configuration', + ], + [ + 'yourself', + { isAdminAssignmentDisabled: false, isService: false }, + true, + 'You cannot change your own admin status', + ], + ])('says why it is disabled for %s', async (_label, user, isSelf, reason) => { + const { container } = renderSwitch({ name: 'u', isAdmin: false, ...user }, isSelf) + expect(container.querySelector('input')?.disabled).toBe(true) + fireEvent.mouseOver(container.querySelector('span')!) + expect(await screen.findByText(reason)).toBeDefined() + }) + }) }) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx index f4c487ccaac..df36b63f381 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx @@ -891,7 +891,9 @@ interface ColumnDisplayProps { isSelf: boolean } -const columns: Table.Column[] = [ +// Exported for testing: a disabled control's explanation lives in the column that +// renders it, so only the call site proves it is there. +export const columns: Table.Column[] = [ { id: 'isActive', label: 'Enabled', @@ -963,6 +965,15 @@ const columns: Table.Column[] = [ openDialog( From bc73eb966010fb2f17174e2377932c62192a573f Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 09:45:21 -0500 Subject: [PATCH 08/17] review: apply the /code-review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - EditRoles: one readOnly expression for title, selector, and actions — the half-applied isService gate left a live Save beside a frozen selector. - Admin column: same isService guard as the Enabled column and roles dialog. - loadAdapter: never latch a rejected chunk import; retry on next read. - totalAvailable (server path): max(initial, available) — initial alone withheld the control when a text query out-returned the truncated list. - Drop useAdapter/AdapterResource: no callers, and a suspending adapter read is the wrong shape for gating an affordance. - CHANGELOG entries for the user-visible fixes. Co-Authored-By: Claude --- catalog/CHANGELOG.md | 5 +++ .../containers/Admin/UsersAndRoles/Users.tsx | 30 ++++++++------- catalog/app/containers/Search/model.ts | 10 +++-- catalog/app/model/DataProducts/hooks.ts | 38 ++++++++----------- catalog/app/model/DataProducts/index.ts | 1 - 5 files changed, 44 insertions(+), 40 deletions(-) diff --git a/catalog/CHANGELOG.md b/catalog/CHANGELOG.md index 48a84989c38..f74676cb310 100644 --- a/catalog/CHANGELOG.md +++ b/catalog/CHANGELOG.md @@ -21,6 +21,11 @@ complete sentence without it. ## Changes +- [Fixed] Athena Queries: a console opened with a `?bucket=` scope — which every legacy per-bucket queries URL redirects to — honors that bucket's `ui.athena.defaultWorkgroup` again; the workspace re-home had silently dropped the preference, landing customers on the alphabetically-first workgroup ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Fixed] Search sidebar: the "Sort by" control no longer disappears while typing in "Find metadata" on stacks with truncated facet lists, and it has an accessible name — its label used to land on a hidden input, reading as an unnamed button to assistive tech ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Fixed] Queries: the query selector announces its label to assistive tech, and no longer claims "Custom" is loaded while its helper text reports the query failed to load ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Fixed] Admin Users: a disabled Enabled or Admin switch explains why on hover — "you can't deactivate yourself" and "managed by the stack" used to render as the same mute, dead control — and the roles dialog reads as read-only end to end for service users rather than offering a Save the registry refuses ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Changed] With the `data-products` preview off, the demo fixture data no longer ships in the bundles a browser downloads on the volumes landing; the adapter behind the preview loads only when the feature is on ([#5217](https://github.com/quiltdata/quilt/pull/5217)) - [Changed] Search sidebar: the package metadata list is sorted from one "Sort by" control, sitting directly above the list ([#5222](https://github.com/quiltdata/quilt/pull/5222)) - [Fixed] Volumes list: the "Shared with" readout no longer overlaps a bucket's tags, and the extra space inside it is gone ([#5220](https://github.com/quiltdata/quilt/pull/5220)) - [Fixed] Front door: the example-query chips show real package handles, a mix of prompts rather than five recent packages, and icons that match the rest of the app ([#5219](https://github.com/quiltdata/quilt/pull/5219)) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx index df36b63f381..325054b26cb 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx @@ -654,6 +654,12 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { [user.extraRoles, user.role], ) + // One expression for the whole dialog. The registry couples these (isService + // implies isRoleAssignmentDisabled), but gating only the selector on the pair + // left the title and a live Save button behind if that ever stops holding -- + // a Save that submits unchanged values and silently changes nothing. + const readOnly = user.isRoleAssignmentDisabled || user.isService + return ( onSubmit={onSubmit} @@ -672,7 +678,7 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { }) => ( <> - {user.isRoleAssignmentDisabled + {readOnly ? `Roles assigned to "${user.name}"` : `Assign roles to "${user.name}"`} @@ -680,14 +686,10 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { name="roles" validate={RoleSelect.validate}> {(props) => ( - // The registry couples these: isService implies - // isRoleAssignmentDisabled. `|| isService` keeps the selector - // read-only even if that stops holding — and only the selector: - // the title and actions still read isRoleAssignmentDisabled. @@ -698,7 +700,7 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { - {user.isRoleAssignmentDisabled ? ( + {readOnly ? ( Close @@ -964,15 +966,17 @@ export const columns: Table.Column[] = [ getDisplay: (_v, u, { openDialog, isSelf }: ColumnDisplayProps) => ( diff --git a/catalog/app/containers/Search/model.ts b/catalog/app/containers/Search/model.ts index 5605557e4eb..326f711cee4 100644 --- a/catalog/app/containers/Search/model.ts +++ b/catalog/app/containers/Search/model.ts @@ -1299,10 +1299,12 @@ function AvailablePackagesMetaFiltersServerFilterQuery({ return React.createElement(AvailablePackagesMetaFiltersGroup, { state, children, - // `initial`, not `available`: `available` is the server-filtered result, and - // the pre-filter count is what keeps `ordering.offered` stable while the - // user types — otherwise the Order-by control vanishes as matches narrow. - totalAvailable: initial.length, + // Neither count alone is the pre-filter total on this path: `available` is + // the server-filtered result (so it vanishes the control as matches narrow), + // and `initial` is the truncated list minus applied filters (so it withholds + // the control when a text query returns far more than the truncated list + // held). Offer whenever either is large enough. + totalAvailable: Math.max(initial.length, available.length), }) } diff --git a/catalog/app/model/DataProducts/hooks.ts b/catalog/app/model/DataProducts/hooks.ts index b3f4092a833..282573c36a0 100644 --- a/catalog/app/model/DataProducts/hooks.ts +++ b/catalog/app/model/DataProducts/hooks.ts @@ -45,7 +45,16 @@ import type { DataProduct } from './types' let adapterPromise: Promise | null = null function loadAdapter(): Promise { if (!adapterPromise) { - adapterPromise = import('./fixtureAdapter').then((m) => m.fixtureAdapter) + adapterPromise = import('./fixtureAdapter').then( + (m) => m.fixtureAdapter, + (e) => { + // Never latch a rejection: a failed chunk fetch (deploy rotated the + // hashes, network blip) must be retryable on the next read, not a + // permanent break until hard reload. + adapterPromise = null + throw e + }, + ) } return adapterPromise } @@ -214,24 +223,9 @@ export function useContents(productId: string, member: string): ContentsResult { ) as ContentsResult } -const AdapterResource = Cache.createResource({ - name: 'DataProducts.adapter', - fetch: () => loadAdapter(), - // @ts-expect-error - key: () => 'adapter', -}) - -/** - * The adapter itself, for the one thing hooks cannot express: asking whether a - * write path exists. - * - * Exposed so a container can call `supportsRequests(useAdapter())` and disable - * its submit affordance honestly, instead of hardcoding "no adapter yet". - * - * Suspends, like the rest: the adapter is loaded on demand so it stays out of - * the chunks a flag-off deployment downloads. Only call it where the product - * screens already suspend -- never to decide whether to show them. - */ -export function useAdapter(): DataProductAdapter { - return Cache.useData(AdapterResource, {}, { suspend: true }) as DataProductAdapter -} +// There was a `useAdapter()` here, exposing the adapter so a container could ask +// `supportsRequests(...)` before offering a submit affordance. It had no callers, +// and once the adapter loads lazily it could only be a suspending read -- which +// is the wrong shape for gating an affordance: it blanks the subtree to decide +// one button's disabled state. Re-add it as a non-suspending capability flag +// resolved alongside data the screen already awaits, not as a bare adapter read. diff --git a/catalog/app/model/DataProducts/index.ts b/catalog/app/model/DataProducts/index.ts index 6584145c1d8..40d7a279906 100644 --- a/catalog/app/model/DataProducts/index.ts +++ b/catalog/app/model/DataProducts/index.ts @@ -32,7 +32,6 @@ export type { } from './adapter' export { supportsBrowsing, supportsFetching, supportsRequests } from './adapter' export { - useAdapter, useConnections, useContents, useEntryBody, From b0b43514344ea58937c55d08f19550c9fef6aa84 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:24:50 -0500 Subject: [PATCH 09/17] fix(athena): keep the ?bucket= scope on in-console links Co-Authored-By: Claude Fable 5 --- .../app/containers/Queries/Athena/Athena.tsx | 9 ++- .../Queries/Athena/History.spec.tsx | 59 +++++++++++++++++++ .../app/containers/Queries/Athena/History.tsx | 16 ++++- 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 catalog/app/containers/Queries/Athena/History.spec.tsx diff --git a/catalog/app/containers/Queries/Athena/Athena.tsx b/catalog/app/containers/Queries/Athena/Athena.tsx index 4fbbc648fd6..6a106bc5d38 100644 --- a/catalog/app/containers/Queries/Athena/Athena.tsx +++ b/catalog/app/containers/Queries/Athena/Athena.tsx @@ -312,12 +312,19 @@ function ResultsBreadcrumbs({ children, className }: ResultsBreadcrumbsProps) { const classes = useResultsBreadcrumbsStyles() const overrideClasses = useOverrideStyles() const { urls } = NamedRoutes.use() + const location = RRDom.useLocation() return (
Query Executions diff --git a/catalog/app/containers/Queries/Athena/History.spec.tsx b/catalog/app/containers/Queries/Athena/History.spec.tsx new file mode 100644 index 00000000000..96d5f62273f --- /dev/null +++ b/catalog/app/containers/Queries/Athena/History.spec.tsx @@ -0,0 +1,59 @@ +import * as React from 'react' +import { MemoryRouter } from 'react-router-dom' +import { render, cleanup } from '@testing-library/react' +import { describe, it, expect, vi, afterEach } from 'vitest' + +import * as NamedRoutes from 'utils/NamedRoutes' +import { queriesAthenaExecution } from 'constants/routes' + +import History from './History' + +vi.mock('constants/config', () => ({ default: {} })) + +vi.mock('containers/Notifications', () => ({ use: () => ({ push: vi.fn() }) })) + +// Keep the real Model helpers (hasValue, etc.); stub only `use`. +vi.mock('./model', async () => { + const utils = await vi.importActual('./model/utils') + return { ...utils, use: () => ({ workgroup: { data: 'primary' }, queryBody: {} }) } +}) + +const succeeded = { + id: 'exec-1', + status: 'SUCCEEDED', + query: 'SELECT 1', + created: new Date(0), + completed: new Date(0), +} + +function renderHistory(entry: string) { + return render( + + + + + , + ) +} + +describe('containers/Queries/Athena/History', () => { + afterEach(() => { + cleanup() + vi.clearAllMocks() + }) + + it('keeps the ?bucket= scope on an execution row link', () => { + // The console branches on ?bucket=, so a bare pathname here remounts the + // whole screen and drops the bucket's preferences. + const { container } = renderHistory('/queries/athena/primary?bucket=my-bucket') + const href = container.querySelector('a')?.getAttribute('href') + expect(href).toBe('/queries/athena/primary/exec-1?bucket=my-bucket') + }) + + it('adds no query string when the console has no scope', () => { + const { container } = renderHistory('/queries/athena/primary') + expect(container.querySelector('a')?.getAttribute('href')).toBe( + '/queries/athena/primary/exec-1', + ) + }) +}) diff --git a/catalog/app/containers/Queries/Athena/History.tsx b/catalog/app/containers/Queries/Athena/History.tsx index 372066016bd..e4761bc6bc7 100644 --- a/catalog/app/containers/Queries/Athena/History.tsx +++ b/catalog/app/containers/Queries/Athena/History.tsx @@ -150,7 +150,7 @@ function Row({ className, children }: RowProps) { interface LinkCellProps { children: React.ReactNode className: string - to?: string + to?: RRDom.LinkProps['to'] } function LinkCell({ children, className, to }: LinkCellProps) { @@ -181,7 +181,7 @@ const useExecutionStyles = M.makeStyles((t) => ({ })) interface ExecutionProps { - to?: string + to?: RRDom.LinkProps['to'] queryExecution: Model.QueryExecution } @@ -251,6 +251,7 @@ interface HistoryProps { export default function History({ executions, onLoadMore }: HistoryProps) { const { urls } = NamedRoutes.use() + const location = RRDom.useLocation() const classes = useStyles() const pageSize = 10 @@ -308,7 +309,16 @@ export default function History({ executions, onLoadMore }: HistoryProps) { key={queryExecution.id} to={ queryExecution.status === 'SUCCEEDED' - ? urls.queriesAthenaExecution(workgroup.data, queryExecution.id) + ? { + // Keep the query string (e.g. the ?bucket= preference + // scope): the console branches on it, so dropping it here + // remounts the whole screen unscoped. + pathname: urls.queriesAthenaExecution( + workgroup.data, + queryExecution.id, + ), + search: location.search, + } : undefined } /> From e4e37a268c358e8b7808560302f7083bce402875 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:32:37 -0500 Subject: [PATCH 10/17] fix(data-products): degrade instead of latching a failed adapter load Co-Authored-By: Claude Fable 5 --- catalog/app/model/DataProducts/hooks.spec.tsx | 71 +++++++++++++++++++ catalog/app/model/DataProducts/hooks.ts | 32 +++++++-- 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 catalog/app/model/DataProducts/hooks.spec.tsx diff --git a/catalog/app/model/DataProducts/hooks.spec.tsx b/catalog/app/model/DataProducts/hooks.spec.tsx new file mode 100644 index 00000000000..84d7511d08e --- /dev/null +++ b/catalog/app/model/DataProducts/hooks.spec.tsx @@ -0,0 +1,71 @@ +import * as React from 'react' +import { render, cleanup, screen, waitFor } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import * as Cache from 'utils/ResourceCache' + +import { useConnections, useProducts } from './hooks' + +// `utils/ResourceCache` is deliberately NOT mocked: the failure mode under test +// is the cache's, not the loader's. It stores a rejected fetch as `AsyncResult +// .Err` and rethrows it on every later read without ever evicting the entry, so +// a loader that rejects blanks the catalog through the root error boundary. + +vi.mock('constants/config', () => ({ default: {} })) +vi.mock('@sentry/react', () => ({ captureException: vi.fn() })) + +const loads = vi.hoisted(() => ({ count: 0 })) + +vi.mock('./fixtureAdapter', () => { + loads.count += 1 + throw new Error('Failed to fetch dynamically imported module') +}) + +function Products() { + return
{useProducts().length}
+} + +function Connections() { + return
{useConnections().length}
+} + +// No error boundary on purpose: a rethrown rejection fails the render outright +// instead of resolving to a count. +function renderCached(children: React.ReactNode) { + return render( + + loading}>{children} + , + ) +} + +describe('model/DataProducts/hooks', () => { + beforeEach(() => { + loads.count = 0 + }) + + afterEach(() => { + cleanup() + vi.clearAllMocks() + }) + + it('degrades to empty data when the adapter chunk fails to load', async () => { + renderCached() + await waitFor(() => expect(screen.getByTestId('products').textContent).toBe('0')) + expect(loads.count).toBe(1) + }) + + it('attempts the import again for a later read', async () => { + // Separate cache resources, so the second read reaches the loader rather + // than being served the first entry. + const { unmount } = renderCached() + await waitFor(() => expect(screen.getByTestId('products').textContent).toBe('0')) + unmount() + + renderCached() + await waitFor(() => expect(screen.getByTestId('connections').textContent).toBe('0')) + + // The rejection was not latched: the second read tried the import again. + expect(loads.count).toBe(2) + }) +}) diff --git a/catalog/app/model/DataProducts/hooks.ts b/catalog/app/model/DataProducts/hooks.ts index 282573c36a0..aa03e9becef 100644 --- a/catalog/app/model/DataProducts/hooks.ts +++ b/catalog/app/model/DataProducts/hooks.ts @@ -16,6 +16,8 @@ * inside the app's root Suspense boundary, which is everywhere these render. */ +import * as Sentry from '@sentry/react' + import * as Cache from 'utils/ResourceCache' import type { ContentsResult, DataProductAdapter, EntryBodyResult } from './adapter' @@ -42,17 +44,39 @@ import type { DataProduct } from './types' * When a GraphQL-backed adapter lands this becomes a build- or config-time * choice here, and no container changes. That is the whole point of the port. */ +/** + * The adapter used when the real one cannot be fetched. + * + * Empty answers rather than a rejection, because the resources below go through + * `ResourceCache`: a stored rejection is rethrown on every later read and never + * evicted, so one failed chunk fetch would blank the catalog through the root + * error boundary until a hard reload. These are the same values the port already + * produces for an adapter that cannot browse or fetch, so no call site needs a + * new branch. It implements neither `listContents` nor `fetchEntry`, so those + * resources take their existing unsupported path. + */ +const unavailableAdapter: DataProductAdapter = { + listProducts: async () => [], + getProduct: async () => null, + listRequests: async () => [], + listConnections: async () => [], +} + let adapterPromise: Promise | null = null function loadAdapter(): Promise { if (!adapterPromise) { adapterPromise = import('./fixtureAdapter').then( (m) => m.fixtureAdapter, (e) => { - // Never latch a rejection: a failed chunk fetch (deploy rotated the - // hashes, network blip) must be retryable on the next read, not a - // permanent break until hard reload. + // Never latch the failure: a failed chunk fetch (deploy rotated the + // hashes, network blip) must be retryable on the next read. adapterPromise = null - throw e + // eslint-disable-next-line no-console + console.error('Error loading the data products adapter:') + // eslint-disable-next-line no-console + console.dir(e) + Sentry.captureException(e) + return unavailableAdapter }, ) } From 2d951b3098e8a61ae117f10fd2baece0f2c4a513 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:41:13 -0500 Subject: [PATCH 11/17] fix(admin): reach the disabled-switch reason by keyboard, gate Role on isService Derive each switch column's `disabled` from its reason so the guard and the explanation cannot drift, and apply the dialog's read-only pair to RoleDisplay. Co-Authored-By: Claude Fable 5 --- .../Admin/UsersAndRoles/Users.spec.tsx | 116 +++++++++++++++++ .../containers/Admin/UsersAndRoles/Users.tsx | 118 ++++++++++-------- 2 files changed, 185 insertions(+), 49 deletions(-) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx index 4cffd590723..eec9762c13d 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.spec.tsx @@ -29,6 +29,27 @@ describe('containers/Admin/UsersAndRoles/Users', () => { ).toBeDefined() }) + it('carries the reason to keyboard and screen-reader users', async () => { + // The switch is disabled, so it is out of the tab order and fires no + // events: without a focusable, named wrapper the reason is mouse-only. + render( + , + ) + const wrapper = screen.getByLabelText('This service user is managed by the stack') + expect(wrapper.getAttribute('tabindex')).toBe('0') + + fireEvent.focus(wrapper) + expect( + await screen.findByText('This service user is managed by the stack'), + ).toBeDefined() + }) + it('keeps the hint on the enabled control', async () => { const { container } = render( { fireEvent.mouseOver(container.querySelector('span')!) expect(await screen.findByText(reason)).toBeDefined() }) + + it('stays editable for an ordinary user', () => { + const { container } = renderSwitch({ + name: 'u', + isAdmin: false, + isAdminAssignmentDisabled: false, + isService: false, + }) + expect(container.querySelector('input')?.disabled).toBe(false) + }) + }) + + describe('the Enabled column switch', () => { + afterEach(cleanup) + + const column = columns.find((c) => c.id === 'isActive')! + + function renderSwitch(user: object, isSelf = false) { + return render( + <> + {column.getDisplay!( + undefined, + user as never, + { + isSelf, + setActive: vi.fn(), + } as never, + )} + , + ) + } + + it.each([ + [ + 'a service user', + { isService: true }, + false, + 'This service user is managed by the stack', + ], + ['yourself', { isService: false }, true, 'You cannot deactivate your own account'], + ])('says why it is disabled for %s', async (_label, user, isSelf, reason) => { + const { container } = renderSwitch({ name: 'u', isActive: true, ...user }, isSelf) + expect(container.querySelector('input')?.disabled).toBe(true) + fireEvent.mouseOver(container.querySelector('span')!) + expect(await screen.findByText(reason)).toBeDefined() + }) + + it('stays editable for an ordinary user', () => { + const { container } = renderSwitch({ name: 'u', isActive: true, isService: false }) + expect(container.querySelector('input')?.disabled).toBe(false) + }) + }) + + describe('the Role column', () => { + afterEach(cleanup) + + const column = columns.find((c) => c.id === 'role')! + + function renderRole(user: object) { + return render( + <> + {column.getDisplay!( + undefined, + { extraRoles: [], ...user } as never, + { + roles: [], + defaultRole: null, + openDialog: vi.fn(), + } as never, + )} + , + ) + } + + // The dialog this opens is read-only for both flags, so the invitation to + // edit must be gated on the same pair. + it.each([ + ['an SSO-managed user', { isRoleAssignmentDisabled: true, isService: false }], + ['a service user', { isRoleAssignmentDisabled: false, isService: true }], + ])('offers only viewing for %s', (_label, user) => { + const { container } = renderRole(user) + expect(container.querySelector('[title]')?.getAttribute('title')).toBe( + 'Click to view', + ) + }) + + it('offers editing for an ordinary user', () => { + const { container } = renderRole({ + isRoleAssignmentDisabled: false, + isService: false, + }) + expect(container.querySelector('[title]')?.getAttribute('title')).toBe( + 'Click to edit', + ) + }) }) }) diff --git a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx index 325054b26cb..b0f336dcacd 100644 --- a/catalog/app/containers/Admin/UsersAndRoles/Users.tsx +++ b/catalog/app/containers/Admin/UsersAndRoles/Users.tsx @@ -655,9 +655,8 @@ function EditRoles({ close, roles, defaultRole, user }: EditRolesProps) { ) // One expression for the whole dialog. The registry couples these (isService - // implies isRoleAssignmentDisabled), but gating only the selector on the pair - // left the title and a live Save button behind if that ever stops holding -- - // a Save that submits unchanged values and silently changes nothing. + // implies isRoleAssignmentDisabled), but nothing here may depend on that: a + // live Save the registry refuses submits unchanged values and reports success. const readOnly = user.isRoleAssignmentDisabled || user.isService return ( @@ -783,8 +782,10 @@ interface EditableSwitchProps { disabled?: boolean /** Why the control is disabled — a disabled switch with no cause is * indistinguishable from a rendering bug, and two different causes (self, - * service user) sit in the same column. */ - disabledReason?: NonNullable + * service user) sit in the same column. A string, not a node: it is also the + * wrapper's accessible name, which is the only way a keyboard or + * screen-reader admin reaches the reason at all. */ + disabledReason?: string checked: boolean onChange: (v: boolean) => void hint: NonNullable @@ -807,8 +808,13 @@ export function EditableSwitch({ if (!disabledReason) return sw return ( - {/* a disabled element fires no events, so the tooltip needs a live wrapper */} - {sw} + {/* A disabled switch fires no events and is out of the tab order, so the + reason needs a live wrapper that is focusable and named — otherwise it + is mouse-only. */} + {/* oxlint-disable-next-line jsx-a11y/no-noninteractive-tabindex -- the span is the only reachable carrier of the reason */} + + {sw} + ) } @@ -865,8 +871,12 @@ function RoleDisplay({ user, roles, defaultRole, openDialog }: RoleDisplayProps) fullWidth: true, }) + // Same pair as the dialog this opens and the switch columns: the registry + // couples the flags, but nothing here may depend on that holding. + const readOnly = user.isRoleAssignmentDisabled || user.isService + return ( - + {user.role?.name ?? emptyRole} {user.extraRoles.length > 0 && +{user.extraRoles.length}} @@ -893,6 +903,26 @@ interface ColumnDisplayProps { isSelf: boolean } +const MANAGED_BY_STACK = 'This service user is managed by the stack' + +// One resolver per switch column, so the guard and its explanation cannot drift: +// `disabled` is derived from whether there is a reason, never stated separately. +function whyEnabledDisabled(user: User, isSelf: boolean): string | undefined { + if (isSelf) return 'You cannot deactivate your own account' + if (user.isService) return MANAGED_BY_STACK + return undefined +} + +function whyAdminDisabled(user: User, isSelf: boolean): string | undefined { + if (isSelf) return 'You cannot change your own admin status' + // `isService` before the SSO flag: the registry couples them, but the guard + // must not depend on that, and "managed by the stack" is the truer reason. + if (user.isService) return MANAGED_BY_STACK + if (user.isAdminAssignmentDisabled) + return 'Admin capabilities for this user are managed by the SSO configuration' + return undefined +} + // Exported for testing: a disabled control's explanation lives in the column that // renders it, so only the call site proves it is there. export const columns: Table.Column[] = [ @@ -900,21 +930,18 @@ export const columns: Table.Column[] = [ id: 'isActive', label: 'Enabled', getValue: (u) => u.isActive, - getDisplay: (_v, u, { setActive, isSelf }: ColumnDisplayProps) => ( - setActive(u.name, active)} - /> - ), + getDisplay: (_v, u, { setActive, isSelf }: ColumnDisplayProps) => { + const reason = whyEnabledDisabled(u, isSelf) + return ( + setActive(u.name, active)} + /> + ) + }, props: { padding: 'none' }, }, { @@ -963,32 +990,25 @@ export const columns: Table.Column[] = [ id: 'isAdmin', label: 'Admin', getValue: (u) => u.isAdmin, - getDisplay: (_v, u, { openDialog, isSelf }: ColumnDisplayProps) => ( - - openDialog( - ({ close }) => , - DIALOG_PROPS, - ).then((res) => { - if (!res) throw new Error('cancel') - }) - } - /> - ), + getDisplay: (_v, u, { openDialog, isSelf }: ColumnDisplayProps) => { + const reason = whyAdminDisabled(u, isSelf) + return ( + + openDialog( + ({ close }) => , + DIALOG_PROPS, + ).then((res) => { + if (!res) throw new Error('cancel') + }) + } + /> + ) + }, props: { padding: 'none' }, }, ] From b36dfec53f89a2e2e0bfe8e38222e588e80c570a Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:46:14 -0500 Subject: [PATCH 12/17] fix(search): stop the facet sort control flickering as the list narrows Make the offer monotonic per mount and withhold it while nothing is displayed, so every call site answers visibility through one helper. Co-Authored-By: Claude Fable 5 --- catalog/app/containers/Search/model.spec.ts | 45 +++++++++++++++++++++ catalog/app/containers/Search/model.ts | 26 +++++++++--- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/catalog/app/containers/Search/model.spec.ts b/catalog/app/containers/Search/model.spec.ts index a0b76e1df6f..082e4b62947 100644 --- a/catalog/app/containers/Search/model.spec.ts +++ b/catalog/app/containers/Search/model.spec.ts @@ -1,3 +1,4 @@ +import { renderHook } from '@testing-library/react-hooks' import { describe, expect, it, vi } from 'vitest' import * as KTree from 'utils/KeyedTree' @@ -414,4 +415,48 @@ describe('containers/Search/model', () => { ) }) }) + + describe('useOrderingOffered', () => { + const T = model.FACET_ORDERING_THRESHOLD + + it('withholds the control below the threshold', () => { + const { result } = renderHook(() => model.useOrderingOffered(T - 1, T - 1)) + expect(result.current).toBe(false) + }) + + it('offers the control at the threshold', () => { + const { result } = renderHook(() => model.useOrderingOffered(T, T)) + expect(result.current).toBe(true) + }) + + it('keeps the control once offered, however far the list narrows', () => { + // The flicker: typing in "Find metadata" narrows the list, and a control + // that reappraised every keystroke would vanish mid-word. + const { result, rerender } = renderHook( + ({ total, shown }) => model.useOrderingOffered(total, shown), + { initialProps: { total: T, shown: T } }, + ) + expect(result.current).toBe(true) + rerender({ total: 1, shown: 1 }) + expect(result.current).toBe(true) + }) + + it('withholds the control while nothing is displayed', () => { + // A live "Sort by" above "No metadata found" sorts nothing. + const { result, rerender } = renderHook( + ({ total, shown }) => model.useOrderingOffered(total, shown), + { initialProps: { total: T, shown: T } }, + ) + expect(result.current).toBe(true) + rerender({ total: T, shown: 0 }) + expect(result.current).toBe(false) + }) + + it('does not carry the offer across mounts', () => { + const { result: first } = renderHook(() => model.useOrderingOffered(T, T)) + expect(first.current).toBe(true) + const { result: second } = renderHook(() => model.useOrderingOffered(1, 1)) + expect(second.current).toBe(false) + }) + }) }) diff --git a/catalog/app/containers/Search/model.ts b/catalog/app/containers/Search/model.ts index 326f711cee4..434f8d8a6d8 100644 --- a/catalog/app/containers/Search/model.ts +++ b/catalog/app/containers/Search/model.ts @@ -1300,10 +1300,9 @@ function AvailablePackagesMetaFiltersServerFilterQuery({ state, children, // Neither count alone is the pre-filter total on this path: `available` is - // the server-filtered result (so it vanishes the control as matches narrow), - // and `initial` is the truncated list minus applied filters (so it withholds - // the control when a text query returns far more than the truncated list - // held). Offer whenever either is large enough. + // the server-filtered result, and `initial` is the truncated list minus + // applied filters — which understates it when a text query matches far more + // than the truncated list held. Take whichever is larger. totalAvailable: Math.max(initial.length, available.length), }) } @@ -1352,6 +1351,23 @@ function AvailablePackagesMetaFiltersClientFilter({ }) } +/** + * Whether to offer the ordering switcher. + * + * Monotonic per mount: once enough fields have been seen the control stays, so + * narrowing the list by typing cannot make it flicker away mid-keystroke. It is + * withheld while nothing is displayed at all, because a live "Sort by" above "No + * metadata found" sorts nothing. + * + * Exported for testing: every visibility rule is here, so the hook is the only + * place the question can be answered. + */ +export function useOrderingOffered(totalAvailable: number, displayed: number): boolean { + const maxSeen = React.useRef(0) + maxSeen.current = Math.max(maxSeen.current, totalAvailable) + return displayed > 0 && maxSeen.current >= FACET_ORDERING_THRESHOLD +} + // Every `Ready` path funnels through here before the tree reaches the panel, so // this is the one place the ordering can own both the sort and the split. function AvailablePackagesMetaFiltersGroup({ @@ -1380,7 +1396,7 @@ function AvailablePackagesMetaFiltersGroup({ [available, ordering], ) - const offered = totalAvailable >= FACET_ORDERING_THRESHOLD + const offered = useOrderingOffered(totalAvailable, available?.length ?? 0) const orderingState = React.useMemo( () => ({ value: ordering, set: setOrdering, offered }), From 4b36a13576174cb3ef4421efd993f15fab68c526 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:50:10 -0500 Subject: [PATCH 13/17] fix(athena): pass the default workgroup as a string, not the prefs object The parsed preferences are rebuilt every provider render, re-firing the workgroup effect throughout the probe fan-out. Co-Authored-By: Claude Fable 5 --- .../containers/Queries/Athena/model/requests.spec.ts | 3 +-- .../app/containers/Queries/Athena/model/requests.ts | 10 ++++++---- catalog/app/containers/Queries/Athena/model/state.tsx | 9 ++++++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/catalog/app/containers/Queries/Athena/model/requests.spec.ts b/catalog/app/containers/Queries/Athena/model/requests.spec.ts index ff41f2b0018..d44ce0cb1a8 100644 --- a/catalog/app/containers/Queries/Athena/model/requests.spec.ts +++ b/catalog/app/containers/Queries/Athena/model/requests.spec.ts @@ -1266,10 +1266,9 @@ describe('containers/Queries/Athena/model/requests', () => { data: { list: ['foo', 'bar'] }, loadMore: noop, } - const preferences = { defaultWorkgroup: 'bar' } const { result, waitFor, unmount } = renderHook(() => - useWrapper([workgroups, undefined, preferences]), + useWrapper([workgroups, undefined, 'bar']), ) await act(async () => { diff --git a/catalog/app/containers/Queries/Athena/model/requests.ts b/catalog/app/containers/Queries/Athena/model/requests.ts index c7ecb8b3c02..8f61c440872 100644 --- a/catalog/app/containers/Queries/Athena/model/requests.ts +++ b/catalog/app/containers/Queries/Athena/model/requests.ts @@ -4,7 +4,6 @@ import * as React from 'react' import * as Sentry from '@sentry/react' import * as AWS from 'utils/AWS' -import * as BucketPreferences from 'utils/BucketPreferences' import Log from 'utils/Logging' import noop from 'utils/noop' @@ -190,7 +189,10 @@ export function useWorkgroups(): Model.DataController> { export function useWorkgroup( workgroups: Model.DataController>, requestedWorkgroup?: Workgroup, - preferences?: BucketPreferences.AthenaPreferences, + // The one preference this needs, as a string rather than the `ui.athena` + // object: the parsed preferences are rebuilt on every provider render, so an + // object here would re-fire this effect throughout the workgroup probe. + defaultWorkgroup?: string, ): Model.DataController { const [data, setData] = React.useState>() React.useEffect(() => { @@ -203,7 +205,7 @@ export function useWorkgroup( } // Stored or default workgroup - const initialWorkgroup = storage.getWorkgroup() || preferences?.defaultWorkgroup + const initialWorkgroup = storage.getWorkgroup() || defaultWorkgroup if (initialWorkgroup && listIncludes(workgroups.data.list, initialWorkgroup)) { setData(initialWorkgroup) return @@ -212,7 +214,7 @@ export function useWorkgroup( // First available workgroup or error. Producer drains to exhaustion, so // an accessible workgroup that exists is in this list. setData(workgroups.data.list[0] || new Error('Workgroup not found')) - }, [preferences, requestedWorkgroup, workgroups]) + }, [defaultWorkgroup, requestedWorkgroup, workgroups]) return React.useMemo(() => ({ data, loadMore: noop }), [data]) } diff --git a/catalog/app/containers/Queries/Athena/model/state.tsx b/catalog/app/containers/Queries/Athena/model/state.tsx index 29aff2967c5..f1ec99012cf 100644 --- a/catalog/app/containers/Queries/Athena/model/state.tsx +++ b/catalog/app/containers/Queries/Athena/model/state.tsx @@ -65,8 +65,7 @@ export const Ctx = React.createContext(null) interface ProviderProps { /** * `ui.athena` from the `?bucket=` scope's preferences. The console is - * workspace-global, so this only arrives when a bucket is in scope — which is - * exactly when `defaultWorkgroup` was ever honored before the re-home. + * workspace-global, so this only arrives when a bucket is in scope. */ preferences?: BucketPreferences.AthenaPreferences children: React.ReactNode @@ -84,7 +83,11 @@ export function Provider({ preferences, children }: ProviderProps) { const execution = requests.useWaitForQueryExecution(queryExecutionId) const workgroups = requests.useWorkgroups() - const workgroup = requests.useWorkgroup(workgroups, workgroupId, preferences) + const workgroup = requests.useWorkgroup( + workgroups, + workgroupId, + preferences?.defaultWorkgroup, + ) const queries = requests.useQueries(workgroup.data) const query = requests.useQuery(queries.data, execution) const queryBody = requests.useQueryBody(query.value, query.setValue, execution) From 4b5efb2b886e2e478be38828fa36fc6db62e4ebe Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:51:46 -0500 Subject: [PATCH 14/17] test(athena): build the prefs fixture through extendDefaults Co-Authored-By: Claude Fable 5 --- .../app/containers/Queries/Athena/Athena.spec.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/catalog/app/containers/Queries/Athena/Athena.spec.tsx b/catalog/app/containers/Queries/Athena/Athena.spec.tsx index b4b1c418968..0347d3fa032 100644 --- a/catalog/app/containers/Queries/Athena/Athena.spec.tsx +++ b/catalog/app/containers/Queries/Athena/Athena.spec.tsx @@ -3,6 +3,8 @@ import { MemoryRouter } from 'react-router-dom' import { render, cleanup, screen } from '@testing-library/react' import { describe, it, expect, vi, afterEach } from 'vitest' +import { extendDefaults } from 'utils/BucketPreferences/BucketPreferences' + import Athena from './Athena' vi.mock('constants/config', () => ({ default: {} })) @@ -63,14 +65,16 @@ describe('containers/Queries/Athena/Athena', () => { }) it('threads ui.athena from the ?bucket= scope into the model', async () => { - // The regression this pins: every legacy /b/:bucket/queries URL redirects - // here with ?bucket= set, and ui.athena.defaultWorkgroup must keep - // applying for those — it silently stopped when the console went global. + // The regression this pins: a legacy /b/:bucket/queries URL redirects here + // with ?bucket= set, and ui.athena.defaultWorkgroup must keep applying for + // those — it silently stopped when the console went global. const { Result } = await vi.importActual( 'utils/BucketPreferences', ) const athena = { defaultWorkgroup: 'analytics-prod' } - prefsResult.mockReturnValue(Result.Ok({ ui: { athena } } as never)) + // Through the real parse pipeline, so a change to how `ui.athena` is + // parsed shows up here rather than being mocked away. + prefsResult.mockReturnValue(Result.Ok(extendDefaults({ ui: { athena } }))) renderAthena('/queries/athena?bucket=my-bucket') expect(screen.getByTestId('prefs-provider').dataset.bucket).toBe('my-bucket') expect(providerProps).toHaveBeenCalledTimes(1) From 53cd485d52cc5437297c706b3d273f1ef9e096d2 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:54:58 -0500 Subject: [PATCH 15/17] fix(queries): carry ?bucket= on the legacy workgroup and execution redirects Only the root shape promoted the bucket, so a legacy workgroup or execution URL landed in the console unscoped and lost the bucket's ui.athena preferences. Co-Authored-By: Claude Fable 5 --- catalog/app/containers/App/queryRedirects.jsx | 28 ++++++++++++++++--- .../containers/App/queryRedirects.spec.tsx | 16 ++++++++--- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/catalog/app/containers/App/queryRedirects.jsx b/catalog/app/containers/App/queryRedirects.jsx index 9d4e5ec6e87..bf51a187db4 100644 --- a/catalog/app/containers/App/queryRedirects.jsx +++ b/catalog/app/containers/App/queryRedirects.jsx @@ -1,6 +1,7 @@ import * as React from 'react' import { Switch, Route, Redirect, useLocation, useParams } from 'react-router-dom' +import mkSearch from 'utils/mkSearch' import * as NamedRoutes from 'utils/NamedRoutes' import parseSearch from 'utils/parseSearch' @@ -9,24 +10,43 @@ import parseSearch from 'utils/parseSearch' // components are extracted from App.jsx unchanged so their redirect targets are // unit-testable; App.jsx wires them at `paths.bucketQueries` exactly as before. +// The bucket segment becomes the console's `?bucket=` scope param on every shape +// below, because that param is what makes the bucket's `ui.athena` preferences +// apply — a workgroup or execution link that dropped it would land the reader in +// the same console, unscoped. +function useScopeSearch() { + const { bucket } = useParams() + const { search } = useLocation() + return mkSearch({ bucket, ...parseSearch(search, true) }) +} + export function AthenaWorkgroupRedirect() { const { workgroup } = useParams() const { urls } = NamedRoutes.use() - return + const search = useScopeSearch() + return } export function AthenaExecutionRedirect() { const { workgroup, queryExecutionId } = useParams() const { urls } = NamedRoutes.use() - return + const search = useScopeSearch() + return ( + + ) } export function AthenaRootRedirect() { const { bucket } = useParams() const { search } = useLocation() const { urls } = NamedRoutes.use() - // The bucket segment becomes the console's `?bucket=` scope param (keeping - // `?table=` tabulator deep links alive); the rest of the search is preserved. + // Through the route builder rather than `useScopeSearch`, so `?table=` + // tabulator deep links keep their declared shape. const params = parseSearch(search, true) return } diff --git a/catalog/app/containers/App/queryRedirects.spec.tsx b/catalog/app/containers/App/queryRedirects.spec.tsx index 1f21b6bc690..dcd898088d7 100644 --- a/catalog/app/containers/App/queryRedirects.spec.tsx +++ b/catalog/app/containers/App/queryRedirects.spec.tsx @@ -72,15 +72,23 @@ describe('containers/App/queryRedirects', () => { expect(landingAt('/b/my-bucket/queries/es')).toBe('/queries/es') }) - it('redirects an athena workgroup, dropping the bucket', () => { + // Every athena shape carries the bucket, because `?bucket=` is what makes that + // bucket's `ui.athena` preferences apply to the console. + it('redirects an athena workgroup, carrying the bucket', () => { expect(landingAt('/b/my-bucket/queries/athena/primary')).toBe( - '/queries/athena/primary', + '/queries/athena/primary?bucket=my-bucket', ) }) - it('redirects an athena query execution', () => { + it('redirects an athena query execution, carrying the bucket', () => { expect(landingAt('/b/my-bucket/queries/athena/primary/exec-1')).toBe( - '/queries/athena/primary/exec-1', + '/queries/athena/primary/exec-1?bucket=my-bucket', + ) + }) + + it('preserves other query params alongside the bucket on a workgroup URL', () => { + expect(landingAt('/b/my-bucket/queries/athena/primary?table=drugs')).toBe( + '/queries/athena/primary?bucket=my-bucket&table=drugs', ) }) }) From 537b778710ad62799c44694e7dea08a26698de66 Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 11:55:57 -0500 Subject: [PATCH 16/17] docs(data-products): drop the useAdapter tombstone Co-Authored-By: Claude Fable 5 --- catalog/app/model/DataProducts/hooks.ts | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/catalog/app/model/DataProducts/hooks.ts b/catalog/app/model/DataProducts/hooks.ts index aa03e9becef..dbf326a3d76 100644 --- a/catalog/app/model/DataProducts/hooks.ts +++ b/catalog/app/model/DataProducts/hooks.ts @@ -44,17 +44,12 @@ import type { DataProduct } from './types' * When a GraphQL-backed adapter lands this becomes a build- or config-time * choice here, and no container changes. That is the whole point of the port. */ -/** - * The adapter used when the real one cannot be fetched. - * - * Empty answers rather than a rejection, because the resources below go through - * `ResourceCache`: a stored rejection is rethrown on every later read and never - * evicted, so one failed chunk fetch would blank the catalog through the root - * error boundary until a hard reload. These are the same values the port already - * produces for an adapter that cannot browse or fetch, so no call site needs a - * new branch. It implements neither `listContents` nor `fetchEntry`, so those - * resources take their existing unsupported path. - */ + +// Stands in when the adapter chunk cannot be fetched. Empty answers rather than a +// rejection: `ResourceCache` stores a rejection and rethrows it on every later +// read without evicting the entry, so one failed fetch would blank the catalog +// through the root error boundary. These match what the port already returns for +// an adapter that cannot browse or fetch, so no call site needs a new branch. const unavailableAdapter: DataProductAdapter = { listProducts: async () => [], getProduct: async () => null, @@ -246,10 +241,3 @@ export function useContents(productId: string, member: string): ContentsResult { { suspend: true }, ) as ContentsResult } - -// There was a `useAdapter()` here, exposing the adapter so a container could ask -// `supportsRequests(...)` before offering a submit affordance. It had no callers, -// and once the adapter loads lazily it could only be a suspending read -- which -// is the wrong shape for gating an affordance: it blanks the subtree to decide -// one button's disabled state. Re-add it as a non-suspending capability flag -// resolved alongside data the screen already awaits, not as a bare adapter read. From 491639d4cc902f0afa7723213ae24f6b9a89ef8f Mon Sep 17 00:00:00 2001 From: Simon Kohnstamm Date: Wed, 26 Aug 2026 12:01:37 -0500 Subject: [PATCH 17/17] docs(changelog): match the #5217 entries to what the fixes now cover Co-Authored-By: Claude Fable 5 --- catalog/CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/catalog/CHANGELOG.md b/catalog/CHANGELOG.md index 3b6d843fcb8..344034f8c74 100644 --- a/catalog/CHANGELOG.md +++ b/catalog/CHANGELOG.md @@ -21,10 +21,10 @@ complete sentence without it. ## Changes -- [Fixed] Athena Queries: a console opened with a `?bucket=` scope — which every legacy per-bucket queries URL redirects to — honors that bucket's `ui.athena.defaultWorkgroup` again; the workspace re-home had silently dropped the preference, landing customers on the alphabetically-first workgroup ([#5217](https://github.com/quiltdata/quilt/pull/5217)) -- [Fixed] Search sidebar: the "Sort by" control no longer disappears while typing in "Find metadata" on stacks with truncated facet lists, and it has an accessible name — its label used to land on a hidden input, reading as an unnamed button to assistive tech ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Fixed] Athena Queries: a console opened with a `?bucket=` scope — which every legacy per-bucket queries URL redirects to — honors that bucket's `ui.athena.defaultWorkgroup` again, and keeps that scope when you follow an execution or breadcrumb link inside the console; the workspace re-home had silently dropped the preference, landing customers on the alphabetically-first workgroup ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Fixed] Search sidebar: the "Sort by" control no longer disappears while typing in "Find metadata" on stacks with truncated facet lists, it is withheld when the query matches nothing rather than offering to sort an empty list, and it has an accessible name — its label used to land on a hidden input, reading as an unnamed button to assistive tech ([#5217](https://github.com/quiltdata/quilt/pull/5217)) - [Fixed] Queries: the query selector announces its label to assistive tech, and no longer claims "Custom" is loaded while its helper text reports the query failed to load ([#5217](https://github.com/quiltdata/quilt/pull/5217)) -- [Fixed] Admin Users: a disabled Enabled or Admin switch explains why on hover — "you can't deactivate yourself" and "managed by the stack" used to render as the same mute, dead control — and the roles dialog reads as read-only end to end for service users rather than offering a Save the registry refuses ([#5217](https://github.com/quiltdata/quilt/pull/5217)) +- [Fixed] Admin Users: a disabled Enabled or Admin switch explains why on hover or keyboard focus — "you can't deactivate yourself" and "managed by the stack" used to render as the same mute, dead control — and the roles dialog and Role column read as read-only end to end for service users rather than offering a Save the registry refuses ([#5217](https://github.com/quiltdata/quilt/pull/5217)) - [Changed] With the `data-products` preview off, the demo fixture data no longer ships in the bundles a browser downloads on the volumes landing; the adapter behind the preview loads only when the feature is on ([#5217](https://github.com/quiltdata/quilt/pull/5217)) - [Removed] The unbaked data-products GraphQL contract (added in [#5203](https://github.com/quiltdata/quilt/pull/5203), never served by a registry) is out of the schema; the `data-products` preview UI is unaffected and keeps reading fixture data ([#5223](https://github.com/quiltdata/quilt/pull/5223)) - [Changed] Search sidebar: the package metadata list is sorted from one "Sort by" control, sitting directly above the list ([#5222](https://github.com/quiltdata/quilt/pull/5222))