Skip to content
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(api): write null values in ModelMutations.create() unless owner field #2679

Merged
merged 2 commits into from
Feb 21, 2023
Merged

fix(api): write null values in ModelMutations.create() unless owner field #2679

merged 2 commits into from
Feb 21, 2023

Conversation

ragingsquirrel3
Copy link
Contributor

@ragingsquirrel3 ragingsquirrel3 commented Feb 17, 2023

This PR has a correction for some logic recently changed in #2504. While the previous PR changed ModelMutations.create() to no longer include any null values in the input object, this PR makes it so only owner fields with null values will be removed from input and other null values will remain.

While removing null owner values is essential to fix the issue mentioned in #2492, removing all null fields from create has some consequences that are difficult for users to understand or predict. Such as:

  • Data created with older versions of model helpers is different than newer versions. Newer versions have empty values in DynamoDB versus explicit nulls from older entries. This makes is so .ne(null) will match newer versions with undefined values (the issue that was reported). While breaking changes in dev preview are expected, I don't think this change has been given proper thought/communication.
  • Because ModelMutations.update() includes null values (impossible to determine if undefined or explicit null there), any model updated for any reason will have all nulls, no undefined fields. For a user, it would be confusing why their data is in 2 states from create/update when the model is in the same state.

Ultimately, the issues is that the codegen models have no way to distinguish between explicit nulls and undefined. Without a solution at that level, this PR makes things more consistent while still preventing the originally reported issue of null values in owner fields.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ragingsquirrel3 ragingsquirrel3 requested a review from a team as a code owner February 17, 2023 22:35
Comment on lines 326 to 327
.where((authRule) => authRule.ownerField != null)
.map((authRule) => authRule.ownerField!)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.where((authRule) => authRule.ownerField != null)
.map((authRule) => authRule.ownerField!)
.map((authRule) => authRule.ownerField)
.whereType<String>() // or `whereNotNull()` from `package:collection`

@ragingsquirrel3 ragingsquirrel3 merged commit 12a8f3b into aws-amplify:next Feb 21, 2023
@ragingsquirrel3 ragingsquirrel3 deleted the fix/api-less-null branch February 21, 2023 18:28
dnys1 pushed a commit that referenced this pull request Mar 7, 2023
### Breaking Changes
- chore(auth)!: Chain stack traces in state machine
- chore(core)!: Chain stack traces for state machines
- feat(api)!: custom primary key support for GraphQL model helpers ([#2606](#2606))

### Fixes
- fix(api): early call of Amplify creates wrong instance of AmplifyClass
- fix(api): write null values in ModelMutations.create() unless owner field ([#2679](#2679))
- fix(auth): Await `signInUri` in Hosted UI platform ([#2706](#2706))
- fix(auth): Transform session expired exceptions ([#2688](#2688))
- fix(storage): GetUrl signing

### Features
- feat(api): GraphQL Subscription Where Filter ([#2650](#2650))
- feat(aws_common): add openRead API for AWSFile
- feat(storage): optimize part size for multipart upload
- feat(storage): web implementation of transfer database using local storage ([#2631](#2631))

Updated-Components: aws_common, Amplify Flutter, Amplify Dart, Amplify UI, DB Common
dnys1 pushed a commit that referenced this pull request Mar 7, 2023
### Breaking Changes
- chore(auth)!: Chain stack traces in state machine
- chore(core)!: Chain stack traces for state machines
- feat(api)!: custom primary key support for GraphQL model helpers ([#2606](#2606))

### Fixes
- fix(api): early call of Amplify creates wrong instance of AmplifyClass
- fix(api): write null values in ModelMutations.create() unless owner field ([#2679](#2679))
- fix(auth): Await `signInUri` in Hosted UI platform ([#2706](#2706))
- fix(auth): Transform session expired exceptions ([#2688](#2688))
- fix(storage): GetUrl signing

### Features
- feat(api): GraphQL Subscription Where Filter ([#2650](#2650))
- feat(aws_common): add openRead API for AWSFile
- feat(storage): optimize part size for multipart upload
- feat(storage): web implementation of transfer database using local storage ([#2631](#2631))

Updated-Components: aws_common, Amplify Flutter, Amplify Dart, Amplify UI, DB Common
dnys1 pushed a commit that referenced this pull request Mar 7, 2023
### Breaking Changes
- chore(auth)!: Chain stack traces in state machine
- chore(core)!: Chain stack traces for state machines
- feat(api)!: custom primary key support for GraphQL model helpers ([#2606](#2606))

### Fixes
- fix(api): early call of Amplify creates wrong instance of AmplifyClass
- fix(api): write null values in ModelMutations.create() unless owner field ([#2679](#2679))
- fix(auth): Await `signInUri` in Hosted UI platform ([#2706](#2706))
- fix(auth): Transform session expired exceptions ([#2688](#2688))
- fix(storage): GetUrl signing

### Features
- feat(api): GraphQL Subscription Where Filter ([#2650](#2650))
- feat(aws_common): add openRead API for AWSFile
- feat(storage): optimize part size for multipart upload
- feat(storage): web implementation of transfer database using local storage ([#2631](#2631))

Updated-Components: aws_common, Amplify Flutter, Amplify Dart, Amplify UI, DB Common
dnys1 pushed a commit that referenced this pull request Mar 7, 2023
### Breaking Changes
- chore(auth)!: Chain stack traces in state machine
- chore(core)!: Chain stack traces for state machines
- feat(api)!: custom primary key support for GraphQL model helpers ([#2606](#2606))

### Fixes
- fix(api): early call of Amplify creates wrong instance of AmplifyClass
- fix(api): write null values in ModelMutations.create() unless owner field ([#2679](#2679))
- fix(auth): Await `signInUri` in Hosted UI platform ([#2706](#2706))
- fix(auth): Transform session expired exceptions ([#2688](#2688))
- fix(storage): GetUrl signing

### Features
- feat(api): GraphQL Subscription Where Filter ([#2650](#2650))
- feat(aws_common): add openRead API for AWSFile
- feat(storage): optimize part size for multipart upload
- feat(storage): web implementation of transfer database using local storage ([#2631](#2631))

Updated-Components: aws_common, Amplify Flutter, Amplify Dart, Amplify UI, DB Common
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants