-
Notifications
You must be signed in to change notification settings - Fork 76
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?
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 passed 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.