Skip to content

Commit

Permalink
add Statements page redux functionality
Browse files Browse the repository at this point in the history
Redux functionality for Statements page is rewritten with respect to
defined behavior in initial implementation in Admin UI. The main reason
to avoid usage of current implementation following:
- redux store is tightly coupled with other parts of application and
cannot be easily broken down into smaller and isolated slice of redux
store.
- it uses 'redux-thunk' which is obsolete library and 'redux-saga' has to
be used instead.

Redux functionality is defined per "module" (feature) to group it by
functionality so all redux logic related to `StatementsPage` component
is defined near component itself.

Common redux logic is defined in `src/reduxStore` which doesn't
belong to any particular feature. For example, redux state which
represents data stored in LocalStorage is defined as common redux
functionality.

Redux actions, reducer and action types stored in the single file
because they all serve single purpose - to define how state can be
updated. In fact, followed this recommendation: https://redux.js.org/style-guide/style-guide#structure-files-as-feature-folders-or-ducks

Optimize types for `fetchData` function

- Optimized input and return types;
- Get rid of `any` types
- Construction of request payload is moved inside of
`fetchData` function to reduce code duplication.

Refactor redux actions creation

- Follow Flux standard actions conventions
- Add `getActionsMap` to generate strongly typed dictionary
of available types based on a list of available actions.

refactor components integration with CC

Initially, statements page components had predefined styles which
didn't allow to adjust component to different pages and styles
were hard coded to specific needs for console DB ui.
The same way internal routing for statements page expected to
change routes starting from based path and hadn't provide any
way to customize this.

This change introduces some level of flexibility for clients
which integrate Statements page.
  • Loading branch information
koorosh committed Nov 12, 2020
1 parent 84c733a commit fc5ee71
Show file tree
Hide file tree
Showing 53 changed files with 1,405 additions and 83 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
},
"rules": {
"@typescript-eslint/interface-name-prefix": "off",
"@typescript-eslint/camelcase": "warn"
"@typescript-eslint/camelcase": "warn",
"@typescript-eslint/no-explicit-any": "warn"
}
}
24 changes: 14 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"types": "dist/types/index.d.ts",
"scripts": {
"build": "npm-run-all build:typescript build:bundle",
"build:bundle": "webpack --display-error-details",
"build:bundle": "NODE_ENV=production webpack --display-error-details",
"build:typescript": "tsc",
"build:watch": "webpack --watch",
"ci": "jest --ci -i",
Expand All @@ -31,6 +31,9 @@
"@cockroachlabs/ui-components": "^0.2.8",
"@popperjs/core": "^2.4.0",
"babel-polyfill": "^6.26.0",
"d3-array": "^2.8.0",
"d3-format": "^2.0.0",
"d3-scale": "^3.2.3",
"highlight.js": "^10.2.0",
"long": "^4.0.0",
"react-helmet": "^5.2.0",
Expand All @@ -55,12 +58,15 @@
"@types/chai": "^4.2.11",
"@types/classnames": "^2.2.9",
"@types/d3": "<4.0.0",
"@types/d3-array": "^2.2.0",
"@types/d3-format": "^2.0.0",
"@types/d3-scale": "^3.2.0",
"@types/enzyme": "^3.10.5",
"@types/jest": "^25.1.2",
"@types/lodash": "^4.14.149",
"@types/long": "^4.0.1",
"@types/node": "^13.7.0",
"@types/react": "^16.9.17",
"@types/react": "^16.9.48",
"@types/react-helmet": "^5.0.5",
"@types/react-redux": "7.1.6",
"@types/react-router": "^5.1.8",
Expand All @@ -79,7 +85,6 @@
"chai": "^4.2.0",
"classnames": "^2.2.6",
"connected-react-router": "^6.8.0",
"d3": "3.5.17",
"enzyme": "^3.11.0",
"enzyme-adapter-react-16": "^1.15.2",
"eslint": "^6.8.0",
Expand All @@ -102,10 +107,10 @@
"npm-run-all": "^4.1.5",
"prettier": "^1.19.1",
"protobufjs": "6.8.6",
"react": "^16.12.0",
"react-dom": "^16.12.0",
"react": "16.9.0",
"react-dom": "16.9.0",
"react-redux": "7.1.3",
"react-router-dom": "^5.2.0",
"react-router-dom": "^5.1.2",
"react-test-renderer": "^16.13.1",
"redux": "4.0.5",
"redux-saga": "1.1.3",
Expand All @@ -122,17 +127,16 @@
"webpackbar": "^4.0.0"
},
"peerDependencies": {
"connected-react-router": "^6.8.0",
"d3": "3.5.17",
"protobufjs": "6.8.6",
"react": "^16.12.0",
"react-dom": "^16.12.0",
"react": "^16.9.0",
"react-dom": "^16.9.0",
"react-redux": "7.1.3",
"react-router-dom": "^5.1.2",
"redux": "4.0.5",
"redux-saga": "1.1.3"
},
"resolutions": {
"@types/react": "^16.9.48",
"node-fetch": "~2.6.1",
"serialize-javascript": "~3.1.0",
"yargs-parser": "~13.1.2"
Expand Down
42 changes: 34 additions & 8 deletions src/api/fetchData.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,35 @@
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";

interface ResponseBuilder<P, R> {
new (properties?: P): R;
encode(message: P, writer?: protobuf.Writer): protobuf.Writer;
interface ProtoBuilder<
P extends ConstructorType,
Prop = FirstConstructorParameter<P>,
R = InstanceType<P>
> {
new (properties?: Prop): R;
encode(message: Prop, writer?: protobuf.Writer): protobuf.Writer;
decode(reader: protobuf.Reader | Uint8Array, length?: number): R;
}

export const fetchData = <Props, Resp, T extends ResponseBuilder<Props, Resp>>(
builder: T,
export function toArrayBuffer(encodedRequest: Uint8Array): ArrayBuffer {
return encodedRequest.buffer.slice(
encodedRequest.byteOffset,
encodedRequest.byteOffset + encodedRequest.byteLength,
);
}

/**
* @param RespBuilder expects protobuf stub class to build decode response;
* @param path relative URL path for requested resource;
* @param ReqBuilder expects protobuf stub to encode request payload. It has to be
* class type, not instance;
* @param reqPayload is request payload object;
**/
export const fetchData = <P extends ProtoBuilder<P>, T extends ProtoBuilder<T>>(
RespBuilder: T,
path: string,
data?: any,
): Promise<Resp> => {
ReqBuilder?: P,
reqPayload?: FirstConstructorParameter<P>,
): Promise<InstanceType<T>> => {
const params: RequestInit = {
headers: {
Accept: "application/x-protobuf",
Expand All @@ -19,6 +38,13 @@ export const fetchData = <Props, Resp, T extends ResponseBuilder<Props, Resp>>(
},
credentials: "same-origin",
};

if (reqPayload) {
const encodedRequest = ReqBuilder.encode(reqPayload).finish();
params.method = "POST";
params.body = toArrayBuffer(encodedRequest);
}

return fetch(path, params)
.then(response => {
if (!response.ok) {
Expand All @@ -28,7 +54,7 @@ export const fetchData = <Props, Resp, T extends ResponseBuilder<Props, Resp>>(
}
return response.arrayBuffer();
})
.then(buffer => builder.decode(new Uint8Array(buffer)))
.then(buffer => RespBuilder.decode(new Uint8Array(buffer)))
.catch(error => {
throw new cockroach.server.serverpb.ResponseError({ error });
});
Expand Down
8 changes: 4 additions & 4 deletions src/barCharts/barChartFactory.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import d3 from "d3";
import { scaleLinear } from "d3-scale";
import { extent as d3Extent } from "d3-array";
import _ from "lodash";
import React from "react";
import { Tooltip } from "src/index";
Expand Down Expand Up @@ -36,13 +37,12 @@ export function barChartFactory<T>(
const getTotal = (d: T) => _.sum(_.map(accessors, ({ value }) => value(d)));
const getTotalWithStdDev = (d: T) => getTotal(d) + stdDevAccessor.value(d);

const extent = d3.extent(
const extent = d3Extent(
rows,
stdDevAccessor ? getTotalWithStdDev : getTotal,
);

const scale = d3.scale
.linear()
const scale = scaleLinear()
.domain([0, extent[1]])
.range([0, 100]);

Expand Down
2 changes: 1 addition & 1 deletion src/barCharts/barCharts.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
retryBarChart,
rowsBarChart,
} from "./barCharts";
import statementsPagePropsFixture from "src/statementsPage/statementsPage.fixture";
import statementsPagePropsFixture from "src/statementsPage/components/statementsPage.fixture";

const { statements } = statementsPagePropsFixture;

Expand Down
1 change: 1 addition & 0 deletions src/barCharts/barCharts.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import React from "react";
import * as protos from "@cockroachlabs/crdb-protobuf-client";
import { stdDevLong } from "src/util/appStats";
import { Duration } from "src/util/format";
Expand Down
5 changes: 2 additions & 3 deletions src/barCharts/latencyBreakdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from "react";
import * as protos from "@cockroachlabs/crdb-protobuf-client";
import { stdDevLong } from "src/util/appStats";
import { NumericStatLegend } from "./numericStatLegend";
import d3 from "d3";
import { scaleLinear } from "d3-scale";
import { Duration } from "src/util/format";
import { Tooltip } from "src/index";
import classNames from "classnames/bind";
Expand Down Expand Up @@ -34,8 +34,7 @@ export function latencyBreakdown(s: StatementStatistics) {

const format = (v: number) => Duration(v * 1e9);

const scale = d3.scale
.linear()
const scale = scaleLinear()
.domain([0, max])
.range([0, 100]);

Expand Down
5 changes: 2 additions & 3 deletions src/barCharts/rowsBrealdown.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import d3 from "d3";
import { scaleLinear } from "d3-scale";
import { stdDevLong } from "src/util/appStats";
import { formatTwoPlaces } from "./utils";
import * as protos from "@cockroachlabs/crdb-protobuf-client";
Expand All @@ -9,8 +9,7 @@ export function rowsBreakdown(s: StatementStatistics) {
const mean = s.stats.num_rows.mean;
const sd = stdDevLong(s.stats.num_rows, s.stats.count);

const scale = d3.scale
.linear()
const scale = scaleLinear()
.domain([0, mean + sd])
.range([0, 100]);

Expand Down
4 changes: 2 additions & 2 deletions src/barCharts/utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import d3 from "d3";
import { format as d3Format } from "d3-format";
import Long from "long";
import { FixLong } from "src/util/fixLong";
import * as protos from "@cockroachlabs/crdb-protobuf-client";
Expand All @@ -11,7 +11,7 @@ export const longToInt = (d: number | Long) =>

export const clamp = (i: number) => (i < 0 ? 0 : i);

export const formatTwoPlaces = d3.format(".2f");
export const formatTwoPlaces = d3Format(".2f");

export function bar(
name: string,
Expand Down
6 changes: 6 additions & 0 deletions src/declaration.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,9 @@ declare module "*.png";
declare module "*.gif";
declare module "*.scss";
declare module "*.svg";

type ConstructorType = new (...args: any) => any;

type FirstConstructorParameter<
P extends ConstructorType
> = ConstructorParameters<P>[0];
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import "antd/dist/antd.less";
import "./protobufInit";
import * as util from "./util";
export * from "./anchor";
export * from "./badge";
Expand Down
2 changes: 1 addition & 1 deletion src/protobufInit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as protobuf from "protobufjs/minimal";
import Long from "long";

protobuf.util.Long = Long as any;
protobuf.util.Long = Long;
protobuf.configure();
29 changes: 29 additions & 0 deletions src/statementsDiagnostics/api/statementsDiagnosticsApi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { cockroach } from "@cockroachlabs/crdb-protobuf-client";
import { fetchData } from "src/api";

const STATEMENT_DIAGNOSTICS_PATH = "_status/stmtdiagreports";
const CREATE_STATEMENT_DIAGNOSTICS_REPORT_PATH = "_status/stmtdiagreports";

type CreateStatementDiagnosticsReportResponseMessage = cockroach.server.serverpb.CreateStatementDiagnosticsReportResponse;

export function getStatementDiagnosticsReports(): Promise<
cockroach.server.serverpb.StatementDiagnosticsReportsResponse
> {
return fetchData(
cockroach.server.serverpb.StatementDiagnosticsReportsResponse,
STATEMENT_DIAGNOSTICS_PATH,
);
}

export function createStatementDiagnosticsReport(
statementsFingerprint: string,
): Promise<CreateStatementDiagnosticsReportResponseMessage> {
return fetchData(
cockroach.server.serverpb.CreateStatementDiagnosticsReportResponse,
CREATE_STATEMENT_DIAGNOSTICS_REPORT_PATH,
cockroach.server.serverpb.CreateStatementDiagnosticsReportRequest,
{
statement_fingerprint: statementsFingerprint,
},
);
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import React, { useState, useCallback, useImperativeHandle } from "react";

import { Modal } from "../modal";
import { Anchor } from "../anchor";
import { Text } from "../text";
import { Modal } from "../../modal";
import { Anchor } from "../../anchor";
import { Text } from "../../text";
import { statementDiagnostics } from "src/util";

export interface ActivateDiagnosticsModalProps {
Expand Down
7 changes: 4 additions & 3 deletions src/statementsDiagnostics/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./activateStatementDiagnosticsModal";
export * from "./diagnosticStatusBadge";
export * from "./diagnosticStatuses";
export * from "./components/activateStatementDiagnosticsModal";
export * from "./components/diagnosticStatusBadge";
export * from "./components/diagnosticStatuses";
export * from "./store";
3 changes: 3 additions & 0 deletions src/statementsDiagnostics/store/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from "./statementDiagnosticsReports.selectors";
export * from "./statementDiagnosticsReports.reducer";
export * from "./statementDiagnosticsReports.sagas";
Loading

0 comments on commit fc5ee71

Please sign in to comment.