better documentation of flow of save in admin
|Reported by:||Jonathan Morgan||Owned by:||nobody|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
It would be good to better document how save in the admin works. Specifically, when you save a model with M2M fields the admin does the following:
- stores the containing model instance in the database.
- if new rows in associated table are created, stores them in the database.
- clears all associated rows for the relation.
- re-adds all that are in the request.
- does not update the association objects that are stored in the instance of the object you are updating (so to look at results once all associations are added, you need to go to the database, can't look at the associations for the field in the containing instance).
When processing M2M, the admin does not compare what is in the request to what is in the database, remove those that have been removed, add any that are new, and ignore those that have not changed. Instead it clears all, then adds back all that are in request (not a bad way to do it in terms of performance). However, this causes the admin to never fire m2m_changed "remove" signals, and it causes the admin to always fire m2m_changed "add" signals, even when there are no changes to an association.
If this is the best way to do this for performance, OK, but it isn't intuitive (especially that the admin would be implemented in such a way as to just not emit a certain type of signal) and it took me a few hours of debugging to figure it out since I couldn't find these details documented anywhere.
For people using these hooks to get around problems they can't address in post-save overriding in the admin (which occurs after model instance is saved, but before M2M are processed), this will be confusing. And in terms of implementation best practices, "remove" signals could be fired in user-defined form processing code and so the signal is valid, but the documentation should describe how the admin works so one knows where the three pre-post pairs of signals occur there (or, in this case, so one can note that the "remove" pair of signals is never fired by the current implementation of the admin, and so you probably should implement a remove handler, but it won't get invoked in the admin, so you'll have to implement your "removed" handling in such a way that it doesn't actually rely on getting a "remove" signal and can be invoked by "add"s and "clear"s instead).