[UIU-2256] Changes to "Users: User loans view, edit, renew (all)" permission set (ui-users.loans.all) Created: 21/Jul/21 Updated: 04/Mar/22 Resolved: 27/Jan/22 |
|
| Status: | Closed |
| Project: | ui-users |
| Components: | None |
| Affects versions: | None |
| Fix versions: | 7.1.0 |
| Type: | Story | Priority: | P2 |
| Reporter: | Erin Nettifee | Assignee: | Matt Connolly |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Attachments: |
|
| Sprint: | Prokopovych - Sprint 131, Prokopovych - Sprint 132 |
| Story Points: | 0.5 |
| Development Team: | Prokopovych |
| Description |
|
The permission set ui-users.loans.all needs to be adjusted to make it more clear what it actually does and to better reflect what the RA SIG would actually use this for when assigning permissions.
We will defer to developers on whether to rename the actual permission name to remove the .all portion of the name or to leave it as-is. Use cases:A library has four types of circulation staff - student workers, senior student workers, full-time staff, and managers.
Right now, this scenario is possible to do, but the naming of the existing permission makes it very easy for SMEs to make mistakes and grant more permissions than desired (particularly with anonymizing.) In general, having the idea of editing a loan in a permission name will always be confusing to RA SMEs - they would not think of the work they do with loans as editing loans, they would use the terminology of the specific workflows instead. E.g., they would say "I marked a loan missing" rather than "I edited the loan record to change the item status to missing," even though the latter is what they are actually doing. This was discussed at the RA SIG on 7-26-2021 and they agreed to this path; patty.wanninger indicated that RA could make the decision on this since these are loans permissions.
|
| Comments |
| Comment by Matt Connolly [ 07/Oct/21 ] |
|
Erin Nettifee After discussing this with some of the other devs, it seems best not to try to change the actual permission name here; changing an existing permission name would probably have undesirable effects for users who already have that permission. Changing the display label isn't a problem, though. But it's a bit odd to have a permission named *.all with a reduced set of sub-permissions. What if we were to leave the original loans.all permission as-is, with a new label that is less confusing, and then create a new permission with the reduced set of abilities as outlined in the issue description above – named something like loans.manage, perhaps? If that doesn't seem sensible, then I suppose we could just live with the discrepancy between the *.all name and what it actually signifies. What do you think?
|
| Comment by Erin Nettifee [ 07/Oct/21 ] |
|
Matt Connolly - yah, that's the challenge here, right? Which I think we discussed with the group when it was pointed, and I offered to take the lead on communicating about it with implementers and making sure there was a note about the permissions change in the release notes. Since the SMEs most likely to need the permission were OK with having to deal with the change, it seemed like it was OK to do even though it's technically a breaking change. The problem was trying to figure out how to communicate what was included in ui-users.loans.all w/o making a display label so long it would wraparound in typical UIs. If I say something like "Users: Loans (All Permissions)," I'm not sure it's totally clear in that context what exactly you're giving someone the ability to do, and without an additional UI piece to describe it better (see https://folio-org.atlassian.net/browse/UXPROD-3159), you're still relying on people knowing how to go to github to understand, for example, that all includes overrides. I also see some desirability in the fact that the change would break permissions, since I think it would make it clear to libraries that people had had additional permissions that they may or may not have been meant to have. Though that's likely not fair to libraries who are actually in production and have to deal with the breaking, so... Which is all to say - I get what you're saying, but unless you have ideas about a way to name ui-loans.all to reflect all the stuff that's in it, I think the SIG would still prefer to have the set reduced to lessen the risk of giving more perms than people should get. Maybe this is what code comments are for? 😃 |
| Comment by Erin Nettifee [ 14/Oct/21 ] |
|
Matt Connolly - just want to make sure I haven't missed anything, is this ready for testing? |
| Comment by Matt Connolly [ 14/Oct/21 ] |
|
Erin Nettifee Yes, it should be! |
| Comment by Erin Nettifee [ 04/Jan/22 ] |
|
I apologize for losing track of this and not testing it, Matt Connolly and Holly Mistlebauer. I tested today on Snapshot. 2 issues found: 1) I can select the Claim Returned button on the open loan screen, and I shouldn't be able to click on it: 2) Renewal fails, with a message in developer tools: |
| Comment by Matt Connolly [ 10/Jan/22 ] |
|
Erin Nettifee, I think I've taken care of those problems now. Please try again.
|
| Comment by Erin Nettifee [ 11/Jan/22 ] |
|
Went on Snapshot to try to retest and unable to get a user password reset to work; asking about it in hosted-ref-envs. |
| Comment by Erin Nettifee [ 27/Jan/22 ] |
|
The password issue on Snapshot was finally resolved today, so I was able to create a test user and validate your changes Matt Connolly. All looks good, so I am closing this ticket. |
| Comment by patty.wanninger [ 04/Mar/22 ] |
|
testrails case C350609 |