-
-
Notifications
You must be signed in to change notification settings - Fork 273
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
[MIG] base_user_role: Migration to 13.0 #46
Conversation
Hey @sebalix, thank you for your Pull Request. It looks like some users haven't signed our Contributor License Agreement, yet.
Appreciation of efforts, |
Hi this is the gihub login of duc.dd@komit-consulting.com: https://github.com/novawish The email address |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @sebalix :) I have added 2 small comments. The rest LGTM.
<odoo> | ||
|
||
<record model="ir.module.category" id="ir_module_category_role"> | ||
<field name='name'>User roles</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don' think this is used anywhere, and I think that we should NOT create this module category as it seems like other similar modules in Odoo core are using the category Tools or Extra Tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This module category aims to be linked to role/group you create in your own custom module (https://github.com/odoo/odoo/blob/13.0/odoo/addons/base/models/res_users.py#L101) if you want. By doing this you'll have the section 'User roles' in the "Access rights" tab on the user form which will regroup all groups related to role. Without it, all these groups will be in the 'Other' section.
I could add a comment above this XML record about that.
base_user_role/readme/USAGE.rst
Outdated
[ This file must be present and contains the usage instructions | ||
for end-users. As all other rst files included in the README, | ||
it MUST NOT contain reStructuredText sections | ||
only body text (paragraphs, lists, tables, etc). Should you need | ||
a more elaborate structure to explain the addon, please create a | ||
Sphinx documentation (which may include this file as a "quick start" | ||
section). ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you should remove this lines
Hi @sebalix is it possible to add these commits in this PR ? https://github.com/OCA/server-tools/pull/1489/commits Not blocking for me but you know perfectly this module, it could be nice Thanks in advance |
ba3ae7e
to
48da5cd
Compare
Thanks @sebalix |
@bealdav I'll continue to work on this the next week. There are a lot of fix and improvements in different branches, all of them need to be consolidated. It should be ok for 13.0 now (I just need to rebase some of them). |
8ab0f13
to
ad5ae3e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally 👍 works as expected
This PR has the |
LGTM |
/ocabot merge |
What a great day to merge this nice PR. Let's do it! |
@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 13.0-ocabot-merge-pr-46-by-pedrobaeza-bump-no. After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red. |
You have to rebase and pass black in pre-commit. See https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-13.0 |
original branch from OCA#46
@sebalix @pedrobaeza @Laurent-Corron what is the status of this migration? I see it did not pass all checks? Thank you for your good work. |
ad5ae3e
to
ce06102
Compare
@pedrobaeza @eggu68 thank you to notice me about this. It's done, all checks are green now. Ready to be merged. |
@pedrobaeza can this be merged to 13.0 now? Thank you. |
/ocabot merge |
Currently translated at 100,0% (37 of 37 strings) Translation: server-backend-11.0/server-backend-11.0-base_user_role Translate-URL: https://translation.odoo-community.org/projects/server-backend-11-0/server-backend-11-0-base_user_role/da/
Updated by Update PO files to match POT (msgmerge) hook in Weblate.
the groups that have changed with respecto the role.
…notebook users line (#1170)
…users.role.line" This reverts commit b537941d5a2d1fcb3fbacc602124c1b3f6e1d495.
ce06102
to
0fdc2ad
Compare
0fdc2ad
to
7615a99
Compare
@pedrobaeza sorry for the delay, it's now rebased + isort.cfg updated to resolve the upcoming conflict. Ready to be merged. |
/ocabot merge |
This PR looks fantastic, let's merge it! |
Congratulations, your PR was merged at c7dc687. Thanks a lot for contributing to OCA. ❤️ |
Syncing from upstream OCA/server-backend (13.0)
No description provided.