-
Notifications
You must be signed in to change notification settings - Fork 79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: un-require required input fields with @default #2867
base: main
Are you sure you want to change the base?
Conversation
0d93765
to
cbc28fd
Compare
const isPrimaryKeyField = | ||
field.directives!.find((dir) => dir.name.value === 'primaryKey') || | ||
(getBaseType(field.type) === 'ID' && field.type.kind === Kind.NON_NULL_TYPE && field.name.value === 'id'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is one more case to consider here which is when the @default
is applied to fields part of a composite primary key. Gen2 doc or this Gen1 doc is more explicit about the schema with composite primary key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a separate check scanning over sortKeyFields
.
The expected behavior here is that "stringValue" should not be required after @default is applied
…n primaryKey fields
…o primary key fields
…omposite key members
…mposite key members
2a46b1d
to
e00975f
Compare
|
||
const validateNotCompositeKeyMember = (config: DefaultValueDirectiveConfiguration): void => { | ||
const objectDirectives = config.object.fields?.flatMap((f) => f.directives); | ||
const primaryKeyDirective = objectDirectives?.find((dir) => dir?.name.value === 'primaryKey'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can we refactor this check to see if the field is a primary key into it's own method that can be re-used between this method and the one above?
Handoff Notes:
Issue
If an input GQL schema has a required field that has a default e.g.
the schema at the end of the transform still considers these fields to be
NonNullable
onCreateInput
types.Reproduction
bin/cdk-test.ts
:cdk synth
orcdk deploy
These inputs are reflected in the AppSync console, where you will not be able to run create mutation without providing a
name
in this case.Q/A
@default
directive is required on the generated create entity mutation input amplify-codegen#390). The lack of recent issues may be because you have to specify a field is required and provide a default, however our gen2 docs state you cannot use both.default()
and.required()
on the same field.PR
Description of changes
Un-require required input fields that have
@default
applied. Also update the corresponding snapshot to document this behavior.Prior to this change, a copy of the input object was being modified but
ctx.output
was not being updated with the new input.CDK / CloudFormation Parameters Changed
n/a
Issue #, if available
aws-amplify/amplify-codegen#390
Description of how you validated changes
Fixed a test snapshot and edited code to pass it.
Checklist
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.