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

Missing fields in update mutation after calling .copyOf() multiple times #9891

Closed
3 tasks done
dmcd2 opened this issue May 10, 2022 · 11 comments · Fixed by #9936
Closed
3 tasks done

Missing fields in update mutation after calling .copyOf() multiple times #9891

dmcd2 opened this issue May 10, 2022 · 11 comments · Fixed by #9936
Assignees
Labels
DataStore Related to DataStore category

Comments

@dmcd2
Copy link

dmcd2 commented May 10, 2022

Before opening, please confirm:

JavaScript Framework

React

Amplify APIs

DataStore

Amplify Categories

api

Environment information

# Put output below this line
  System:
    OS: macOS 12.3
    CPU: (8) arm64 Apple M1
    Memory: 140.45 MB / 8.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 17.4.0 - /opt/homebrew/bin/node
    Yarn: 1.22.10 - /opt/homebrew/bin/yarn
    npm: 8.3.1 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 101.0.4951.54
    Edge: 101.0.1210.39
    Firefox: 99.0.1
    Safari: 15.4
  npmPackages:
    @aws-amplify/ui-react: ^2.15.8 => 2.15.8 
    @aws-amplify/ui-react-internal:  undefined ()
    @aws-amplify/ui-react-legacy:  undefined ()
    @babel/cli: ^7.12.1 => 7.17.10 
    @babel/core: ^7.12.3 => 7.17.10 
    @babel/preset-env: ^7.12.1 => 7.17.10 
    @date-io/date-fns: ^1.3.13 => 1.3.13 
    @material-ui/core: ^4.11.0 => 4.12.4 
    @material-ui/lab: ^4.0.0-alpha.56 => 4.0.0-alpha.61 
    @material-ui/pickers: ^3.2.9 => 3.3.10 
    @material-ui/styles: ^4.10.0 => 4.11.5 
    @types/expect-puppeteer: ^4.4.7 => 4.4.7 
    @types/jest-environment-puppeteer: ^5.0.0 => 5.0.2 
    @types/puppeteer: ^5.4.4 => 5.4.6 
    @types/react-router: ^5.1.18 => 5.1.18 
    @welldone-software/why-did-you-render: ^3.3.9 => 3.6.0 
    aws-amplify: ^4.3.21 => 4.3.21 
    date-fns: ^2.16.1 => 2.28.0 
    dayjs: ^1.9.3 => 1.11.1 
    graphql-tag: ^2.11.0 => 2.12.6 
    i18n-iso-countries: ^7.3.0 => 7.4.0 
    i18next: ^21.6.5 => 21.6.16 
    i18next-browser-languagedetector: ^6.1.2 => 6.1.4 
    ini: ^1.3.5 => 1.3.8 
    inquirer: ^6.5.1 => 6.5.2 
    jest-puppeteer: ^6.1.0 => 6.1.0 
    lodash.clonedeep: ^4.5.0 => 4.5.0 
    lodash.get: ^4.4.2 => 4.4.2 
    lodash.isequal: ^4.5.0 => 4.5.0 
    lodash.merge: ^4.6.2 => 4.6.2 
    lodash.mergewith: ^4.6.2 => 4.6.2 
    lodash.set: ^4.3.2 => 4.3.2 
    match-sorter: ^4.2.1 => 4.2.1 
    mdi-material-ui: ^6.20.0 => 6.24.0 
    ncp: ^2.0.0 => 2.0.0 
    notistack: ^1.0.10 => 1.0.10 
    ol: ^6.3.1 => 6.14.1 
    prop-types: ^15.7.2 => 15.8.1 
    puppeteer: ^13.1.3 => 13.7.0 
    react: ^16.14.0 => 16.14.0 (17.0.2)
    react-beautiful-dnd: ^13.1.0 => 13.1.0 
    react-dom: ^16.14.0 => 16.14.0 
    react-error-overlay: 6.0.9 => 6.0.9 (6.0.11)
    react-i18next: ^11.15.3 => 11.16.7 
    react-image-crop: ^8.6.6 => 8.6.12 
    react-number-format: ^4.4.1 => 4.9.3 
    react-router-dom: ^5.2.0 => 5.3.1 
    react-scripts: ^5.0.1 => 5.0.1 
    react-select-country-list: ^2.0.1 => 2.2.3 
    react-virtualized-auto-sizer: ^1.0.2 => 1.0.6 
    react-window: ^1.8.5 => 1.8.7 
    svg-country-flags: ^1.2.9 => 1.2.10 
    uuid: ^3.3.3 => 3.4.0 (3.3.2, 8.3.2, 7.0.3)
    yargs: ^17.3.1 => 17.4.1 (16.2.0, 15.4.1)
  npmGlobalPackages:
    @aws-amplify/cli: 8.0.2
    @wdio/cli: 7.1.2
    node-gyp: 7.1.2
    npm: 8.3.1
    serverless: 2.67.0
    yarn: 1.22.10

Describe the bug

