Skip to content

Commit

Permalink
Merge #51910
Browse files Browse the repository at this point in the history
51910: ui: use StatementsPage from admin-ui-components r=koorosh a=koorosh

Depends on cockroachdb/admin-ui-components#9
Depends on cockroachdb/yarn-vendored#27
Depends on cockroachdb/yarn-vendored#28

`StatementsPage` component is replaced with previously extracted version of StatementsPage in `admin-ui-components`.
The new version (extracted) of `StatementsPage` has several changes that required to make the following adjustments:
- `ActivateStatementDiagnostics` modal isn't connected components anymore so dispatching actions are pulled up to connected StatementsPage wrapper.
- Previously, analytics functions (which track user interaction) were
called directly from React components that made this functionality tightly coupled.
Now, sagas used to watch actions related to user interaction and then call async logic in sagas.
-- components expose only callback functions which are related to the
the logic component is responsible;
-- connected component is responsible to dispatch actions with proper
payload to be properly then handled by sagas.




Co-authored-by: Andrii Vorobiov <and.vorobiov@gmail.com>
  • Loading branch information
craig[bot] and koorosh committed Aug 5, 2020
2 parents 37c10c4 + b816b22 commit 8f7a596
Show file tree
Hide file tree
Showing 16 changed files with 222 additions and 391 deletions.
1 change: 1 addition & 0 deletions pkg/ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"cypress:update-snapshots": "yarn cypress run --env updateSnapshots=true --spec 'cypress/integration/**/*.visual.spec.ts'"
},
"dependencies": {
"@cockroachlabs/admin-ui-components": "^0.1.9",
"analytics-node": "^3.4.0-beta.1",
"antd": "^3.25.2",
"babel-polyfill": "^6.26.0",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ui/src/app.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
} from "src/views/databases/containers/databases";
import { TableMain } from "src/views/databases/containers/tableDetails";
import { DataDistributionPage } from "src/views/cluster/containers/dataDistribution";
import { StatementsPage } from "src/views/statements/statementsPage";
import { StatementsPage } from "@cockroachlabs/admin-ui-components";
import { StatementDetails } from "src/views/statements/statementDetails";
import Debug from "src/views/reports/containers/debug";
import { ReduxDebug } from "src/views/reports/containers/redux";
Expand Down
46 changes: 46 additions & 0 deletions pkg/ui/src/redux/analyticsActions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import { PayloadAction } from "src/interfaces/action";

export const TRACK_STATEMENTS_SEARCH = "cockroachui/analytics/TRACK_STATEMENTS_SEARCH";
export const TRACK_STATEMENTS_PAGINATION = "cockroachui/analytics/TRACK_STATEMENTS_PAGINATION";
export const TRACK_TABLE_SORT = "cockroachui/analytics/TRACK_TABLE_SORT";

export interface TableSortActionPayload {
tableName: string;
columnName: string;
ascending?: boolean;
}

export function trackStatementsSearchAction(searchResults: number): PayloadAction<number> {
return {
type: TRACK_STATEMENTS_SEARCH,
payload: searchResults,
};
}

export function trackStatementsPaginationAction(pageNum: number): PayloadAction<number> {
return {
type: TRACK_STATEMENTS_PAGINATION,
payload: pageNum,
};
}

export function trackTableSortAction(tableName: string, columnName: string, ascending?: boolean): PayloadAction<TableSortActionPayload> {
return {
type: TRACK_TABLE_SORT,
payload: {
tableName,
columnName,
ascending,
},
};
}
63 changes: 63 additions & 0 deletions pkg/ui/src/redux/analyticsSagas.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import { all, call, takeEvery } from "redux-saga/effects";
import { PayloadAction } from "src/interfaces/action";
import {
CREATE_STATEMENT_DIAGNOSTICS_REPORT,
DiagnosticsReportPayload,
OPEN_STATEMENT_DIAGNOSTICS_MODAL,
} from "src/redux/statements";
import {
trackActivateDiagnostics,
trackDiagnosticsModalOpen,
trackPaginate,
trackSearch,
trackTableSort,
} from "src/util/analytics";
import {
TRACK_STATEMENTS_SEARCH,
TRACK_STATEMENTS_PAGINATION,
TRACK_TABLE_SORT,
TableSortActionPayload,
} from "./analyticsActions";

export function* trackActivateStatementsDiagnostics(action: PayloadAction<DiagnosticsReportPayload>) {
const { statementFingerprint } = action.payload;
yield call(trackActivateDiagnostics, statementFingerprint);
}

