[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: PNG File screenshot-1.png    
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. 

  • Subpermission changes: remove ui-users.loans.edit and add ui-users.loans.change-due-date
  • Remove from ui-users.loans.all the ability to:
    • declare an item lost
    • mark an item claim returned
    • mark a claim returned item missing
    • anonymize a user's loans
    • renew an item through override 
  • Change the display name (and associated translations) for ui-users.loans.all to "Users: User loans view, change due date, renew"

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.

  • Student workers should be able to view and renew loans. 
  • Senior student workers should be able to do everything a student worker can do + change a due date.
  • Full time staff should be able to do everything a senior student worker can do + declare an item lost and renew an item through override.
  • Managers should be able to do everything full-time staff should be able to do + mark an item claim returned.
  • Only the most senior of the managers should be able to anonymize loans.

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:
HTTP request to "http://10.36.1.161:9130/manualblocks" failed, status code: 403, response: "Access requires permission: manualblocks.collection.get"

Comment by Matt Connolly [ 10/Jan/22 ]

Erin Nettifee, I think I've taken care of those problems now. Please try again.

  1. The Claim Returned button actually was not protected by a permissions check, but I've added that now.
  2. I think this is probably an older bug that we didn't catch in testing before, but I've added the requisite permission to the .all permissions set.
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

Generated at Fri Feb 09 00:30:03 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.