Access to an instance during create/update_model about flask-admin

[baverman]

Closed by 95ee8eb for #64

[smlynch]

I ran into an issue trying to set a required property on my model using the new on_model_change callback because self.session.flush() is called before it. What is the reason an explicit flush is called before the callback, and can it either be removed or moved to after the callback?

[mrjoes]

You're right - you should have chance to modify something in the callback.

[techniq]

Great, thanks (I would have submitted a pull request, but wasn't sure the reasoning behind the flush). Is the explicit flush even needed here?

[baverman]

Is the explicit flush even needed here?

Yeah. That's the main point for the whole callback. After flush model has an "id" and one can handle update/create cases in generic way. For example you can fill depended models with created foreign key.

[baverman]

I ran into an issue trying to set a required property on my model using the new on_model_change callback because self.session.flush() is called before it.

Use create/update_model.

[techniq]

Hmm, I guess this conflicts with my intended use of on_model_change for setting other required fields on the model not by the form before committing

If I override create_model/update_model, I would have to completely override it's implementation (copy/paste).

Maybe instead of 1 callback/event (on_model_change), there is before_model_change and after_model_change. Since this hasn't been part of a release yet, it wouldn't break compatibility (except for us running off git).

[mrjoes]

I think we should omit flushing for these callbacks: there are cases when you need to modify model before it will be saved. And for some other cases it will work without explicit flushing.

SQLAlchemy is smart enough to flush dependencies in most of the cases: https://gist.github.com/3941123

However, if you will try to access user.id without flushing, it will be set to None.

I think it is OK, as new version of flask-admin is not yet released and it is possible to mention in documentation that callback happens before model is comitted, so if you need id in on_model_change, flush session.

Adding another on_before_model_change does not look pretty to me.

Thoughts?

[baverman]

Sorry, I forgot that overriding of create_model have no any sense.

Maybe instead of 1 callback/event (on_model_change), there is before_model_change and after_model_change

Boo. Very bad idea.

I agree. Removing flush is the best solution. Clients still can call it by themselves.

[techniq]

I agree as well.