Bug 15788

Summary: Split borrowers permission into create/edit and delete
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: PatronsAssignee: Martin Renvoize <martin.renvoize>
Status: RESOLVED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: david, dcook, fiona.borthwick, gmcharlt, jonathan.druart+koha, jzairo, katrin.fischer, kyle.m.hall, kyle, martin.renvoize, nick, sally.healey, sbcornell, tomascohen
Version: masterKeywords: Academy, Manual
Hardware: All   
OS: All   
URL: http://devs.bywatersolutions.com/projects/patron-edit-permission/
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
This enhancement allows administrators to control, at a more fine-grained level, which users may delete patron records. This patch introduces a new `delete_borrowers` user permission.
Version(s) released in:
21.11.00
Attachments: Bug 15788: Add delete_borrowers permission
Bug 15788: Use delete_borrowers permission
Bug 15788: Add delete_borrowers permission
Bug 15788: Use delete_borrowers permission
Bug 15788: Add delete_borrowers permission
Bug 15788: Use delete_borrowers permission
Bug 15788: Add delete_borrowers permission
Bug 15788: Use delete_borrowers permission
Bug 15788: (QA follow-up) Simplify query in update
Bug 15788: Make the API require the new permission

Description Nicole C. Engard 2016-02-10 18:49:02 UTC
Goal : Add ability to control if a staff user can create new patrons vs editing existing patrons

Work to be done:
1) Add new borrowers sub-permission ‘modify’
2) Add new borrowers sub-permission ‘create’
3) Implement new permissions for patron editor/creator



Looking for sponsors here: http://devs.bywatersolutions.com/projects/patron-edit-permission/
Comment 1 Nick Clemens 2018-01-26 10:56:42 UTC
I think ideally we would add a third 'delete' permission, updated site link:
http://devs.bywatersolutions.com/2016/11/30/Split-borrowers-permission-into-create-edit/
Comment 2 Katrin Fischer 2018-05-16 23:35:01 UTC
*** Bug 8535 has been marked as a duplicate of this bug. ***
Comment 3 Jessie Zairo 2019-08-01 19:25:41 UTC
I think in addition to giving a delete permission, we should also had a merge patron permission.
Comment 4 Sarah Cornell 2019-08-21 17:44:53 UTC
We just had our first accidental borrower deletion in almost 2 years, so I'm pleased to see this is already suggested!

I'd like to see permissions broken down thus:

create
modify
renew
delete
merge

The funding link is broken.  Is there still a call out for cosponsors?
Comment 5 Katrin Fischer 2020-04-18 22:29:38 UTC
I agree about delete and merge - but I am unsure about splitting create and modify. 

Say you create a new patron, but you discover you have typoed the email address or phone number after saving - you won't be able to fix it, if you don't have create and modify. 

I feel like they both should go together, but wonder if there is a use case where splitting makes sense?
Comment 6 Fiona Borthwick 2020-09-16 11:32:17 UTC
One of our large public library customers is due to go live next month and are currently working out their staff permissions.
This exact question has been asked - can they give the DELETE permission to some members of staff but not to others?

This would  be a very welcome enhancement as we really don't offer many patron permissions at all at present.
Comment 7 Martin Renvoize 2021-01-12 14:27:12 UTC
Created attachment 115091 [details] [review]
Bug 15788: Add delete_borrowers permission
Comment 8 Martin Renvoize 2021-01-12 14:27:16 UTC
Created attachment 115092 [details] [review]
Bug 15788: Use delete_borrowers permission

Link the new delete_borrowers sub-permission to the delete actions.

Test plan
1/ Remove the delete_borrowers permission from a staff user
2/ Check that the user cannot use the 'Delete' option from the members
menu.
Comment 9 Martin Renvoize 2021-01-12 14:33:00 UTC
These patches simply add a 'delete_borrowers' sub-permissions and bind the option to delete borrowers to it.  All other 'edit_borrowers' options should remain intact.
Comment 10 Martin Renvoize 2021-02-03 09:11:41 UTC
Anyone fancy testing this?
Comment 11 David Nind 2021-03-17 11:13:18 UTC
After applying the patch and the database update:

- You can't expand any of the "Show details" to see the sub permissions for a patron.

- There is an error in the web developer tools console:

   Uncaught ReferenceError: CAN_user_borrowers_delete_borrowers is not defined
      <anonymous> Ember
      jQuery 4
          i
          fireWith
          ready
          J

(A link is also provided to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Not_defined )
Comment 12 Martin Renvoize 2021-03-26 15:01:37 UTC
Created attachment 118865 [details] [review]
Bug 15788: Add delete_borrowers permission
Comment 13 Martin Renvoize 2021-03-26 15:01:41 UTC
Created attachment 118866 [details] [review]
Bug 15788: Use delete_borrowers permission

Link the new delete_borrowers sub-permission to the delete actions.

Test plan
1/ Remove the delete_borrowers permission from a staff user
2/ Check that the user cannot use the 'Delete' option from the members
menu.
Comment 14 Martin Renvoize 2021-03-26 15:04:38 UTC
Thanks for testing David.. I found the issue, I'd neglected to add one of the files to the final commit!

All sorted now, should be working this time around.. Doh!
Comment 15 David Nind 2021-03-26 18:43:10 UTC
Created attachment 118874 [details] [review]
Bug 15788: Add delete_borrowers permission

Signed-off-by: David Nind <david@davidnind.com>
Comment 16 David Nind 2021-03-26 18:43:14 UTC
Created attachment 118875 [details] [review]
Bug 15788: Use delete_borrowers permission

Link the new delete_borrowers sub-permission to the delete actions.

Test plan
1/ Remove the delete_borrowers permission from a staff user
2/ Check that the user cannot use the 'Delete' option from the members
menu.

Signed-off-by: David Nind <david@davidnind.com>
Comment 17 Tomás Cohen Arazi 2021-06-12 13:59:58 UTC
Created attachment 121887 [details] [review]
Bug 15788: Add delete_borrowers permission

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2021-06-12 14:00:23 UTC
Created attachment 121888 [details] [review]
Bug 15788: Use delete_borrowers permission

Link the new delete_borrowers sub-permission to the delete actions.

Test plan
1/ Remove the delete_borrowers permission from a staff user
2/ Check that the user cannot use the 'Delete' option from the members
menu.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Tomás Cohen Arazi 2021-06-12 14:00:29 UTC
Created attachment 121889 [details] [review]
Bug 15788: (QA follow-up) Simplify query in update

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Tomás Cohen Arazi 2021-06-12 14:00:36 UTC
Created attachment 121890 [details] [review]
Bug 15788: Make the API require the new permission

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Jonathan Druart 2021-06-21 12:01:14 UTC
Looks like we are missing renew and merge. It would have been good to split this bug report.

Pushing as it anyway.
Comment 22 Jonathan Druart 2021-06-21 12:03:30 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 23 Kyle M Hall 2021-07-02 13:42:38 UTC
Enhancement not backported to 21.05.x