export function* trackOpenStatementsDiagnostics(action: PayloadAction<DiagnosticsReportPayload>) {
const { statementFingerprint } = action.payload;
yield call(trackDiagnosticsModalOpen, statementFingerprint);
}

export function* trackStatementsSearch(action: PayloadAction<number>) {
yield call(trackSearch, action.payload);
}

export function* trackStatementsPagination(action: PayloadAction<number>) {
yield call(trackPaginate, action.payload);
}

export function* trackTableSortChange(action: PayloadAction<TableSortActionPayload>) {
const { tableName, columnName, ascending } = action.payload;
yield call(trackTableSort, tableName, columnName, ascending);
}

export function* analyticsSaga() {
yield all([
takeEvery(CREATE_STATEMENT_DIAGNOSTICS_REPORT, trackActivateStatementsDiagnostics),
takeEvery(OPEN_STATEMENT_DIAGNOSTICS_MODAL, trackOpenStatementsDiagnostics),
takeEvery(TRACK_STATEMENTS_SEARCH, trackStatementsSearch),
takeEvery(TRACK_STATEMENTS_PAGINATION, trackStatementsPagination),
takeEvery(TRACK_TABLE_SORT, trackTableSortChange),
]);
}
2 changes: 2 additions & 0 deletions pkg/ui/src/redux/sagas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ import { queryMetricsSaga } from "./metrics";
import { localSettingsSaga } from "./localsettings";
import { customAnalyticsSaga } from "./customAnalytics";
import { statementsSaga } from "./statements";
import { analyticsSaga } from "./analyticsSagas";

export default function* rootSaga() {
yield all([
fork(queryMetricsSaga),
fork(localSettingsSaga),
fork(customAnalyticsSaga),
fork(statementsSaga),
fork(analyticsSaga),
]);
}
10 changes: 10 additions & 0 deletions pkg/ui/src/redux/statements/statementsActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { PayloadAction } from "src/interfaces/action";
export const CREATE_STATEMENT_DIAGNOSTICS_REPORT = "cockroachui/statements/CREATE_STATEMENT_DIAGNOSTICS_REPORT";
export const CREATE_STATEMENT_DIAGNOSTICS_COMPLETE = "cockroachui/statements/CREATE_STATEMENT_DIAGNOSTICS_COMPLETE";
export const CREATE_STATEMENT_DIAGNOSTICS_FAILED = "cockroachui/statements/CREATE_STATEMENT_DIAGNOSTICS_FAILED";
export const OPEN_STATEMENT_DIAGNOSTICS_MODAL = "cockroachui/statements/OPEN_STATEMENT_DIAGNOSTICS_MODAL";

export type DiagnosticsReportPayload = {
statementFingerprint: string;
Expand All @@ -39,3 +40,12 @@ export function createStatementDiagnosticsReportFailedAction(): Action {
type: CREATE_STATEMENT_DIAGNOSTICS_FAILED,
};
}

