Closed ginty closed 10 years ago
This is indeed a bug. Take a look at #22. It's an update rather than create there, but it's the same issue.
What do you guys think is the desired behavior here:
flush()
is successful before updating the parent.flush()
and rollback if it fails.flush()
call.One of the main reasons for keeping things isolated is to support cancel
behavior. If a flush
is called it becomes clear the intent is not to cancel.
@jasonkriss thoughts?
I think it should be either (1) or (2), and would prefer (1).
(3) is flawed because it allows errors from the child session to bleed into the main session which does not provide the level of encapsulation that you would expect from the child session
It should definitely be able to handle the conceptual case where a user tries to do something that saves to the server -> it then fails for whatever reason and gives the user feedback -> the user then decides to abandon the change.
I prefer (1) for the example case of a user updating their profile. Say they make some changes that are deemed to be invalid upon saving, the username is already taken for example, in this case you would not want the username attached to the main session to be updated to the new value.
Also by not updating the parent until the flush is successful it will mean that you can still cancel the change late in the day by simply walking away from the child session.
Thanks.
I tend to agree with @ginty for the most part here. Although, I guess not familiar enough with the internals yet to understand if there would be any advantages to (3). The example of a user changing their username only to find out it is already taken, is definitely the kind of thing that happens quite a bit. In that case, it's crucial to get back the original value. It's obvious how this would work in (1) and (2), but how would it work in (3)?
@ghempton are you leaning towards a particular solution?
I think I might ultimately support multiple isolation levels. I can see the argument for all 3 cases. The reason why #3 still appeals to me is that it allows for optimistically assuming the updates succeed (which I think is important). This also is related to @jasonkriss example in #51 when editing a profile inline.
I would like to propose a variation on (1). Since there is not an actual transaction, on flush(), only those records that are successfully persisted are moved to the parent session. Any records that failed persisting remain in the child session to either be fixed and persisted, or abandoned.
At any rate, I like (3) the least as it pollutes the parent session whenever a flush() fails. (2) just seems like a more complicated variation of (1).
Just my two cents.
This has since been cleared up. See https://github.com/GroupTalent/epf/issues/59
It has always seemed annoying to have to clear up un-saved models when leaving a create route and I thought that the child session capabilities for EPF would simplify this considerably.
e.g. with code like this the new object could be created upon entering the route, and if the route is left via any means other than the save action the child session and it's contained object would just get forgotten about and garbage collected.
However this doesn't work because every time the child is flushed the new record is put into the parent session before trying to persist it. If this fails then an unsaved record is left in the parent session and not isolated to the child as I would have expected.
Is this a bug or are my expectations wrong?
Thanks!