fix(@aws-amplify/datastore): update mutation input - diff with DB instead of patches #7935
Conversation
Codecov Report
@@ Coverage Diff @@
## main #7935 +/- ##
==========================================
- Coverage 74.24% 74.24% -0.01%
==========================================
Files 215 215
Lines 13474 13471 -3
Branches 2647 2647
==========================================
- Hits 10004 10001 -3
Misses 3272 3272
Partials 198 198
Continue to review full report at Codecov.
|
|
This pull request introduces 1 alert when merging 075a95e into 0218cf5 - view on LGTM.com new alerts:
|
iartemiev
force-pushed
the
ds-update-mutation-fix
branch
2 times, most recently
from
2012fe9 to
cdf0300
Compare
… instead of patches
iartemiev
force-pushed
the
ds-update-mutation-fix
branch
from
cdf0300 to
e7a087a
Compare
|
|
||
| // It is me | ||
| if (id === model.id) { | ||
| // Even if the parent is an INSERT, the child might not be, so we need to get its key |
There was a problem hiding this comment.
Could you explain this a little more for me? Is this a cascading save method? Just trying to figure out why you are talking about parent/child here.
There was a problem hiding this comment.
Yes, exactly this is cascading save logic.
FWIW I just slightly refactored it to return the locally stored record (i.e., the previous state that we then diff against in the Storage Class) in the [StorageAdapter].save method. That comment and most of the business logic were already there, this PR is not changing the cascading save logic itself.
There was a problem hiding this comment.
Interesting. We don't have cascading save on Android (and I don't think on iOS either). We should probably add that to our roadmap.
There was a problem hiding this comment.
Overall, this looks good! Very similar to the Android implementation. I just had one minor question for you (above), and as mentioned in Slack, please confirm that when update mutations are created while offline, they are properly sent to AppSync when they come back online! As long as that works properly, ship it!
|
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Issue #, if available:
#7840
This PR changes the way we derive the correct update mutation input when using DataStore. Relying on Immer patches makes it difficult to resolve field data for connections and to include the correct fields when updating relational data. E.g., in the linked issue. Using a diff between the updated record and the previous record is a simpler and more reliable approach.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.