[12.0] migrage base multi company + improve performance #123

florian-dacosta · 2019-05-13T15:41:23Z

Standard migration + improve performance applying idea proposed by @sbidoul in #109

* Create new module to provide base multi company logic and mixin * Add deactivation by company mixin * Add company_id/ids handling * Add break after company is found Squashed commits: [854cc36] Increase test coverage [770bd71] Revert hook view create back to model init [40e803e] Fix apples and oranges [7a4dfb4] Use registry correctly [6e9f170] Switch company_id to computed & move company aliased view creation to post init hook [faa4fc9] Remove active functionality [fecfb59] Add explicit tests for active and inactive searches

* Revert "Revert hook view create back to model init" This reverts commit 770bd71. * [FIX] base_multi_company: Always create the view into a pre_init_hook to avoid error in log

Squashed commits: [4c17d04] auto_join company_ids

…ch domain on company_id/company_ids (+1 squashed commit) Squashed commits: [fe161fe] [ADD] setup.py

- Test if a company is set in inverse method (+1 squashed commit) Squashed commits: [d670f30] [FIX] fix init hooks as company_id is not stored anymore

* Add implementation instructions to ReadMe

[FIX] Fix issue based on the computation of company_id for multi_company_abstract models [FIX] Fix issue based on the computation of company_id for multi_company_abstract models [FIX] Fix issue based on the computation of company_id for multi_company_abstract models [FIX] Bad branch version pushed before [FIX] Bad branch version pushed before [FIX] manifest Fix flake Flake8 on OCA tests

… are bound to some company

Currently translated at 88.9% (8 of 9 strings) Translation: multi-company-10.0/multi-company-10.0-base_multi_company Translate-URL: https://translation.odoo-community.org/projects/multi-company-10-0/multi-company-10-0-base_multi_company/pt/

Odoo now checks if the user who creates the transient model is the same that is accessing its data, so we need to create it with the same user.

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

* Abstract model shouldn't have default for `company_ids`. It will be got by default `company_id` value. * Record rule based on `company_id` makes the evaluation slower. Switch to `company_ids` field. * Add `@api.depends` to `company_id` computation for proper refresh. * Fix tests that were not correct but luckily previously suceeded. Now that other code has been removed, they have been uncovered.

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

tbaden

please use the fragment readme

florian-dacosta · 2019-05-14T09:14:05Z

@tbaden There is no "Implementation" section and I don't really see a suitable section in the readme files for this part. Any idea?

pedrobaeza · 2019-05-14T09:21:40Z

Put it inside usage section with ~~~~ underlining.

florian-dacosta · 2019-05-14T09:29:09Z

@pedrobaeza Thanks for the hint, I'll do that

florian-dacosta · 2019-05-16T20:51:07Z

@tbaden Readme has been split.

tbaden

LGTM 👍

LoisRForgeFlow

LGTM

OCA-git-bot · 2019-05-22T10:00:03Z

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). 🤖

kittiu · 2019-06-18T10:28:41Z

base_multi_company/hooks.py

+        return
+    rule.write({
+        'active': True,
+        'domain_force': (


@florian-dacosta I am doing #136 which has error from this rule writing. In partner_multi_company, it will error when I create a new company which also create res.partner.
Can you change to following, because normally we need to have term "or company_id = False" to avoid that error.

Suggested change

'domain_force': (

'domain_force': (

"['|', '|', ('company_ids', 'in', user.company_id.ids),"

" ('visible_for_all_companies', '=', True), ('company_id', '=', False)]"

),

I think now that a better technique is to duplicate the rule, and deactivate the old one, and on uninstall, delete the new one, and activate the old.

Allow me to confirm my understanding,
The domain_force in this base_multi_company should stay like this, and the changes per mentioned goes to partner_multi_company, correct?

Also, this one can be merged soon to fix dependency? :)

@pedrobaeza I think the problem is not the modification of the rule, but the value in domain_force that raise an error when it should not, in a specific case.
It is True that it could be cleaner to do it this way (activate/deactivate) but it won't change this particular problem.

@kittiu I checked a little, I am not sure why it fails exactly, and only in this particular case.
It seems the visible_for_all_companies field is computed too late...
Anyway, I don't think that changing the domain for all models/modules is the good solution.
Indeed, the changes have been made to improve the poor performance of this rule, and adding ('company_id', '=', False) once again revert the performance improvments.

So the domain could eventually be changed in partner_multi_company, to fix this very particular problem, but it will have an impact on performance.

Another ideal which could be implemented in this base module is to add a default value True to visible_for_all_companies field.
This way, the record rule won't fail during creation if visible_for_all_companies is not computed yet.
And if it is computed, it will get the right value...
@pedrobaeza any comment about this?

@florian-dacosta as you mentioned about default=True, I try add 2 lines in your abstract class, and it works fine.

@kittiu Actually the second line is not necessary.
I made the change

florian-dacosta · 2019-06-19T09:00:48Z

Test are failing, but it seems it is not because of this module.
Still, I don't understand why...

kittiu · 2019-06-24T05:36:06Z

@florian-dacosta I also can't find the exact problem. But I found that, if we run test_multi_company_abstract, we will get this error,

2019-06-24 05:34:21,915 5396 CRITICAL odoo12_base_multi_company odoo.service.server: Failed to initialize database `odoo12_base_multi_company`. 
Traceback (most recent call last):
  File "/home/kittiu/workspace/odoo12/odoo/service/server.py", line 1116, in preload_registries
    registry = Registry.new(dbname, update_module=update_module)
  File "/home/kittiu/workspace/odoo12/odoo/modules/registry.py", line 86, in new
    odoo.modules.load_modules(registry._db, force_demo, status, update_module)
  File "/home/kittiu/workspace/odoo12/odoo/modules/loading.py", line 497, in load_modules
    registry.init_models(cr, list(models_to_check), {'models_to_check': True})
  File "/home/kittiu/workspace/odoo12/odoo/modules/registry.py", line 301, in init_models
    func()
  File "/home/kittiu/workspace/odoo12/odoo/models.py", line 281, in _reflect
    self.env['ir.model']._reflect_model(self)
  File "/home/kittiu/workspace/odoo12/odoo/addons/base/models/ir_model.py", line 274, in _reflect_model
    ids = query_update(cr, self._table, params, ['model'])
  File "/home/kittiu/workspace/odoo12/odoo/addons/base/models/ir_model.py", line 65, in query_update
    cr.execute(query, values)
  File "/home/kittiu/workspace/odoo12/odoo/sql_db.py", line 147, in wrapper
    raise psycopg2.OperationalError(msg)
psycopg2.OperationalError: Unable to use a closed cursor.

florian-dacosta · 2019-07-06T14:08:26Z

Well I was wrong, the failing was really because of this module.
I did not meet any error in local using pytests, that's why I did not see it.
Anyway, I have fixed the issue, I think this PR is now ready to merge.
@kittiu Could do make a review?

kittiu · 2019-07-07T04:42:21Z

@florian-dacosta thanks for the fix. Now it works, and also works with partner_multi_company too #136

florian-dacosta · 2019-09-09T13:07:44Z

@kittiu
Since you seem satisfied with the migration ,could you approve the PR?

kittiu · 2019-09-10T03:52:46Z

@florian-dacosta how to approve, I don't have rights :p

florian-dacosta · 2019-09-10T08:44:06Z

@kittiu Of course you have!
If you click on "file changed", on the top of the PR, to see the changes made in the PR, you will see a button named "Review changes".
Then you can comment, approve, or request changes, depending if you think it is all good or not.

florian-dacosta · 2019-09-19T16:36:11Z

@pedrobaeza Could you merge this?

pedrobaeza · 2019-09-20T13:36:35Z

/ocabot merge

OCA-git-bot · 2019-09-20T13:36:48Z

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-123-by-pedrobaeza-bump-no, awaiting test results.

Signed-off-by pedrobaeza

OCA-git-bot · 2019-09-20T14:03:35Z

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

amkarthik · 2020-08-31T11:14:22Z

@pedrobaeza @florian-dacosta
It seems that this fix is missing v13 branch
Any specific reason for that?

pedrobaeza · 2020-08-31T11:15:43Z

This is not a fix, but a migration PR. Anyway, you can do a PR with anything you consider for improving the module.

amkarthik · 2020-08-31T11:18:08Z

@pedrobaeza
Thanks

lasley and others added 23 commits May 4, 2019 15:59

10.0 base multi company lmi (#1)

91ce90f

* Revert "Revert hook view create back to model init" This reverts commit 770bd71. * [FIX] base_multi_company: Always create the view into a pre_init_hook to avoid error in log

PR fixes (+1 squashed commit)

21c7c41

Squashed commits: [4c17d04] auto_join company_ids

[FIX] base_multi_company: Allow child_of/parent_of operator into sear…

737dec5

…ch domain on company_id/company_ids (+1 squashed commit) Squashed commits: [fe161fe] [ADD] setup.py

- Remove depends as company_id is not stored anymore

123cb62

- Test if a company is set in inverse method (+1 squashed commit) Squashed commits: [d670f30] [FIX] fix init hooks as company_id is not stored anymore

OCA Transbot updated translations from Transifex

1b854f5

[IMP] base_multi_company: Improve ReadMe

0787cdb

* Add implementation instructions to ReadMe

[MIG] *_multi_company: OpenUpgrade migration scripts

1fd4688

[FIX] Insert in company_ids relation's table only ids of records that…

da5f3bf

… are bound to some company

[UPD] Update base_multi_company.pot

952faa2

Translated using Weblate (Portuguese)

dbfbfd2

Currently translated at 88.9% (8 of 9 strings) Translation: multi-company-10.0/multi-company-10.0-base_multi_company Translate-URL: https://translation.odoo-community.org/projects/multi-company-10-0/multi-company-10-0-base_multi_company/pt/

[MIG] base_multi_company: Migration to 11.0

81f46a4

[UPD] Update base_multi_company.pot

5c50b39

[FIX] base_multi_company: Fix tests

bcfee2c

Odoo now checks if the user who creates the transient model is the same that is accessing its data, so we need to create it with the same user.

Update translation files

9cee705

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

[UPD] Update base_multi_company.pot

b9ba8d0

Update translation files

432acd1

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

[MIG] partner_multi_company: Finish migration to 11.0

ad34fea

[UPD] Update base_multi_company.pot

1f1c34c

Update translation files

a92fac9

Updated by Update PO files to match POT (msgmerge) hook in Weblate.

Migrate base_multi_company to v12

a9a8441

florian-dacosta mentioned this pull request May 13, 2019

[12.0] migrate product_multi_company #124

Merged

pedrobaeza added this to the 12.0 milestone May 13, 2019

OCA-git-bot mentioned this pull request May 13, 2019

Migration to version 12.0 #112

Closed

9 tasks

tbaden suggested changes May 13, 2019

View reviewed changes

tbaden approved these changes May 17, 2019

View reviewed changes

kittiu mentioned this pull request May 19, 2019

[12.0][MIG] partner_multi_company #127

Closed

LoisRForgeFlow approved these changes May 22, 2019

View reviewed changes

OCA-git-bot added approved ready to merge labels May 22, 2019

kittiu reviewed Jun 18, 2019

View reviewed changes

kittiu mentioned this pull request Jun 18, 2019

[12.0][MIG] partner_multi_company #136

Merged

1 task

Improve performance adding a techical field used in ir rules

2be4f5a

florian-dacosta force-pushed the 12.0-mig-base_multi_company branch from fc8ab72 to 2be4f5a Compare June 19, 2019 08:33

fixup! Migrate base_multi_company to v12

a474721

kittiu approved these changes Sep 10, 2019

View reviewed changes

OCA-git-bot added a commit that referenced this pull request Sep 20, 2019

Merge PR #123 into 12.0

0f053dc

Signed-off-by pedrobaeza

OCA-git-bot merged commit a474721 into OCA:12.0 Sep 20, 2019

OCA-git-bot added the merged 🎉 label Sep 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[12.0] migrage base multi company + improve performance #123

[12.0] migrage base multi company + improve performance #123

florian-dacosta commented May 13, 2019

tbaden left a comment

florian-dacosta commented May 14, 2019

pedrobaeza commented May 14, 2019

florian-dacosta commented May 14, 2019

florian-dacosta commented May 16, 2019

tbaden left a comment

LoisRForgeFlow left a comment

OCA-git-bot commented May 22, 2019

kittiu Jun 18, 2019 •

edited

Loading

pedrobaeza Jun 18, 2019

kittiu Jun 18, 2019

kittiu Jun 18, 2019

florian-dacosta Jun 18, 2019

kittiu Jun 19, 2019 •

edited

Loading

florian-dacosta Jun 19, 2019

florian-dacosta commented Jun 19, 2019

kittiu commented Jun 24, 2019

florian-dacosta commented Jul 6, 2019

kittiu commented Jul 7, 2019

florian-dacosta commented Sep 9, 2019

kittiu commented Sep 10, 2019

florian-dacosta commented Sep 10, 2019

florian-dacosta commented Sep 19, 2019

pedrobaeza commented Sep 20, 2019

OCA-git-bot commented Sep 20, 2019

OCA-git-bot commented Sep 20, 2019

amkarthik commented Aug 31, 2020

pedrobaeza commented Aug 31, 2020

amkarthik commented Aug 31, 2020

[12.0] migrage base multi company + improve performance #123

[12.0] migrage base multi company + improve performance #123

Conversation

florian-dacosta commented May 13, 2019

tbaden left a comment

Choose a reason for hiding this comment

florian-dacosta commented May 14, 2019

pedrobaeza commented May 14, 2019

florian-dacosta commented May 14, 2019

florian-dacosta commented May 16, 2019

tbaden left a comment

Choose a reason for hiding this comment

LoisRForgeFlow left a comment

Choose a reason for hiding this comment

OCA-git-bot commented May 22, 2019

kittiu Jun 18, 2019 • edited Loading

Choose a reason for hiding this comment

pedrobaeza Jun 18, 2019

Choose a reason for hiding this comment

kittiu Jun 18, 2019

Choose a reason for hiding this comment

kittiu Jun 18, 2019

Choose a reason for hiding this comment

florian-dacosta Jun 18, 2019

Choose a reason for hiding this comment

kittiu Jun 19, 2019 • edited Loading

Choose a reason for hiding this comment

florian-dacosta Jun 19, 2019

Choose a reason for hiding this comment

florian-dacosta commented Jun 19, 2019

kittiu commented Jun 24, 2019

florian-dacosta commented Jul 6, 2019

kittiu commented Jul 7, 2019

florian-dacosta commented Sep 9, 2019

kittiu commented Sep 10, 2019

florian-dacosta commented Sep 10, 2019

florian-dacosta commented Sep 19, 2019

pedrobaeza commented Sep 20, 2019

OCA-git-bot commented Sep 20, 2019

OCA-git-bot commented Sep 20, 2019

amkarthik commented Aug 31, 2020

pedrobaeza commented Aug 31, 2020

amkarthik commented Aug 31, 2020

kittiu Jun 18, 2019 •

edited

Loading

kittiu Jun 19, 2019 •

edited

Loading