Skip to content

Commit

Permalink
Merge pull request #640 from gadget-inc/mill/dontBlockSubmitWithExclu…
Browse files Browse the repository at this point in the history
…dedRequiredFields

Fixed bug with AutoForm include/exclude system which prevented submission due to validations getting applied to hidden fields
  • Loading branch information
MillanWangGadget authored Sep 13, 2024
2 parents 0b52c96 + 405b4ed commit 1413914
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 17 deletions.
5 changes: 5 additions & 0 deletions packages/react/.changeset/nasty-crews-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@gadgetinc/react": patch
---

Fixed bug with AutoForm include/exclude system which prevented submission due to validations getting applied to hidden fields
7 changes: 2 additions & 5 deletions packages/react/cypress/component/auto/form/AutoForm.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,8 @@ describeForEachAutoAdapter("AutoForm", ({ name, adapter: { AutoForm }, wrapper }
});

it("can render a rich text editor for markdown content", async () => {
cy.mountWithWrapper(<AutoForm action={api.widget.create} include={["name", "inventoryCount", "description"]} />, wrapper);
cy.mountWithWrapper(<AutoForm action={api.widget.create} include={["description"]} />, wrapper);

cy.get(`input[name="widget.name"]`).type("test record");
cy.get(`input[name="widget.inventoryCount"]`).type("42");
cy.get(`[aria-label="editable markdown"] > p`).type("# foobar\n## foobaz");

cy.intercept("POST", `${api.connection.options.endpoint}?operation=createWidget`, {
Expand All @@ -256,8 +254,7 @@ describeForEachAutoAdapter("AutoForm", ({ name, adapter: { AutoForm }, wrapper }

cy.wait("@createWidget").then((interception) => {
const { variables } = interception.request.body;
expect(variables.widget.name).to.equal("test record");
expect(variables.widget.inventoryCount).to.equal(42);

expect(variables.widget.description).to.deep.equal({ markdown: "# foobar\n\n## foobaz" });
});
});
Expand Down
5 changes: 3 additions & 2 deletions packages/react/spec/auto/PolarisAutoForm.stories.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const UpsertRecordWithoutFindBy = {
export const Excluded = {
args: {
action: api.widget.create,
exclude: ["birthday", "roles"],
exclude: ["birthday", "roles", "name"],
},
};

Expand All @@ -89,7 +89,8 @@ export const ExcludedWithDefaultValues = {
export const Included = {
args: {
action: api.widget.create,
include: ["name", "inventoryCount"],
// Inventory is required and not included. This will be a server-side error since it can be set in the action file code
include: ["name"],
},
};

Expand Down
11 changes: 7 additions & 4 deletions packages/react/src/auto/AutoForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ export type AutoFormProps<
/**
* React hook for getting the validation schema for a list of fields
*/
export const useValidationResolver = (metadata: ActionMetadata | GlobalActionMetadata | undefined) => {
const useValidationResolver = (metadata: ActionMetadata | GlobalActionMetadata | undefined, pathsToValidate: string[]) => {
return useMemo(() => {
if (!metadata) return undefined;
const action = isActionMetadata(metadata) ? metadata.action : metadata;
return yupResolver(validationSchema(action.inputFields));
}, [metadata]);
return yupResolver(validationSchema(action.inputFields, pathsToValidate));
}, [metadata, pathsToValidate]);
};

/**
Expand Down Expand Up @@ -180,7 +180,10 @@ export const useAutoForm = <
} = useActionForm(action, {
defaultValues: defaultValues as any,
findBy: "findBy" in props ? props.findBy : undefined,
resolver: useValidationResolver(metadata),
resolver: useValidationResolver(
metadata,
fields.map(({ path }) => path)
),
send: () => {
const fieldsToSend = fields
.filter(({ path, metadata }) => {
Expand Down
25 changes: 19 additions & 6 deletions packages/react/src/validationSchema.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { ISchema } from "yup";
import { MixedSchema, NumberSchema, StringSchema, array, boolean, date, mixed, number, object, string } from "yup";
import { fileSizeValidationErrorMessage } from "./auto/hooks/useFileInputController.js";
import type {
FieldMetadataFragment,
GadgetEnumConfig,
GadgetGenericFieldValidation,
GadgetObjectFieldConfig,
Expand All @@ -12,8 +13,10 @@ import type {
import { GadgetFieldType } from "./internal/gql/graphql.js";
import type { FieldMetadata } from "./metadata.js";

const validatorForField = (field: FieldMetadata) => {
const validatorForField = (field: FieldMetadata, pathsToValidate: string[] = [], currentFieldPath = "") => {
let validator;
const path = currentFieldPath ? `${currentFieldPath}.${field.apiIdentifier}` : field.apiIdentifier;

switch (field.fieldType) {
case GadgetFieldType.Boolean: {
validator = boolean();
Expand Down Expand Up @@ -91,7 +94,7 @@ const validatorForField = (field: FieldMetadata) => {
}
case GadgetFieldType.Object: {
const config = field.configuration as GadgetObjectFieldConfig;
validator = validationSchema(config.fields as any);
validator = validationSchema(config.fields as any, pathsToValidate, path);
break;
}
case GadgetFieldType.RichText: {
Expand Down Expand Up @@ -129,13 +132,23 @@ const validatorForField = (field: FieldMetadata) => {
}
}

if (field.requiredArgumentForInput) {
validator = applyValidationsToInputField(field, validator, pathsToValidate.includes(path));

return validator;
};

const applyValidationsToInputField = (field: FieldMetadataFragment, validator: any, pathRequiresValidation: boolean) => {
if (field.requiredArgumentForInput && pathRequiresValidation) {
if (field.fieldType === GadgetFieldType.RichText) {
validator = object({ markdown: string().required() });
}
validator = validator.required(`${field.name} is required`);
} else {
validator = (validator.nullable() as any).default(null);
validator = validator.nullable().default(null);
}

if (!pathRequiresValidation) {
return validator;
}

for (const validation of field.configuration.validations) {
Expand Down Expand Up @@ -251,10 +264,10 @@ export const isFailedJSONParse = (value: any): value is FailedJSONParse => {
/**
* Build a Yup validation schema given some fields metadata for validating that a data object conforms to the schema at runtime
*/
export const validationSchema = (fields: FieldMetadata[]) => {
export const validationSchema = (fields: FieldMetadata[], pathsToValidate: string[] = [], currentPath = "") => {
const validators: Record<string, ISchema<any>> = {};
for (const field of fields) {
validators[field.apiIdentifier] = validatorForField(field);
validators[field.apiIdentifier] = validatorForField(field, pathsToValidate, currentPath);
}
return object(validators);
};
Expand Down

0 comments on commit 1413914

Please sign in to comment.