data icon indicating copy to clipboard operation
data copied to clipboard

pushPayload does not change hasMany relationship when going to empty state

Open danielspaniel opened this issue 8 years ago • 8 comments

I have a model which hasMany things, then I pushPayload with data that says .. I have no things anymore. Ember data ignores that and keeps the hasMany things.

The key activities are that:

  • I have a model with no things
  • then I set some things model.set('things', things)
  • then I pushPayload to remove them.

Here is a twiddle to show the problem:

https://ember-twiddle.com/26204b7fa7fc185dde464418cd33df95?openFiles=routes.application.js%2C

danielspaniel avatar Feb 15 '17 21:02 danielspaniel

@danielspaniel https://ember-twiddle.com/53d305379774c4756d576107c98ae6d4?openFiles=routes.application.js%2C

In the above twiddle you can comment / uncomment line 48 (ie the this.store.pushPayload line) to see things either cleared or not cleared.

There are two changes as compared to your original twiddle.

  1. the initial relationships data is populated, the relationships are not merely sieloaded in the included payload
  2. the manyarray itself is used and not overwritten by the live array from peekAll (ie the person.set('things', things) line is commented).

Please let me know if I have misunderstood the issue your twiddle aimed to highlight, but I believe the core error in the above was in the payload and not an ember data bug.

hjdivad avatar Feb 16 '17 03:02 hjdivad

The issue with the first twiddle is that the relationship is being added as "dirty state", it won't be cleared because no save attempt has been made (so obviously a server update would not know about it).

runspired avatar Feb 16 '17 03:02 runspired

The reason I flag this as a bug is because even in a dirty state if you pushPayload with values for the hasMany then the relationship will update itself to add those new rows. By your logic in a "dirty state" relationship pushing data to a relationship would always have no affect.

Here is a gist to show what I mean: https://ember-twiddle.com/983bf2f8ac50eaf7c5f1ffd3bdc6839b?openFiles=routes.application.js%2C

But I know what your going to say ... Hey Dan, you see, the start state is empty, you set some things, then pushPayload tries to clear them, but since this hasMany does not "know" about those 2 new things, it can't remove them.

2 flaws in that argument. The biggest flaw: pushPayload resets the state of the model to root.loaded.saved, so in essence you now can not even save those 2 new things, because the model thinks. Ok I am all good.

Also, the belongsTo relationship is happy to clear any "dirty state" ( new model ) you set when you pushPayload.

So, it seems like pushPayload is just not quite 100% clear on it's mission for what to do with hasMany because usually pushPayload just sets the state to what you pushed, no matter what state your in .. no questions asked, and resets the state of the model to root.loaded.saved. It is a way to update the model immediately to a new state, and call it saved. Which works for 95% of cases except the hasMany.

danielspaniel avatar Feb 16 '17 11:02 danielspaniel

Any news here?

Exelord avatar Mar 16 '17 19:03 Exelord

@Exelord mostly fixed. There's one case for "current vs canonical" where we get it wrong that has not been fixed which is this test PR: https://github.com/emberjs/data/pull/5269 and is the case mentioned by @danielspaniel in his last comment. Once the RecordData RFC lands I will be doing a pass through the relationship layer making things nice.

runspired avatar Apr 19 '18 07:04 runspired

I believe #4852 improves this situation, once that lands I will add more tests and hopefully this issue will be resolved :)

runspired avatar Apr 22 '18 20:04 runspired

I ran into the same issue with ember-data Version 3.8: empty relationship arrays in the payload didn't clear the relationship. What is the state of this issue?

Dirk-27 avatar Jun 04 '19 08:06 Dirk-27

I have the same issue

wuarmin avatar Jul 13 '20 14:07 wuarmin

@Dirk-27 the state of this issue is there is nothing more we can do for push api's. It is equally incorrect to discard local mutations as it is to not discard them: it just boils down to what your app wants.

While not explicitly enabled by the new cache design in emberjs/rfcs#854 once @ember-data/graph is made public rollback will be a public API. Custom cache implementations can also choose today to do one behavior vs the other.

runspired avatar Dec 07 '22 18:12 runspired

Also closing this as at this point this request is more about relationship-rollback than it is about updating the state. The store forking behavior introduced in emberjs/rfcs#854 allows applications to better choose whether to display remote or mutated state as desired, so rollback and this issue are dead artifacts of a previous era of APIs.

runspired avatar Dec 07 '22 18:12 runspired