fix(datastore): fix has-one associations #1676
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 tasks
Codecov Report
@@ Coverage Diff @@
## main #1676 +/- ##
==========================================
+ Coverage 59.19% 59.27% +0.07%
==========================================
Files 716 716
Lines 21786 21808 +22
==========================================
+ Hits 12897 12926 +29
+ Misses 8889 8882 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
diegocstn
commented
Mar 16, 2022
| @@ -58,7 +58,7 @@ class DataStoreConnectionScenario2FlutterTests: SyncEngineFlutterIntegrationTest | |||
| return | |||
| } | |||
| let saveTeamCompleted = expectation(description: "save team completed") | |||
| plugin.save(team.model, modelSchema: Team1.schema) { result in | |||
There was a problem hiding this comment.
fixed as in this scenario we are testing models Team2 and Project2
diegocstn
commented
Mar 16, 2022
Comment on lines
+19
to
+22
| ModelRegistry.register(modelType: Project2V2.self) | ||
| ModelRegistry.register(modelType: Team2V2.self) | ||
| ModelRegistry.register(modelType: Record.self) | ||
| ModelRegistry.register(modelType: RecordCover.self) |
There was a problem hiding this comment.
those models were missing, so the selection set in queries wasn't properly generated (see GraphQL queries below)
lawmicha
reviewed
Mar 16, 2022
| && lhs.project1V2TeamId == rhs.project1V2TeamId | ||
| // && lhs.team == rhs.team // TODO: Should the Project have the team? | ||
|
|
||
| // && lhs.project1V2TeamId == rhs.project1V2TeamId |
There was a problem hiding this comment.
should this check lhs.team == rhs.team?
There was a problem hiding this comment.
yes, thanks for catching that
| } | ||
| } | ||
|
|
||
| /// This is a temporary workaround to circumvent a Codegen issue where |
There was a problem hiding this comment.
can we add a link to more information to track the removal of this temp workaround? perhaps an issue in our repo to describe the situation
|
Please merge this! People running in iOS land are feeling the pain, there's been lots of activity in the Discord server about this issue. |
igorTimofiejczyk
added a commit
to AnimealProject/animeal_iOS
that referenced
this pull request
Aug 31, 2022
Amplify have known issue which prevent to fill properties with .hasOne connection Here the issue: aws-amplify/amplify-swift#1107 And here the prepared fix: aws-amplify/amplify-swift#1676 Since fix was on github for while without any changes I switched the to the forked version of amplify
igorTimofiejczyk
added a commit
to AnimealProject/animeal_iOS
that referenced
this pull request
Aug 31, 2022
Amplify have known issue which prevent to fill properties with .hasOne connection Here the issue: aws-amplify/amplify-swift#1107 And here the prepared fix: aws-amplify/amplify-swift#1676 Since fix was on github for while without any changes I switched the to the forked version of amplify
igorTimofiejczyk
added a commit
to AnimealProject/animeal_iOS
that referenced
this pull request
Aug 31, 2022
Amplify have known issue which prevent to fill properties with .hasOne connection Here the issue: aws-amplify/amplify-swift#1107 And here the prepared fix: aws-amplify/amplify-swift#1676 Since fix was on github for while without any changes I switched the to the forked version of amplify
igorTimofiejczyk
added a commit
to AnimealProject/animeal_iOS
that referenced
this pull request
Sep 1, 2022
Amplify have known issue which prevent to fill properties with .hasOne connection Here the issue: aws-amplify/amplify-swift#1107 And here the prepared fix: aws-amplify/amplify-swift#1676 Since fix was on github for while without any changes I switched the to the forked version of amplify
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
This PR addresses different issues related to the
has-onemodel relationships in DataStore.The associated Model in a has-one relationship now is eagerly-loaded as we do for belongs-to (#1107)
Models generated with the new version of the GraphQL transformer requires consumers to pass the value of the foreign key to the initializer therefore resulting in a poor dx. While a codegen change is needed to fully address this issue, this PR introduces the necessary changes to have the foreign key value persisted even when only an instance of the associated model is passed to the initializer (#1623).
This change will then restore the previously working dx:
Check points: (check or cross out if not relevant)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.