export function createOpenDiagnosticsModalAction(statementFingerprint: string): PayloadAction<DiagnosticsReportPayload> {
return {
type: OPEN_STATEMENT_DIAGNOSTICS_MODAL,
payload: {
statementFingerprint,
},
};
}
7 changes: 3 additions & 4 deletions pkg/ui/src/util/analytics/trackTableSort.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,9 @@ describe("trackTableSort", () => {
it("should send the correct payload", () => {
const spy = sandbox.spy();
const tableName = "Test table";
const column = { title: "test", cell: (x: number) => x };
const sortSetting = { sortKey: "whatever", ascending: true };
const title = "test";

track(spy)(tableName, column, sortSetting);
track(spy)(tableName, title, "asc");

const sent = spy.getCall(0).args[0];
const table = get(sent, "properties.tableName");
Expand All @@ -56,7 +55,7 @@ describe("trackTableSort", () => {
assert.isTrue(table === tableName, "Table name matches given table name");

assert.isTrue(isString(columnName), "Column name is a string");
assert.isTrue(column.title === columnName, "Column name matches given column name");
assert.isTrue(title === columnName, "Column name matches given column name");

assert.isTrue(isString(direction), "Sort direction is a string");
assert.isTrue(
Expand Down
20 changes: 8 additions & 12 deletions pkg/ui/src/util/analytics/trackTableSort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,12 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.
import { analytics } from "src/redux/analytics";
import { SortableColumn, SortSetting } from "src/views/shared/components/sortabletable";

export const track = (fn: Function) => (
name?: String,
col?: SortableColumn,
sortSetting?: SortSetting,
tableName = "",
columnName = "",
sortDirection: "asc" | "desc" = undefined,
) => {
const tableName = name || "";
const columnName = col && col.title || "";
const sortDirection = sortSetting && (sortSetting.ascending) ? "asc" : "desc";

fn({
event: "Table Sort",
properties: {
Expand All @@ -30,10 +25,11 @@ export const track = (fn: Function) => (
};

export default function trackTableSort(
name?: String,
col?: SortableColumn,
sortSetting?: SortSetting,
tableName?: string,
columnTitle?: string,
ascending?: boolean,
) {
const boundTrack = analytics.track.bind(analytics);
track(boundTrack)(name, col, sortSetting);
const sortDirection = ascending && ascending ? "asc" : "desc";
track(boundTrack)(tableName, columnTitle, sortDirection);
}
5 changes: 4 additions & 1 deletion pkg/ui/src/views/shared/components/sortabletable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,10 @@ export class SortableTable extends React.Component<TableProps> {
if (!isUndefined(c.sortKey)) {
classes.push(cx("sort-table__cell--sortable"));
onClick = () => {
trackTableSort(className, c, sortSetting);
// TODO (koorosh): `title` field has ReactNode type isn't correct field to
// track column name. `SortableColumn` has to be imported from `@cockroachlabs/admin-ui-components`
// package which has extended field to track column name.
trackTableSort(className, c.title.toString(), sortSetting.ascending);
this.clickSort(c.sortKey);
};
if (c.sortKey === sortSetting.sortKey) {
Expand Down
14 changes: 9 additions & 5 deletions pkg/ui/src/views/statements/statements.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import Long from "long";
import moment from "moment";
import { RouteComponentProps } from "react-router-dom";
import * as H from "history";
import { merge } from "lodash";

import "src/protobufInit";
import * as protos from "src/js/protos";
Expand All @@ -21,6 +22,7 @@ import { appAttr, statementAttr } from "src/util/constants";
import { selectStatements, selectApps, selectTotalFingerprints, selectLastReset } from "./statementsPage";
import { selectStatement } from "./statementDetails";
import ISensitiveInfo = protos.cockroach.sql.ISensitiveInfo;
import { AdminUIState, createAdminUIStore } from "src/redux/state";

const INTERNAL_STATEMENT_PREFIX = "$ internal";

Expand Down Expand Up @@ -462,8 +464,9 @@ function makeEmptySensitiveInfo(): ISensitiveInfo {
};
}

function makeInvalidState() {
return {
function makeInvalidState(): AdminUIState {
const store = createAdminUIStore(H.createMemoryHistory());
return merge(store.getState(), {
cachedData: {
statements: {
inFlight: true,
Expand All @@ -474,11 +477,12 @@ function makeInvalidState() {
valid: false,
},
},
};
});
}

function makeStateWithStatementsAndLastReset(statements: CollectedStatementStatistics[], lastReset: number) {
return {
const store = createAdminUIStore(H.createMemoryHistory());
return merge(store.getState(), {
cachedData: {
statements: {
data: protos.cockroach.server.serverpb.StatementsResponse.fromObject({
Expand All @@ -497,7 +501,7 @@ function makeStateWithStatementsAndLastReset(statements: CollectedStatementStati
valid: false,
},
},
};
});
}

function makeStateWithStatements(statements: CollectedStatementStatistics[]) {
Expand Down
5 changes: 4 additions & 1 deletion pkg/ui/src/views/statements/statementsPage.fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

import { StatementsPageProps } from "./statementsPage";
import { StatementsPageProps } from "@cockroachlabs/admin-ui-components";
import { createMemoryHistory } from "history";
import Long from "long";
import * as protos from "src/js/protos";
Expand Down Expand Up @@ -392,6 +392,9 @@ const statementsPagePropsFixture: StatementsPageProps = {
dismissAlertMessage: () => {},
refreshStatementDiagnosticsRequests: (() => {}) as (typeof refreshStatementDiagnosticsRequests),
refreshStatements: (() => {}) as (typeof refreshStatements),
onActivateStatementDiagnostics: _ => {},
onDiagnosticsModalOpen: _ => {},
onPageChanged: _ => {},
};

export default statementsPagePropsFixture;
34 changes: 0 additions & 34 deletions pkg/ui/src/views/statements/statementsPage.spec.tsx

This file was deleted.

23 changes: 0 additions & 23 deletions pkg/ui/src/views/statements/statementsPage.stories.tsx

This file was deleted.

Loading

0 comments on commit 8f7a596

Please sign in to comment.