Skip to content

Commit

Permalink
ui: move latestQuery, latestFormattedQuery from redux to local state
Browse files Browse the repository at this point in the history
Previously, the fields `latestQuery` and `latestFormattedQuery`
representing the latest non-empty query string for a statement viewed
from the detaisl page was being stored in redux. The purpose of these
fields was to preserve the query when changing tabs in the stmt
details page. Saving this in the redux store was unnecessary and so
this commit moves these fields to the stmt details local state.

Release note: None
  • Loading branch information
xinhaoz committed Sep 26, 2022
1 parent d390148 commit 27299e0
Show file tree
Hide file tree
Showing 9 changed files with 44 additions and 156 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,6 @@ export const getStatementDetailsPropsFixture = (
key: "Custom",
},
statementFingerprintID: "4705782015019656142",
latestQuery: "SELECT * FROM crdb_internal.node_build_info",
latestFormattedQuery: "SELECT * FROM crdb_internal.node_build_info\n",
statementDetails: withData ? statementDetailsData : statementDetailsNoData,
statementsError: null,
nodeNames: {
Expand All @@ -809,8 +807,6 @@ export const getStatementDetailsPropsFixture = (
dismissStatementDiagnosticsAlertMessage: noop,
onTimeScaleChange: noop,
createStatementDiagnosticsReport: noop,
onStatementDetailsQueryChange: noop,
onStatementDetailsFormattedQueryChange: noop,
uiConfig: {
showStatementDiagnosticsLink: true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,5 @@ storiesOf("StatementDetails", module)
)
.add("No data for this time frame; no cached statement", () => {
const props = getStatementDetailsPropsFixture(false);
props.latestQuery = "";
props.latestFormattedQuery = "";
return <StatementDetails {...props} />;
});
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,20 @@ export type StatementDetailsProps = StatementDetailsOwnProps &

export interface StatementDetailsState {
currentTab?: string;

/**
* The latest non-null query text associated with the statement fingerprint in the URL.
* We save this to preserve this data when the time frame changes such that there is no
* longer data for this statement fingerprint in the selected time frame.
*/
query: string;

/**
* The latest non-null formatted query associated with the statement fingerprint in the URL.
* We save this to preserve data when the time frame changes such that there is no longer
* data for this statement fingerprint in the selected time frame.
*/
formattedQuery: string;
}

export type NodesSummary = {
Expand Down Expand Up @@ -127,16 +141,12 @@ export interface StatementDetailsDispatchProps {
ascending: boolean,
) => void;
onBackToStatementsClick?: () => void;
onStatementDetailsQueryChange: (query: string) => void;
onStatementDetailsFormattedQueryChange: (formattedQuery: string) => void;
}

export interface StatementDetailsStateProps {
statementFingerprintID: string;
statementDetails: StatementDetailsResponse;
isLoading: boolean;
latestQuery: string;
latestFormattedQuery: string;
statementsError: Error | null;
timeScale: TimeScale;
nodeNames: { [nodeId: string]: string };
Expand Down Expand Up @@ -207,11 +217,15 @@ export class StatementDetails extends React.Component<
StatementDetailsState
> {
activateDiagnosticsRef: React.RefObject<ActivateDiagnosticsModalRef>;

constructor(props: StatementDetailsProps) {
super(props);
const searchParams = new URLSearchParams(props.history.location.search);
this.state = {
currentTab: searchParams.get("tab") || "overview",
query: this.props.statementDetails?.statement?.metadata?.query,
formattedQuery:
this.props.statementDetails?.statement?.metadata?.formatted_query,
};
this.activateDiagnosticsRef = React.createRef();

Expand Down Expand Up @@ -286,39 +300,30 @@ export class StatementDetails extends React.Component<
}
}

// If new, non-empty-string query text is available (derived from the statement details response),
// cache the query text.
// If a new, non-empty-string query text is available
// (derived from the statement details response), save the query text.
const newQuery =
this.props.statementDetails?.statement?.metadata?.query ||
this.state.query;
const newFormattedQuery =
this.props.statementDetails?.statement?.metadata?.formatted_query ||
this.state.formattedQuery;
if (
this.props.statementDetails &&
this.props.statementDetails.statement.metadata.query != "" &&
this.props.latestQuery !=
this.props.statementDetails.statement.metadata.query
newQuery !== this.state.query ||
newFormattedQuery !== this.state.formattedQuery
) {
this.props.onStatementDetailsQueryChange(
this.props.statementDetails.statement.metadata.query,
);
}

// If a new, non-empty-string formatted query text is available (derived from the statement details response),
// cache the query text.
if (
this.props.statementDetails &&
this.props.statementDetails.statement.metadata.formatted_query != "" &&
this.props.latestFormattedQuery !=
this.props.statementDetails.statement.metadata.formatted_query
) {
this.props.onStatementDetailsFormattedQueryChange(
this.props.statementDetails.statement.metadata.formatted_query,
);
this.setState({
query: newQuery,
formattedQuery: newFormattedQuery,
});
}

// If the statementFingerprintID (derived from the URL) changes, invalidate the cached query texts.
// This is necessary for when the new statementFingerprintID does not have data for the given time frame.
// The new query text and the formatted query text would be an empty string, and we need to invalidate the old
// query text and formatted query text.
if (this.props.statementFingerprintID != prevProps.statementFingerprintID) {
this.props.onStatementDetailsQueryChange("");
this.props.onStatementDetailsFormattedQueryChange("");
this.setState({ query: null, formattedQuery: null });
}
}

Expand Down Expand Up @@ -466,11 +471,11 @@ export class StatementDetails extends React.Component<
</PageConfigItem>
</PageConfig>
<section className={cx("section")}>
{this.props.latestFormattedQuery && (
{this.state.formattedQuery && (
<Row gutter={24}>
<Col className="gutter-row" span={24}>
<SqlBox
value={this.props.latestFormattedQuery}
value={this.state.formattedQuery}
size={SqlBoxSize.custom}
/>
</Col>
Expand Down Expand Up @@ -614,7 +619,7 @@ export class StatementDetails extends React.Component<
<Row gutter={24}>
<Col className="gutter-row" span={24}>
<SqlBox
value={this.props.latestFormattedQuery}
value={this.state.formattedQuery}
size={SqlBoxSize.custom}
/>
</Col>
Expand Down Expand Up @@ -764,7 +769,7 @@ export class StatementDetails extends React.Component<
};

renderDiagnosticsTabContent = (hasData: boolean): React.ReactElement => {
if (!hasData && !this.props.latestQuery) {
if (!hasData && !this.state.query) {
return this.renderNoDataTabContent();
}

Expand All @@ -774,7 +779,9 @@ export class StatementDetails extends React.Component<
diagnosticsReports={this.props.diagnosticsReports}
dismissAlertMessage={this.props.dismissStatementDiagnosticsAlertMessage}
hasData={this.hasDiagnosticReports()}
statementFingerprint={this.props.latestQuery}
statementFingerprint={
this.props.statementDetails?.statement?.metadata?.query
}
onDownloadDiagnosticBundleClick={this.props.onDiagnosticBundleDownload}
onDiagnosticCancelRequestClick={this.props.onDiagnosticCancelRequest}
showDiagnosticsViewLink={
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,11 @@ const mapStateToProps = (state: AppState, props: RouteComponentProps) => {
state,
props,
);
const statementFingerprint = statementDetails?.statement.metadata.query;
return {
statementFingerprintID: getMatchParamByName(props.match, statementAttr),
statementDetails,
isLoading: isLoading,
latestQuery: state.adminUI.sqlDetailsStats.latestQuery,
latestFormattedQuery: state.adminUI.sqlDetailsStats.latestFormattedQuery,
statementsError: lastError,
timeScale: selectTimeScale(state),
nodeNames: selectIsTenant(state) ? {} : nodeDisplayNameByIDSelector(state),
Expand All @@ -76,7 +75,7 @@ const mapStateToProps = (state: AppState, props: RouteComponentProps) => {
? []
: selectDiagnosticsReportsByStatementFingerprint(
state,
state.adminUI.sqlDetailsStats.latestQuery,
statementFingerprint,
),
uiConfig: selectStatementDetailsUiConfig(state),
isTenant: selectIsTenant(state),
Expand Down Expand Up @@ -162,14 +161,6 @@ const mapDispatchToProps = (
}),
);
},
onStatementDetailsQueryChange: (latestQuery: string) => {
dispatch(sqlDetailsStatsActions.setLatestQuery(latestQuery));
},
onStatementDetailsFormattedQueryChange: (latestFormattedQuery: string) => {
dispatch(
sqlDetailsStatsActions.setLatestFormattedQuery(latestFormattedQuery),
);
},
onSortingChange: (tableName, columnName) =>
dispatch(
analyticsActions.track({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,10 @@ export type SQLDetailsStatsReducerState = {
cachedData: {
[id: string]: SQLDetailsStatsState;
};
latestQuery: string;
latestFormattedQuery: string;
};

const initialState: SQLDetailsStatsReducerState = {
cachedData: {},
latestQuery: "",
latestFormattedQuery: "",
};

const sqlDetailsStatsSlice = createSlice({
Expand Down Expand Up @@ -103,12 +99,6 @@ const sqlDetailsStatsSlice = createSlice({
inFlight: true,
};
},
setLatestQuery: (state, action: PayloadAction<string>) => {
state.latestQuery = action.payload;
},
setLatestFormattedQuery: (state, action: PayloadAction<string>) => {
state.latestFormattedQuery = action.payload;
},
},
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,8 +667,6 @@ describe("SQLDetailsStats sagas", () => {
inFlight: false,
},
},
latestQuery: "",
latestFormattedQuery: "",
})
.run();
});
Expand All @@ -694,8 +692,6 @@ describe("SQLDetailsStats sagas", () => {
inFlight: false,
},
},
latestQuery: "",
latestFormattedQuery: "",
})
.run();
});
Expand Down
78 changes: 0 additions & 78 deletions pkg/ui/workspaces/db-console/src/redux/sqlActivity.ts

This file was deleted.

3 changes: 0 additions & 3 deletions pkg/ui/workspaces/db-console/src/redux/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { apiReducersReducer, APIReducersState } from "./apiReducers";
import { hoverReducer, HoverState } from "./hover";
import { localSettingsReducer, LocalSettingsState } from "./localsettings";
import { metricsReducer, MetricsState } from "./metrics";
import { sqlActivityReducer, SqlActivityState } from "src/redux/sqlActivity";
import { queryManagerReducer, QueryManagerState } from "./queryManager/reducer";
import { timeScaleReducer, TimeScaleState } from "./timeScale";
import { uiDataReducer, UIDataState } from "./uiData";
Expand All @@ -44,7 +43,6 @@ export interface AdminUIState {
hover: HoverState;
localSettings: LocalSettingsState;
metrics: MetricsState;
sqlActivity: SqlActivityState;

queryManager: QueryManagerState;
router: RouterState;
Expand All @@ -65,7 +63,6 @@ export function createAdminUIStore(historyInst: History<any>) {
hover: hoverReducer,
localSettings: localSettingsReducer,
metrics: metricsReducer,
sqlActivity: sqlActivityReducer,

queryManager: queryManagerReducer,
router: routerReducer,
Expand Down
Loading

0 comments on commit 27299e0

Please sign in to comment.