Make DS.Model reload-able #546

Closed
wants to merge 2 commits into from

2 participants

@darthdeus
Ember.js member

This is a first step of making models able to expire and be updated from the server, as mentioned in #545

I'm mostly looking for a feedback, since this is my first contribution to ember. I don't want to merge this yet.

darthdeus added some commits Dec 22, 2012
@darthdeus darthdeus Add .reload() to DS.Model
This is a first step of making models able to expire
and be updated from the server.
60e8c9d
@darthdeus darthdeus Add tests for DS.Model#reload() ae67630
@darthdeus
Ember.js member

Just now I realized the test never fails ... in my manual testing I was using RESTAdapter and updaing the data server side, and using adapter.load to force a new request which reloads the data. I'm not sure how to simulate that in the test thought.

@darthdeus
Ember.js member

After a few days spent digging around the source code, I realized that I probably won't be able to implement the whole #545 TTL thing.

On the other hand, I found the reload() method very useful in my development, so I'd like to merge this.

If someone could provide some feedback that would be great :)

@tchak
Ember.js member

The reloading was discussed a few times. One of the concerns is data merging. What should happen if there is conflicts between current client state and server state? There is some work planed on data merging facility, we could expect reloading to land at the same moment.

@darthdeus
Ember.js member

@tchak If you're intentionally reloading, then it should replace all of the data stored on the client side, because you are basically saying "I want the fresh copy of XYZ".

For example if you have a news block in a footer, it's ok to just reload the data and throw the old copy away, because the only reason why you're doing it is to get the new data.

@tchak
Ember.js member

This is what is happening currently. Except that at the time you reloading a user can be editing your record, what will you do in this case? And there is a use case for partial data loading.

@darthdeus
Ember.js member

Yes that is tricky, but you can either chose not to reload when a user is editing, or just reload in the case when this just can't happen.

This reload() is supposed to be only for read only data which changes often or externaly. Nothing more than a syntactic sugar.

@tchak
Ember.js member

@darthdeus If all you want is some "sugar", your current solution is juste fine. Most of us, have a similar implementation in our app. But for inclusion in core, a reload mechanism must have all the bells and whistles. We can not juste throw in a solution that will work in "some" cases. Most developers do not know, do not want to know and should not have to know what they are doing :)

We also should have a way to mark the record as reloading.

@darthdeus
Ember.js member

@tchak Ok let's close this and keep the discussion in the main issue #545. It is still an issue, but this implementation isn't helping.

@darthdeus darthdeus closed this Jan 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment