Skip to content

Commit

Permalink
Miscellaneous UI bugfixes and refactoring (#19)
Browse files Browse the repository at this point in the history
* Handle empty segmenter type

* Refactor code

* Fix bug whereby ListExperiments returns custom segmenters with identical timestamps

Co-authored-by: Krithika Sundararajan <krithika.sundararajan@go-jek.com>
Co-authored-by: ewezy <ziyi.ewe@gojek.com>
  • Loading branch information
3 people authored Jul 19, 2022
1 parent 7755258 commit 8aa97d0
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 130 deletions.
6 changes: 4 additions & 2 deletions management-service/services/segmenter_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,10 @@ func (svc *segmenterService) ListSegmenters(
}
// UpdatedAt and CreatedAt fields are manually updated for custom segmenters but not global segmenters since
// global segmenters do not contain these fields
formattedCustomSegmenter.UpdatedAt = &customSegmenter.UpdatedAt
formattedCustomSegmenter.CreatedAt = &customSegmenter.CreatedAt
customSegmenterUpdatedAt := customSegmenter.UpdatedAt
customSegmenterCreatedAt := customSegmenter.CreatedAt
formattedCustomSegmenter.UpdatedAt = &customSegmenterUpdatedAt
formattedCustomSegmenter.CreatedAt = &customSegmenterCreatedAt
if params.Status == nil || string(*params.Status) == string(*formattedCustomSegmenter.Status) {
allSegmenters = append(allSegmenters, formattedCustomSegmenter)
}
Expand Down
2 changes: 1 addition & 1 deletion ui/src/experiments/components/form/CreateExperimentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const CreateExperimentForm = ({ projectId, onCancel, onSuccess }) => {
const { data: experiment } = useContext(FormContext);
const { segmenterConfig, getSegmenterOptions } = useContext(SegmenterContext);

// retrieve name-type (in caps) mappings for active segmenters specified for this project
// retrieve name-type mappings for active segmenters specified for this project
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function(
map,
obj
Expand Down
2 changes: 1 addition & 1 deletion ui/src/experiments/components/form/EditExperimentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const EditExperimentForm = ({ projectId, onCancel, onSuccess }) => {
const { data: experiment } = useContext(FormContext);
const { segmenterConfig, getSegmenterOptions } = useContext(SegmenterContext);

// retrieve name-type (in caps) mappings for active segmenters specified for this project
// retrieve name-type mappings for active segmenters specified for this project
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function(
map,
obj
Expand Down
59 changes: 2 additions & 57 deletions ui/src/experiments/components/form/validation/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
experimentTiers,
experimentTypes,
} from "experiments/components/typeOptions";
import { segmentConfigSchema } from "segments/components/form/validation/schema";

const nameRegex = /^[A-Za-z\d][\w\d \-()#$%&:.]*[\w\d\-()#$%&:.]$/;
const nameRegexDescription =
Expand Down Expand Up @@ -169,63 +170,7 @@ const schema = [
}),
}),
yup.object().shape({
segment: yup.lazy((obj) =>
yup.object().shape(
Object.keys(obj).reduce((acc, key) => {
return {
...acc,
[key]: yup
.array()
.when(
"$requiredSegmenterNames",
(requiredSegmenterNames, schema) => {
if (requiredSegmenterNames.includes(key)) {
return schema
.required(`Segmenter ${key} is required`)
.min(
1,
`Segmenter ${key} should at least have 1 valid value`
);
}
}
)
.when("$segmenterTypes", (segmenterTypes) => {
switch (segmenterTypes[key].toUpperCase()) {
case "BOOL":
return yup.array(
yup
.bool()
.typeError("Array elements must all be of type: BOOL")
);
case "INTEGER":
return yup.array(
yup
.number()
.integer()
.typeError(
"Array elements must all be of type: INTEGER"
)
);
case "REAL":
return yup.array(
yup
.number()
.typeError("Array elements must all be of type: REAL")
);
case "STRING":
return yup.array(
yup
.string()
.typeError("Array elements must all be of type: STRING")
);
default:
return yup.array(); // Type is unknown for deactivated segmenters
}
}),
};
}, {})
)
),
segment: segmentConfigSchema,
}),
yup.object().shape({
treatments: yup
Expand Down
14 changes: 12 additions & 2 deletions ui/src/segments/components/form/CreateSegmentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,17 @@ import schema from "./validation/schema";
export const CreateSegmentForm = ({ projectId, onCancel, onSuccess }) => {
const validationSchema = useMemo(() => schema, []);
const { data: segment } = useContext(FormContext);
const { segmenterConfig } = useContext(SegmenterContext);
const { segmenterConfig, getSegmenterOptions } = useContext(SegmenterContext);

// retrieve name-type mappings for active segmenters specified for this project
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function(
map,
obj
) {
map[obj.name] = obj.type;
return map;
},
{});
const requiredSegmenterNames = useMemo(
() =>
segmenterConfig
Expand Down Expand Up @@ -50,7 +60,7 @@ export const CreateSegmentForm = ({ projectId, onCancel, onSuccess }) => {
iconType: "package",
children: <ConfigurationStep projectId={projectId} isEdit={false} />,
validationSchema: validationSchema[0],
validationContext: { requiredSegmenterNames },
validationContext: { requiredSegmenterNames, segmenterTypes },
},
];

Expand Down
14 changes: 12 additions & 2 deletions ui/src/segments/components/form/EditSegmentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,17 @@ import schema from "./validation/schema";
export const EditSegmentForm = ({ projectId, onCancel, onSuccess }) => {
const validationSchema = useMemo(() => schema, []);
const { data: segment } = useContext(FormContext);
const { segmenterConfig } = useContext(SegmenterContext);
const { segmenterConfig, getSegmenterOptions } = useContext(SegmenterContext);

// retrieve name-type mappings for active segmenters specified for this project
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function(
map,
obj
) {
map[obj.name] = obj.type;
return map;
},
{});
const requiredSegmenterNames = useMemo(
() =>
segmenterConfig
Expand Down Expand Up @@ -50,7 +60,7 @@ export const EditSegmentForm = ({ projectId, onCancel, onSuccess }) => {
iconType: "apmTrace",
children: <ConfigurationStep projectId={projectId} isEdit={true} />,
validationSchema: validationSchema[0],
validationContext: { requiredSegmenterNames },
validationContext: { requiredSegmenterNames, segmenterTypes },
},
];

Expand Down
81 changes: 57 additions & 24 deletions ui/src/segments/components/form/validation/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,62 @@
import * as yup from "yup";

const segmentNameRegex = /^[A-Za-z\d][\w\d \-()#$%&:.]*[\w\d\-()#$%&:.]$/;
export const segmentConfigSchema = yup.lazy((obj) =>
yup.object().shape(
Object.keys(obj).reduce((acc, key) => {
return {
...acc,
[key]: yup.mixed()
.when("$segmenterTypes", (segmenterTypes) => {
switch ((segmenterTypes[key] || "").toUpperCase()) {
case "BOOL":
return yup.array(
yup
.bool()
.typeError("Array elements must all be of type: BOOL")
);
case "INTEGER":
return yup.array(
yup
.number()
.integer()
.typeError(
"Array elements must all be of type: INTEGER"
)
);
case "REAL":
return yup.array(
yup
.number()
.typeError("Array elements must all be of type: REAL")
);
case "STRING":
return yup.array(
yup
.string()
.typeError("Array elements must all be of type: STRING")
);
default:
return yup.array(); // Type is unknown for deactivated segmenters
}
})
.when(
"$requiredSegmenterNames",
(requiredSegmenterNames, schema) => {
if (requiredSegmenterNames.includes(key)) {
return schema
.required(`Segmenter ${key} is required`)
.min(
1,
`Segmenter ${key} should at least have 1 valid value`
);
}
return schema;
}
),
};
}, {})
));

const schema = [
yup.object().shape({
Expand All @@ -14,30 +70,7 @@ const schema = [
segmentNameRegex,
"Name must begin with an alphanumeric character and have no trailing spaces and can contain letters, numbers, blank spaces and the following symbols: -_()#$%&:."
),
segment: yup.lazy((obj) =>
yup.object().shape(
Object.keys(obj).reduce((acc, key) => {
return {
...acc,
[key]: yup
.array()
.when(
"$requiredSegmenterNames",
(requiredSegmenterNames, schema) => {
if (requiredSegmenterNames.includes(key)) {
return schema
.required(`Segmenter ${key} is required`)
.min(
1,
`Segmenter ${key} should at least have 1 valid value`
);
}
}
),
};
}, {})
)
),
segment: segmentConfigSchema,
}),
];

Expand Down
13 changes: 13 additions & 0 deletions ui/src/services/segmenter/SegmenterScope.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export const getSegmenterScope = (scope) => {
const status = {
project: {
label: "Project",
color: "primary",
},
global: {
label: "Global",
color: "secondary",
},
};
return status[scope];
};
4 changes: 2 additions & 2 deletions ui/src/services/segmenter/SegmenterStatus.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
export const getSegmenterStatus = (segmenter) => {
const statusMapping = {
inactive: {
label: "inactive",
label: "Inactive",
color: "#6A717D",
healthColor: "subdued",
iconType: "cross",
},
active: {
label: "active",
label: "Active",
color: "#017D73",
healthColor: "success",
iconType: "check",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import isEqual from "lodash/isEqual";
import sortBy from "lodash/sortBy";

import { StatusBadge } from "components/status_badge/StatusBadge";
import { getSegmenterScope } from "services/segmenter/SegmenterScope";

import "./SegmenterCard.scss";

Expand Down Expand Up @@ -62,20 +63,6 @@ const VariablesMappingPanel = ({
);
};

const getScopeBadge = (scope) => {
const status = {
project: {
label: "Project",
color: "primary",
},
global: {
label: "Global",
color: "secondary",
},
};
return status[scope];
};

export const SegmenterCard = ({
id,
name,
Expand All @@ -93,12 +80,12 @@ export const SegmenterCard = ({
const buttonContent = isDragging ? (
<>
<EuiTextColor color="accent">{displayName}</EuiTextColor>
<StatusBadge status={getScopeBadge(scope)} />
<StatusBadge status={getSegmenterScope(scope)} />
</>
) : (
<>
{displayName}
<StatusBadge status={getScopeBadge(scope)} />
<StatusBadge status={getSegmenterScope(scope)} />
</>
);

Expand Down Expand Up @@ -138,7 +125,7 @@ export const SegmenterCard = ({
) : (
<>
{displayName}
<StatusBadge status={getScopeBadge(scope)} />
<StatusBadge status={getSegmenterScope(scope)} />
</>
)}
</EuiPanel>
Expand Down
27 changes: 5 additions & 22 deletions ui/src/settings/segmenters/details/SegmenterDetailsView.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { Redirect, Router } from "@reach/router";
import { PageTitle } from "components/page/PageTitle";
import { StatusBadge } from "components/status_badge/StatusBadge";
import { useXpApi } from "hooks/useXpApi";
import { getSegmenterStatus } from "services/segmenter/SegmenterStatus";
import { SegmentersConfigView } from "settings/segmenters/details/config/SegmentersConfigView";
import { SegmenterActions } from "settings/segmenters/details/SegmenterActions";
import EditSegmenterView from "settings/segmenters/edit/EditSegmenterView";
Expand All @@ -35,24 +36,6 @@ const SegmenterDetailsView = ({ projectId, segmenterName, ...props }) => {
},
] = useXpApi(`/projects/${projectId}/segmenters/${segmenterName}`, {}, []);

const getStatusBadge = (item) => {
const status = {
active: {
label: "Active",
color: "#017D73",
healthColor: "success",
iconType: "check",
},
inactive: {
label: "Inactive",
color: "#6A717D",
healthColor: "subdued",
iconType: "cross",
},
};
return status[item.status];
};

// ../../segmenters is required, with pure .. it will end up with /segmenters/ and the tab routing is bugged
useEffect(() => {
!!segmenter &&
Expand Down Expand Up @@ -86,7 +69,7 @@ const SegmenterDetailsView = ({ projectId, segmenterName, ...props }) => {
icon="package"
title={segmenter.name}
postpend={
<StatusBadge status={getStatusBadge(segmenter)} />
<StatusBadge status={getSegmenterStatus(segmenter)} />
}
/>
</EuiPageHeaderSection>
Expand All @@ -103,9 +86,9 @@ const SegmenterDetailsView = ({ projectId, segmenterName, ...props }) => {
actions={
segmenter.scope === "project"
? getActions({
name: segmenterName,
projectId: projectId,
})
name: segmenterName,
projectId: projectId,
})
: null
}
selectedTab={props["*"]}
Expand Down

0 comments on commit 8aa97d0

Please sign in to comment.