Skip to content

CNS-120 In Console, cluster list now pulls data from SUBSCRIBE #38014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jdonelson wants to merge 1 commit into
base: main
Choose a base branch
from
+71 −72
Open

CNS-120 In Console, cluster list now pulls data from SUBSCRIBE #38014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 30 additions & 40 deletions console/src/platform/clusters/ClusterRoutes.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,14 @@ import React from "react";

import { Cluster } from "~/api/materialize/cluster/clusterList";
import { ErrorCode } from "~/api/materialize/types";
import {
buildSqlQueryHandlerV2,
mapKyselyToTabular,
} from "~/api/mocks/buildSqlQueryHandler";
import server from "~/api/mocks/server";
import { getStore } from "~/jotai";
import { allClusters } from "~/store/allClusters";
import {
clustersFetchColumns,
emptyClustersResponse,
} from "~/test/clusterQueryBuilders";
import { mockSubscribeState } from "~/test/mockSubscribe";
import { renderComponent, RenderWithPathname } from "~/test/utils";

import ClusterRoutes from "./ClusterRoutes";
import { buildClusterServerResponse } from "./clustersTestUtils";
import { CLUSTERS_FETCH_ERROR_MESSAGE } from "./constants";
import { clusterQueryKeys } from "./queries";

vi.mock("~/platform/clusters/ClusterDetail", () => ({
default: function () {
Expand Down Expand Up @@ -73,38 +63,18 @@ const validCluster: Cluster = {
latestStatusUpdate: "2024-01-01T00:00:00.000Z",
};

export const noSystemObjectClustersResponse = buildSqlQueryHandlerV2({
queryKey: clusterQueryKeys.list({
includeSystemObjects: false,
}),
results: mapKyselyToTabular({
columns: clustersFetchColumns,
rows: [
buildClusterServerResponse({ id: "u1", name: "default" }),
buildClusterServerResponse({ id: "u2", name: "user_cluster" }),
],
}),
});

export const errorClustersResponse = buildSqlQueryHandlerV2({
queryKey: clusterQueryKeys.list({ includeSystemObjects: false }),
results: {
error: {
message: "Something went wrong",
code: ErrorCode.INTERNAL_ERROR,
},
notices: [],
},
});

describe("ClusterRoutes", () => {
beforeEach(() => {
const store = getStore();
store.set(allClusters, mockSubscribeState({ data: [validCluster] }));
});

it("shows a spinner initially", async () => {
server.use(emptyClustersResponse);
const store = getStore();
store.set(
allClusters,
mockSubscribeState<Cluster>({ data: [], snapshotComplete: false }),
);
renderComponent(<ClusterRoutes />);

expect(await screen.findByText("Clusters")).toBeVisible();
Expand All @@ -114,22 +84,42 @@ describe("ClusterRoutes", () => {
});

it("shows the empty state when there are no results", async () => {
server.use(emptyClustersResponse);
const store = getStore();
store.set(allClusters, mockSubscribeState<Cluster>({ data: [] }));
renderComponent(<ClusterRoutes />);

expect(await screen.findByText("No available clusters")).toBeVisible();
});

it("shows an error state when clusters fail to load", async () => {
server.use(errorClustersResponse);
it("shows an error state when the clusters subscribe fails", async () => {
const store = getStore();
store.set(
allClusters,
mockSubscribeState<Cluster>({
data: [],
snapshotComplete: false,
error: {
code: ErrorCode.INTERNAL_ERROR,
message: "Something went wrong",
},
}),
);
renderComponent(<ClusterRoutes />);

expect(await screen.findByText(CLUSTERS_FETCH_ERROR_MESSAGE)).toBeVisible();
});

it("renders the cluster list", async () => {

@leedqin leedqin Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The system-cluster filtering is client-side logic now, so it'd be good to cover it: seed the atom with an s-id cluster plus user clusters and assert it's hidden by default / shown with the toggle.

// The cluster routes use the unfiltered response and the list uses the filtered response
server.use(noSystemObjectClustersResponse);
const store = getStore();
store.set(
allClusters,
mockSubscribeState({
data: [
buildClusterServerResponse({ id: "u1", name: "default" }),
buildClusterServerResponse({ id: "u2", name: "user_cluster" }),
],
}),
);
renderComponent(<ClusterRoutes />);

expect(await screen.findByText("Clusters")).toBeVisible();
Expand Down
73 changes: 41 additions & 32 deletions console/src/platform/clusters/ClustersList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import useLatestOfflineReplica, {
import { AppErrorBoundary } from "~/components/AppErrorBoundary";
import { CodeBlock } from "~/components/copyableComponents";
import DeleteObjectMenuItem from "~/components/DeleteObjectMenuItem";
import ErrorBox from "~/components/ErrorBox";
import { LoadingContainer } from "~/components/LoadingContainer";
import OverflowMenu, { OVERFLOW_BUTTON_WIDTH } from "~/components/OverflowMenu";
import { sortingFunctions } from "~/components/Table/tableColumnBuilders";
Expand All @@ -50,6 +51,7 @@ import {
} from "~/layouts/listPageComponents";
import docUrls from "~/mz-doc-urls.json";
import { relativeClusterPath } from "~/platform/routeHelpers";
import { useAllClusters } from "~/store/allClusters";
import WarningIcon from "~/svg/WarningIcon";
import { truncateMaxWidth } from "~/theme/components/Table";
import {
Expand All @@ -59,7 +61,7 @@ import {

import AlterClusterMenuItem from "./AlterClusterMenuItem";
import { CLUSTERS_FETCH_ERROR_MESSAGE } from "./constants";
import { useClusters } from "./queries";
import { useOwners } from "./queries";
import { useShowSystemObjects } from "./useShowSystemObjects";

const createClusterSuggestion = {
Expand All @@ -73,7 +75,6 @@ const createClusterSuggestion = {
* Read from `info.table.options.meta` and cast to this shape inside cells.
*/
interface ClusterTableMeta {
refetchClusters: () => void;
offlineReplicaMap: LatestOfflineReplicaMap | undefined;
}

Expand Down Expand Up @@ -137,13 +138,7 @@ const LastStatusChangeCell = ({
);
};

const ClusterActionsCell = ({
cluster,
refetchClusters,
}: {
cluster: ClusterWithOwnership;
refetchClusters: () => void;
}) => (
const ClusterActionsCell = ({ cluster }: { cluster: ClusterWithOwnership }) => (
<OverflowMenu
items={[
{
Expand All @@ -154,7 +149,8 @@ const ClusterActionsCell = ({
<DeleteObjectMenuItem
key="delete-object"
selectedObject={cluster}
onSuccessAction={refetchClusters}
// the subscribe drops the row from our list
onSuccessAction={() => undefined}
objectType="CLUSTER"
/>
</>
Expand Down Expand Up @@ -210,15 +206,7 @@ const columns = [
columnHelper.display({
id: "actions",
header: "",
cell: (info) => {
const meta = info.table.options.meta as ClusterTableMeta;
return (
<ClusterActionsCell
cluster={info.row.original}
refetchClusters={meta.refetchClusters}
/>
);
},
cell: (info) => <ClusterActionsCell cluster={info.row.original} />,
enableSorting: false,
size: OVERFLOW_BUTTON_WIDTH,
}),
Expand All @@ -229,20 +217,42 @@ const ClustersListContent = ({
}: {
showSystemObjects: boolean;
}) => {
const { data: clusters, refetch } = useClusters({
includeSystemObjects: showSystemObjects,
});
const { data: clusters, snapshotComplete, isError } = useAllClusters();
const { data: ownersById, isPending: isOwnersPending } = useOwners();

const orderedClusters = React.useMemo(() => {
if (!clusters) return [];
const systemClusters = clusters.filter((c) => isSystemCluster(c.id));
const nonSystemClusters = clusters
const visibleClusters = clusters

@leedqin leedqin Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same filter + sort already exists in ClusterDetailBreadcrumbs. Maybe pull it into a helper in store/allClusters.ts, e.g. visibleClusters(clusters, showSystemObjects)

export function visibleClusters(clusters: Cluster[], showSystemObjects: boolean) {
  return clusters
    .filter((c) => showSystemObjects || !isSystemCluster(c.id))
    .sort((a, b) => a.name.localeCompare(b.name));
}

@jdonelson jdonelson Aug 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of minor differences between the two screens. On the cluster list page, in the default sort state, we push system clusters to the bottom of the list. In the menu on the detail page, we sort by name without separating the 2 cluster types.

Also, on the List page, we filter out clusters that aren't owned by the user. We don't filter on the Detail page.

I suggest we update the detail page menu to include the filtering and to separate the 2 types of cluster, but that's just my uninformed opinion. For that matter, maybe we should move the Unowned filter higher up in the chain so that we use it in the query or when writing to the store?

@leedqin leedqin Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of clarifications:

The list doesn't actually filter unowned clusters out. getOwners() returns every role with an isOwner boolean, so no rows are dropped, isOwner only gates the Alter/Delete items in the row's overflow menu. So there's no unowned filter to move up the chain.

Moving ownership into the subscribe also isn't possible: has_role() and mz_is_superuser() can't run in a dataflow, which is why ownership is a separate polled query joined client-side. The allClusters atom is also shared by route validation, this breadcrumb menu, and the Shell dropdown, which all need every cluster regardless of ownership.

On sorting, the difference is smaller than it looks: the table applies initialSorting: name asc, so in the default state rows are re-sorted by name and the list's system-last grouping only shows once a user cycles sorting off. Given that, I'd keep the helper to the shared core (system-visibility filter + name sort) and let the list layer its grouping on top. Grouping system clusters last in this menu too seems fine if you want it, no strong opinion from me either.

@jdonelson jdonelson Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh gotcha OK. I see. I think I was conflating owned and system clusters. And yep, I realized that filtering higher in the chain didn't make sense after I re-read my comment. Thanks!

.filter((c) => showSystemObjects || !isSystemCluster(c.id))
.map((c) => ({
...c,
// Treat an in-flight owners query as non-owner so owner-only menu items
// stay hidden until ownership is known.
isOwner: !isOwnersPending && (ownersById?.get(c.ownerId) ?? false),

@leedqin leedqin Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think we could move this check into useOwners hook and share this check across ClustersList.tsx, ClusterDetail and ClusterReplica?

}));
// The subscribe upserts by id, so the atom's order is arbitrary. Sort each
// group by name and keep system clusters at the end.
const byName = (a: ClusterWithOwnership, b: ClusterWithOwnership) =>
a.name.localeCompare(b.name);
const systemClusters = visibleClusters
.filter((c) => isSystemCluster(c.id))
.sort(byName);
const nonSystemClusters = visibleClusters
.filter((c) => !isSystemCluster(c.id))
.sort((a, b) => a.name.localeCompare(b.name));
.sort(byName);
return [...nonSystemClusters, ...systemClusters];
}, [clusters]);
}, [clusters, isOwnersPending, ownersById, showSystemObjects]);

if (isError) {
return <ErrorBox message={CLUSTERS_FETCH_ERROR_MESSAGE} />;
}

// The atom starts out empty, so the empty state has to wait for the snapshot
// or it would flash before the first rows arrive.
if (!snapshotComplete) {
return <LoadingContainer />;
}

if (clusters !== null && clusters.length === 0) {
if (orderedClusters.length === 0) {
return (
<EmptyListWrapper>
<EmptyListHeader>
Expand Down Expand Up @@ -270,19 +280,18 @@ const ClustersListContent = ({
);
}

return <ClusterTable clusters={orderedClusters} refetchClusters={refetch} />;
return <ClusterTable clusters={orderedClusters} />;
};

interface ClusterTableProps {
clusters: ClusterWithOwnership[];
refetchClusters: () => void;
}

const ClusterTable = ({ clusters, refetchClusters }: ClusterTableProps) => {
const ClusterTable = ({ clusters }: ClusterTableProps) => {
const { data: offlineReplicaMap, error: offlineReplicaError } =
useLatestOfflineReplica();

const meta: ClusterTableMeta = { refetchClusters, offlineReplicaMap };
const meta: ClusterTableMeta = { offlineReplicaMap };

const table = useUniversalTable({
data: clusters,
Expand Down
Loading