From 8378383e660cd741b7eb49ac7b1de344c1f733da Mon Sep 17 00:00:00 2001 From: "ningjie.lee" Date: Fri, 28 Oct 2022 17:59:20 +0800 Subject: [PATCH 1/3] add protocol for prediction_context display --- .../form/EditExperimentEngineConfig.js | 5 +- .../form/EditStandardEnsemblerConfig.js | 29 +++-- .../LinkedExperimentsContextMenu.js | 56 +++++---- .../standard_ensembler/LinkedRoutesTable.js | 111 ++++++++++++------ .../standard_ensembler/RouteNamePathFlyout.js | 42 +++---- .../standard_ensembler/RouteNamePathRow.js | 19 ++- .../variables_config/VariableConfigRow.js | 8 +- .../variables_config/VariablesConfigPanel.js | 6 +- .../components/FieldSourceFormLabel.js | 45 ++++--- 9 files changed, 202 insertions(+), 119 deletions(-) diff --git a/ui/src/turing/components/form/EditExperimentEngineConfig.js b/ui/src/turing/components/form/EditExperimentEngineConfig.js index f13afc03..c06bcdb2 100644 --- a/ui/src/turing/components/form/EditExperimentEngineConfig.js +++ b/ui/src/turing/components/form/EditExperimentEngineConfig.js @@ -14,6 +14,7 @@ import { VariablesConfigPanel } from "./variables_config/VariablesConfigPanel"; const EditExperimentEngineConfigComponent = ({ projectId, + protocol, config = {}, onChangeHandler, errors = {}, @@ -41,6 +42,7 @@ const EditExperimentEngineConfigComponent = ({ @@ -50,7 +52,8 @@ const EditExperimentEngineConfigComponent = ({ + iconType="alert" + >

{ "Please complete onboarding to Turing experiments to configure the router." diff --git a/ui/src/turing/components/form/EditStandardEnsemblerConfig.js b/ui/src/turing/components/form/EditStandardEnsemblerConfig.js index 721eb9be..d82ccc65 100644 --- a/ui/src/turing/components/form/EditStandardEnsemblerConfig.js +++ b/ui/src/turing/components/form/EditStandardEnsemblerConfig.js @@ -1,12 +1,20 @@ import React, { useContext, useRef } from "react"; -import { EuiCallOut, EuiFlexItem, EuiLoadingChart, EuiHorizontalRule } from "@elastic/eui"; +import { + EuiCallOut, + EuiFlexItem, + EuiHorizontalRule, + EuiLoadingChart, +} from "@elastic/eui"; import { OverlayMask } from "@gojek/mlp-ui"; import { Panel } from "components/panel/Panel"; import { ConfigProvider, useConfig } from "config"; -import ProjectContext, { ProjectContextProvider } from "providers/project/context"; import { ExperimentContextProvider } from "providers/experiment/context"; +import ProjectContext, { + ProjectContextProvider, +} from "providers/project/context"; + import { LinkedRoutesTable } from "./standard_ensembler/LinkedRoutesTable"; import { RouteNamePathRow } from "./standard_ensembler/RouteNamePathRow"; @@ -17,7 +25,9 @@ const EditStandardEnsemblerConfigComponent = ({ onChange, errors, }) => { - const { appConfig: { routeNamePathPrefix } } = useConfig(); + const { + appConfig: { routeNamePathPrefix }, + } = useConfig(); const { isProjectOnboarded, isLoaded } = useContext(ProjectContext); const overlayRef = useRef(); @@ -40,7 +50,9 @@ const EditStandardEnsemblerConfigComponent = ({ @@ -49,9 +61,12 @@ const EditStandardEnsemblerConfigComponent = ({ + iconType={"alert"} + >

- {"Please complete onboarding to Turing experiments to configure the standard ensembler."} + { + "Please complete onboarding to Turing experiments to configure the standard ensembler." + }

@@ -74,7 +89,7 @@ const EditStandardEnsemblerConfig = (props) => { - ) + ); }; export default EditStandardEnsemblerConfig; diff --git a/ui/src/turing/components/form/standard_ensembler/LinkedExperimentsContextMenu.js b/ui/src/turing/components/form/standard_ensembler/LinkedExperimentsContextMenu.js index 130b6a16..069d0f70 100644 --- a/ui/src/turing/components/form/standard_ensembler/LinkedExperimentsContextMenu.js +++ b/ui/src/turing/components/form/standard_ensembler/LinkedExperimentsContextMenu.js @@ -1,18 +1,18 @@ import React from "react"; import { - EuiContextMenu, - EuiPopover, EuiButtonEmpty, + EuiContextMenu, + EuiIcon, EuiLink, - EuiIcon + EuiPopover, } from "@elastic/eui"; import { useToggle } from "@gojek/mlp-ui"; export const LinkedExperimentsContextMenu = ({ projectId, linkedExperiments, - experimentStatus + experimentStatus, }) => { const [isPopoverOpen, togglePopover] = useToggle(); @@ -41,28 +41,32 @@ export const LinkedExperimentsContextMenu = ({ > ( - { - name: ( - - {e.name} - - ), - icon: , - size: "s", - toolTipContent: "Open experiment details page", - toolTipPosition: "right", - } - )) - } - ] - } + panels={[ + { + id: 0, + title: `Linked ${ + experimentStatus[0].toUpperCase() + experimentStatus.slice(1) + } Experiments`, + items: Object.values(linkedExperiments[experimentStatus]).map( + (e) => ({ + name: ( + + {e.name} + + ), + icon: , + size: "s", + toolTipContent: "Open experiment details page", + toolTipPosition: "right", + }) + ), + }, + ]} /> ); -}; \ No newline at end of file +}; diff --git a/ui/src/turing/components/form/standard_ensembler/LinkedRoutesTable.js b/ui/src/turing/components/form/standard_ensembler/LinkedRoutesTable.js index d81b4ce3..3edff65d 100644 --- a/ui/src/turing/components/form/standard_ensembler/LinkedRoutesTable.js +++ b/ui/src/turing/components/form/standard_ensembler/LinkedRoutesTable.js @@ -2,56 +2,83 @@ import React, { useContext, useEffect, useState } from "react"; import { EuiFlexItem, + EuiIcon, + EuiInMemoryTable, EuiLoadingChart, EuiTextAlign, - EuiInMemoryTable, - EuiIcon, EuiTextColor, } from "@elastic/eui"; +import ExperimentContext from "providers/experiment/context"; import { getExperimentStatus } from "services/experiment/ExperimentStatus"; + import { LinkedExperimentsContextMenu } from "./LinkedExperimentsContextMenu"; -import ExperimentContext from "providers/experiment/context"; export const LinkedRoutesTable = ({ projectId, routes, treatmentConfigRouteNamePath, }) => { - const { allExperiments, isAllExperimentsLoaded } = useContext(ExperimentContext) + const { allExperiments, isAllExperimentsLoaded } = + useContext(ExperimentContext); - const [routeToExperimentMappings, setRouteToExperimentMappings] = useState(routes.reduce((m, r) => {m[r.id] = {running: {}, scheduled: {}}; return m}, {})); + const [routeToExperimentMappings, setRouteToExperimentMappings] = useState( + routes.reduce((m, r) => { + m[r.id] = { running: {}, scheduled: {} }; + return m; + }, {}) + ); - const getRouteName = (config, path) => path.split('.').reduce((obj, key) => obj && obj[key], config); + const getRouteName = (config, path) => + path.split(".").reduce((obj, key) => obj && obj[key], config); // this stringified value of routes below allows the React effect below to mimic a deep comparison when changes to the // array routes are made - const stringifiedRoutes = routes.map(e => e.id).join(); + const stringifiedRoutes = routes.map((e) => e.id).join(); // reset loaded routeToExperimentMappings if treatmentConfigRouteNamePath or routes changes useEffect(() => { if (isAllExperimentsLoaded) { - let newRouteToExperimentMappings = routes.reduce((m, r) => {m[r.id] = {running: {}, scheduled: {}}; return m}, {}); + let newRouteToExperimentMappings = routes.reduce((m, r) => { + m[r.id] = { running: {}, scheduled: {} }; + return m; + }, {}); for (let experiment of allExperiments) { for (let treatment of experiment.treatments) { - let configRouteName = getRouteName(treatment.configuration, treatmentConfigRouteNamePath); - if (typeof configRouteName === 'string' && configRouteName in newRouteToExperimentMappings) { - newRouteToExperimentMappings[configRouteName][getExperimentStatus(experiment).label.toLowerCase()][experiment.id] = experiment; + let configRouteName = getRouteName( + treatment.configuration, + treatmentConfigRouteNamePath + ); + if ( + typeof configRouteName === "string" && + configRouteName in newRouteToExperimentMappings + ) { + newRouteToExperimentMappings[configRouteName][ + getExperimentStatus(experiment).label.toLowerCase() + ][experiment.id] = experiment; } } } setRouteToExperimentMappings(newRouteToExperimentMappings); } - }, [treatmentConfigRouteNamePath, stringifiedRoutes, routes, isAllExperimentsLoaded, allExperiments]); + }, [ + treatmentConfigRouteNamePath, + stringifiedRoutes, + routes, + isAllExperimentsLoaded, + allExperiments, + ]); const columns = [ { field: "id", width: "5px", render: (id) => { - const isAssigned = routeToExperimentMappings[id] ? - Object.keys(routeToExperimentMappings[id].running).length + - Object.keys(routeToExperimentMappings[id].scheduled).length > 0 : false; + const isAssigned = routeToExperimentMappings[id] + ? Object.keys(routeToExperimentMappings[id].running).length + + Object.keys(routeToExperimentMappings[id].scheduled).length > + 0 + : false; return ( { - const isAssigned = routeToExperimentMappings[id] ? - Object.keys(routeToExperimentMappings[id].running).length + - Object.keys(routeToExperimentMappings[id].scheduled).length > 0 : false; - return ({id}); + const isAssigned = routeToExperimentMappings[id] + ? Object.keys(routeToExperimentMappings[id].running).length + + Object.keys(routeToExperimentMappings[id].scheduled).length > + 0 + : false; + return ( + + {id} + + ); }, }, { field: "id", width: "35%", name: "Running Experiments", - render: (id) => routeToExperimentMappings[id] && ( - - ), + render: (id) => + routeToExperimentMappings[id] && ( + + ), }, { field: "id", width: "35%", name: "Scheduled Experiments", - render: (id) => routeToExperimentMappings[id] && ( - - ) + render: (id) => + routeToExperimentMappings[id] && ( + + ), }, ]; @@ -108,9 +143,9 @@ export const LinkedRoutesTable = ({ isSelectable={false} /> - ) : ( - - - - ); + ) : ( + + + + ); }; diff --git a/ui/src/turing/components/form/standard_ensembler/RouteNamePathFlyout.js b/ui/src/turing/components/form/standard_ensembler/RouteNamePathFlyout.js index 340ebabd..bdc36b88 100644 --- a/ui/src/turing/components/form/standard_ensembler/RouteNamePathFlyout.js +++ b/ui/src/turing/components/form/standard_ensembler/RouteNamePathFlyout.js @@ -1,27 +1,25 @@ import React from "react"; import { + EuiCode, + EuiCodeBlock, EuiFlexItem, EuiFlyout, EuiFlyoutBody, EuiFlyoutHeader, EuiText, EuiTitle, - EuiCode, - EuiCodeBlock } from "@elastic/eui"; -import {useConfig} from "config"; + +import { useConfig } from "config"; const RouteNamePathFlyout = ({ onClose }) => { - const { appConfig: { routeNamePathPrefix } } = useConfig(); + const { + appConfig: { routeNamePathPrefix }, + } = useConfig(); return ( - + @@ -34,28 +32,30 @@ const RouteNamePathFlyout = ({ onClose }) => {

- The prefix in the grayed-out area specifies the path prefix that gets appended to a user-defined - treatment configuration. + The prefix in the grayed-out area specifies the path prefix that + gets appended to a user-defined treatment configuration.

- This path prefix reflects the nesting of the treatment configuration within - the final response payload that the Turing Router finally receives. + This path prefix reflects the nesting of the treatment + configuration within the final response payload that the Turing + Router finally receives.

- In Turing Experiments' case, if the user-defined treatment configuration is: + In Turing Experiments' case, if the user-defined treatment + configuration is:

{`{ "route_name": "control", ... -}` - } +}`}

- the client response that gets sent back to the Turing Router is actually as follows: + the client response that gets sent back to the Turing Router is + actually as follows:

{`{ @@ -67,13 +67,13 @@ const RouteNamePathFlyout = ({ onClose }) => { .... } ... -}` - } +}`}

Hence, the path prefix is automatically specified as - {routeNamePathPrefix} (including the final period). + {routeNamePathPrefix}{" "} + (including the final period).

diff --git a/ui/src/turing/components/form/standard_ensembler/RouteNamePathRow.js b/ui/src/turing/components/form/standard_ensembler/RouteNamePathRow.js index 145412b5..653b7650 100644 --- a/ui/src/turing/components/form/standard_ensembler/RouteNamePathRow.js +++ b/ui/src/turing/components/form/standard_ensembler/RouteNamePathRow.js @@ -1,7 +1,14 @@ import React, { Fragment } from "react"; -import { EuiFlexItem, EuiText, EuiFormRow, EuiFieldText, EuiButtonIcon } from "@elastic/eui"; +import { + EuiButtonIcon, + EuiFieldText, + EuiFlexItem, + EuiFormRow, + EuiText, +} from "@elastic/eui"; import { FormLabelWithToolTip, useToggle } from "@gojek/mlp-ui"; + import RouteNamePathFlyout from "./RouteNamePathFlyout"; export const RouteNamePathRow = ({ @@ -25,7 +32,8 @@ export const RouteNamePathRow = ({ } isInvalid={!!errors} error={errors} - display="row"> + display="row" + > , - {routeNamePathPrefix} + , + {routeNamePathPrefix}, ]} /> diff --git a/ui/src/turing/components/form/variables_config/VariableConfigRow.js b/ui/src/turing/components/form/variables_config/VariableConfigRow.js index a5904942..31b3ec0a 100644 --- a/ui/src/turing/components/form/variables_config/VariableConfigRow.js +++ b/ui/src/turing/components/form/variables_config/VariableConfigRow.js @@ -17,6 +17,7 @@ export const VariableConfigRow = ({ name, field, fieldSrc, + protocol, onChangeHandler, error = {}, }) => { @@ -35,12 +36,14 @@ export const VariableConfigRow = ({ direction="row" gutterSize="m" alignItems="center" - className="euiFlexGroup--experimentVariableRow"> + className="euiFlexGroup--experimentVariableRow" + > + error={error.field || ""} + > onChangeFieldSource(e)} /> diff --git a/ui/src/turing/components/form/variables_config/VariablesConfigPanel.js b/ui/src/turing/components/form/variables_config/VariablesConfigPanel.js index f4425a4e..699a75d7 100644 --- a/ui/src/turing/components/form/variables_config/VariablesConfigPanel.js +++ b/ui/src/turing/components/form/variables_config/VariablesConfigPanel.js @@ -10,6 +10,7 @@ import { VariableConfigRow } from "./VariableConfigRow"; export const VariablesConfigPanel = ({ variables = [], + protocol, onChangeHandler, errors = {}, }) => { @@ -46,8 +47,8 @@ export const VariablesConfigPanel = ({ size="m" content="Specify how the experiment variables may be parsed from the request." /> - }> - + } + > @@ -55,6 +56,7 @@ export const VariablesConfigPanel = ({ { +export const FieldSourceFormLabel = ({ + value, + protocol, + onChange, + readOnly, +}) => { const [isOpen, togglePopover] = useToggle(); + const fieldSourceOptions = [ + { + value: "none", + inputDisplay: "None", + }, + { + value: "header", + inputDisplay: "Header", + }, + { + value: "payload", + // Display is change to Prediction Context to be consistent with Turing Traffic rule + // backend value stays the same as payload, because XP is not supporting gRPC + inputDisplay: protocol === "UPI_V1" ? "Prediction Context" : "Payload", + }, + ]; + const panels = flattenPanelTree({ id: 0, items: fieldSourceOptions.map((option) => ({ @@ -57,13 +64,15 @@ export const FieldSourceFormLabel = ({ value, onChange, readOnly }) => { iconType="arrowDown" iconSide="right" className="fieldSourceLabel" - onClick={togglePopover}> + onClick={togglePopover} + > {selectedOption.inputDisplay} } isOpen={isOpen} closePopover={togglePopover} - panelPaddingSize="s"> + panelPaddingSize="s" + > Date: Wed, 2 Nov 2022 17:47:47 +0800 Subject: [PATCH 2/3] update labels for detail view --- .../ExperimentEngineConfigDetails.js | 10 +++-- .../variables_config/VariablesConfigGroup.js | 9 +++- .../components/FieldSourceFormLabel.js | 44 ++++++++++++------- ui/src/turing/components/utils/helper.js | 11 +++++ 4 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 ui/src/turing/components/utils/helper.js diff --git a/ui/src/turing/components/configuration/ExperimentEngineConfigDetails.js b/ui/src/turing/components/configuration/ExperimentEngineConfigDetails.js index 457cf9f0..2293c623 100644 --- a/ui/src/turing/components/configuration/ExperimentEngineConfigDetails.js +++ b/ui/src/turing/components/configuration/ExperimentEngineConfigDetails.js @@ -8,7 +8,7 @@ import { ConfigProvider } from "config"; import { ExperimentsConfigGroup } from "./experiments_config/ExperimentsConfigGroup"; import { VariablesConfigGroup } from "./variables_config/VariablesConfigGroup"; -const ExperimentEngineConfigDetails = ({ projectId, config }) => ( +const ExperimentEngineConfigDetails = ({ projectId, protocol, config }) => ( @@ -20,8 +20,12 @@ const ExperimentEngineConfigDetails = ({ projectId, config }) => ( - + className="experimentVariablesPanel" + > + diff --git a/ui/src/turing/components/configuration/variables_config/VariablesConfigGroup.js b/ui/src/turing/components/configuration/variables_config/VariablesConfigGroup.js index e2820792..89731e7e 100644 --- a/ui/src/turing/components/configuration/variables_config/VariablesConfigGroup.js +++ b/ui/src/turing/components/configuration/variables_config/VariablesConfigGroup.js @@ -1,8 +1,10 @@ import React from "react"; -import { EuiBasicTable, EuiTextColor, EuiTitle } from "@elastic/eui"; +import { EuiBasicTable, EuiText, EuiTextColor, EuiTitle } from "@elastic/eui"; -export const VariablesConfigGroup = ({ variables }) => { +import { mapProtocolLabel } from "turing/components/utils/helper"; + +export const VariablesConfigGroup = ({ variables, protocol }) => { const columns = [ { field: "name", @@ -28,6 +30,9 @@ export const VariablesConfigGroup = ({ variables }) => { field: "field_source", name: "Source", width: "30%", + render: (value) => ( + {mapProtocolLabel(protocol, value)} + ), }, ]; diff --git a/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js b/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js index d68e0b6f..7f9990dc 100644 --- a/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js +++ b/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js @@ -8,6 +8,11 @@ import { } from "@elastic/eui"; import { flattenPanelTree, useToggle } from "@gojek/mlp-ui"; +import { + capitalizeFirstLetter, + mapProtocolLabel, +} from "turing/components/utils/helper"; + import "./FieldSourceFormLabel.scss"; export const FieldSourceFormLabel = ({ @@ -18,22 +23,27 @@ export const FieldSourceFormLabel = ({ }) => { const [isOpen, togglePopover] = useToggle(); - const fieldSourceOptions = [ - { - value: "none", - inputDisplay: "None", - }, - { - value: "header", - inputDisplay: "Header", - }, - { - value: "payload", - // Display is change to Prediction Context to be consistent with Turing Traffic rule - // backend value stays the same as payload, because XP is not supporting gRPC - inputDisplay: protocol === "UPI_V1" ? "Prediction Context" : "Payload", - }, - ]; + const fieldSourceOptions = useMemo( + () => [ + { + value: "none", + inputDisplay: "None", + }, + { + value: "header", + inputDisplay: "Header", + }, + { + value: "payload", + // Display is change to Prediction Context to be consistent with Turing Traffic rule + // backend value stays the same as payload, because XP is not supporting gRPC + inputDisplay: capitalizeFirstLetter( + mapProtocolLabel(protocol, "payload") + ), + }, + ], + [protocol] + ); const panels = flattenPanelTree({ id: 0, @@ -49,7 +59,7 @@ export const FieldSourceFormLabel = ({ const selectedOption = useMemo( () => fieldSourceOptions.find((o) => o.value === value), - [value] + [value, fieldSourceOptions] ); return readOnly ? ( diff --git a/ui/src/turing/components/utils/helper.js b/ui/src/turing/components/utils/helper.js new file mode 100644 index 00000000..ad52b500 --- /dev/null +++ b/ui/src/turing/components/utils/helper.js @@ -0,0 +1,11 @@ +export const mapProtocolLabel = (protocol, value) => { + if (protocol === "UPI_V1" && value === "payload") { + return "prediction context"; + } + + return value; +}; + +export const capitalizeFirstLetter = (string) => { + return string.replace(/(^\w|\s\w)/g, (m) => m.toUpperCase()); +}; From 369f33e202eea095c293ab583aba4e0fa320ee22 Mon Sep 17 00:00:00 2001 From: "ningjie.lee" Date: Thu, 3 Nov 2022 18:51:38 +0800 Subject: [PATCH 3/3] remove custom func and use lodash --- .../components/FieldSourceFormLabel.js | 10 +++------- ui/src/turing/components/utils/helper.js | 4 ---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js b/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js index 7f9990dc..709c002c 100644 --- a/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js +++ b/ui/src/turing/components/form/variables_config/components/FieldSourceFormLabel.js @@ -7,11 +7,9 @@ import { EuiPopover, } from "@elastic/eui"; import { flattenPanelTree, useToggle } from "@gojek/mlp-ui"; +import startCase from "lodash/startCase"; -import { - capitalizeFirstLetter, - mapProtocolLabel, -} from "turing/components/utils/helper"; +import { mapProtocolLabel } from "turing/components/utils/helper"; import "./FieldSourceFormLabel.scss"; @@ -37,9 +35,7 @@ export const FieldSourceFormLabel = ({ value: "payload", // Display is change to Prediction Context to be consistent with Turing Traffic rule // backend value stays the same as payload, because XP is not supporting gRPC - inputDisplay: capitalizeFirstLetter( - mapProtocolLabel(protocol, "payload") - ), + inputDisplay: startCase(mapProtocolLabel(protocol, "payload")), }, ], [protocol] diff --git a/ui/src/turing/components/utils/helper.js b/ui/src/turing/components/utils/helper.js index ad52b500..874b33fb 100644 --- a/ui/src/turing/components/utils/helper.js +++ b/ui/src/turing/components/utils/helper.js @@ -5,7 +5,3 @@ export const mapProtocolLabel = (protocol, value) => { return value; }; - -export const capitalizeFirstLetter = (string) => { - return string.replace(/(^\w|\s\w)/g, (m) => m.toUpperCase()); -};