Skip to content

Conversation

@grindtildeath
Copy link
Contributor

In case a implementation module, eg sale_exception, breaks a function override, ie does not call super on sale.order.action_confirm in case an exception rule applies, it is still possible that another module that is not in the implementation module's dependency, does modify existing the same object through the same function's override that is called before through MRO.

As the exception rule is only written in base_exception module, such modifications would be still be committed although the exception was triggered.

The only way to rollback everything that happened is to raise an exception that is going to rollback the transaction. However, we still want to commit the write of the exception rule and not to propagate the exception back to the webclient.

Since we cannot alter the env in the retrying function (that handles rollback), we do not have any other choice than to use a dedicated cursor in the dispatching function to process the request, since it could trigger the exception rule, and to use the original env to write the exception rule, and have the webclient being refreshed to display the exception.

This might break the latest feature to pop up the wizard, but since this is still a POC, we could fix that afterwards in case this POC is accepted.

In case a implementation module, eg sale_exception, breaks a function
override, ie does not call super on sale.order.action_confirm in
case an exception rule applies, it is still possible that another
module that is not in the implementation module's dependency, does
modify existing the same object through the same function's override
that is called before through MRO.

As the exception rule is only written in base_exception module,
such modifications would be still be committed although the exception
was triggered.

The only way to rollback everything that happened is to raise an exception
that is going to rollback the transaction. However, we still want to
commit the write of the exception rule and not to propagate the exception
back to the webclient.

Since we cannot alter the env in the retrying function (that handles
rollback), we do not have any other choice than to use a dedicated cursor
in the dispatching function to process the request, since it could trigger
the exception rule, and to use the original env to write the exception
rule, and have the webclient being refreshed to display the exception.

This might break the latest feature to pop up the wizard, but since
this is still a POC, we could fix that afterwards in case this POC
is accepted.
@OCA-git-bot
Copy link
Contributor

Hi @sebastienbeau, @hparfr,
some modules you are maintaining are being modified, check this out!

Comment on lines +34 to +37
for rule_id, (model, res_ids) in to_add.items():
old_env[model].browse(res_ids).write({"exception_ids": [(4, rule_id)]})
for rule_id, (model, res_ids) in to_remove.items():
old_env[model].browse(res_ids).write({"exception_ids": [(4, rule_id)]})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beware:

  1. in the old behavior, old rules were removed before new ones got added, while in here you do the opposite: is this a wanted change?
  2. the command value should be 3 when removing rules, 4 when adding them, while in here you're always using 4: can you please switch to Command.[un]link(ID) instead to make it easier to read and debug (and also, possibly add tests)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants