Skip to content

[19.0][MIG] base_user_role_company: Migration to 19.0#445

Open
clement-gouin wants to merge 29 commits into
OCA:19.0from
clement-gouin:19.0-mig-base_user_role_company
Open

[19.0][MIG] base_user_role_company: Migration to 19.0#445
clement-gouin wants to merge 29 commits into
OCA:19.0from
clement-gouin:19.0-mig-base_user_role_company

Conversation

@clement-gouin
Copy link
Copy Markdown

@clement-gouin clement-gouin commented Mar 31, 2026

NOTE: UPDATED COPY OF STALE #383

(there were no updates since 15th of october 2025)

Scope

  • base_user_role_company only

Summary

  • Migrate to Odoo 19.0 following OCA guide.
  • Override parent unique constraint with company-aware one: UNIQUE(user_id, role_id, company_id).
  • Tests aligned with 19.0 (group_ids API).

(differences from original):

Pre-commit

  • Verified locally; no additional changes applied beyond auto-fixes.

Tests

# init
docker network create -d bridge odoo_net
docker run --rm --name odoo_db --network=odoo_net --hostname odoo_db --env POSTGRES_DB=odoo --env POSTGRES_USER=odoo --env POSTGRES_PASSWORD=odoo -d postgres:18

# test
docker run --rm -it -v $PWD:/mnt/extra-addons --network=odoo_net odoo:19 \
    /usr/bin/odoo server --db_host odoo_db -d odoo --db_password odoo  --db_user odoo --without-demo --log-level=info --stop-after-init --no-http \
    -i base_user_role,base_user_role_company --test-enable --test-tags "/base_user_role_company"

# shutdown
docker kill odoo_db
docker network rm odoo_net

results:

2026-03-31 10:09:22,219 1 INFO odoo odoo.tests.stats: base_user_role_company: 9 tests 0.55s 834 queries 
2026-03-31 10:09:22,219 1 INFO odoo odoo.tests.result: 0 failed, 0 error(s) of 5 tests when loading database 'odoo' 

@clement-gouin clement-gouin mentioned this pull request Mar 31, 2026
7 tasks
@clement-gouin clement-gouin force-pushed the 19.0-mig-base_user_role_company branch from f32d3af to c0b2709 Compare March 31, 2026 10:12
@clement-gouin
Copy link
Copy Markdown
Author

I'm looking at coverage on codecov and I will try to add more tests to meet the requirements

Comment on lines +11 to +13
def web_load_menus(self, lang=None):
# v19 signature: (lang=None). Keep behavior of disabling menu HTTP cache.
response = super().web_load_menus(lang=lang)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

better to use args / kwargs syntax, to make the code more resilient in the futur.
don't you think ?

Copy link
Copy Markdown
Author

@clement-gouin clement-gouin Mar 31, 2026

Choose a reason for hiding this comment

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

Sure thing, I've updated the PR with args/kwargs on every super() call

@clement-gouin clement-gouin force-pushed the 19.0-mig-base_user_role_company branch 4 times, most recently from 9718772 to a4aff05 Compare March 31, 2026 13:04
@clement-gouin
Copy link
Copy Markdown
Author

Added more tests and less edits from base code

@clement-gouin clement-gouin force-pushed the 19.0-mig-base_user_role_company branch 2 times, most recently from 3e564da to 0250393 Compare March 31, 2026 13:33
@clement-gouin
Copy link
Copy Markdown
Author

Can I get any update on this subject ? I've tested this branch on a real 19.0 production database and it works the same as 18.0

Copy link
Copy Markdown
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Could you squash all migration commits (red box) and also sqush the CI commits like the yellow marked (there are more)?

Image

I'll test when runboat is available.

Comment on lines +40 to +43
(0, 0, {"role_id": self.roleA.id}),
(0, 0, {"role_id": self.roleB.id, "company_id": self.company1.id}),
(0, 0, {"role_id": self.roleC.id, "company_id": self.company1.id}),
(0, 0, {"role_id": self.roleC.id, "company_id": self.company2.id}),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this syntax is more readable...

from odoo import Command

Suggested change
(0, 0, {"role_id": self.roleA.id}),
(0, 0, {"role_id": self.roleB.id, "company_id": self.company1.id}),
(0, 0, {"role_id": self.roleC.id, "company_id": self.company1.id}),
(0, 0, {"role_id": self.roleC.id, "company_id": self.company2.id}),
Command.create({"role_id": self.roleA.id}),
Command.create({"role_id": self.roleB.id, "company_id": self.company1.id}),
Command.create({"role_id": self.roleC.id, "company_id": self.company1.id}),
Command.create({"role_id": self.roleC.id, "company_id": self.company2.id}),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'v updated the tests in the last commit

Comment on lines +90 to +92
"company_ids": [(6, 0, [self.company1.id, self.company2.id])],
"role_line_ids": [
(0, 0, {"role_id": self.roleA.id, "company_id": self.company3.id}),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
"company_ids": [(6, 0, [self.company1.id, self.company2.id])],
"role_line_ids": [
(0, 0, {"role_id": self.roleA.id, "company_id": self.company3.id}),
"company_ids": [Command.set([self.company1.id, self.company2.id])],
"role_line_ids": [
Command.create({"role_id": self.roleA.id, "company_id": self.company3.id}),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'v updated the tests in the last commit

Comment on lines +28 to +39
"company_ids": [(fields.Command.set([cls.company1.id]))],
"role_line_ids": [
(fields.Command.create({"role_id": cls.roleA.id})),
(
fields.Command.create(
{
"role_id": cls.roleB.id,
"date_to": fields.Date.today() + timedelta(days=1),
}
)
),
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

from odoo import Command

Suggested change
"company_ids": [(fields.Command.set([cls.company1.id]))],
"role_line_ids": [
(fields.Command.create({"role_id": cls.roleA.id})),
(
fields.Command.create(
{
"role_id": cls.roleB.id,
"date_to": fields.Date.today() + timedelta(days=1),
}
)
),
],
"company_ids": [(Command.set([cls.company1.id]))],
"role_line_ids": [
(Command.create({"role_id": cls.roleA.id})),
(
Command.create(
{
"role_id": cls.roleB.id,
"date_to": fields.Date.today() + timedelta(days=1),
}
)
),
],

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'v updated the tests in the last commit

@clement-gouin clement-gouin force-pushed the 19.0-mig-base_user_role_company branch from 0250393 to c4b52ef Compare May 21, 2026 07:38
@OCA-git-bot OCA-git-bot added series:19.0 mod:base_user_role_company Module base_user_role_company labels May 21, 2026
@clement-gouin
Copy link
Copy Markdown
Author

Could you squash all migration commits (red box) and also sqush the CI commits like the yellow marked (there are more)?
Image

I'll test when runboat is available.

Done.

@clement-gouin clement-gouin force-pushed the 19.0-mig-base_user_role_company branch from c4b52ef to 19c3920 Compare May 21, 2026 07:43
@clement-gouin
Copy link
Copy Markdown
Author

Hi, I've just:

  • Rebased the PR with the latest version of the 19.0 branch
  • Squashed mentioned commits
  • Applied mentioned tests syntax

@CRogos
Copy link
Copy Markdown
Contributor

CRogos commented May 21, 2026

@clement-gouin there is still more to squash, as already mentioned. (oca-travis, OCA-git-bot, oca-transbot, ...)
https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

Chandresh-OSI and others added 9 commits May 21, 2026 13:54
Currently translated at 100.0% (11 of 11 strings)

Translation: server-backend-14.0/server-backend-14.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-14-0/server-backend-14-0-base_user_role_company/it/
Issue found on logout / relogin.
The user groups were applied correctly, but the main menu showed apps
the user did not have access to.

This was related to the menu caching mechanisn, that was disabled here.
Currently translated at 90.9% (10 of 11 strings)

Translation: server-backend-14.0/server-backend-14.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-14-0/server-backend-14-0-base_user_role_company/it/
Urvisha-OSI and others added 20 commits May 21, 2026 13:54
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-backend-16.0/server-backend-16.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-backend-16.0/server-backend-16.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/es/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-backend-16.0/server-backend-16.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/pt/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-backend-16.0/server-backend-16.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/es/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-backend-16.0/server-backend-16.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-16-0/server-backend-16-0-base_user_role_company/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: server-backend-18.0/server-backend-18.0-base_user_role_company
Translate-URL: https://translation.odoo-community.org/projects/server-backend-18-0/server-backend-18-0-base_user_role_company/
the _company module is an extra module used in a multi company context (and only in some situation). It is totally useless in mono company. As 2 modules exists, it doesn't makes sense to make the second auto_installable
@clement-gouin clement-gouin force-pushed the 19.0-mig-base_user_role_company branch from 19c3920 to 216db4d Compare May 21, 2026 11:54
@clement-gouin
Copy link
Copy Markdown
Author

@clement-gouin there is still more to squash, as already mentioned. (oca-travis, OCA-git-bot, oca-transbot, ...) https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate

My bad, I thought the contributor of #383 was already following the guidelines, I didn't check prior to my edits.

This is now done as indicated in the documentation.

Copy link
Copy Markdown
Contributor

@CRogos CRogos left a comment

Choose a reason for hiding this comment

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

Migration LGTM, but I am not very aware of the security perspective. Wouldn't it be a good idea to add some demo data for easier testing?

Some improvement suggestions:
Shall we change this to a tag view like in the user page?

Image Image

Should we add optional columns (companies also as tag cloud)?

Image

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

Labels

mod:base_user_role_company Module base_user_role_company series:19.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.