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

[16.0][FIX] base_multi_company: support in False search operators #760

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

yajo
Copy link
Member

@yajo yajo commented Jan 29, 2025

Before this commit, when doing a read with a domain such as the one used by upstream base.res_partner_rule, the read failed to find records without company.

This is now fixed in the base module, adding tested support for such operators. The only relevant part of the hooks that were provided to workaround the issue is extracted to a new post_init_hook. All other hooks are marked as deprecated.

Instructions for functional tests: This refactor is internal and should not be noticed functionally. The modules product_multi_company and partner_multi_company should work just as always. If so, then this is good.

@moduon MT-8863 MT-8873

@OCA-git-bot
Copy link
Contributor

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

yajo added a commit to moduon/multi-company that referenced this pull request Jan 29, 2025
Now that `base_multi_company` supports `('company_id', 'in', some_ids + [False])` domains since OCA#760, it's time to remove the (un)install hooks that changed those rules.

The fix is important when migrating a database from previous versions where these hooks didn't exist. In those cases, users wouldn't be able to browse some partners, getting a weird `AccessError`.

FWIW, [upstream rules changed for performance][2], so the uninstall hook had to be changed nevertheless.

[2]: odoo/odoo@a04df5c#diff-1f2cafa0e26df218210181c72e599c279fd093c4215d1dc1da5ef8875195e6c2L23-R23

@moduon MT-8863
@yajo yajo marked this pull request as draft January 29, 2025 11:45
@yajo yajo force-pushed the 16.0-base_multi_company-fix_in_false branch 3 times, most recently from 2805d27 to d34f488 Compare January 29, 2025 11:56
@yajo yajo marked this pull request as ready for review January 29, 2025 12:02
yajo added a commit to moduon/multi-company that referenced this pull request Jan 29, 2025
… company assignments

Remove lots of code thanks to OCA#760.

Also, call the new `fill_company_ids` hook, to respect company assignments done before the module installation.

@moduon MT-8863
@yajo yajo force-pushed the 16.0-base_multi_company-fix_in_false branch from d34f488 to 6875f5a Compare January 29, 2025 12:10
Copy link
Member

@edlopen edlopen left a comment

Choose a reason for hiding this comment

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

Code review here. LGTM!

Before this commit, when doing a read with a domain such as [the one used by upstream `base.res_partner_rule`][1], the read failed to find records without company.

This is now fixed in the base module, adding tested support for such operators. The only relevant part of the hooks that were provided to workaround the issue is extracted to a new `post_init_hook`. All other hooks are marked as deprecated.

[1]: https://github.com/odoo/odoo/blob/db072461cddced2a8f65a64fb6d2ddf0dd79b38e/odoo/addons/base/security/base_security.xml#L23

@moduon MT-8863 MT-8873
@yajo yajo force-pushed the 16.0-base_multi_company-fix_in_false branch from 6875f5a to aee7d28 Compare January 29, 2025 12:47
yajo added a commit to moduon/multi-company that referenced this pull request Jan 29, 2025
… company assignments

Remove lots of code thanks to OCA#760.

Also, call the new `fill_company_ids` hook, to respect company assignments done before the module installation.

@moduon MT-8873
Copy link

@loida-vm loida-vm left a comment

Choose a reason for hiding this comment

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

Functional review done.

I have verified that both in the Products module
imagen
and in Contacts
imagen
it is allowed to select the companies we want for sharing.

LGTM!
Thanks @yajo

@rafaelbn
Copy link
Member

This module is already migrated to 17 and the is a PR for 18.

@victoralmau @pedrobaeza @chienandalu could you review please? We can fordward port to all versions.

Thank you! 😄

Note Odoo 18 migration #708

@pedrobaeza pedrobaeza added this to the 16.0 milestone Feb 11, 2025
@pedrobaeza pedrobaeza changed the title [FIX] base_multi_company: support in False search operators [16.0][FIX] base_multi_company: support in False search operators Feb 11, 2025
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge major

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-760-by-pedrobaeza-bump-major, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit f6aa60a into OCA:16.0 Feb 11, 2025
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 93c447e. Thanks a lot for contributing to OCA. ❤️

@yajo yajo deleted the 16.0-base_multi_company-fix_in_false branch February 12, 2025 09:21
yajo added a commit to moduon/multi-company that referenced this pull request Feb 12, 2025
Now that `base_multi_company` supports `('company_id', 'in', some_ids + [False])` domains since OCA#760, it's time to remove the (un)install hooks that changed those rules.

The fix is important when migrating a database from previous versions where these hooks didn't exist. In those cases, users wouldn't be able to browse some partners, getting a weird `AccessError`.

FWIW, [upstream rules changed for performance][2], so the uninstall hook had to be changed nevertheless.

[2]: odoo/odoo@a04df5c#diff-1f2cafa0e26df218210181c72e599c279fd093c4215d1dc1da5ef8875195e6c2L23-R23

@moduon MT-8863
yajo added a commit to moduon/multi-company that referenced this pull request Feb 12, 2025
… company assignments

Remove lots of code thanks to OCA#760.

Also, call the new `fill_company_ids` hook, to respect company assignments done before the module installation.

@moduon MT-8873
@yajo
Copy link
Member Author

yajo commented Feb 12, 2025

Thanks everyone!

Now that this is merged, please review #761 and #762, which are the refactors for modules that implement this same behavior on partners and products respectively.

Just like here, the refactor is internal, and if modules just work as always, then it's OK.

yajo added a commit to moduon/multi-company that referenced this pull request Feb 14, 2025
… company assignments

Remove lots of code thanks to OCA#760.

Also, call the new `fill_company_ids` hook, to respect company assignments done before the module installation.

@moduon MT-8873
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants