Skip to content

Commit

Permalink
Custom Segmenters: Miscellaneous UI Bugfixes (#18)
Browse files Browse the repository at this point in the history
* Do not parse segmenter values in experiments before posting

* Cater to segmenter type in lowercase

Co-authored-by: Krithika Sundararajan <krithika.sundararajan@go-jek.com>
  • Loading branch information
krithika369 and Krithika Sundararajan authored Jul 15, 2022
1 parent 5956ff6 commit 9e810d7
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 37 deletions.
18 changes: 4 additions & 14 deletions ui/src/experiments/components/form/CreateExperimentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { GeneralStep } from "./steps/GeneralStep";
import { SegmentStep } from "./steps/SegmentStep";
import { TreatmentsStep } from "./steps/TreatmentsStep";
import schema from "./validation/schema";
import { parseSegmenterValue } from "services/experiment/Segment";

export const CreateExperimentForm = ({ projectId, onCancel, onSuccess }) => {
const validationSchema = useMemo(() => schema, []);
Expand All @@ -20,14 +19,14 @@ export const CreateExperimentForm = ({ projectId, onCancel, onSuccess }) => {
const { segmenterConfig, getSegmenterOptions } = useContext(SegmenterContext);

// retrieve name-type (in caps) mappings for active segmenters specified for this project
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function (
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function(
map,
obj
) {
map[obj.name] = obj.type.toUpperCase();
map[obj.name] = obj.type;
return map;
},
{});
{});

const requiredSegmenterNames = useMemo(
() =>
Expand All @@ -46,16 +45,7 @@ export const CreateExperimentForm = ({ projectId, onCancel, onSuccess }) => {
{},
false
);
const onSubmit = () => {
for (const key of Object.keys(experiment.segment)) {
experiment.segment[key] = experiment.segment[key].map(
(segmenterValue) => {
return parseSegmenterValue(segmenterValue, segmenterTypes[key]);
}
);
}
return submitForm({ body: experiment.stringify() }).promise;
};
const onSubmit = () => submitForm({ body: experiment.stringify() }).promise;

useEffect(() => {
if (submissionResponse.isLoaded && !submissionResponse.error) {
Expand Down
12 changes: 3 additions & 9 deletions ui/src/experiments/components/form/EditExperimentForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { ConfigSectionTitle } from "components/config_section/ConfigSectionTitle
import { useXpApi } from "hooks/useXpApi";
import SegmenterContext from "providers/segmenters/context";
import SettingsContext from "providers/settings/context";
import { parseSegmenterValue } from "services/experiment/Segment";

import { GeneralStep } from "./steps/GeneralStep";
import { SegmentStep } from "./steps/SegmentStep";
Expand All @@ -21,14 +20,14 @@ export const EditExperimentForm = ({ projectId, onCancel, onSuccess }) => {
const { segmenterConfig, getSegmenterOptions } = useContext(SegmenterContext);

// retrieve name-type (in caps) mappings for active segmenters specified for this project
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function (
const segmenterTypes = getSegmenterOptions(segmenterConfig).reduce(function(
map,
obj
) {
map[obj.name] = obj.type.toUpperCase();
map[obj.name] = obj.type;
return map;
},
{});
{});

const requiredSegmenterNames = useMemo(
() =>
Expand All @@ -53,11 +52,6 @@ export const EditExperimentForm = ({ projectId, onCancel, onSuccess }) => {
if (!settings.segmenters.names.includes(key)) {
delete experiment.segment[key];
}
experiment.segment[key] = experiment.segment[key].map(
(segmenterValue) => {
return parseSegmenterValue(segmenterValue, segmenterTypes[key]);
}
);
}
return submitForm({ body: experiment.stringify() }).promise;
};
Expand Down
18 changes: 9 additions & 9 deletions ui/src/experiments/components/form/validation/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const experimentTierValues = experimentTiers.map((e) => e.value);
// arrow format, for access to `this` from the invocation context:
// https://stackoverflow.com/a/33308151

const validateABTreatmentTraffic = function (items) {
const validateABTreatmentTraffic = function(items) {
const sum = items.reduce(
(total, e) => total + (!!e.traffic ? e.traffic : 0),
0
Expand All @@ -45,7 +45,7 @@ const validateABTreatmentTraffic = function (items) {
return !!errors.length ? new yup.ValidationError(errors) : true;
};

const validateSwitchbackTreatmentTraffic = function (items) {
const validateSwitchbackTreatmentTraffic = function(items) {
const sum = items.reduce(
(total, e) => total + (!!e.traffic ? e.traffic : 0),
0
Expand Down Expand Up @@ -74,7 +74,7 @@ const validateSwitchbackTreatmentTraffic = function (items) {
return !!errors.length ? new yup.ValidationError(errors) : true;
};

const validateTreatmentNames = function (items) {
const validateTreatmentNames = function(items) {
const uniqueNamesMap = items.reduce((acc, item) => {
const current = item.name in acc ? acc[item.name] : 0;
// If name is set, increment the count
Expand Down Expand Up @@ -143,10 +143,10 @@ const schema = [
interval: yup.mixed().when("type", (type, schema) => {
return type === "Switchback"
? yup
.number()
.required("Interval is required for Switchback experiments")
.integer("Interval must be an integer")
.min(5, "Interval cannot be lower than 5 minutes")
.number()
.required("Interval is required for Switchback experiments")
.integer("Interval must be an integer")
.min(5, "Interval cannot be lower than 5 minutes")
: schema;
}),
tier: yup
Expand Down Expand Up @@ -190,7 +190,7 @@ const schema = [
}
)
.when("$segmenterTypes", (segmenterTypes) => {
switch (segmenterTypes[key]) {
switch (segmenterTypes[key].toUpperCase()) {
case "BOOL":
return yup.array(
yup
Expand Down Expand Up @@ -219,7 +219,7 @@ const schema = [
.typeError("Array elements must all be of type: STRING")
);
default:
return false;
return yup.array(); // Type is unknown for deactivated segmenters
}
}),
};
Expand Down
4 changes: 2 additions & 2 deletions ui/src/services/experiment/Segment.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
var jsonBig = require(`json-bigint`);

export class Segment {}
export class Segment { }

export const parseSegmenterValue = (value, type) => {
let parsedValue;
switch (type) {
switch (type.toUpperCase()) {
case "BOOL":
parsedValue = value.toLowerCase() === "true";
break;
Expand Down
3 changes: 1 addition & 2 deletions ui/src/services/segmenter/Segmenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export class Segmenter {
const clone = cloneDeep(json);
let obj = merge(new Segmenter(""), clone);

obj.type = obj.type.toLowerCase();
obj.options =
obj?.options != null && Object.keys(obj.options).length !== 0
? JSON.stringify(obj?.options)
Expand Down Expand Up @@ -74,7 +73,7 @@ export const newConstraint = (segmenter) => {
: "[]",
options:
segmenter?.options != null &&
Object.keys(segmenter.options).length !== 0
Object.keys(segmenter.options).length !== 0
? JSON.stringify(segmenter?.options)
: "",
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export const SegmenterGeneralPanel = ({
label={
<FormLabelWithToolTip
label="Multi-Valued"
content="Specify if this segmenter can take on multiple values."
content="If selected, multiple values of the segmenter can be chosen at once, in segment configurations."
/>
}
checked={multiValued}
Expand Down

0 comments on commit 9e810d7

Please sign in to comment.