Bug 32682

Summary: Add permission for viewing patron reading history
Product: Koha Reporter: Donna <bwsdonna>
Component: CirculationAssignee: Lucas Gass (lukeg) <lucas>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: andrew, catrina, dcook, gmcharlt, hebah, kebliss, kyle, lisette, lucas, michael.adamyk, sbrown, trevor.diamond
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 40364    
Attachments: Bug 32682: Add permission for viewing patron reading history
Bug 32682: Implement reading history permission
Bug 32682: Adjust atomicupdate permission
Bug 32682: Fix atomicupdate
Bug 32682: Add permission for viewing patron reading history
Bug 32682: Add permission for viewing patron reading history
Bug 32682: Fix typo
Bug 32682: Implement reading history permission
Bug 32682: Add permission for viewing patron reading history
Bug 32682: Implement reading history permission
Bug 32682: Implement reading history permission
Bug 32682: Add new permission to t/Koha/Auth/Permissions.t

Description Donna 2023-01-19 18:21:33 UTC
With intranetreadinghistory it is an all or nothing option - there are often situations where staff will need to view this, but not necessarily every staff member needs to see the information.  It would be great to add the ability to view this to the permissions so libraries can decided which staff have this ability.
Comment 1 trevor.diamond 2025-04-07 19:25:27 UTC
Hear here!  Great idea and as a consortium we are wholeheartedly behind this kind of development.
Comment 2 David Flater 2025-07-07 15:46:07 UTC
Created attachment 183844 [details] [review]
Bug 32682: Add permission for viewing patron reading history
Comment 3 David Flater 2025-07-07 15:46:09 UTC
Created attachment 183845 [details] [review]
Bug 32682: Implement reading history permission
Comment 4 David Flater 2025-07-07 15:46:12 UTC
Created attachment 183846 [details] [review]
Bug 32682: Adjust atomicupdate permission
Comment 5 Owen Leonard 2025-07-07 16:57:14 UTC
Is this ready for testing?
Comment 6 David Flater 2025-07-08 21:32:30 UTC
It's not quite ready yet--I'm still working on both a test plan and migrating over old functionality so that libraries don't experience a change in expected functionality.
Comment 7 David Cook 2025-07-09 00:20:23 UTC
Comment on attachment 183844 [details] [review]
Bug 32682: Add permission for viewing patron reading history

Review of attachment 183844 [details] [review]:
-----------------------------------------------------------------

::: installer/data/mysql/atomicupdate/bug_32682_add_permission.pl
@@ +24,5 @@
> +        #     SELECT borrowernumber, 10, 'view_checkout_history' FROM user_permissions WHERE code = 'remaining_permissions'
> +        # }
> +        # );
> +
> +        # TODO: migrate libraries' "intranetreadinghistory" syspref (will this require us to not fully remove it until we're sure everyone has migrated off of it?)

This is an interesting question. Technically, I think that you have to keep the intranetreadinghistory because it's impossible to replace it with permissions only. For instance, maybe you disable intranetreadinghistory via Apache configuration because you don't want anyone - even superlibrarians to be able to read the checkout history.

If you have libraries with intranetreadinghistory turned off, then these patches will actually cause a regression where anyone with all borrowers permissions will be able to see the reading history when they couldn't previously.
Comment 8 David Cook 2025-07-09 00:22:35 UTC
Failing due to my above comment.

On the plus side, with a small adjustment, I think that these patches would be nearly good to go. 

I suppose the only scenario not covered would be where intranetreadinghistory is enabled and you have limited borrower permissions and you used to be able to read the checkout history and now you can't...

I suppose you could add another SQL update that adds the view_checkout_history permission if they had permission to view the pages that this is modifying...
Comment 9 David Flater 2025-07-23 20:07:58 UTC
Created attachment 184571 [details] [review]
Bug 32682: Fix atomicupdate
Comment 10 David Flater 2025-07-23 20:08:01 UTC
Created attachment 184572 [details] [review]
Bug 32682: Add permission for viewing patron reading history

Create or modify a staff user with edit_borrowers permssion enabled ( Staff A )
Create or modify a staff user without edit_borrowers permssion enabled ( Staff B )
Ensure the "intranetreadinghistory" syspref is enabled
Check out an assortment of items to any patron
Apply patch, update_database, then restart_all
Check the permissions of the staff user A and ensure that they have been automatically assigned the new "view_checkout_history" permission
Check the permissions of the staff user B and ensure that they have NOT been automatically assigned the new "view_checkout_history" permission
View the same patron and ensure you can still see their checkout history as Staff A and as a superlibrarain
Disable the "intranetreadinghistory" syspref and remove the "view_checkout_history" permission of the staff user
update_database, then restart_all, and check the staff user (Staff A and Staff B). Neither should have the "view_checkout_history" permission
L:i
Comment 11 Lucas Gass (lukeg) 2025-07-24 17:56:54 UTC
In issuehistory.tt:

[% SET show_patron_column = (CAN_user_borrowers_view_checkout_history && Koha.Preference('intranetreadinghistory')) AND CAN_user_circulate_circulate_remaining_permissions %]

Should we still be checking for circulate_remaining_permissions ?
Comment 12 David Flater 2025-07-29 17:21:22 UTC
Created attachment 184832 [details] [review]
Bug 32682: Add permission for viewing patron reading history

Create or modify a staff user with edit_borrowers permssion enabled ( Staff A )
Create or modify a staff user without edit_borrowers permssion enabled ( Staff B )
Ensure the "intranetreadinghistory" syspref is enabled
Check out an assortment of items to any patron
Apply patch, update_database, then restart_all
Check the permissions of the staff user A and ensure that they have been automatically assigned the new "view_checkout_history" permission
Check the permissions of the staff user B and ensure that they have NOT been automatically assigned the new "view_checkout_history" permission
View the same patron and ensure you can still see their checkout history as Staff A and as a superlibrarain
Disable the "intranetreadinghistory" syspref and remove the "view_checkout_history" permission of the staff user
update_database, then restart_all, and check the staff user (Staff A and Staff B). Neither should have the "view_checkout_history" permission
L:i
Comment 13 David Flater 2025-07-29 17:33:37 UTC
Created attachment 184833 [details] [review]
Bug 32682: Fix typo
Comment 14 Andrew Fuerste-Henry 2025-09-16 18:12:19 UTC
I can't get these patches to apply, the atomicupdate is giving errors.
Comment 15 Lucas Gass (lukeg) 2025-09-25 20:01:04 UTC
Created attachment 186944 [details] [review]
Bug 32682: Implement reading history permission
Comment 16 Lucas Gass (lukeg) 2025-09-25 20:01:28 UTC
Created attachment 186945 [details] [review]
Bug 32682: Add permission for viewing patron reading history
Comment 17 Lucas Gass (lukeg) 2025-09-25 20:01:31 UTC
Created attachment 186946 [details] [review]
Bug 32682: Implement reading history permission
Comment 18 Lucas Gass (lukeg) 2025-09-25 20:05:11 UTC
Created attachment 186947 [details] [review]
Bug 32682: Implement reading history permission

To test:
1. Create or modify a staff user with edit_borrowers permssion enabled ( Staff A )
2. Create or modify a staff user without edit_borrowers permssion enabled ( Staff B )
3. Ensure the "intranetreadinghistory" syspref is enabled
4. Check out an assortment of items to any patron
5. Apply patch, update_database, then restart_all
6. Check the permissions of the staff user A and ensure that they have been automatically assigned the new "view_checkout_history" permission
7. Check the permissions of the staff user B and ensure that they have NOT been automatically assigned the new "view_checkout_history" permission
8. View the same patron and ensure you can still see their checkout history as Staff A and as a superlibrarain
9. Disable the "intranetreadinghistory" syspref and remove the "view_checkout_history" permission of the staff user
Comment 19 Lucas Gass (lukeg) 2025-09-25 20:05:44 UTC
Squashed and rebased for David. This should again be ready for testing.
Comment 20 Lucas Gass (lukeg) 2025-10-08 16:48:00 UTC
Created attachment 187591 [details] [review]
Bug 32682: Add new permission to t/Koha/Auth/Permissions.t
Comment 21 Lisette Scheer 2025-10-09 17:34:00 UTC
Hi Lucas, you shouldn't use C4::Context in DB updates. 

https://wiki.koha-community.org/wiki/Database_updates


So do not call Koha:: or C4:: methods/subroutines or use of Koha::Database->schema.
Comment 22 Lucas Gass (lukeg) 2025-10-09 18:10:02 UTC
(In reply to Lisette Scheer from comment #21)
> Hi Lucas, you shouldn't use C4::Context in DB updates. 
> 
> https://wiki.koha-community.org/wiki/Database_updates
> 
> 
> So do not call Koha:: or C4:: methods/subroutines or use of
> Koha::Database->schema.

Lisette, what do you suggest instead?