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

Model.changedAttributes not cleaned out after save #8329

Closed
timmorey opened this issue Nov 30, 2022 · 5 comments · Fixed by #8335
Closed

Model.changedAttributes not cleaned out after save #8329

timmorey opened this issue Nov 30, 2022 · 5 comments · Fixed by #8335
Labels
🏷️ bug This PR primarily fixes a reported issue

Comments

@timmorey
Copy link
Contributor

timmorey commented Nov 30, 2022

Reproduction

I have a repro case in a simple ember application: https://github.com/timmorey/ember-data-changed-attrs. You can see the failure by cloning and then running npm install && npm run test.

The application defines a single model and configures the application REST adapter/serializer. ember-cli-mirage provides a mock backend to support the model. The test case is coded as an acceptance test that will fail when it hits the issue, and succeed if changedAttribute behaves as expected.

See test case in https://github.com/timmorey/ember-data-changed-attrs/blob/main/tests/acceptance/model-changed-attributes-test.js. A basic summary:

const thing = await store.findRecord('thing', '1');
thing.name = 'new name';
// thing.changedAttributes() now returns { name: ... }
await thing.save();
// thing.changedAttributes() still returns { name: ... }

I'm also going to try to take a stab at coding a failing test in ember-data itself, but I've never done that before...

Description

The Model.changedAttributes() method continues to indicate that attributes have been changed after the changes to the model have been saved. In previous versions of ember-data (< 4.7.0), this method would indicate that there are no changed attributes after a save().

This is confounding some logic we have in another application, where we look at changedAttributes() in the adapter to determine how to save the changes.

Versions

I cannot reproduce in 4.6.3, but can in 4.7.0. Also validated that the issue is reproducible in 4.8.3 and 4.9.0-beta.3.

@runspired
Copy link
Contributor

a failing test would be great

@timmorey
Copy link
Contributor Author

timmorey commented Dec 6, 2022

It took me a few tries to figure out how to run the test suite, but I think I've got a failing test to work off now: #8335

runspired added a commit that referenced this issue Dec 7, 2022
…#8335)

* Add failing test for #8329

* fix lint

* fix: clear changes if no local mutations exist

Co-authored-by: Chris Thoburn <runspired@users.noreply.github.com>
@kiosion
Copy link
Contributor

kiosion commented Apr 12, 2023

Was this issue fixed? I can still reproduce it under v4.11 - modifying then saving a record has changedAttributes() showing the saved changes

@runspired
Copy link
Contributor

@kiosion it was. If we didn't backport it then its in 4.12 only.

@RosarioAleCali
Copy link

Hi @runspired, would it be possible to backport the fix for 4.8? It would be much appreciated, thanks!

@runspired runspired added 🏷️ bug This PR primarily fixes a reported issue and removed Bug labels Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bug This PR primarily fixes a reported issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants