Downloads Documentation Community
Contribute
Contribute Demo






Show Sidebar
Login | Register


Ticket #936 (closed task: fixed)

Opened 2 years ago

Last modified 3 months ago

Role Management should populate privilege check boxes with a role's inherited privileges.

Reported by: bmckown Assigned to: Keelhaul
Priority: minor Milestone: OpenMRS 1.6
Component: OpenMRS Code Base Keywords: role, privilege
Cc: jmiranda Introductory Ticket: 0
Code Review Status: Approved

Description

The admin/users/roleForm.jsp lists a series of check boxes for the permissions granted to this role. These check boxes should be populated with a role's inherited privileges. Currently there is no way of knowing what privileges are inherited without browsing to the page of the child role. (Parent inherits from child.) This would be best fixed in conjunction with 227.

Attachments

privs.png (30.5 kB) - added by Keelhaul on 03/08/09 05:05:20.
Screenshot showing directly added vs inherited privileges
privilegeInheritance.patch (6.7 kB) - added by Keelhaul on 03/18/09 18:39:32.

Change History

07/24/08 02:41:42 changed by Keelhaul

It'd be nice if the inherited privs could also have greyed out checkboxes to avoid any confusion.

03/08/09 05:01:19 changed by Keelhaul

  • owner changed from bmckown to Keelhaul.
  • intro_ticket changed.
  • status changed from new to assigned.
  • review_status changed.

I added a patch that will check and disable all boxes for privs that have been inherited from other roles.

Note that although the boxes are checked the inherited privs will not be added to the current role, it's just for visual.

03/08/09 05:05:20 changed by Keelhaul

  • attachment privs.png added.

Screenshot showing directly added vs inherited privileges

03/09/09 15:21:53 changed by bwolfe

A quick visual review shows this looks good. The only comment is that there should be some help text somewhere so the user knows why some boxes are already checked.

I'll let Brian do the final review so he can test out the patch and make sure it looks like how he envisioned it.

03/18/09 17:09:31 changed by bmckown

I think it is very nice, Keelhaul. This is indeed what we (you/I) had envisioned, right? Really this is what I had thought when writing the ticket - no more or less that that.

It seems intuitive to me that among the boxes already checked, those that cannot be un-checked are inherited and those that can be un-checked pertain to the present role. In the case of multiple inheritance of roles we will not know necessarily what privilege was inherited from which role... but at least with this patch we do know all of the privileges assigned to a role from looking at one page.

Nice work and thanks, Keelhaul.

03/18/09 17:29:16 changed by Keelhaul

No problem. I was getting annoyed by the status quo every time I had to assign privileges to roles, so I finally decided to do something about it. =P

I really couldn't think of a good way to visualize exactly which privilege was inherited from which role without adding many more parameters to ListPicker.

03/18/09 18:39:32 changed by Keelhaul

  • attachment privilegeInheritance.patch added.

03/18/09 18:41:43 changed by Keelhaul

Patch update: added a line above the privilege box table explaining why some boxes are greyed out (as per Ben's request).

05/11/09 18:02:08 changed by jmiranda

  • cc set to jmiranda.
  • review_status set to Needs Review.

07/30/09 18:02:49 changed by bwolfe

Bumping this to the 1.6 milestone.

Brian, if/when you get a trunk deployed to test this, feel free to apply it. If you don't think that will be before 1.6 is released, Justin or I can take it on. :-)

07/30/09 18:04:58 changed by bwolfe

  • milestone changed from OpenMRS 1.5 to OpenMRS 1.6.

12/08/09 16:45:14 changed by bwolfe

  • status changed from assigned to closed.
  • review_status changed from Needs Review to Approved.
  • resolution set to fixed.

Committed to trunk in [11413]. Thanks Andrey!