Reminder: Create & find trunk-related issues here. Legacy Trac tickets are still available. Problems? Workflow feedback? Need a module project? Open a support ticket.

OpenMRS Trunk

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

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Could Could
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: OpenMRS 1.6.0
  • Component/s: None
  • Description:
    Hide

    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 [ticket:227].

    Show
    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 [ticket:227].
  1. privilegeInheritance.patch
    (7 kB)
    Burke Mamlin
    2009-03-18 18:39:32 EDT
  1. privs.png
    (30 kB)

Activity

Hide
Andrey Kozhushkov added a comment - 2008-07-24 02:41:42 EDT

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

Show
Andrey Kozhushkov added a comment - 2008-07-24 02:41:42 EDT It'd be nice if the inherited privs could also have greyed out checkboxes to avoid any confusion.
Hide
Andrey Kozhushkov added a comment - 2009-03-08 05:01:19 EDT

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.

Show
Andrey Kozhushkov added a comment - 2009-03-08 05:01:19 EDT 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.
Hide
Andrey Kozhushkov added a comment - 2009-03-08 05:05:20 EDT

Screenshot showing directly added vs inherited privileges

Show
Andrey Kozhushkov added a comment - 2009-03-08 05:05:20 EDT Screenshot showing directly added vs inherited privileges
Hide
Ben Wolfe added a comment - 2009-03-09 15:21:53 EDT

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.

Show
Ben Wolfe added a comment - 2009-03-09 15:21:53 EDT 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.
Hide
zBrian zMcKown added a comment - 2009-03-18 17:09:31 EDT

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.

Show
zBrian zMcKown added a comment - 2009-03-18 17:09:31 EDT 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.
Hide
Andrey Kozhushkov added a comment - 2009-03-18 17:29:16 EDT

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.

Show
Andrey Kozhushkov added a comment - 2009-03-18 17:29:16 EDT 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.
Hide
Andrey Kozhushkov added a comment - 2009-03-18 18:41:43 EDT

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

Show
Andrey Kozhushkov added a comment - 2009-03-18 18:41:43 EDT Patch update: added a line above the privilege box table explaining why some boxes are greyed out (as per Ben's request).
Hide
Ben Wolfe added a comment - 2009-07-30 18:02:49 EDT

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.

Show
Ben Wolfe added a comment - 2009-07-30 18:02:49 EDT 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.
Hide
Ben Wolfe added a comment - 2009-12-08 16:45:14 EST

Committed to trunk in rev:11413. Thanks Andrey!

Show
Ben Wolfe added a comment - 2009-12-08 16:45:14 EST Committed to trunk in rev:11413. Thanks Andrey!

People

Dates

  • Created:
    2008-07-23 21:33:37 EDT
    Updated:
    2010-07-08 16:21:12 EDT
    Resolved:
    2010-07-01 22:42:11 EDT