Skip to content

Commit

Permalink
Merge pull request #135993 from kyle-a-wong/backportrelease-24.3-135941
Browse files Browse the repository at this point in the history
release-24.3: ui: fix bug where index details page link is broken in cc console
  • Loading branch information
kyle-a-wong authored Nov 22, 2024
2 parents 40285ab + a95df2c commit a915efc
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 89 deletions.
2 changes: 1 addition & 1 deletion pkg/ui/workspaces/cluster-ui/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@cockroachlabs/cluster-ui",
"version": "24.3.2",
"version": "24.3.3",
"description": "Cluster UI is a library of large features shared between CockroachDB and CockroachCloud",
"repository": {
"type": "git",
Expand Down
5 changes: 3 additions & 2 deletions pkg/ui/workspaces/cluster-ui/src/breadcrumbs/breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ export const Breadcrumbs: FunctionComponent<BreadcrumbsProps> = ({
if (items.length === 0) {
return null;
}
const lastItem = items.pop();
const lastItem = items.slice(-1)[0];
const itemsWithoutLast = items.slice(0, -1);
return (
<div className={cx("breadcrumbs")}>
{items.map(({ link, name, onClick = () => {} }) => (
{itemsWithoutLast.map(({ link, name, onClick = () => {} }) => (
<div className={cx("breadcrumbs__item")} key={link}>
<Link
className={cx("breadcrumbs__item--link")}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const IndexStatsLink: React.FC<Props> = ({
const isCockroachCloud = useContext(CockroachCloudContext);

const linkUrl = isCockroachCloud
? `${location.pathname}/${indexName}`
? `/databases/${encodeURIComponent(dbName)}/null/${encodeURIComponent(escSchemaQualifiedTableName)}/${EncodeUriName(indexName)}`
: `/database/${encodeURIComponent(dbName)}/table/${encodeURIComponent(escSchemaQualifiedTableName)}/index/${EncodeUriName(indexName)}`;
return <Link to={linkUrl}>{indexName}</Link>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { Dispatch } from "redux";

import { actions as indexStatsActions } from "src/store/indexStats/indexStats.reducer";

import { BreadcrumbItem } from "../breadcrumbs";
import { AppState, uiConfigActions } from "../store";
import { actions as analyticsActions } from "../store/analytics";
import {
Expand All @@ -31,7 +30,6 @@ import {
getMatchParamByName,
indexNameAttr,
longToInt,
schemaNameAttr,
tableNameAttr,
TimestampToMoment,
} from "../util";
Expand All @@ -45,38 +43,11 @@ import {

import RecommendationType = cockroach.sql.IndexRecommendation.RecommendationType;

// Note: if the managed-service routes to the index detail or the previous
// database pages change, the breadcrumbs displayed here need to be updated.
// TODO(thomas): ensure callers are splitting schema/table name correctly
function createManagedServiceBreadcrumbs(
database: string,
schema: string,
table: string,
index: string,
): BreadcrumbItem[] {
return [
{ link: "legacy/databases", name: "Databases" },
{
link: `/databases/${database}`,
name: "Tables",
},
{
link: `/databases/${database}/${schema}/${table}`,
name: `Table: ${table}`,
},
{
link: `/databases/${database}/${schema}/${table}/${index}`,
name: `Index: ${index}`,
},
];
}

const mapStateToProps = (
state: AppState,
props: RouteComponentProps,
): IndexDetailsPageData => {
const databaseName = getMatchParamByName(props.match, databaseNameAttr);
const schemaName = getMatchParamByName(props.match, schemaNameAttr);
const tableName = getMatchParamByName(props.match, tableNameAttr);
const indexName = getMatchParamByName(props.match, indexNameAttr);

Expand All @@ -94,14 +65,7 @@ const mapStateToProps = (
"Unknown") as RecType,
reason: indexRec.reason,
}));

return {
breadcrumbItems: createManagedServiceBreadcrumbs(
databaseName,
schemaName,
tableName,
indexName,
),
databaseName,
hasAdminRole: selectHasAdminRole(state),
hasViewActivityRedactedRole: selectHasViewActivityRedactedRole(state),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
// Use of this software is governed by the CockroachDB Software License
// included in the /LICENSE file.

import { expect } from "chai";
import "@testing-library/jest-dom";
import { render } from "@testing-library/react";
import { shallow } from "enzyme";
import moment from "moment";
import React from "react";
import { MemoryRouter, Route, Switch } from "react-router-dom";

import { IndexDetailsPage, IndexDetailsPageProps, util } from "../index";

Expand All @@ -31,13 +33,12 @@ describe("IndexDetailsPage", () => {
createStatement: "",
totalReads: 0,
indexRecommendations: [],
tableID: undefined,
tableID: "2",
indexID: undefined,
lastRead: util.minDate,
lastReset: util.minDate,
databaseID: 1,
},
breadcrumbItems: null,
isTenant: false,
refreshUserSQLRoles: () => {},
onTimeScaleChange: () => {},
Expand All @@ -46,7 +47,7 @@ describe("IndexDetailsPage", () => {
it("should call refreshNodes if isTenant is false", () => {
const mockCallback = jest.fn(() => {});
shallow(<IndexDetailsPage {...props} refreshNodes={mockCallback} />);
expect(mockCallback.mock.calls).to.have.length(1);
expect(mockCallback.mock.calls).toHaveLength(1);
});
it("should not call refreshNodes if isTenant is true", () => {
const mockCallback = jest.fn(() => {});
Expand All @@ -58,6 +59,22 @@ describe("IndexDetailsPage", () => {
isTenant={true}
/>,
);
expect(mockCallback.mock.calls).to.have.length(0);
expect(mockCallback.mock.calls).toHaveLength(0);
});
it("should render bread crumbs", () => {
const { container } = render(
<MemoryRouter initialEntries={["/"]}>
<Switch>
<Route path="/">
<IndexDetailsPage {...props} isTenant={false} />
</Route>
</Switch>
</MemoryRouter>,
);
const itemLinks = container.getElementsByClassName("item-link");
expect(itemLinks).toHaveLength(3);
expect(itemLinks[0].getAttribute("href")).toEqual("/databases");
expect(itemLinks[1].getAttribute("href")).toEqual("/databases/1");
expect(itemLinks[2].getAttribute("href")).toEqual("/table/2");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,6 @@ const withData: IndexDetailsPageProps = {
},
],
},
breadcrumbItems: [
{ link: "/legacy/databases", name: "Databases" },
{
link: `/databases/story_db`,
name: "Tables",
},
{
link: `/database/story_db/$public/story_table`,
name: `Table: story_table`,
},
{
link: `/database/story_db/public/story_table/story_index`,
name: `Index: story_index`,
},
],
refreshIndexStats: () => {},
resetIndexUsageStats: () => {},
refreshNodes: () => {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// included in the /LICENSE file.

import { Caution, Search as IndexIcon } from "@cockroachlabs/icons";
import { Heading } from "@cockroachlabs/ui-components";
import { Heading, Icon } from "@cockroachlabs/ui-components";
import { Col, Row, Tooltip } from "antd";
import classNames from "classnames/bind";
import flatMap from "lodash/flatMap";
Expand All @@ -29,10 +29,8 @@ import {
StatementsListRequestFromDetails,
StatementsUsingIndexRequest,
} from "../api/indexDetailsApi";
import { BreadcrumbItem, Breadcrumbs } from "../breadcrumbs";
import { commonStyles } from "../common";
import { CockroachCloudContext } from "../contexts";
import { CaretRight } from "../icon/caretRight";
import { Pagination } from "../pagination";
import {
calculateActiveFilters,
Expand All @@ -41,6 +39,7 @@ import {
Filters,
} from "../queryFilter";
import { Search } from "../search";
import Breadcrumbs from "../sharedFromCloud/breadcrumbs";
import LoadingError from "../sqlActivity/errorComponent";
import { filterStatementsData } from "../sqlActivity/util";
import { EmptyStatementsPlaceholder } from "../statementsPage/emptyStatementsPlaceholder";
Expand All @@ -64,8 +63,6 @@ import {
Count,
DATE_FORMAT_24_TZ,
EncodeDatabaseTableIndexUri,
EncodeDatabaseTableUri,
EncodeDatabaseUri,
performanceTuningRecipes,
unique,
unset,
Expand Down Expand Up @@ -110,7 +107,6 @@ export interface IndexDetailsPageData {
tableName: string;
indexName: string;
details: IndexDetails;
breadcrumbItems: BreadcrumbItem[];
isTenant: UIConfigState["isTenant"];
hasViewActivityRedactedRole?: UIConfigState["hasViewActivityRedactedRole"];
hasAdminRole?: UIConfigState["hasAdminRole"];
Expand Down Expand Up @@ -381,34 +377,19 @@ export class IndexDetailsPage extends React.Component<
}

private renderBreadcrumbs() {
if (this.props.breadcrumbItems) {
return (
<Breadcrumbs
items={this.props.breadcrumbItems}
divider={<CaretRight className={cx("icon--xxs", "icon--primary")} />}
/>
);
}

const isCockroachCloud = this.context;
// If no props are passed, render db-console breadcrumb links by default.
return (
<Breadcrumbs
items={[
{ link: DB_PAGE_PATH, name: "Databases" },
{
link: isCockroachCloud
? EncodeDatabaseUri(this.props.databaseName)
: databaseDetailsPagePath(this.props.details.databaseID),
link: databaseDetailsPagePath(this.props.details.databaseID),
name: this.props.databaseName,
},
{
link: isCockroachCloud
? EncodeDatabaseTableUri(
this.props.databaseName,
this.props.tableName,
)
: tableDetailsPagePath(parseInt(this.props.details.tableID, 10)),
link: tableDetailsPagePath(
parseInt(this.props.details.tableID, 10),
),
name: `Table: ${this.props.tableName}`,
},
{
Expand All @@ -420,7 +401,8 @@ export class IndexDetailsPage extends React.Component<
name: `Index: ${this.props.indexName}`,
},
]}
divider={<CaretRight className={cx("icon--xxs", "icon--primary")} />}
divider={<Icon iconName="CaretRight" size="tiny" />}
className={cx("header-breadcrumbs")}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ describe("Index Details Page", function () {
lastReset: util.minDate,
databaseID: undefined,
},
breadcrumbItems: null,
},
false,
);
Expand Down Expand Up @@ -217,7 +216,6 @@ describe("Index Details Page", function () {
indexRecommendations: [],
databaseID: 10,
},
breadcrumbItems: null,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ export const mapStateToProps = (
indexRecommendations,
databaseID: stats?.data?.database_id,
},
breadcrumbItems: null,
};
};

Expand Down

0 comments on commit a915efc

Please sign in to comment.