apply some modification to the values used for ModelStorage.create and ModelStorage.write (ex: standardize the format of email, put in uppercase the country code, set number from a sequence etc.)
trigger an action on modified records (ex: clear a cache, call workflow transition of linked record etc.)
check if record can be modified (ex: forbid modification of posted move)
Proposal
I propose to add 3 new hooks in ModelStorage:
preprocess_values(cls, mode, values) which is called by ModelStorage.create and ModelStorage.write with each values and must return the modified value (preferably a copy if modified).
on_modification(cls, mode, records, field_names=None) which is called by ModelStorage.create, ModelStorage.write at the end and by ModelStorage.delete at the beginning. The field_names contains at least all the fields that have been modified.
check_modification(cls, records, values=None) which is called only by ModelStorage.write and ModelStorage.delete at the beginning.
I’m missing one hook that allows to trigger a method on related records once the records have been deleted.
For example on AccountMoveLine.delete we need to call Move.validate_move on the linked moves but once the lines have been deleted.
So I propose to add another hook on ModelStorage:
on_delete(cls, records) which is called at the beginning of ModelStorage.delete and returns a list of methods (aka callbacks). The methods are called at the end of ModelStorage.delete.
We also need to the same when such trigger must be called on write:
on_write(cls, records, values) which is called at the beginning of ModelStorage.write and returns a list of methods. The methods are called at the end of ModelStorage.write.
The design is complete but I have updated only ir, res and some account* modules.
I had for now to keep just 2 create extension in ActionMixin (because of the same id hack) and account.move.line (because of the creation on the fly of moves).
We have some checks that are only to be enforced for external user and other checks that should always be enforced for data integrity.
So I think we should follow the proposal of Verify access right only for RPC and have this API:
check_modification(cls, records, values=None, external=False) which is called without check access but external is set to True when check access is True.
Converting modules to this pattern, I found that I have to use on_modification to check the creation of records and raise AccessError.
This does not feel right. on_modification should be use to trigger same kind of callbacks and check_modification should be responsible to raise AccessError.
I have a final issue with the pattern, that we have applied a few times, of cancelling records before delete.
I wanted to do the cancel call in on_modification because it is a kind of “trigger” and checking the state in check_modification.
The problem is that check_modification is called before on_modification when deleting as it is obvious that we want to ensure we have the right to delete before triggering other events.
I could not find a good design for this case because doing the cancellation in check_modification is wrong as it is not a check and more over such transition will trigger a write during the delete which seems odd.
Inverting the order between check_modification and on_modification for deletion will create a different behavior than the other methods and again it will be odd to make write during the deletion.
So I came to the conclusion that it should be better to have the user explicitly cancel the record before deleting it.