Bug 37155 - Remove unnecessary unblessing of patron in CanItemBeReserved
Summary: Remove unnecessary unblessing of patron in CanItemBeReserved
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks: 37154
  Show dependency treegraph
 
Reported: 2024-06-21 16:09 UTC by Nick Clemens (kidclamp)
Modified: 2024-07-25 11:23 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:


Attachments
Bug 37155: Refactor GetAgeRestrictions (8.18 KB, patch)
2024-06-21 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37155: Remove use of unblessed patron (3.19 KB, patch)
2024-06-21 16:11 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37155: Refactor GetAgeRestrictions (8.22 KB, patch)
2024-06-21 19:53 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37155: Remove use of unblessed patron (3.24 KB, patch)
2024-06-21 19:53 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 37155: Refactor GetAgeRestrictions (8.29 KB, patch)
2024-06-28 15:37 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 37155: Remove use of unblessed patron (3.30 KB, patch)
2024-06-28 15:37 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!