From e3e8917ec13025c8a4eee76aa32d6f610941670f Mon Sep 17 00:00:00 2001 From: Marylia Gutierrez Date: Wed, 10 Nov 2021 13:26:52 -0500 Subject: [PATCH] ui: save sort on cache for Statement page 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 --- .../statementsPage/statementsPage.fixture.ts | 5 +++ .../statementsPage.selectors.ts | 5 +++ .../statementsPage/statementsPage.spec.tsx | 4 +-- .../src/statementsPage/statementsPage.tsx | 32 +++++++++---------- .../statementsPageConnected.tsx | 19 +++++++++-- .../localStorage/localStorage.reducer.ts | 14 ++++++++ .../statements/statementsPage.fixture.ts | 4 +++ .../src/views/statements/statementsPage.tsx | 20 ++++++++++-- 8 files changed, 78 insertions(+), 25 deletions(-) diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts index b54f4673c249..15bd566bae09 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts @@ -267,6 +267,10 @@ const statementsPagePropsFixture: StatementsPageProps = { "3": "gcp-us-west1", "4": "gcp-europe-west1", }, + sortSetting: { + ascending: false, + columnTitle: "executionCount" + }, statements: [ { label: @@ -738,6 +742,7 @@ const statementsPagePropsFixture: StatementsPageProps = { onSearchComplete: noop, onDiagnosticsReportDownload: noop, onColumnsChange: noop, + onSortingChange: noop, }; export const statementsPagePropsWithRequestError: StatementsPageProps = { diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts index 76cdb5a663bf..201cfbe01a1d 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts @@ -240,3 +240,8 @@ export const selectDateRange = createSelector( Moment, ], ); + +export const selectSortSetting = createSelector( + localStorageSelector, + localStorage => localStorage["sortSetting/StatementsPage"], +); diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.spec.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.spec.tsx index 6254c894dcec..e70548843e98 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.spec.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.spec.tsx @@ -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); }); }); }); diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx index 8ae71c882289..302d1b503a16 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx @@ -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; @@ -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, @@ -168,24 +163,26 @@ export class StatementsPage extends React.Component< getStateFromHistory = (): Partial => { 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(), @@ -422,6 +419,7 @@ export class StatementsPage extends React.Component< onColumnsChange, nodeRegions, isTenant, + sortSetting, } = this.props; const appAttrValue = queryByName(location, appAttr); const selectedApp = appAttrValue || ""; @@ -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={ ({ refreshStatements: (req?: StatementsRequest) => @@ -115,7 +117,11 @@ export const ConnectedStatementsPage = withRouter( value, }), ), - onSortingChange: (tableName: string, columnName: string) => + onSortingChange: ( + tableName: string, + columnName: string, + ascending: boolean, + ) => { dispatch( analyticsActions.track({ name: "Column Sorted", @@ -123,7 +129,14 @@ export const ConnectedStatementsPage = withRouter( tableName, columnName, }), - ), + ); + dispatch( + localStorageActions.update({ + key: "sortSetting/StatementsPage", + value: { columnTitle: columnName, ascending: ascending }, + }), + ); + }, onStatementClick: () => dispatch( analyticsActions.track({ diff --git a/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts b/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts index 27cde4cb3f9d..e28dbc27fda8 100644 --- a/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts +++ b/pkg/ui/workspaces/cluster-ui/src/store/localStorage/localStorage.reducer.ts @@ -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 = { @@ -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": @@ -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({ diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.fixture.ts b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.fixture.ts index c88157015361..dddcb5a9e106 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.fixture.ts +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.fixture.ts @@ -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", diff --git a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx index 5b678ca6bdfb..8ee854f89d31 100644 --- a/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx +++ b/pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx @@ -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"; @@ -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, @@ -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.