Wishlist: Custom profile group display in patron registration.

Bug #1744756 reported by Bill Erickson
16
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

Evergreen 3.0 / Wishlist

We have a chunk of local code that re-orders the patron profile selector, hides some profiles, and squashes part of the tree down for ease of staff navigation in the patron registration / edit form. The most commonly used profiles are at the top, profiles that are never applied in that interface are hidden, etc. I'm hoping to turn this into a proper EG feature that other sites can use (and of course make our local code easier to maintain).

My first thought was to create a parallel profile tree that supports sibling-level node ordering. The structure of the tree could be anything -- it could even be a flat list. By using a separate tree, sites could create patron registration profile selectors that are faster/easier to navigate than the full profile tree. In the absence of a custom tree, the default tree would still be used.

Revision history for this message
Bill Erickson (berick) wrote :
Revision history for this message
Terran McCanna (tmccanna) wrote :

Great! I customized ours to default to our most commonly used profile, but if they need to use any other type they currently have to scroll back and forth to find the one they need. This change should make that much nicer.

Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Terran. Glad to know this will be useful.

Kyle Huckins (khuckins)
Changed in evergreen:
assignee: nobody → Kyle Huckins (khuckins)
assignee: Kyle Huckins (khuckins) → nobody
Revision history for this message
Kyle Huckins (khuckins) wrote :

I've pushed up a collab branch for this here: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/khuckins/lp1744756-custom-profile-group-display

Included is a new local admin UI for permission group display entries, allowing you to order them based on the selected workstation. A new table has been added to the permissions schema, and will require manual entry of entries to be added in before the UI is functional. If there are no entries, the profile group selector will act as it normally does. When there are entries, it will instead use the custom order as defined in the admin UI. Generally, you'll want to add entries to the table for each OU and each group(So you'd add a Catalogers entry for every OU, for example). I've also added a commit with release notes.

In the new UI, you may add and remove entries, as well as move them up and down within their current branch of the permission group tree. You may remove an entry from one branch and add it to another - for example, you may make Volunteers a root entry, or an entry under patrons, instead of having it stuck under staff.

tags: added: pullrequest webstaffclient
Kyle Huckins (khuckins)
tags: removed: pullrequest
tags: added: pullrequest
Revision history for this message
Kyle Huckins (khuckins) wrote :

I've amended the commits in this collab branch to update the add/remove functionality to be a bit more intuitive. Now when adding a display entry, you may choose any of the permission groups that aren't already represented on the tree for the Org Unit you're currently trying to add to. Adding and removing both make DB calls to add/delete respectively. rather than switching a flag on and off.

Bill Erickson (berick)
Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
Revision history for this message
Bill Erickson (berick) wrote :

1. Tested creating and deleting custom trees.
2. Confirmed custom trees and appear (and not) as expected in the patron editor.
3. Created patron with a custom tree active.

Pushed a sign-off branch with one additional bugfix commit to repair a few minor things. Signoff needed for that.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1744756-custom-profile-group-display-signoff1

Changed in evergreen:
milestone: none → 3.2-beta
assignee: Bill Erickson (berick) → nobody
status: New → Confirmed
Revision history for this message
Bill Erickson (berick) wrote :

Note to other testers, the Tree Control widget used in the admin UI throws an error when it's loaded. I confirmed this error happens in other UI's the implement a tree control. I tried a few things to get rid of the error with no luck. Likely the dependency needs an update.

Revision history for this message
Bill Erickson (berick) wrote :

To clarify my previous comment, the error appears to be harmless (just annoying).

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've tested the above bugfix commit and everything seems to work great. I've pushed a signoff branch: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/khuckins/lp1744756-custom-profile-group-display-signoff2

Kathy Lussier (klussier)
Changed in evergreen:
assignee: nobody → Kathy Lussier (klussier)
Revision history for this message
Kathy Lussier (klussier) wrote :

I'm just starting to test this branch now. I notice when I retrieve the Permission Group Tree Entries interface, I see the following in the browser console:

