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 retrieved by findRecord does not update the client on delete #322

Open
kavika13 opened this issue Oct 15, 2015 · 5 comments
Open

Model retrieved by findRecord does not update the client on delete #322

kavika13 opened this issue Oct 15, 2015 · 5 comments

Comments

@kavika13
Copy link

If you retrieve a model by findRecord('something', someId) on one client, then delete that object through a different client, or on the firebase server itself, then the first client doesn't receive any update.

If you transition to a route that fetches that model, then the model is returned in its previous state to the client (since the local cache doesn't get updated).

If you tried to retrieve a model via findRecord('something', someId) on one client, and find it didn't exist (then return a null), then create that object through another client, then the first client doesn't receive an update that the object got created. This might be harder to deal with, because it is keeping around refs to objects that don't currently exist on the server simply because it fetched them. This is probably more useful in a compound ("hash") model.

There is a didDelete method on a model that I'd expect to be called when the object is deleted, and I'd expect the model instance to somehow be purged from the local cache. The challenge there is what to do if it gets recreated.

At the very least, I'd hope to handle the single item delete case so the client can dynamically respond (ask to create it again, do a redirect, post a warning, whatever is necessary).

I think this could be rectified by using 'on' instead of 'once' for the object when fetching it, but I am concerned about E2E expectations on behavior here, and especially in dealing with leaking references. I don't have enough experience hacking on a data adapter to know that I got my changes right, or what the impact could be on existing clients.

I think this can be worked around by doing a query for the object by key, since the list/query hooks work well, but this does lead to a clunky implementation when you're really trying to implement single-item data access. It also wouldn't work if you're trying to restrict read access to sibling objects.

I think you can also work around it by calling the underlying ref's on hook: model.ref().on('value', function() { ... });. I saw this recommended as a work-around for Firebase's transaction support on another bug. This seems like it shouldn't be a permanent solution, though, because unlike transaction, Ember Data has an interface to support notifications and updating on delete.

@kavika13 kavika13 changed the title Model retrieved by findRecord does not update the client Model retrieved by findRecord does not update the client on delete Oct 15, 2015
@tstirrat
Copy link
Contributor

tstirrat commented Dec 1, 2015

This is related to #145 in how we handle deleted records. We need to unload them on delete, but there is a problem if the local record has changes, too.

@tstirrat
Copy link
Contributor

tstirrat commented Dec 1, 2015

Actually, sorry, this is slightly different. reopening so that I can track it.

@tstirrat tstirrat reopened this Dec 1, 2015
@cs3b
Copy link

cs3b commented Feb 8, 2016

on v1.6.4 this is still a problem - how it is possible to handle it, where to start?

@gabrielgrant
Copy link
Contributor

I think this bug description is slightly inaccurate: the record is updated on the client, but it isn't removed -- it remains in the store, with isDeletedand hasDirtyttribues set to true.

@tstirrat I think you're right that this is caused by the same root problem as #145 -- when a record is deleted in FB, the corresponding record should be unloaded from the ED store instead of deleted. Even with that, though, I'm not sure the didDelete event @kavika13 is asking for would fire.

@cs3b The quick-fix workaround is to filter your records to hide deleted records, either using the ember-data-live-filter addon or, if you don't want the addon, as described here

For more background, see the blog post about this change is ember-data 2.0

@kavika13
Copy link
Author

kavika13 commented Jul 18, 2016

when a record is deleted in FB, the corresponding record should be unloaded from the ED store instead of deleted. Even with that, though, I'm not sure the didDelete event @kavika13 is asking for would fire.

Anything I was stating above was just me trying to be thorough in reporting my "expected behavior" vs "actual behavior", so you could see the crux of the problem. I leave it up to you guys if it makes sense for didDelete to fire in this case.

Re the Ember Data 2.0 blog post -

Having flexibility to allow local changes to trump remote changes (but inform the user they're doing so) is great. Whatever the final solution for this issue, I'd also like simpler workflows (peers win, especially if you haven't edited anything locally) to be simple and painless to code. The more off-path I have to go to make Firebase "do its thing", the less interested I'll be in Ember + Firebase for rapid prototyping (which is half the reason I'm interested in FB to begin with). I can't really tell at this point if this is different from the current plan or implementation without digging in more, but I thought I'd chime in naively with my preference anyhow :) Thanks in advance for humoring me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants