Skip to content

Commit

Permalink
ui: save sort on cache for Statement page
Browse files Browse the repository at this point in the history
Previously, a sort selection was not maintained when
the page change (e.g. coming back from Statement details).
This commits saves the selected value to be used.

Partially adressed #68199

Release note: None
  • Loading branch information
maryliag committed Nov 17, 2021
1 parent 525322a commit e3e8917
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ const statementsPagePropsFixture: StatementsPageProps = {
"3": "gcp-us-west1",
"4": "gcp-europe-west1",
},
sortSetting: {
ascending: false,
columnTitle: "executionCount"
},
statements: [
{
label:
Expand Down Expand Up @@ -738,6 +742,7 @@ const statementsPagePropsFixture: StatementsPageProps = {
onSearchComplete: noop,
onDiagnosticsReportDownload: noop,
onColumnsChange: noop,
onSortingChange: noop,
};

export const statementsPagePropsWithRequestError: StatementsPageProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,8 @@ export const selectDateRange = createSelector(
Moment,
],
);

export const selectSortSetting = createSelector(
localStorageSelector,
localStorage => localStorage["sortSetting/StatementsPage"],
);
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ describe("StatementsPage", () => {
const statementsPageInstance = statementsPageWrapper.instance();

assert.equal(
statementsPageInstance.state.sortSetting.columnTitle,
statementsPageInstance.props.sortSetting.columnTitle,
"executionCount",
);
assert.equal(statementsPageInstance.state.sortSetting.ascending, false);
assert.equal(statementsPageInstance.props.sortSetting.ascending, false);
});
});
});
32 changes: 15 additions & 17 deletions pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ export interface StatementsPageStateProps {
lastReset: string;
columns: string[];
nodeRegions: { [key: string]: string };
sortSetting: SortSetting;
isTenant?: UIConfigState["isTenant"];
}