vendor.bundle.js:6 Error: [$injector:unpr] Unknown provider: eProvider <- e <- treeRightClickDirective
https://errors.angularjs.org/1.6.10/$injector/unpr?p0=eProvider%20%3C-%20e%20%3C-%20treeRightClickDirective
    at vendor.bundle.js:6
    at vendor.bundle.js:6
    at Object.i [as get] (vendor.bundle.js:6)
    at vendor.bundle.js:6
    at i (vendor.bundle.js:6)
    at r (vendor.bundle.js:6)
    at Object.a [as invoke] (vendor.bundle.js:6)
    at vendor.bundle.js:6
    at o (vendor.bundle.js:6)
    at Object.<anonymous> (vendor.bundle.js:6)

I haven't noticed any problems using the interface as a result of the error.

Revision history for this message
Kathy Lussier (klussier) wrote :

I love this feature, and I know our circ staff will love it too! I do have a couple of questions.

1. Using a Concerto database, I was able to create a tree for CONS and for SYS2, but these trees had no impact on the patron registration interface. Is that expected? It would be highly useful if a tree owned by the CONS served as a default for all child org units. A tree for a specific org unit could then override the one created at the consortium level. This would be highly useful for our consortium with 150+ libraries where the majority of libraries are probably using the same permission groups because it wouldn't be required to create a tree for each branch-level org unit. I imagine it might also be useful for creating a tree for an entire system when the branch libraries all use the same permission groups. If the trees are not supposed to work that way, then I would recommend removing org units that cannot be used as registered workstations from the selector in the admin interface.

2. The docs say 'If you want a particular Org Unit to not have access to specific entries, you may remove an entry. Removing an entry will remove it from view. The entry will remain in the database, marked as disabled. Select an entry and press the *Remove* button.'

However, this isn't what I'm seeing. When I click the remove button in the interface, it removes the entry from the table, thereby removing the entry from the view. It has the same visible effect for the end user, but the only way I've been able to set an entry to disabled is directly through the database. Maybe the disabled field isn't needed?

Kathy Lussier (klussier)
Changed in evergreen:
assignee: Kathy Lussier (klussier) → nobody
Revision history for this message
Bill Erickson (berick) wrote :

Thanks, Kathy.

Regarding the error, see https://bugs.launchpad.net/evergreen/+bug/1744756/comments/7

I agree having the custom trees inherit from parent configurations makes sense.

I suspect the "disabled" field is no longer used after the last last round of refactoring, but I'll leave that to Kyle confirm.

Revision history for this message
Kyle Huckins (khuckins) wrote :

It looks like that line in the documentation managed to slip by after the refactor. Thanks for catching that! The disabled field is references in a small amount of places in the code, but it looks like they could be easily removed or tweaked, as the disabled column should have no effect on anything anymore.

Revision history for this message
Michele Morgan (mmorgan) wrote :

Our libraries will love this feature!

I have a concerns about the permissions, though. The permissions required to administer the custom profile group display are:

CREATE_PERM
UPDATE_PERM
DELETE_PERM

at the CONS level. These are the same permissions as those required to administer the Evergreen perm_list. We could not grant local admins at our libraries access to configure the custom display because they would also be able to administer the perm_list and we wouldn't want anyone to accidentally delete a permission from the system.

We certainly can manage the custom displays for our libraries, but if there were unique permissions for administering the custom display, we could allow libraries to manage their own.

Revision history for this message
Bill Erickson (berick) wrote :

Agreed a new permission is a good idea. I propose we create a single MANAGE_CUSTOM_PERM_GRP_TREE permission to cover the update actions.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I'm pushed up a commit to the signoff branch which no longer creates the disabled field, adds the newly proposed permission and applies to to CUD operations in the IDL for the display entry class, and fixes the documentation to address the correct add/remove behavior.

Revision history for this message
Kathy Lussier (klussier) wrote :

Thanks Kyle!

Is there any likelihood that #1 from comment 11 could be addressed? Ideally, child org units without a tree could inherit from their parent's config since most things in Evergreen work that way. But if that can't be done, org units that cannot be set as working locations (is this driven by the 'can have users flag'?) should be disabled so that people don't create trees that don't work.

Revision history for this message
Kyle Huckins (khuckins) wrote :

I've pushed up a commit that addresses #1 from comment 11. It works by checking for a tree on the patron edit UI, and moving upward through the parent ou chain. If nothing is found, it'll default to the regular behavior, otherwise it will draw from the nearest parent's tree

Revision history for this message
Kathy Lussier (klussier) wrote :

Awesome! This is all working as expected. Merged to master for inclusion in release 3.2.

Thanks so much Kyle, Bill and Michele!

Changed in evergreen:
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.