[ADD] 8.0 - New module 'base_user_role' to manage user roles efficiently #608

sebalix · 2016-11-15T13:40:18Z

Note : we are using this module on version 6.0, 7.0 and 8.0 I just added the support for dates recently on the 8.0 version. I'll migrate it to 9.0 and 10.0 soon.

Related issue: #146

User roles

This module was written to extend the standard functionality regarding users and groups management. It helps creating well-defined user roles and associating them to users.

It can become very hard to maintain a large number of user profiles over time, juggling with many technical groups. For this purpose, this module will help you to:

define functional roles by aggregating low-level groups,
set user accounts with the predefined roles (roles are cumulative),
update groups of all relevant user accounts (all at once),
ensure that user accounts will have the groups defined in their roles (nothing more, nothing less). In other words, you can not set groups manually on a user as long as there is roles configured on it,
activate/deactivate roles depending on the date (useful to plan holidays, etc)
get a quick overview of roles and the related user accounts.

That way you make clear the different responsabilities within a company, and are able to add and update user accounts in a scalable and reliable way.

Configuration

To configure this module, you need to go to Configuration / Users / Roles, and create a new role. From there, you can add groups to compose your role, and then associate users to it.

sebalix · 2016-11-15T13:51:07Z

About Travis: flake8 errors and tests failure not related to this module.

jcdrubay · 2016-11-16T06:58:07Z

Very good contribution (high quality of code, useful, well documented).
I hope it can be reviewed and merged soon 👍

lasley

This is awesome, thanks @sebalix

Code, no test. Comments inline

lasley · 2016-11-16T07:07:54Z

base_user_role/__openerp__.py

+    'license': 'AGPL-3',
+    'maintainer': 'ABF OSIELL',
+    'website': 'http://www.osiell.com',
+    'depends': [


You can remove this key. base dependency is implicit.

No, you should depend on your module at least in base, but base is implicit when depending on other module.

lasley · 2016-11-16T07:12:53Z

base_user_role/models/role.py

+        'res.users', string=u"Users", compute='_compute_user_ids')
+
+    @api.multi
+    @api.depends('line_ids')


line_ids.user_id

lasley · 2016-11-16T07:20:29Z

base_user_role/models/role.py

+    def update_users(self):
+        """Update all the users concerned by the roles identified by `ids`."""
+        users = self.mapped('user_ids')
+        if users:


There shouldn't be need for this guard, mapped will just return an empty Recordset that should work with the other method just fine due to record loop

lasley · 2016-11-16T07:24:20Z

base_user_role/models/role.py

+        'res.users', string=u"User")
+    date_from = fields.Date(u"From")
+    date_to = fields.Date(u"To")
+    is_active = fields.Boolean(u"Active", compute='_compute_is_active')


active is the Odoo norm for this col - although I'm with you on prefixing bool cols with linking verbs like is, has, etc. It also comes with some defaults filters and such, so makes it worth using despite the hurt to naming OCD

That said, it also needs to be store=True to allow all of that

Indeed, but I was unable to set a working domain on the XML views to display all the lines (as we don't want to hide them from the administrator in any case, we just want to filter them when setting groups). If I set the domain on the Python field, it works fine, but then it does not dispense to filter the lines in the ResUsers.set_groups_from_roles method. And as I've seen no benefit to use the standard active field I ended up adding my own.

Or I could rename it to 'is_enabled'?

I like the is_enabled idea - it is far enough away from active that people won't be trying to change it all the time. I know I would have probably done this during a migration had I not already gotten the reasoning behind it.

lasley · 2016-11-16T07:25:38Z

base_user_role/models/user.py

+        'res.users.role', string=u"Roles", compute='_compute_role_ids')
+
+    @api.multi
+    @api.depends('role_line_ids')


role_line_ids.role_id

lasley · 2016-11-16T07:36:40Z

Oh please also check Travis and Runbot 😉

jcdrubay · 2016-11-16T07:50:06Z

base_user_role/security/ir.model.access.csv

@@ -0,0 +1,2 @@
+id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
+access_res_users_role,access_res_users_role,model_res_users_role,"base.group_erp_manager",1,1,1,1


From travis https://travis-ci.org/OCA/server-tools/jobs/176051212#L1703

The model res.users.role.line has no access rules, consider adding one. E.g.

access_res_users_role_line,access_res_users_role_line,model_res_users_role_line,"base.group_erp_manager",1,0,0,0

sebalix · 2016-11-16T07:51:38Z

Thank you for the review, I'll fix these points soon

susport · 2016-11-17T12:56:29Z

base_user_role/models/role.py

+        'res.users.role', string=u"Role", ondelete='cascade')
+    user_id = fields.Many2one(
+        'res.users', string=u"User")
+    date_from = fields.Date(u"From")


Date from may be default the date now of the create moment.
Good contribution.
👍

Dates are optional, no dates = role activated immediately

sebalix · 2016-11-18T13:12:13Z

I think all the points are fixed.

lasley · 2016-11-18T17:42:33Z

Travis is still mad 🔴

sebalix · 2016-11-18T20:46:13Z

