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

[18.0][MIG] base_technical_features: Migration to 18.0 #953

Merged
merged 41 commits into from
Oct 25, 2024

Conversation

Kimkhoi3010
Copy link

@Kimkhoi3010 Kimkhoi3010 commented Oct 15, 2024

I found only 1 commit to port but it was ignored, I added it to blacklist: [17.0][FW] base_technical_features: port from 14.0

Reason for change:

The original logic in _postprocess_access_rights removes base.group_no_one for non-admin users, which restricts their access to certain UI components. Extending _get_view ensures that technical users (part of base_technical_features.group_technical_features) still have access to these components without needing full admin privileges.

StefanRijnhart and others added 30 commits October 15, 2024 11:05
Using new base model inheritance
Currently translated at 75.0% (6 of 8 strings)

Translation: server-ux-11.0/server-ux-11.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-base_technical_features/ar/
Currently translated at 75.0% (6 of 8 strings)

Translation: server-ux-11.0/server-ux-11.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-base_technical_features/da/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-11.0/server-ux-11.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-base_technical_features/es/
Currently translated at 100,0% (8 of 8 strings)

Translation: server-ux-11.0/server-ux-11.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-base_technical_features/ca/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/it/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/it/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/pt/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/zh_CN/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/pt_BR/
Currently translated at 87.5% (7 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/hr/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-12.0/server-ux-12.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-12-0/server-ux-12-0-base_technical_features/hr/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-14.0/server-ux-14.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-14-0/server-ux-14-0-base_technical_features/es/
Currently translated at 100.0% (8 of 8 strings)

Translation: server-ux-14.0/server-ux-14.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-14-0/server-ux-14-0-base_technical_features/nl/
Ivorra78 and others added 9 commits October 15, 2024 11:05
Currently translated at 100.0% (8 of 8 strings)

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

Translation: server-ux-17.0/server-ux-17.0-base_technical_features
Translate-URL: https://translation.odoo-community.org/projects/server-ux-17-0/server-ux-17-0-base_technical_features/it/
…sers

On databases with a large number of users, loading all users of base.group_no_one can be slow.
Moreover, the 'technical_features' and 'show_technical_features' are always computed for a single record
@pedrobaeza
Copy link
Member

/ocabot migration base_technical_features

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Oct 15, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 15, 2024
17 tasks
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Thanks :) Some comments below

base_technical_features/models/base.py Outdated Show resolved Hide resolved
Comment on lines +11 to +12
"""Set debug = True, so that group_no_one is not filtered out of the
user's groups"""
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Author

Choose a reason for hiding this comment

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

That is the docstring available from this commit: 204294

Comment on lines 9 to 13
def user_has_groups(self, groups):
"""Return True for users in the technical features group when
membership of the original group is checked, even if debug mode
is not enabled.
@api.model
def _get_view(self, view_id=None, view_type="form", **options):
Copy link
Member

Choose a reason for hiding this comment

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

Is it enogh to just replace the group in view?

arch, view = super()._get_view(view_id=view_id, view_type=view_type, **options)
arch_str = etree.tostring(arch, encoding="unicode")
arch_str = arch_str.replace(
"base.group_no_one", "base_technical_features.group_technical_features"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this would be more correct, as otherwise a user with no base_technical_features.group_technical_features wouldn't access to the regular base.group_no_one behavior in debug mode

Suggested change
"base.group_no_one", "base_technical_features.group_technical_features"
"base.group_no_one", "base.group_no_one,base_technical_features.group_technical_features"

Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG overall, minor remark.

@chienandalu good for you?

@api.model
def _get_view(self, view_id=None, view_type="form", **options):
arch, view = super()._get_view(view_id=view_id, view_type=view_type, **options)
arch_str = etree.tostring(arch, encoding="unicode")
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't be better to check if the string is there 1st?

Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

👍

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-953-by-pedrobaeza-bump-nobump, awaiting test results.

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

@OCA-git-bot OCA-git-bot merged commit 91dca12 into OCA:18.0 Oct 25, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

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.