Skip to content

Commit b6a678b

Browse files
airhornsairhorns
authored
Merge pull request #850 from gadget-inc/mill/fixPathingOnHasManyThroughForm
Fix pathing of validations on AutoHasManyThroughForm
2 parents a2a1b9f + bae5640 commit b6a678b

File tree

10 files changed

+146
-53
lines changed

10 files changed

+146
-53
lines changed

‎packages/react/.changeset/fair-rabbits-lick.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@gadgetinc/react": patch
3+
---
4+
5+
- Fixed a bug with AutoHasManyThroughForm where input validations were not working, causing type errors to occur on tha API for non-string fields

‎packages/react/cypress/component/auto/form/AutoHasManyThroughForm.cy.tsx

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ describeForEachAutoAdapter(
7676
<AutoHasManyThroughJoinModelForm>
7777
<AutoInput field="effectiveFrom" />
7878
<AutoInput field="effectiveTo" />
79+
<AutoInput field="attempt" />
7980
</AutoHasManyThroughJoinModelForm>
8081
</AutoHasManyThroughForm>
8182
<AutoSubmit id="submit" />
@@ -91,7 +92,9 @@ describeForEachAutoAdapter(
9192
cy.contains("Emma Williams").click();
9293
cy.contains("Add Students").click();
9394

95+
cy.wait(1000);
9496
cy.get('[id="deleteButton_students.0"]').click();
97+
cy.wait(1000);
9598

9699
expectUpdateActionSubmissionVariables({
97100
course: {
@@ -101,18 +104,41 @@ describeForEachAutoAdapter(
101104
// Updated second
102105
{
103106
update: {
104-
effectiveFrom: "2025-02-18",
105-
effectiveTo: "2025-02-22",
107+
effectiveFrom: "2025-02-18T00:00:00.000Z",
108+
effectiveTo: "2025-02-22T00:00:00.000Z",
106109
id: "50",
107-
student: { update: { firstName: "Benjamin", id: "43", lastName: "Martin" } },
110+
attempt: 1,
111+
student: {
112+
update: {
113+
department: null,
114+
firstName: "Benjamin",
115+
id: "43",
116+
lastName: "Martin",
117+
year: null,
118+
},
119+
},
108120
},
109121
},
110122
// Created third
111-
{ create: { student: { _link: "10" } } },
123+
{
124+
create: {
125+
effectiveFrom: null,
126+
effectiveTo: null,
127+
attempt: 2,
128+
student: { _link: "10" },
129+
},
130+
},
112131
],
113132
},
114133
id: "3",
115134
});
135+
136+
cy.get('[id="submit"]').click();
137+
cy.contains("Attempt is required").should("exist"); // Validation for blank field
138+
139+
cy.get('[id="course.registrations.0.attempt"]').type("1");
140+
cy.get('[id="course.registrations.1.attempt"]').type("2");
141+
116142
cy.get('[id="submit"]').click();
117143
cy.wait("@updateCourse");
118144
});
@@ -128,6 +154,7 @@ describeForEachAutoAdapter(
128154
>
129155
<AutoInput field="firstName" />
130156
<AutoInput field="lastName" />
157+
<AutoInput field="year" />
131158
</AutoHasManyThroughForm>
132159
<AutoSubmit id="submit" />
133160
</AutoForm>,
@@ -137,9 +164,13 @@ describeForEachAutoAdapter(
137164
cy.wait("@course");
138165
cy.wait("@students");
139166

167+
cy.wait(1000);
140168
cy.get('[id="deleteButton_students.0"]').click();
169+
cy.wait(1000);
141170
cy.get('[name="course.registrations.0.student.firstName"]').click().type("- updated");
142171
cy.get('[name="course.registrations.0.student.lastName"]').click().type("- updated");
172+
cy.get('[name="course.registrations.0.student.year"]').click().type("- updated");
173+
cy.get('[name="course.registrations.0.student.year"]').click().type("1");
143174

144175
expectUpdateActionSubmissionVariables({
145176
course: {
@@ -159,6 +190,7 @@ describeForEachAutoAdapter(
159190
firstName: "Benjamin- updated",
160191
id: "43",
161192
lastName: "Martin- updated",
193+
year: 1,
162194
},
163195
},
164196
},
@@ -245,7 +277,7 @@ const RealUniversityCourseMetadata = {
245277
fieldType: "HasManyThrough",
246278
requiredArgumentForInput: false,
247279
sortable: false,
248-
filterable: false,
280+
filterable: true,
249281
__typename: "GadgetModelField",
250282
configuration: {
251283
__typename: "GadgetHasManyThroughConfig",
@@ -285,7 +317,7 @@ const RealUniversityCourseMetadata = {
285317
fieldType: "HasManyThrough",
286318
requiredArgumentForInput: false,
287319
sortable: false,
288-
filterable: false,
320+
filterable: true,
289321
__typename: "GadgetModelField",
290322
configuration: {
291323
__typename: "GadgetHasManyThroughConfig",
@@ -367,7 +399,7 @@ const RealUniversityCourseMetadata = {
367399
fieldType: "HasMany",
368400
requiredArgumentForInput: false,
369401
sortable: false,
370-
filterable: false,
402+
filterable: true,
371403
__typename: "GadgetModelField",
372404
configuration: {
373405
__typename: "GadgetHasManyConfig",
@@ -393,7 +425,7 @@ const RealUniversityCourseMetadata = {
393425
fieldType: "HasMany",
394426
requiredArgumentForInput: false,
395427
sortable: false,
396-
filterable: false,
428+
filterable: true,
397429
__typename: "GadgetModelField",
398430
configuration: {
399431
__typename: "GadgetHasManyConfig",
@@ -661,7 +693,7 @@ const RealUniversityCourseMetadata = {
661693
fieldType: "HasManyThrough",
662694
requiredArgumentForInput: false,
663695
sortable: false,
664-
filterable: false,
696+
filterable: true,
665697
__typename: "GadgetModelField",
666698
configuration: {
667699
__typename: "GadgetHasManyThroughConfig",
@@ -743,7 +775,7 @@ const RealUniversityCourseMetadata = {
743775
fieldType: "HasMany",
744776
requiredArgumentForInput: false,
745777
sortable: false,
746-
filterable: false,
778+
filterable: true,
747779
__typename: "GadgetModelField",
748780
configuration: {
749781
__typename: "GadgetHasManyConfig",
@@ -806,11 +838,26 @@ const RealUniversityCourseMetadata = {
806838
],
807839
},
808840
},
841+
{
842+
name: "Attempt",
843+
apiIdentifier: "attempt",
844+
fieldType: "Number",
845+
requiredArgumentForInput: true,
846+
sortable: true,
847+
filterable: true,
848+
__typename: "GadgetModelField",
849+
configuration: {
850+
__typename: "GadgetNumberConfig",
851+
fieldType: "Number",
852+
validations: [],
853+
decimals: null,
854+
},
855+
},
809856
{
810857
name: "Effective from",
811858
apiIdentifier: "effectiveFrom",
812859
fieldType: "DateTime",
813-
requiredArgumentForInput: true,
860+
requiredArgumentForInput: false,
814861
sortable: true,
815862
filterable: true,
816863
__typename: "GadgetModelField",
@@ -1064,7 +1111,7 @@ const RealUniversityCourseMetadata = {
10641111
fieldType: "HasManyThrough",
10651112
requiredArgumentForInput: false,
10661113
sortable: false,
1067-
filterable: false,
1114+
filterable: true,
10681115
__typename: "GadgetModelField",
10691116
configuration: {
10701117
__typename: "GadgetHasManyThroughConfig",
@@ -1146,7 +1193,7 @@ const RealUniversityCourseMetadata = {
11461193
fieldType: "HasMany",
11471194
requiredArgumentForInput: false,
11481195
sortable: false,
1149-
filterable: false,
1196+
filterable: true,
11501197
__typename: "GadgetModelField",
11511198
configuration: {
11521199
__typename: "GadgetHasManyConfig",
@@ -1236,7 +1283,7 @@ const RealUniversityCourseMetadata = {
12361283
fieldType: "HasMany",
12371284
requiredArgumentForInput: false,
12381285
sortable: false,
1239-
filterable: false,
1286+
filterable: true,
12401287
__typename: "GadgetModelField",
12411288
configuration: {
12421289
__typename: "GadgetHasManyConfig",
@@ -1262,7 +1309,7 @@ const RealUniversityCourseMetadata = {
12621309
fieldType: "HasManyThrough",
12631310
requiredArgumentForInput: false,
12641311
sortable: false,
1265-
filterable: false,
1312+
filterable: true,
12661313
__typename: "GadgetModelField",
12671314
configuration: {
12681315
__typename: "GadgetHasManyThroughConfig",
@@ -1302,7 +1349,7 @@ const RealUniversityCourseMetadata = {
13021349
fieldType: "HasMany",
13031350
requiredArgumentForInput: false,
13041351
sortable: false,
1305-
filterable: false,
1352+
filterable: true,
13061353
__typename: "GadgetModelField",
13071354
configuration: {
13081355
__typename: "GadgetHasManyConfig",
@@ -1328,7 +1375,7 @@ const RealUniversityCourseMetadata = {
13281375
fieldType: "HasManyThrough",
13291376
requiredArgumentForInput: false,
13301377
sortable: false,
1331-
filterable: false,
1378+
filterable: true,
13321379
__typename: "GadgetModelField",
13331380
configuration: {
13341381
__typename: "GadgetHasManyThroughConfig",

‎packages/react/spec/auto/storybook/form/AutoForm.stories.tsx

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,7 @@ import React, { useState } from "react";
22
import { Provider } from "../../../../src/GadgetProvider.js";
33
import { testApi as api } from "../../../apis.js";
44
import { StorybookErrorBoundary } from "../StorybookErrorBoundary.js";
5-
import {
6-
AutoHasManyThroughInput,
7-
AutoInput,
8-
AutoSubmit,
9-
Button,
10-
SelectableDesignSystemAutoFormStory,
11-
} from "./SelectableDesignSystemAutoFormStory.js";
5+
import { AutoInput, AutoSubmit, Button, SelectableDesignSystemAutoFormStory } from "./SelectableDesignSystemAutoFormStory.js";
126

137
// More on how to set up stories at: https://storybook.js.org/docs/writing-stories#default-export
148
export default {
@@ -219,12 +213,5 @@ export const hasManyThrough: any = {
219213
name: "HasManyThrough fields",
220214
args: {
221215
action: api.hasManyThrough.baseModel.create,
222-
children: (
223-
<>
224-
<AutoInput field="baseModelName" />
225-
<AutoHasManyThroughInput field="baseModelHmtField" />
226-
<AutoSubmit />
227-
</>
228-
),
229216
},
230217
};

‎packages/react/spec/auto/storybook/form/AutoRelationshipForm.stories.tsx

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,14 @@ export const UpdateWithHasManyThrough: any = {
174174
},
175175
};
176176

177+
export const UpdateWithHasManyThroughSiblingFieldsOnly: any = {
178+
render: (args: any) => <ExampleCourseHasManyThroughSiblingFieldsOnlyForm {...args} />,
179+
args: {
180+
action: api.university.course.update,
181+
findBy: "3",
182+
},
183+
};
184+
177185
const ExampleCourseCreateRelatedForm = (props: any) => {
178186
return (
179187
<>
@@ -213,16 +221,11 @@ const ExampleCourseCreateRelatedForm = (props: any) => {
213221
}}
214222
>
215223
<InlineStack gap="300">
216-
{/* Fields on the join model. The prefix is the model API id of the join model */}
217-
218224
<AutoHasManyThroughJoinModelForm>
219225
<AutoInput field="effectiveTo" />
220226
<AutoInput field="effectiveFrom" />
227+
<AutoInput field="attempt" />
221228
</AutoHasManyThroughJoinModelForm>
222-
223-
{/* Fields on the sibling model. No prefix */}
224-
<AutoInput field="firstName" />
225-
<AutoInput field="lastName" />
226229
</InlineStack>
227230
</AutoHasManyThroughForm>
228231
</Card>
@@ -242,6 +245,35 @@ const ExampleCourseCreateRelatedForm = (props: any) => {
242245
);
243246
};
244247

248+
const ExampleCourseHasManyThroughSiblingFieldsOnlyForm = (props: any) => {
249+
return (
250+
<>
251+
<SelectableDesignSystemAutoFormStory {...props}>
252+
<SubmitResultBanner />
253+
254+
<Card>
255+
<AutoHasManyThroughForm
256+
field="students"
257+
recordLabel={{
258+
primary: ["firstName", "lastName"],
259+
secondary: "year",
260+
tertiary: "department",
261+
}}
262+
>
263+
<InlineStack gap="300">
264+
<AutoInput field="firstName" />
265+
<AutoInput field="lastName" />
266+
<AutoInput field="year" />
267+
</InlineStack>
268+
</AutoHasManyThroughForm>
269+
</Card>
270+
271+
<AutoSubmit />
272+
</SelectableDesignSystemAutoFormStory>
273+
</>
274+
);
275+
};
276+
245277
export const CreateWithHasManyThroughSelfReferential: any = {
246278
render: (args: any) => <ExampleTweeterFollowerCreateRelatedForm {...args} />,
247279
args: {

‎packages/react/src/auto/AutoForm.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,11 @@ export const useAutoForm = <
592592
metadata,
593593
fetchingMetadata,
594594
metadataError,
595-
fields: fields.filter(removeIdFieldsUnlessUpsertWithoutFindBy(isUpsertWithFindBy)),
595+
fields: fields.filter(
596+
(field) =>
597+
falseForIdFieldsUnlessUpsertWithoutFindBy({ isUpsertWithFindBy, metadata: field.metadata }) &&
598+
!isJoinModelHasManyField(field.metadata)
599+
),
596600
submit,
597601
formError,
598602
pauseExistingRecordLookup,
@@ -692,12 +696,19 @@ const aggregatePathsFromRecordLabel = (recordLabel: RecordLabel, getFieldsToSele
692696
return Array.from(selectedPaths);
693697
};
694698

695-
const removeIdFieldsUnlessUpsertWithoutFindBy = (isUpsertWithFindBy?: boolean) => {
696-
return (field: { metadata: FieldMetadata }) => {
697-
return field.metadata.fieldType === FieldType.Id ? !isUpsertWithFindBy : true;
698-
};
699+
const falseForIdFieldsUnlessUpsertWithoutFindBy = (props: { isUpsertWithFindBy?: boolean; metadata: FieldMetadata }) => {
700+
const { isUpsertWithFindBy, metadata } = props;
701+
return metadata.fieldType === FieldType.Id ? !isUpsertWithFindBy : true;
699702
};
700703

704+
/**
705+
* Removes `hasMany` fields that emerge from `hasManyThrough` fields that are not actually model fields
706+
*/
707+
const isJoinModelHasManyField = (field: FieldMetadata) =>
708+
field.fieldType === FieldType.HasMany &&
709+
field.configuration.__typename === "GadgetHasManyConfig" &&
710+
field.configuration.isJoinModelHasManyField;
711+
701712
const validateFindBy = (params: {
702713
operatesWithRecordId: boolean;
703714
hasFindBy: boolean;

0 commit comments

Comments
 (0)