search

getoutreach / epf

A framework for keeping your Ember.js apps in sync.
http://epf.io
MIT License
369 stars 33 forks source link

model rollback functionality #58

Closed g13013 closed 9 years ago

g13013 commented 11 years ago

basic model rollback functionality

ghempton commented 11 years ago

This looks like a great start! Couple of comments:

  1. Could you rename rollbackModel to just rollback? In general, the Model suffix is only appended if the name of the method is a reserved javascript keyword (as is the case for delete).
  2. In response to your TODO comment, I would like to make rollback and flush both behave differently based on parameters:
    • If there are no args, operate on all dirty models in the session.
    • If there is one singular arg, operate just on that model.
    • If the arg is an enumerable, operate on each element in the array.
  3. Tests?

Thanks!

g13013 commented 11 years ago

Totally agree, i'll work on it and keep you informed!

ghempton commented 11 years ago

Cool, feel free to add me on gchat: ghempton@gmail.com if you want any help!

g13013 commented 11 years ago

I think i did a mistake, dirty models include new models, according to you what does make more sense, rollback all dirty models including new models and make the function undo all new changes to session, or make it behave only on shadowed models and keep new models ? for now i add a new commit that act only on shadowed models with more robust tests.

g13013 commented 11 years ago

sorry for the multi commits, I discovered epf/ed code a week ago so it took me a while to undestand how it works. Any comments?

ghempton commented 11 years ago

This looks great! A couple minor nitpicks/questions:

  1. We should remove from originals as well as shadows (this will actually not make a difference but might as well do a full cleanup.)
  2. What does rollback mean for new models?
g13013 commented 11 years ago

We should remove from originals as well as shadows (this will actually not make a difference but might as well do a full cleanup.)

I was wondering if the originals could do any difference if we remove them as well, thanks for the clarification.

What does rollback mean for new models?

For me rollback in case there is no arguments will only rollback dirty models except for the new models, as dirty models include new models, i wasn't sure if new models as they are marked in the session as 'dirty' have to be deleted, but this sounds more like a 'reset' behavior so i think there is 3 choices:

  1. We undo changes on all models and remove new once (reset behavior).
  2. We only undo changes on dirty non new models, and provide another method 'reset', witch will rollback models and delete new once.
  3. We rollback only dirty non new models and let the user manage the new models and delete them all manually if he need to do so.

to prevent confusion the behavior must be clear. personally i would chose option 2.

what do you think ?

ghempton commented 11 years ago

I just realized that we need to do a full rollback on relationships as well as attributes (right now it's just attrs). Once this happens, rolled back new records will effectively be "orphans" as they won't have relationships with any existing records. At this point maybe we should just detach them from the session? They could be re-used if the user did a session.add.

g13013 commented 11 years ago

I just realized that we need to do a full rollback on relationships as well as attributes (right now it's just attrs)

I need to read the code further more to understand how relationships are managed!

rolled back new records will effectively be "orphans" as they won't have relationships with any existing records. At this point maybe we should just detach them from the session? They could be re-used if the user did a session.add

sounds an even better idea, if I understood, when we rollback a model, its new added relationships have to be unlinked by detaching the session! right ? if so, this will solve a part of the new models problem, what about new models that do not have any parents / relationships? should we remove/detach them as well?

g13013 commented 11 years ago

Do you plan to merge when relationships are managed or is it ok, if no, can you explain to me please what is the rollback scenario for rolled back models? thanks.

ghempton commented 11 years ago

I think we should do it right with relationships. In the case of a new model I think we should just reset the state of the session as if the model wasn't there. In this case that means removing them from the session.

denisnazarov commented 10 years ago

Was wondering if any new progress has been made on the rollback functionality.