You mean I have to fix lint errors of others modules in this one? Tests fail on fetchmail_notify_error_to_sender.

lasley

Hmm yeah ok the build looks broken on 8.0 branch in the same way, so you are correct that we should not be fixing it here.

sebalix · 2016-11-19T18:21:02Z

I fixed the other modules in #616 to get green builds.

sebalix · 2016-11-19T22:54:40Z

Rebased

sbidoul · 2016-11-20T12:06:33Z

@adrienpeiffer you should have a look at this.

andhit-r · 2016-12-08T13:04:38Z

Tested functionality on runbot 👍

andhit-r · 2016-12-09T02:26:07Z

@mikevhe18 @azmimr67 you guys should take a look at this PR and help review

lasley · 2016-12-09T05:45:40Z

We're approved for merge. @sebalix - do you prefer to squash your own commits or would you rather me do it? I don't think I've gotten this preference from you yet, but sorry if I'm double asking.

sebalix · 2016-12-09T08:22:27Z

@lasley You can squash them, no trouble

lasley · 2016-12-09T17:07:03Z

Thanks for the awesome new module @sebalix! Do you have any plans to migrate it forward?

…CA#608) * [ADD] New module 'base_user_role' * [FIX] base_user_role - Review * [FIX] base_user_role - Review s/is_active/is_enabled/ * [FIX] base_user_role - Review s/is_active/is_enabled/ * [IMP] base_user_role - Translations updated (template + FR) * [FIX] base_user_role - Lint

sebalix · 2016-12-09T21:01:17Z

@lasley Thank you! Yes, I plan to migrate it to v9 (see #641) and v10 right after.

…CA#608) * [ADD] New module 'base_user_role' * [FIX] base_user_role - Review * [FIX] base_user_role - Review s/is_active/is_enabled/ * [FIX] base_user_role - Review s/is_active/is_enabled/ * [IMP] base_user_role - Translations updated (template + FR) * [FIX] base_user_role - Lint

Syncing from upstream OCA/server-tools (13.0)

sebalix force-pushed the 8.0-base_user_role branch from a2f2ac1 to 9d85bf6 Compare November 15, 2016 13:42

lasley suggested changes Nov 16, 2016

View reviewed changes

lasley added the needs review label Nov 16, 2016

lasley added this to the 8.0 milestone Nov 16, 2016

jcdrubay reviewed Nov 16, 2016

View reviewed changes

susport reviewed Nov 17, 2016

View reviewed changes

lasley approved these changes Nov 18, 2016

View reviewed changes

sebalix force-pushed the 8.0-base_user_role branch from 7a32246 to 7fbd3b1 Compare November 19, 2016 22:40

sebalix added 6 commits November 19, 2016 23:43

[ADD] New module 'base_user_role'

bc35a1c

[FIX] base_user_role - Review

cbed5a5

[FIX] base_user_role - Review s/is_active/is_enabled/

df1ca76

[FIX] base_user_role - Review s/is_active/is_enabled/

5b8053e

[IMP] base_user_role - Translations updated (template + FR)

4295c2c

[FIX] base_user_role - Lint

6227edf

sebalix force-pushed the 8.0-base_user_role branch from 7fbd3b1 to 6227edf Compare November 19, 2016 22:44

lasley merged commit 7a272ae into OCA:8.0 Dec 9, 2016

sebalix deleted the 8.0-base_user_role branch December 9, 2016 21:02

SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024

Merge pull request OCA#608 from OCA/13.0

21480b4

Syncing from upstream OCA/server-tools (13.0)

		@@ -0,0 +1,2 @@
		id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink
		access_res_users_role,access_res_users_role,model_res_users_role,"base.group_erp_manager",1,1,1,1

Uh oh!

[ADD] 8.0 - New module 'base_user_role' to manage user roles efficiently #608

[ADD] 8.0 - New module 'base_user_role' to manage user roles efficiently #608

Uh oh!

Conversation

sebalix commented Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User roles

Configuration

Uh oh!

sebalix commented Nov 15, 2016

Uh oh!

jcdrubay commented Nov 16, 2016

Uh oh!

lasley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasley Nov 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lasley commented Nov 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebalix commented Nov 16, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sebalix commented Nov 18, 2016

Uh oh!

lasley commented Nov 18, 2016

Uh oh!

sebalix commented Nov 18, 2016

Uh oh!

lasley left a comment

Choose a reason for hiding this comment

Uh oh!

sebalix commented Nov 19, 2016

Uh oh!

sebalix commented Nov 19, 2016

Uh oh!

sbidoul commented Nov 20, 2016

Uh oh!

andhit-r commented Dec 8, 2016

Uh oh!

andhit-r commented Dec 9, 2016

Uh oh!

lasley commented Dec 9, 2016

Uh oh!

sebalix commented Dec 9, 2016

Uh oh!

lasley commented Dec 9, 2016

Uh oh!

sebalix commented Dec 9, 2016

Uh oh!

Uh oh!

sebalix commented Nov 15, 2016 •

edited

Loading

lasley Nov 17, 2016 •

edited

Loading