Bug 37155

Summary: Remove unnecessary unblessing of patron in CanItemBeReserved
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Architecture, internals, and plumbingAssignee: Nick Clemens (kidclamp) <nick>
Status: RESOLVED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: blawlor, caroline.cyr-la-rose, fridolin.somers, lucas
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:
24.11.00,24.05.06
Circulation function:
Bug Depends on:    
Bug Blocks: 37154    
Attachments: Bug 37155: Refactor GetAgeRestrictions
Bug 37155: Remove use of unblessed patron
Bug 37155: Refactor GetAgeRestrictions
Bug 37155: Remove use of unblessed patron
Bug 37155: Refactor GetAgeRestrictions
Bug 37155: Remove use of unblessed patron

Description Nick Clemens (kidclamp) 2024-06-21 16:09:36 UTC
We unbless the patron here to pass to GetAgeRestriction which needs only the patron's dob to perform age calculations
Comment 1 Nick Clemens (kidclamp) 2024-06-21 16:11:02 UTC
Created attachment 167967 [details] [review]
Bug 37155: Refactor GetAgeRestrictions

This routine currently takes the agerestriction value from biblioitems and an unblessed borrower object
and uses the date of birth to calculate whether the ptrons DOB is before or after the minimum value required
against the age restriction

We have a routine in the patron object to get the patron's age - we cna use this against the parsed agerestriction
value in a simple comparison and remove the need to unbless and pass the patron.

FIXME: We should move this to a biblioitems or biblio object method

To test:
0 - In Admin -> Koha to MARC mapping, set biblioitems.agerestriction to 521,a
1 - Set syspref AgeRestrictionMarker to 'Age'
2 - Edit a record and set 521$a to 'Age 14'
3 - Add an item or copy the barcode of the item on that record
4 - Attempt to checkout item to Lisa Charles in sample data, or a 15 year old patron
5 - It should checkout fine
6 - Check in item
7 - Edit patron  Joyce Gaines to set age to 13 DOB:06/20/2011, or create a 13 year old patron
8 - Attempt to checkout item
9 - Item is blocked
10 - Apply patch
11 - Repeat tests, confirm no change
Comment 2 Nick Clemens (kidclamp) 2024-06-21 16:11:03 UTC
Created attachment 167968 [details] [review]
Bug 37155: Remove use of unblessed patron

This removes the unblessing of the patron object and uses fields form the patron

To test:
Confirm tests still pass:
prove -v t/db_dependent/Holds.t t/db_dependent/Circulation.t t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t t/db_dependent/Reserves.t t/db_dependent/api/v1/holds.t
Comment 3 Brendan Lawlor 2024-06-21 19:53:13 UTC
Created attachment 167991 [details] [review]
Bug 37155: Refactor GetAgeRestrictions

This routine currently takes the agerestriction value from biblioitems and an unblessed borrower object
and uses the date of birth to calculate whether the ptrons DOB is before or after the minimum value required
against the age restriction

We have a routine in the patron object to get the patron's age - we cna use this against the parsed agerestriction
value in a simple comparison and remove the need to unbless and pass the patron.

FIXME: We should move this to a biblioitems or biblio object method

To test:
0 - In Admin -> Koha to MARC mapping, set biblioitems.agerestriction to 521,a
1 - Set syspref AgeRestrictionMarker to 'Age'
2 - Edit a record and set 521$a to 'Age 14'
3 - Add an item or copy the barcode of the item on that record
4 - Attempt to checkout item to Lisa Charles in sample data, or a 15 year old patron
5 - It should checkout fine
6 - Check in item
7 - Edit patron  Joyce Gaines to set age to 13 DOB:06/20/2011, or create a 13 year old patron
8 - Attempt to checkout item
9 - Item is blocked
10 - Apply patch
11 - Repeat tests, confirm no change

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 4 Brendan Lawlor 2024-06-21 19:53:15 UTC
Created attachment 167992 [details] [review]
Bug 37155: Remove use of unblessed patron

This removes the unblessing of the patron object and uses fields form the patron

To test:
Confirm tests still pass:
prove -v t/db_dependent/Holds.t t/db_dependent/Circulation.t t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t t/db_dependent/Reserves.t t/db_dependent/api/v1/holds.t

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 5 Brendan Lawlor 2024-06-21 19:55:03 UTC
Tested before and after patch and confirm no changes in behavior.

prove -v t/db_dependent/Holds.t t/db_dependent/Circulation.t t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t t/db_dependent/Reserves.t t/db_dependent/api/v1/holds.t

All tests successful.
Files=5, Tests=252, 58 wallclock secs ( 0.24 usr  0.07 sys + 44.99 cusr  4.90 csys = 50.20 CPU)
Result: PASS
Comment 6 Martin Renvoize (ashimema) 2024-06-28 15:37:50 UTC
Created attachment 168265 [details] [review]
Bug 37155: Refactor GetAgeRestrictions

This routine currently takes the agerestriction value from biblioitems and an unblessed borrower object
and uses the date of birth to calculate whether the ptrons DOB is before or after the minimum value required
against the age restriction

We have a routine in the patron object to get the patron's age - we cna use this against the parsed agerestriction
value in a simple comparison and remove the need to unbless and pass the patron.

FIXME: We should move this to a biblioitems or biblio object method

To test:
0 - In Admin -> Koha to MARC mapping, set biblioitems.agerestriction to 521,a
1 - Set syspref AgeRestrictionMarker to 'Age'
2 - Edit a record and set 521$a to 'Age 14'
3 - Add an item or copy the barcode of the item on that record
4 - Attempt to checkout item to Lisa Charles in sample data, or a 15 year old patron
5 - It should checkout fine
6 - Check in item
7 - Edit patron  Joyce Gaines to set age to 13 DOB:06/20/2011, or create a 13 year old patron
8 - Attempt to checkout item
9 - Item is blocked
10 - Apply patch
11 - Repeat tests, confirm no change

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2024-06-28 15:37:53 UTC
Created attachment 168266 [details] [review]
Bug 37155: Remove use of unblessed patron

This removes the unblessing of the patron object and uses fields form the patron

To test:
Confirm tests still pass:
prove -v t/db_dependent/Holds.t t/db_dependent/Circulation.t t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t t/db_dependent/Reserves.t t/db_dependent/api/v1/holds.t

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize (ashimema) 2024-06-28 15:38:33 UTC
It's great to see cleanups like this taking place, thanks Nick.

All working great and no regressions were spotted in testing.

QA scripts happy.

Passing QA
Comment 9 Katrin Fischer 2024-07-01 16:58:50 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 10 Lucas Gass (lukeg) 2024-11-18 17:26:58 UTC
Backported to 24.05.x for upcoming 24.05.06
Comment 11 Fridolin Somers 2024-12-02 11:14:34 UTC
I prefer not backport this refactoring to 23.11.x for stability
Comment 12 Caroline Cyr La Rose 2025-04-18 19:25:23 UTC
Internal architecture, nothing to add/edit in the manual.