During an update from amplify-aws 3.3.14 to 4.3.21 we have noticed a change in behaviour in which fields are included in an update mutation. It appears that fields that we have updated in our model objects are not being included in the resulting mutation when DataStore.save() is called.

On further investigation we have narrowed this down to the fact that our model objects are copied multiple times before the DataStore.save(). Only the fields updated in the last Model.copyOf() are present in the mutation, any changes made before that are missing.

If I console.log the the contents of the model from the last .copyOf() I can see all the fields I expect to be present. The mutation sent to AppSync does not match this.

This is a definite change in behaviour from 3.3.14 where we had no issues in saving data. Another difference is that update mutations now only contain (or are supposed) the fields that have changed rather than providing the full data as was the case in older versions. That looks to be an optimisation which makes sense.

Expected behavior

What I would expect to happen is that all changes made through the use of Model.copyOf() are present in an update mutation. Model objects can be copied multiple times with different fields changed each time. When DataStore.save() is called all the changes are rolled up into a single update mutation.

Reproduction steps

  1. Query DataStore for an object (data)
  2. Create a new instance of the data using Model.copyOf(data, updated => ... ) and update some fields (dataCopy1)
  3. Create a new instance of the data using Model.copyOf(dataCopy1, updated => ...) and update some fields (dataCopy2)
  4. Save dataCopy2

Examine the update mutation in browser Network tab or print out outboxMutationEnqueued, only the fields set in step 3 are present.

Code Snippet

// Put your code below this line.

 const newObject = Object.copyOf(object, updated => {
      updated.abc = 23;
      updated.def = 24;
      updated.xyz = 10.0;
    });
    const newObject2 = Object.copyOf(newObject, updated => {
      updated.abc = 42;
      updated.def = 43;
    });
    console.log('NEW OBJECT', newObject);
    console.log('NEW OBJECT2', newObject2);

    await DataStore.save(newObject2);

    // Expect to have fields, abc=42, def=43 and xyz = 10.0 
    // Mutation contains only updates to abc and def.

Log output

// Put your logs below this line


aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@chrisbonifacio chrisbonifacio added DataStore Related to DataStore category pending-triage Issue is pending triage labels May 10, 2022
@dmcd2
Copy link
Author

dmcd2 commented May 23, 2022

Hi @chrisbonifacio, is anyone from the Amplify team able to look into this issue for us? It's a blocker for us updating to the latest version of aws-amplify. Thanks.

@undefobj undefobj added the p1 label May 23, 2022
@abdallahshaban557
Copy link
Contributor

Hi @dmcd2 - Our team is currently looking into this! We will update you with our findings.

@chrisbonifacio
Copy link
Member

@dmcd2 In the meantime while we look into this, would you be able to aggregate the changes into one call of copyOf before passing it to DataStore.save? Or, perform intermediate saves in between changes to the copy?

@chrisbonifacio chrisbonifacio added pending-response and removed pending-triage Issue is pending triage labels May 24, 2022
@chrisbonifacio
Copy link
Member

In case this might be a use case we're missing, could you explain why you had to use Model.copyOf in this manner before using it as an argument to DataStore.save?

@dmcd2
Copy link
Author

dmcd2 commented May 26, 2022

@chrisbonifacio We're using copyOf() in the context of a UI dialog. As the user makes changes in UI we update the corresponding fields in the model with their update. At the point of saving we also update some metadata in the model to capture who and when the change was made, so there are at least two copyOf() calls before the save.

@dpilch
Copy link
Member

dpilch commented May 27, 2022

@dmcd2 thanks for the information on your use case.

It looks like this feature was not originally officially supported and happened to work due to the implementation at the time. We are researching the possible impacts of adding official support to all platforms. As a workaround, you could maintain a separate state of changes and call copyOf once after all changes have settled.

// given original { name: 'original', planet: 'original' }
const hero = await DataStore.query(Hero, 'id-123');
const changes = {};

// user makes changes to name and planet
changes.name = 'Obi-Wan Kenobi';
changes.planet = 'Tatooine';

// ===============================

// later user makes another change to name
changes.name = 'Ben Kenobi'

const heroCopy = Hero.copyOf(hero, draft => {
  Object.assign(draft, changes);
});
await DataStore.save(heroCopy);
// will save final hero as { name: 'Ben Kenobi', planet: 'Tatooine' }

@dmcd2
Copy link
Author

dmcd2 commented Jun 2, 2022

@dpilch Are you going to support this behaviour? I saw there was a PR to address this. We've been using the older version of amplify for a while now so we could wait for an update rather than changing our application at this point.

@dpilch
Copy link
Member

dpilch commented Jun 2, 2022

We are still discussing support. I'll update here when the decision is reached.

@dpilch
Copy link
Member

dpilch commented Jun 9, 2022

@dmcd2, We have decided to support consecutive copyOf chains with the functionality that you described. We will work on getting the fix merged to unblock you.

@dmcd2
Copy link
Author

dmcd2 commented Jun 10, 2022

@dpilch That's great, thank you.

@github-actions
Copy link

This issue 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 amplify-help forum.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DataStore Related to DataStore category
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants