-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
Changes to fragment array are not present in the owner record's changedAttributes #330
Comments
Names on person are changed from |
I'm seeing a similar issue with my fragmentArrays. I make a change to one of the fragments in the fragmentArray and the parent model will return |
Try with the new beta version I think it might fix your issues 😉 |
I actually do have the same issue than @mattgraham1995 on version |
I have the same "bug" : make a change in a fragmentArray (add or remove an item), I find record._internalModel._recordData.setDirtyAttribute(key, fragment); But in In - record._internalModel._recordData._attributes[key] = fragment;
-
+ record._internalModel._recordData.setDirtyAttribute(key, fragment); If I revert this change, the bug is resolved... I think the ember-data @cohitre What do you think about it? Perhaps should we revert this change? If OK, I can make a PR. |
An Ember Twiddle to see the bug: https://ember-twiddle.com/93d84d0a212054ff2a0fc146b27917ae |
I have a fix-issue-330 branch in my repo but I can not PR to this repo because I need a "release-4.0.0" branch here to PR into it. Thanks. |
@jakesjews by any chance, do you know why the "release-4.0.0" branch has disappeared? I need the fix discussed above to be merged, as well |
This is a temporary patch
No news ? I try a new call :) @jakesjews @araddon @cohitre @markhayden. Is it possible to create a "release-4.0.0" branch to PR a fix about this issue? Thanks. |
If you need the changes right now your best bet is making a fork and branch from commit f1c7060 |
Hello @jakesjews.
Yes I read about adopting this addon. It's a good news. But does the procedure block bugfixes on this repository?
That's what I did :). But wouldn't it be better to centralize bugfixes here? Regards, |
Yes it would but there is some more to the problem as well. I can't actually merge anything right now. I don't really have much time to work through all the stuff hence moving the repo to adopted-addons. |
Hello, It seems the migration to adopted has been made. Really good 👍 . Maybe can I give you my fix for this issue? Would it be possible to create a release-4.0.0 branch so that I can push a PR? Thank you. |
You can submit a PR against the master branch, no need for another branch. |
Hello, Thank you for you message. This PR is for v4.0.0 not 5.0. See https://github.com/mpirio/ember-data-model-fragments/tree/fix-issue-330 |
changedAttributes
is not returning changes in fragment array on the model.Ember version used 3.8
Addon version used is v4.0.0
I forked this addon(v4.0.0) and added a unit test case to verify the issue. The test case is not complete, I asserted owner record's changedAttributes with empty string to know what value is returned.
Value returned by changedAttributes is not appropriate. Old property is a fragment object. But new property is a simple object.
You can find the corresponding test case in nhemanth007@44ad92d
names
is a fragment array on person model. String version of data returned byperson.changedAttributes().names
is[{"type":"name","options":{},"name":"names","owner":{"title":null,"nickName":null,"name":null,"names":[{"first":"Cat","last":"Stark"}],"addresses":[],"titles":[],"hobbies":null,"houses":[],"children":[],"strings":[],"numbers":[],"booleans":[]},"_objectsDirtyIndex":-1,"_objects":[{"first":"Cat","last":"Stark"}],"_lengthDirty":false,"_length":1,"_arrangedContent":[{"first":"Cat","last":"Stark"}],"_originalState":[{"first":"Cat","last":"Stark"}]},{"first":"Cat","last":"Stark"}]
The text was updated successfully, but these errors were encountered: