Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
73795: ui: save sort settings on cache for Database page r=maryliag a=maryliag

Previously, sort setting on Databases page were not
being stored. With this commits we save the info about sort settings,
so the value is the same when the user goes back to those pages.
This commit also updates the value on query params and that value
take priority over the cached valued.

Partially addresses cockroachdb#68199

Release note: None

Co-authored-by: Marylia Gutierrez <[email protected]>
  • Loading branch information
craig[bot] and maryliag committed Dec 14, 2021
2 parents 14367d7 + 4e07fa9 commit f116a74
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,61 @@ import { withBackground, withRouterProvider } from "src/storybook/decorators";
import { randomName } from "src/storybook/fixtures";
import { DatabasesPage, DatabasesPageProps } from "./databasesPage";

import * as H from "history";
const history = H.createHashHistory();

const withLoadingIndicator: DatabasesPageProps = {
loading: true,
loaded: false,
databases: [],
sortSetting: {
ascending: false,
columnTitle: "name",
},
onSortingChange: () => {},
refreshDatabases: () => {},
refreshDatabaseDetails: () => {},
refreshTableStats: () => {},
location: history.location,
history,
match: {
url: "",
path: history.location.pathname,
isExact: false,
params: {},
},
};

const withoutData: DatabasesPageProps = {
loading: false,
loaded: true,
databases: [],
sortSetting: {
ascending: false,
columnTitle: "name",
},
onSortingChange: () => {},
refreshDatabases: () => {},
refreshDatabaseDetails: () => {},
refreshTableStats: () => {},
location: history.location,
history,
match: {
url: "",
path: history.location.pathname,
isExact: false,
params: {},
},
};

const withData: DatabasesPageProps = {
loading: false,
loaded: true,
showNodeRegionsColumn: true,
sortSetting: {
ascending: false,
columnTitle: "name",
},
databases: _.map(Array(42), _item => {
return {
loading: false,
Expand All @@ -51,10 +84,18 @@ const withData: DatabasesPageProps = {
"gcp-europe-west1(n8), gcp-us-east1(n1), gcp-us-west1(n6)",
};
}),

onSortingChange: () => {},
refreshDatabases: () => {},
refreshDatabaseDetails: () => {},
refreshTableStats: () => {},
location: history.location,
history,
match: {
url: "",
path: history.location.pathname,
isExact: false,
params: {},
},
};

storiesOf("Databases Page", module)
Expand Down
67 changes: 48 additions & 19 deletions pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// licenses/APL.txt.

import React from "react";
import { Link } from "react-router-dom";
import { Link, RouteComponentProps } from "react-router-dom";
import { Tooltip } from "antd";
import classNames from "classnames/bind";
import _ from "lodash";
Expand All @@ -30,6 +30,7 @@ import {
baseHeadingClasses,
statisticsClasses,
} from "src/transactionsPage/transactionsPageClasses";
import { syncHistory } from "../util";

const cx = classNames.bind(styles);
const sortableTableCx = classNames.bind(sortableTableStyles);
Expand All @@ -46,6 +47,7 @@ const sortableTableCx = classNames.bind(sortableTableStyles);
// interface DatabasesPageData {
// loading: boolean;
// loaded: boolean;
// sortSetting: SortSetting;
// databases: { // DatabasesPageDataDatabase[]
// loading: boolean;
// loaded: boolean;
Expand All @@ -64,6 +66,7 @@ export interface DatabasesPageData {
loading: boolean;
loaded: boolean;
databases: DatabasesPageDataDatabase[];
sortSetting: SortSetting;
showNodeRegionsColumn?: boolean;
}

Expand Down Expand Up @@ -97,13 +100,19 @@ export interface DatabasesPageActions {
refreshDatabaseDetails: (database: string) => void;
refreshTableStats: (database: string, table: string) => void;
refreshNodes?: () => void;
onSortingChange?: (
name: string,
columnTitle: string,
ascending: boolean,
) => void;
}

export type DatabasesPageProps = DatabasesPageData & DatabasesPageActions;
export type DatabasesPageProps = DatabasesPageData &
DatabasesPageActions &
RouteComponentProps<unknown>;

interface DatabasesPageState {
pagination: ISortedTablePagination;
sortSetting: SortSetting;
}

class DatabasesSortedTable extends SortedTable<DatabasesPageDataDatabase> {}
Expand All @@ -120,22 +129,33 @@ export class DatabasesPage extends React.Component<
current: 1,
pageSize: 20,
},
sortSetting: {
ascending: true,
columnTitle: null,
},
};

const { history } = this.props;
const searchParams = new URLSearchParams(history.location.search);
const ascending = (searchParams.get("ascending") || undefined) === "true";
const columnTitle = searchParams.get("columnTitle") || undefined;
const sortSetting = this.props.sortSetting;

if (
this.props.onSortingChange &&
columnTitle &&
(sortSetting.columnTitle != columnTitle ||
sortSetting.ascending != ascending)
) {
this.props.onSortingChange("Databases", columnTitle, ascending);
}
}

componentDidMount() {
componentDidMount(): void {
this.refresh();
}

componentDidUpdate() {
componentDidUpdate(): void {
this.refresh();
}

private refresh() {
private refresh(): void {
if (this.props.refreshNodes != null) {
this.props.refreshNodes();
}
Expand All @@ -157,13 +177,22 @@ export class DatabasesPage extends React.Component<
});
}

private changePage(current: number) {
changePage = (current: number): void => {
this.setState({ pagination: { ...this.state.pagination, current } });
}
};

private changeSortSetting(sortSetting: SortSetting) {
this.setState({ sortSetting });
}
changeSortSetting = (ss: SortSetting): void => {
syncHistory(
{
ascending: ss.ascending.toString(),
columnTitle: ss.columnTitle,
},
this.props.history,
);
if (this.props.onSortingChange) {
this.props.onSortingChange("Databases", ss.columnTitle, ss.ascending);
}
};

private columns: ColumnDescriptor<DatabasesPageDataDatabase>[] = [
{
Expand Down Expand Up @@ -244,7 +273,7 @@ export class DatabasesPage extends React.Component<
},
];

render() {
render(): React.ReactElement {
this.columns.find(
c => c.name === "nodeRegions",
).showByDefault = this.props.showNodeRegionsColumn;
Expand Down Expand Up @@ -273,8 +302,8 @@ export class DatabasesPage extends React.Component<
className={cx("databases-table")}
data={this.props.databases}
columns={displayColumns}
sortSetting={this.state.sortSetting}
onChangeSortSetting={this.changeSortSetting.bind(this)}
sortSetting={this.props.sortSetting}
onChangeSortSetting={this.changeSortSetting}
pagination={this.state.pagination}
loading={this.props.loading}
renderNoResult={
Expand All @@ -292,7 +321,7 @@ export class DatabasesPage extends React.Component<
pageSize={this.state.pagination.pageSize}
current={this.state.pagination.current}
total={this.props.databases.length}
onChange={this.changePage.bind(this)}
onChange={this.changePage}
/>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ describe("Databases Page", function() {
loading: false,
loaded: false,
databases: [],
sortSetting: { ascending: true, columnTitle: "name" },
showNodeRegionsColumn: false,
});
});
Expand Down Expand Up @@ -131,6 +132,7 @@ describe("Databases Page", function() {
missingTables: [],
},
],
sortSetting: { ascending: true, columnTitle: "name" },
showNodeRegionsColumn: false,
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import _ from "lodash";
import { createSelector } from "reselect";
import { LocalSetting } from "src/redux/localsettings";
import {
DatabasesPageData,
DatabasesPageDataDatabase,
Expand Down Expand Up @@ -43,6 +44,12 @@ const selectLoaded = createSelector(
databases => databases.valid,
);

const sortSettingLocalSetting = new LocalSetting(
"sortSetting/DatabasesPage",
(state: AdminUIState) => state.localSettings,
{ ascending: true, columnTitle: "name" },
);

const selectDatabases = createSelector(
(state: AdminUIState) => state.cachedData.databases.data?.databases,
(state: AdminUIState) => state.cachedData.databaseDetails,
Expand Down Expand Up @@ -115,21 +122,28 @@ export const mapStateToProps = (state: AdminUIState): DatabasesPageData => ({
loading: selectLoading(state),
loaded: selectLoaded(state),
databases: selectDatabases(state),
sortSetting: sortSettingLocalSetting.selector(state),
showNodeRegionsColumn: selectIsMoreThanOneNode(state),
});

export const mapDispatchToProps = {
refreshDatabases,

refreshDatabaseDetails: (database: string) => {
return refreshDatabaseDetails(
new DatabaseDetailsRequest({ database, include_stats: true }),
);
},

refreshTableStats: (database: string, table: string) => {
return refreshTableStats(new TableStatsRequest({ database, table }));
},

refreshNodes,
onSortingChange: (
_tableName: string,
columnName: string,
ascending: boolean,
) =>
sortSettingLocalSetting.set({
ascending: ascending,
columnTitle: columnName,
}),
};

0 comments on commit f116a74

Please sign in to comment.