Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

15.0 mig base exception fix #2319

Closed
wants to merge 60 commits into from

Conversation

damdam-s
Copy link
Member

tentative to fix #2279

@damdam-s damdam-s force-pushed the 15.0-mig-base_exception_adhoc_fix branch 2 times, most recently from 9877b58 to 7150145 Compare March 23, 2022 09:56
mourad-ehm and others added 28 commits March 23, 2022 15:06
* Fix menu in base_exception

* Fix base_exception/views/base_exception_view.xml
…lled by constraint methods

'detect_exception' can be called on an empty recordset.
Defines a new mechanism to build odoo classes that are only
defined during testing.
- Show menu only to Exception Rule Managers
- Use sequence and active widgets on tree view
- Updated form to use sheet
improve the perfs dramastically when there is a lot of records
No more empty fields after changing type.
Add a note about safe_eval
[IMP] Computed exception descriptions field, to display better help messages

[IMP] Exceptions shouldn't be copied
This recently added feature is counter intuitive, error prone and is
already causing bugs in sale_workflow.
The goal of the modified method is to create or remove the relationship
(in the M2m relation tabel) between the tested model (such as
sale_order) and the exception rules. When the ORM writes on
ExceptionRule.sale_ids (using the example of sale_exception), it will
first proceeds with these updates:

* an UPDATE on exception_rule to set the write_date
* INSERT or DELETE on the relation table
* but then, as "write" is called on the exception rule, the ORM will
  trigger the api.depends to recompute all the "main_exception_ids"
  of the records (sales, ...) related to it, leading to an UPDATE
  for each sale order

We end up with RowExclusiveLock on such records:

* All the records of the relation table added / deleted for the current
sale order
* All the records of exception_rule matching the current sale order
* All the records of sale_order related to the exception rules matching
the current sale order

The first one is expected, the next 2 are not. We can remove the lock on
the exception_rule table by removing `_log_access`, however in any case,
the main_exception_ids computed field will continue to lock many sale
orders, effectively preventing 2 sales orders with the same exception
to be confirmed at the same time.

Reversing the write by writing on SaleOrder instead of ExceptionRule
fixes the 2 unexpected locks. It should not result in more queries: the
"to remove" part generates a DELETE on the relation table for the rule
to remove and the "to add" part generates an INSERT for the rule to add,
both will be exactly the same in both cases.

Related to OCA#1642
Replaces OCA#1638
JordiBForgeFlow and others added 12 commits March 23, 2022 15:06
By default exceptions can be ignored by the click of a button.
Users begin human they will just click that button what ever
the internal rules.

So this PR adds the option to set specific exceptions as blocking.

When exceptions are detected if one of them is blocking, the user
will not be able to ignore them.
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-tools-14.0/server-tools-14.0-base_exception
Translate-URL: https://translation.odoo-community.org/projects/server-tools-14-0/server-tools-14-0-base_exception/
@damdam-s damdam-s force-pushed the 15.0-mig-base_exception_adhoc_fix branch from fafedc2 to 58d1b96 Compare March 23, 2022 14:06
@damdam-s
Copy link
Member Author

superseeds #2279

splits base_exception module in 2 modules:

  • base_exception for the behaviour
  • test_base_exception for the tests

I did that because model loading "on the fly" does not work

Copy link
Contributor

@nicomacr nicomacr left a comment

Choose a reason for hiding this comment

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

LGTM

@augusto-weiss augusto-weiss mentioned this pull request May 3, 2022
54 tasks
@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@mylbco
Copy link

mylbco commented May 31, 2022

When will this be merged ?

super(TestBaseException, cls).setUpClass()
cls.loader = FakeModelLoader(cls.env, cls.__module__)
cls.loader.backup_registry()

Copy link
Contributor

Choose a reason for hiding this comment

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

do not remove tests

)

cls.exception_rule._fields["model"].selection.append(
("base.exception.test.purchase.line", "Purchase Order Line")
Copy link
Contributor

Choose a reason for hiding this comment

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

do not remove tests

Copy link
Contributor

Choose a reason for hiding this comment

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

I did a new PR #2366 that solves this problem

@pedrobaeza
Copy link
Member

Superseded by #2366

@pedrobaeza pedrobaeza closed this Jul 12, 2022
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.