export interface StatementsPageState {
sortSetting: SortSetting;
search?: string;
pagination: ISortedTablePagination;
filters?: Filters;
Expand Down Expand Up @@ -142,11 +142,6 @@ export class StatementsPage extends React.Component<
this.props.history.location.search,
);
const defaultState = {
sortSetting: {
// Sort by Execution Count column as default option.
ascending: false,
columnTitle: "executionCount",
},
pagination: {
pageSize: 20,
current: 1,
Expand All @@ -168,24 +163,26 @@ export class StatementsPage extends React.Component<
getStateFromHistory = (): Partial<StatementsPageState> => {
const { history } = this.props;
const searchParams = new URLSearchParams(history.location.search);
const ascending = searchParams.get("ascending") || undefined;
const columnTitle = searchParams.get("columnTitle") || undefined;
const searchQuery = searchParams.get("q") || undefined;
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("Statements", columnTitle, ascending);
}

return {
sortSetting: {
ascending: ascending === "true",
columnTitle,
},
search: searchQuery,
};
};

changeSortSetting = (ss: SortSetting): void => {
this.setState({
sortSetting: ss,
});

syncHistory(
{
ascending: ss.ascending.toString(),
Expand Down Expand Up @@ -422,6 +419,7 @@ export class StatementsPage extends React.Component<
onColumnsChange,
nodeRegions,
isTenant,
sortSetting,
} = this.props;
const appAttrValue = queryByName(location, appAttr);
const selectedApp = appAttrValue || "";
Expand Down Expand Up @@ -545,7 +543,7 @@ export class StatementsPage extends React.Component<
className="statements-table"
data={data}
columns={displayColumns}
sortSetting={this.state.sortSetting}
sortSetting={sortSetting}
onChangeSortSetting={this.changeSortSetting}
renderNoResult={
<EmptyStatementsPlaceholder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
selectTotalFingerprints,
selectColumns,
selectDateRange,
selectSortSetting,
} from "./statementsPage.selectors";
import { selectIsTenant } from "../store/uiConfig";
import { AggregateStatistics } from "../statementsTable";
Expand All @@ -55,8 +56,9 @@ export const ConnectedStatementsPage = withRouter(
lastReset: selectLastReset(state),
columns: selectColumns(state),
nodeRegions: selectIsTenant(state) ? {} : nodeRegionsByIDSelector(state),
isTenant: selectIsTenant(state),
dateRange: selectDateRange(state),
sortSetting: selectSortSetting(state),
isTenant: selectIsTenant(state),
}),
(dispatch: Dispatch) => ({
refreshStatements: (req?: StatementsRequest) =>
Expand Down Expand Up @@ -115,15 +117,26 @@ export const ConnectedStatementsPage = withRouter(
value,
}),
),
onSortingChange: (tableName: string, columnName: string) =>
onSortingChange: (
tableName: string,
columnName: string,
ascending: boolean,
) => {
dispatch(
analyticsActions.track({
name: "Column Sorted",
page: "Statements",
tableName,
columnName,
}),
),
);
dispatch(
localStorageActions.update({
key: "sortSetting/StatementsPage",
value: { columnTitle: columnName, ascending: ascending },
}),
);
},
onStatementClick: () =>
dispatch(
analyticsActions.track({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,17 @@ type StatementsDateRangeState = {
end: number;
};

type SortSetting = {
ascending: boolean;
columnTitle: string;
};

export type LocalStorageState = {
"adminUi/showDiagnosticsModal": boolean;
"showColumns/StatementsPage": string;
"showColumns/TransactionPage": string;
"dateRange/StatementsPage": StatementsDateRangeState;
"sortSetting/StatementsPage": SortSetting;
};

type Payload = {
Expand All @@ -37,6 +43,11 @@ const defaultDateRange: StatementsDateRangeState = {
end: moment.utc().unix() + 60, // Add 1 minute to account for potential lag.
};

const defaultSortSetting: SortSetting = {
ascending: false,
columnTitle: "executionCount",
};

// TODO (koorosh): initial state should be restored from preserved keys in LocalStorage
const initialState: LocalStorageState = {
"adminUi/showDiagnosticsModal":
Expand All @@ -49,6 +60,9 @@ const initialState: LocalStorageState = {
"dateRange/StatementsPage":
JSON.parse(localStorage.getItem("dateRange/StatementsPage")) ||
defaultDateRange,
"sortSetting/StatementsPage":
JSON.parse(localStorage.getItem("sortSetting/StatementsPage")) ||
defaultSortSetting,
};

const localStorageSlice = createSlice({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,10 @@ const statementsPagePropsFixture: StatementsPageProps = {
"3": "gcp-us-west1",
"4": "gcp-europe-west1",
},
sortSetting: {
ascending: false,
columnTitle: "executionCount",
},
columns: null,
match: {
path: "/statements",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {
trackDownloadDiagnosticsBundleAction,
trackStatementsPaginationAction,
trackStatementsSearchAction,
trackTableSortAction,
} from "src/redux/analyticsActions";
import { resetSQLStatsAction } from "src/redux/sqlStats";
import { LocalSetting } from "src/redux/localsettings";
Expand Down Expand Up @@ -235,18 +234,25 @@ export const statementColumnsLocalSetting = new LocalSetting(
null,
);

export const sortSettingLocalSetting = new LocalSetting(
"sortSetting/StatementsPage",
(state: AdminUIState) => state.localSettings,
{ ascending: false, columnTitle: "executionCount" },
);

export default withRouter(
connect(
(state: AdminUIState, props: RouteComponentProps) => ({
statements: selectStatements(state, props),
statementsError: state.cachedData.statements.lastError,
dateRange: selectDateRange(state),
apps: selectApps(state),
databases: selectDatabases(state),
totalFingerprints: selectTotalFingerprints(state),
lastReset: selectLastReset(state),
columns: statementColumnsLocalSetting.selectorToArray(state),
nodeRegions: nodeRegionsByIDSelector(state),
dateRange: selectDateRange(state),
sortSetting: sortSettingLocalSetting.selector(state),
}),
{
refreshStatements: refreshStatements,
Expand All @@ -260,7 +266,15 @@ export default withRouter(
onSearchComplete: (results: AggregateStatistics[]) =>
trackStatementsSearchAction(results.length),
onPageChanged: trackStatementsPaginationAction,
onSortingChange: trackTableSortAction,
onSortingChange: (
_tableName: string,
columnName: string,
ascending: boolean,
) =>
sortSettingLocalSetting.set({
ascending: ascending,
columnTitle: columnName,
}),
onDiagnosticsReportDownload: (report: IStatementDiagnosticsReport) =>
trackDownloadDiagnosticsBundleAction(report.statement_fingerprint),
// We use `null` when the value was never set and it will show all columns.
Expand Down

0 comments on commit e3e8917

Please sign in to comment.