feat(api): specify include or exclude tables option to generate schema #1824
Conversation
| if (!existingDocument) { | ||
| return document; | ||
| } |
There was a problem hiding this comment.
This check shouldn't be required, since you don't specify as optional, and also explicitly already check this in the caller.
| newObjectWrapper.fields.push(new FieldWrapper(relationalField)); | ||
| }); | ||
|
|
||
| if (relationalFields.length > 0) { |
There was a problem hiding this comment.
This branch creates a potentially bizarre modal behavior. If I'm reading this right if I go make changes to my Model after import, then upon the next import those changes are scrubbed, but if those changes ALSO include a relational update then those changes are maintained (not just the relationship).
Is that the desired behavior, or should we be doing the merge one layer deeper (e.g. only maintain relational fields in the colliding models?
There was a problem hiding this comment.
That a great question. This logic copies only the relationship. newObject is the regenerated GraphQL type and existingObject is the current state of the GraphQL type from schema.rds.graphql file. We only copy the relational fields from existing type to the new type in this method (line 56-58).
I'm adding a test specific to this scenario.
However, I had other concerns that I will review with Rene and address in a follow up if required.
- What if the table belongs to the related type is deleted? Should we still retain the relationship?
- What if one of the references fields removed?
There was a problem hiding this comment.
Got it - thank you for explaining. I missed that on my first read, but makes sense after reviewing.
| newObjectWrapper.fields.push(new FieldWrapper(relationalField)); | ||
| }); | ||
|
|
||
| if (relationalFields.length > 0) { |
There was a problem hiding this comment.
Got it - thank you for explaining. I missed that on my first read, but makes sense after reviewing.
|
|
||
| export const applySchemaOverrides = (document: DocumentNode, existingDocument?: DocumentNode | undefined): DocumentNode => { | ||
| if (!existingDocument) { | ||
| return document; | ||
| } | ||
|
|
||
| let updatedDocument = preserveRelationalDirectives(document, existingDocument) as any; |
There was a problem hiding this comment.
One alternative here is to extend the schema visitor pattern we use for FieldDefinition to ObjectTypeDefinition as well, this could save us from multiple mutations of updatedDocument. Non-blocking suggestion.
sundersc
merged commit 5fe85d3
into
aws-amplify:feature/rds-support-preview2
Description of changes
Include/Exclude Option
Example:
or
This change also includes preserving customer added relational fields on schema regeneration.
CDK / CloudFormation Parameters Changed
NA
Issue #, if available
NA
Description of how you validated changes
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.