Skip to content

Commit 1c83ad3

Browse files
fix(ui): Fix yup validation bugs (caraml-dev#387)
* Fix incorrect client id error being displayed * Update yup validation schemas * Update mlp * Undo unnecessary line changes * Temporarily remove codecov checks for api server
1 parent dd0b448 commit 1c83ad3

File tree

7 files changed

+55
-70
lines changed

7 files changed

+55
-70
lines changed

.github/workflows/turing.yaml

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,14 +263,6 @@ jobs:
263263
working-directory: api
264264
args: --timeout 3m --verbose
265265

266-
- uses: codecov/codecov-action@v4
267-
with:
268-
flags: api-test
269-
name: api-test
270-
token: ${{ secrets.CODECOV_TOKEN }}
271-
working-directory: api
272-
codecov_yml_path: ../.github/workflows/codecov-config/codecov.yml
273-
274266
test-engines-router:
275267
runs-on: ubuntu-latest
276268
defaults:

ui/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"//": "[@sentry/browser] pinned to 7.118.0 because craco/module federation has issues resolving this dependency; see",
88
"//": "https://github.com/caraml-dev/turing/pull/384#discussion_r1666418144 for more details",
99
"dependencies": {
10-
"@caraml-dev/ui-lib": "^1.13.0-build.3-a234b6b",
10+
"@caraml-dev/ui-lib": "^1.13.0-build.4-09c363a",
1111
"@elastic/datemath": "^5.0.3",
1212
"@elastic/eui": "88.2.0",
1313
"@emotion/css": "^11.11.2",

ui/src/components/validation.js

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,25 @@
11
import * as yup from "yup";
22

3-
const httpHeaderSchema = yup
3+
const httpHeaderSchema = (_) => yup
44
.string()
55
.required('Valid Request Header value is required, e.g. "x-session-id"');
66

7-
const jsonPathSchema = yup
7+
const jsonPathSchema = (_) => yup
88
.string()
9-
.required(
10-
'Valid Request Payload json path is required, e.g. "my_object.session_id"'
11-
);
9+
.required('Valid Request Payload json path is required, e.g. "my_object.session_id"');
1210

13-
const predictionContextSchema = yup
14-
.string()
15-
.required(
16-
'Valid Prediction Context value is required, e.g. "model_name"'
17-
);
11+
const predictionContextSchema = (_) => yup
12+
.string()
13+
.required('Valid Prediction Context value is required, e.g. "model_name"');
1814

19-
export const fieldSourceSchema = yup
20-
.mixed()
15+
export const fieldSourceSchema = (_) => yup
16+
.string()
2117
.required("Valid Field Source should be selected")
2218
.oneOf(["header", "payload", "prediction_context"], "Valid Field Source should be selected");
2319

24-
export const fieldSchema = (fieldSource) =>
20+
export const fieldSchema = (fieldSource) => (_) =>
2521
yup
26-
.mixed()
22+
.string()
2723
.when(fieldSource, {
2824
is: "header",
2925
then: httpHeaderSchema,

ui/src/router/components/form/components/experiment_config/components/client_config/ClientConfigPanel.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ export const ClientConfigPanel = ({ client, onChangeHandler, errors = {} }) => {
7474
content="Select your Client ID from the provided list"
7575
/>
7676
}
77-
isInvalid={!!errors.id}
78-
error={errors.id}
77+
isInvalid={!!errors.username}
78+
error={errors.username}
7979
display="row">
8080
<EuiComboBoxSelect
8181
fullWidth

ui/src/router/components/form/components/experiment_config/validation/schema.js

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,21 +15,17 @@ const experimentSchema = yup.object().shape({
1515

1616
const variableSchema = yup.object().shape({
1717
required: yup.boolean(),
18-
field_source: yup.mixed().when("required", {
18+
field_source: yup.string().when("required", {
1919
is: true,
20-
then: fieldSourceSchema.required(
21-
"Select the field source for the required variable"
22-
),
20+
then: fieldSourceSchema,
2321
}),
24-
field: yup.mixed().when("required", {
22+
field: yup.string().when("required", {
2523
is: true,
26-
then: fieldSchema("field_source").required(
27-
"Specify the field name for the required variable"
28-
),
24+
then: fieldSchema("field_source"),
2925
}),
3026
});
3127

32-
const standardExperimentConfigSchema = (engineProps) =>
28+
const standardExperimentConfigSchema = (engineProps) => (_) =>
3329
yup.object().shape({
3430
client: engineProps?.standard_experiment_manager_config
3531
?.client_selection_enabled

ui/src/router/components/form/validation/schema.js

Lines changed: 33 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -130,25 +130,25 @@ const routeSchema = yup.object().shape({
130130
});
131131

132132
const validRouteSchema = yup
133-
.mixed()
133+
.object()
134134
.test("valid-route", "Valid route is required", function(value) {
135135
const configSchema = this.from.slice(-1).pop();
136136
const { routes } = configSchema.value.config;
137137
return routes.map((r) => r.id).includes(value);
138138
});
139139

140140
const ruleConditionSchema = yup.object().shape({
141-
field_source: fieldSourceSchema,
142-
field: fieldSchema("field_source"),
141+
field_source: fieldSourceSchema(),
142+
field: fieldSchema("field_source")(),
143143
operator: yup
144-
.mixed()
144+
.string()
145145
.oneOf(["in"], "One of supported operators should be specified"),
146146
values: yup
147147
.array(yup.string())
148-
.required("At least one value should be provided"),
148+
.min(1, "At least one value should be provided"),
149149
});
150150

151-
const defaultTrafficRuleSchema = yup.object().shape({
151+
const defaultTrafficRuleSchema = (_) => yup.object().shape({
152152
routes: yup
153153
.array()
154154
.of(validRouteSchema)
@@ -244,7 +244,7 @@ const autoscalingPolicySchema = yup.object().shape({
244244

245245
const enricherSchema = yup.object().shape({
246246
type: yup
247-
.mixed()
247+
.string()
248248
.required("Valid Enricher type should be selected")
249249
.oneOf(["nop", "docker"], "Valid Enricher type should be selected"),
250250
});
@@ -256,7 +256,7 @@ const dockerImageSchema = yup
256256
"Valid Docker Image value should be provided, e.g. kennethreitz/httpbin:latest"
257257
);
258258

259-
const dockerDeploymentSchema = (maxAllowedReplica) =>
259+
const dockerDeploymentSchema = (maxAllowedReplica) => (_) =>
260260
yup.object().shape({
261261
image: dockerImageSchema.required("Docker Image is required"),
262262
endpoint: yup.string().required("Endpoint value is required"),
@@ -271,7 +271,7 @@ const dockerDeploymentSchema = (maxAllowedReplica) =>
271271
autoscaling_policy: autoscalingPolicySchema,
272272
});
273273

274-
const pyfuncDeploymentSchema = (maxAllowedReplica) =>
274+
const pyfuncDeploymentSchema = (maxAllowedReplica) => (_) =>
275275
yup.object().shape({
276276
project_id: yup.number().integer().required("Project ID is required"),
277277
ensembler_id: yup.number().integer().required("Ensembler ID is required"),
@@ -287,7 +287,7 @@ const mappingSchema = yup.object().shape({
287287
route: yup.string().required("Treatment needs to be mapped back to a route"),
288288
});
289289

290-
const standardEnsemblerConfigSchema = yup
290+
const standardEnsemblerConfigSchema = (_) => yup
291291
.object()
292292
.shape({
293293
route_name_path: yup.string().nullable(),
@@ -310,7 +310,7 @@ const standardEnsemblerConfigSchema = yup
310310
}
311311
);
312312

313-
const bigQueryConfigSchema = yup.object().shape({
313+
const bigQueryConfigSchema = (_) => yup.object().shape({
314314
table: yup
315315
.string()
316316
.required("BigQuery table name is required")
@@ -321,7 +321,7 @@ const bigQueryConfigSchema = yup.object().shape({
321321
service_account_secret: yup.string().required("Service Account is required"),
322322
});
323323

324-
const kafkaConfigSchema = yup.object().shape({
324+
const kafkaConfigSchema = (_) => yup.object().shape({
325325
brokers: yup
326326
.string()
327327
.required("Kafka broker(s) is required")
@@ -337,7 +337,7 @@ const kafkaConfigSchema = yup.object().shape({
337337
"A valid Kafka topic name may only contain letters, numbers, dot, hyphen or underscore"
338338
),
339339
serialization_format: yup
340-
.mixed()
340+
.string()
341341
.required("Serialzation format should be selected")
342342
.oneOf(
343343
["json", "protobuf"],
@@ -364,10 +364,9 @@ const schema = (maxAllowedReplica) => [
364364
.test("unique-rule-names", validateRuleNames),
365365
routes: yup
366366
.array(routeSchema)
367-
.required()
368367
.unique("id", "Route Id must be unique")
369368
.min(1, "At least one route should be configured")
370-
.when(["rules"], (rules, schema) => {
369+
.when(["rules"], ([rules], schema) => {
371370
if (rules.length > 0) {
372371
return schema.test("no-dangling-routes", validateDanglingRoutes);
373372
}
@@ -391,20 +390,19 @@ const schema = (maxAllowedReplica) => [
391390
config: yup.object().shape({
392391
experiment_engine: yup.object().shape({
393392
type: yup
394-
.mixed()
393+
.string()
395394
.required("Valid Experiment Engine should be selected")
396-
.when("$experimentEngineOptions", (options, schema) =>
395+
.when("$experimentEngineOptions", ([options], schema) =>
397396
schema.oneOf(options, "Valid Experiment Engine should be selected")
398397
),
399-
config: yup.mixed().when("type", (engine, schema) =>
398+
config: yup.object().when("type", ([engine], schema) =>
400399
engine === "nop"
401400
? schema
402-
: yup
403-
.mixed()
404-
.when("$getEngineProperties", (getEngineProperties) => {
401+
: schema
402+
.when("$getEngineProperties", ([getEngineProperties], schema) => {
405403
const engineProps = getEngineProperties(engine);
406404
return engineProps.type === "standard"
407-
? standardExperimentConfigSchema(engineProps)
405+
? standardExperimentConfigSchema(engineProps)(schema)
408406
: engineProps.custom_experiment_manager_config
409407
?.parsed_experiment_config_schema || schema;
410408
})
@@ -418,7 +416,7 @@ const schema = (maxAllowedReplica) => [
418416
switch (value.type) {
419417
case "docker":
420418
return enricherSchema.concat(
421-
dockerDeploymentSchema(maxAllowedReplica)
419+
dockerDeploymentSchema(maxAllowedReplica)()
422420
);
423421
default:
424422
return enricherSchema;
@@ -430,27 +428,30 @@ const schema = (maxAllowedReplica) => [
430428
config: yup.object().shape({
431429
ensembler: yup.object().shape({
432430
type: yup
433-
.mixed()
431+
.string()
434432
.required("Valid Ensembler type should be selected")
435433
.oneOf(
436434
["nop", "docker", "standard", "pyfunc"],
437435
"Valid Ensembler type should be selected"
438436
),
439-
nop_config: yup.mixed().when("type", {
437+
nop_config: yup.object().when("type", {
440438
is: "nop",
441-
then: yup.object().shape({
439+
then: (_) => yup.object().shape({
442440
final_response_route_id: validRouteSchema,
443441
}),
444442
}),
445-
docker_config: yup.mixed().when("type", {
443+
some_field: yup.string().when("some_other_field", ([some_other_field], schema) =>
444+
some_other_field ? yup.string().required("this is needed") : schema
445+
),
446+
docker_config: yup.object().when("type", {
446447
is: "docker",
447448
then: dockerDeploymentSchema(maxAllowedReplica),
448449
}),
449-
standard_config: yup.mixed().when("type", {
450+
standard_config: yup.object().when("type", {
450451
is: "standard",
451452
then: standardEnsemblerConfigSchema,
452453
}),
453-
pyfunc_config: yup.mixed().when("type", {
454+
pyfunc_config: yup.object().when("type", {
454455
is: "pyfunc",
455456
then: pyfuncDeploymentSchema(maxAllowedReplica),
456457
}),
@@ -461,17 +462,17 @@ const schema = (maxAllowedReplica) => [
461462
config: yup.object().shape({
462463
log_config: yup.object().shape({
463464
result_logger_type: yup
464-
.mixed()
465+
.string()
465466
.required("Valid Results Logging type should be selected")
466467
.oneOf(
467468
["nop", "bigquery", "kafka"],
468469
"Valid Results Logging type should be selected"
469470
),
470-
bigquery_config: yup.mixed().when("result_logger_type", {
471+
bigquery_config: yup.object().when("result_logger_type", {
471472
is: "bigquery",
472473
then: bigQueryConfigSchema,
473474
}),
474-
kafka_config: yup.mixed().when("result_logger_type", {
475+
kafka_config: yup.object().when("result_logger_type", {
475476
is: "kafka",
476477
then: kafkaConfigSchema,
477478
}),

ui/yarn.lock

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1267,10 +1267,10 @@
12671267
resolved "https://registry.yarnpkg.com/@bcoe/v8-coverage/-/v8-coverage-0.2.3.tgz#75a2e8b51cb758a7553d6804a5932d7aace75c39"
12681268
integrity sha512-0hYQ8SB4Db5zvZB4axdMHGwEaQjkZzFjQiN9LVYvIFB2nSUHW9tYpxWriPrWDASIxiaXax83REcLxuSdnGPZtw==
12691269

1270-
"@caraml-dev/ui-lib@^1.13.0-build.3-a234b6b":
1271-
version "1.13.0-build.3-a234b6b"
1272-
resolved "https://registry.yarnpkg.com/@caraml-dev/ui-lib/-/ui-lib-1.13.0-build.3-a234b6b.tgz#f24d09d00c77f1288953da8f06893970cdeeda13"
1273-
integrity sha512-aiOWz2QNKJ3uK8No8r9FAfHHQNHe9cvSV5Bgznj7PRaTMpJbfDDSY8U+z+B60SjdtZ7qeeTiC14cEpmv/Ot07Q==
1270+
"@caraml-dev/ui-lib@^1.13.0-build.4-09c363a":
1271+
version "1.13.0-build.4-09c363a"
1272+
resolved "https://registry.yarnpkg.com/@caraml-dev/ui-lib/-/ui-lib-1.13.0-build.4-09c363a.tgz#c80987fa5c7989450095d354acf51a023ca94e15"
1273+
integrity sha512-kqb/5koS2IQtdLJIh5tk+t30ZX2GvvQO7kAuE9oKXKlwBPr83Q+lvLxcvSg+rxILYxjOUeNn2izoej0ZGMDoRg==
12741274
dependencies:
12751275
"@react-oauth/google" "0.12.1"
12761276
classnames "^2.5.1"

0 commit comments

Comments
 (0)