Bug 32496 - Reduce unnecessary unblessings of objects in Circulation.pm
Summary: Reduce unnecessary unblessings of objects in Circulation.pm
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: David Gustafsson
QA Contact: Marcel de Rooy
URL:
Keywords: rel_23_11_candidate
Depends on: 31735
Blocks: 33746 34016
  Show dependency tree / graph
 
Reported: 2022-12-19 17:47 UTC by David Gustafsson
Modified: 2024-07-04 20:37 UTC (History)
7 users (show)

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

Attachments
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (165.88 KB, patch)
2022-12-19 18:21 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Optimize OPAC checkouts view (4.12 KB, patch)
2022-12-19 18:21 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (162.75 KB, patch)
2022-12-21 18:17 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (161.71 KB, patch)
2022-12-22 17:42 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (211.64 KB, patch)
2023-01-04 14:48 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (201.70 KB, patch)
2023-01-04 16:11 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (197.25 KB, patch)
2023-02-22 14:17 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (200.01 KB, patch)
2023-02-22 15:00 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (200.77 KB, patch)
2023-05-03 18:20 UTC, David Gustafsson 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (201.23 KB, patch)
2023-07-04 14:04 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 32496: Some fixes (2.28 KB, patch)
2023-07-04 14:04 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (201.12 KB, patch)
2023-07-16 13:01 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 32496: Some fixes (2.34 KB, patch)
2023-07-16 13:01 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 32496: (QA follow-up) Fix POD and DateTime use (1.77 KB, patch)
2023-07-16 13:01 UTC, Katrin Fischer 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (200.85 KB, patch)
2023-09-22 09:54 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 32496: Some fixes (2.34 KB, patch)
2023-09-22 09:54 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 32496: (QA follow-up) Fix POD and DateTime use (1.77 KB, patch)
2023-09-22 09:54 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm (200.83 KB, patch)
2023-09-22 12:34 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 32496: Some fixes (2.43 KB, patch)
2023-09-22 12:34 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 32496: (QA follow-up) Fix POD and DateTime use (1.86 KB, patch)
2023-09-22 12:34 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Bug 32496: (follow-up) Fix t/db_dependent/SIP/Message.t (2.18 KB, patch)
2023-09-22 14:32 UTC, Tomás Cohen Arazi (tcohen) 		Details | Diff | Splinter Review
Bug 32496: Fix t/db_dependent/SIP/ILS.t (1.36 KB, patch)
2023-09-22 14:56 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Bug 32496: Fix tests (7.77 KB, patch)
2023-09-22 15:03 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (18) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description David Gustafsson 2022-12-19 17:47:37 UTC 
There are a lot of instances in of unnecessary unblesssings in Circulation.pm (and also in some other parts of the C4 namespace), for legacy reasons I assume as it was an convenient way not having to adept the code to much after changing to the object oriented interface. This patch builds on Bug 31735 and cleans up a large part, but not all, of these uncalled-for unblessings. I focused mainly on refactoring CanBookBeRenewed and the subroutines it calls as this is run for each checked out item and has on of the highest performance impacts of the circulation related subroutines. The performance gains are pretty modest though, so the main motivation is to clean up the code (and in some instances avoid having to re-fetch the object where a method needs to be called, and an unblessed hashref was passed in).
Comment 1 David Gustafsson 2022-12-19 18:21:19 UTC 
Created attachment 144722 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t

Sponsored-by: Gothenburg University Library
Comment 2 David Gustafsson 2022-12-19 18:21:24 UTC 
Created attachment 144723 [details] [review]
Bug 32496: Optimize OPAC checkouts view
Comment 3 David Gustafsson 2022-12-21 18:17:16 UTC 
Created attachment 144789 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t

Sponsored-by: Gothenburg University Library
Comment 4 David Gustafsson 2022-12-22 17:42:35 UTC 
Created attachment 144804 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t

Sponsored-by: Gothenburg University Library
Comment 5 David Gustafsson 2023-01-03 17:10:48 UTC 
Found tests that still needs fixing, working on this and then should be ready for review.
Comment 6 David Gustafsson 2023-01-04 14:48:21 UTC 
Created attachment 145024 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library
Comment 7 Jonathan Druart 2023-01-04 15:05:06 UTC 
I like the idea, but the patch contains unrelated changes that may slow down its integration.

For instance:

-                C4::Circulation::ReturnLostItem( $self->borrowernumber,
-                    $debit->itemnumber );
+                C4::Circulation::ReturnLostItem( $self->borrowernumber, $debit->itemnumber );

In Koha::Hold::cancel
-        my ( undef, $next_hold ) = C4::Reserves::CheckReserves( $self->itemnumber );
+        my ( undef, $next_hold ) = CheckReserves( $self->item );

I am suspecting this last one will explode, CheckReserves is not imported.
Comment 8 Jonathan Druart 2023-01-04 15:14:12 UTC 
As well as:

* unrelated change: item_issued_to_other_patron

* 
-    my $issue = AddIssue( $patron_borrower->unblessed, $item_1->barcode);
+    my $issue = AddIssue( $patron->unblessed, $item_1->barcode);

+        my $issue = AddIssue( $renewing_borrower_obj->unblessed, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { 
auto_renew => 1 } );

And other AddIssue calls.

Should not unbless here.
Comment 9 Jonathan Druart 2023-01-04 15:14:47 UTC 
Sorry those 2 last comments were for the first patch of bug 31735.
Comment 10 Jonathan Druart 2023-01-04 15:18:22 UTC 
And, sorry again. I actually mixed the 2 reviews, I had the patches from the 2 bugs applied, reviewed 31735 and compared with the code with both applied.

The missing unblessed calls comment is not relevant.
Comment 11 David Gustafsson 2023-01-04 16:11:27 UTC 
Created attachment 145027 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library
Comment 12 David Gustafsson 2023-01-04 16:30:23 UTC 
Ok, thanks for spotting that. The changes pertaining to CheckReserves have not been moved to bug 31735. In Koha/Hold.pm now using the fully qualified sub name for CheckResvere, there is a use line for C4::Reserves that can probably be removed but keeping it not to cause further issue. For some reason imported subroutines seems not to work inside of object methods so you are probably right it would have crashed. Fixing the  formatting of C4::Circulation::ReturnLostItem( $self->borrowernumber,
                    $debit->itemnumber );
feels like such a minor change that would hardly merit it's own bug report, also noticed the if above is also incorrectly formatted. I can revert this change though.
Comment 13 Jonathan Druart 2023-01-04 16:37:39 UTC 
(In reply to David Gustafsson from comment #12)
> Fixing the  formatting of
> C4::Circulation::ReturnLostItem( $self->borrowernumber,
>                     $debit->itemnumber );
> feels like such a minor change that would hardly merit it's own bug report,
> also noticed the if above is also incorrectly formatted. I can revert this
> change though.

Actually it does not need to be fixed, it's the expected format. The line is longer than x (I guess 80 is the default) chars and so perltidy is splitting it into 2 lines.
Comment 14 David Gustafsson 2023-01-04 16:54:46 UTC 
Ok, there are other lines exceeding that line number in that file, but should in that case I would have formatted it as

C4::Circulation::ReturnLostItem(                       
    $self->borrowernumber,
    $debit->itemnumber    
);

Also realize there are formatting/style related changes left. Took the opportunity to fix a few of the most egregious ones while going the code as would never get around to beak out into separate issues. I know it's not best practice, but figured would not be to much of an issue.
Comment 15 David Gustafsson 2023-02-22 14:17:33 UTC 
Created attachment 147158 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library
Comment 16 David Gustafsson 2023-02-22 15:00:30 UTC 
Created attachment 147162 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library
Comment 17 David Gustafsson 2023-02-22 15:01:30 UTC 
Ran all tests and these fail:

t/db_dependent/Reserves.t                           (Wstat: 768 Tests: 77 Failed: 3)
  Failed tests:  26-27, 29
  Non-zero exit status: 3
t/db_dependent/selenium/regressions.t               (Wstat: 5632 Tests: 0 Failed: 0)
  Non-zero exit status: 22
  Parse errors: Bad plan.  You planned 8 tests but ran 0.
t/db_dependent/Koha/Plugins/Recall_hooks.t          (Wstat: 256 Tests: 4 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
t/db_dependent/Koha/Recalls.t                       (Wstat: 256 Tests: 20 Failed: 1)
  Failed test:  12
  Non-zero exit status: 1
t/db_dependent/Koha/Recall.t                        (Wstat: 256 Tests: 27 Failed: 1)
  Failed test:  15
  Non-zero exit status: 1

Also failing on master though so either some problem with my koha-testing-docker instance, or tests are broken in master.
Comment 18 David Gustafsson 2023-05-03 18:20:14 UTC 
Created attachment 150614 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library
Comment 19 David Gustafsson 2023-05-03 18:21:49 UTC 
Rebased against master, though need ensure tests still pass before ready for signoff.
Comment 20 David Gustafsson 2023-05-04 12:53:46 UTC 
Tests seems to pass, so again ready for signoff.
Comment 21 Jonathan Druart 2023-06-08 13:50:55 UTC 
This one should be pushed early in the release cycle.
Comment 22 Emily Lamancusa (emlam) 2023-06-22 14:04:38 UTC 
Looks like this needs another rebase:
Applying: Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm
error: sha1 information is lacking or useless (C4/Circulation.pm).
error: could not build fake ancestor
Comment 23 Jonathan Druart 2023-07-04 14:04:09 UTC 
Created attachment 153017 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 24 Jonathan Druart 2023-07-04 14:04:12 UTC 
Created attachment 153018 [details] [review]
Bug 32496: Some fixes

This can be squashed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 25 Katrin Fischer 2023-07-16 13:01:31 UTC 
Created attachment 153517 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 26 Katrin Fischer 2023-07-16 13:01:35 UTC 
Created attachment 153518 [details] [review]
Bug 32496: Some fixes

This can be squashed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2023-07-16 13:01:39 UTC 
Created attachment 153519 [details] [review]
Bug 32496: (QA follow-up) Fix POD and DateTime use

 FAIL	t/db_dependent/Koha/Object.t
   FAIL	  forbidden patterns
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 991)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 28 Katrin Fischer 2023-07-16 13:03:23 UTC 
I am not sure what/how to test... but I just started anyway :)

1) QA test tools

 FAIL	t/db_dependent/Koha/Object.t
   FAIL	  forbidden patterns
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 991)

Fixed in a follow-up.


2) Unit tests 

Passed.

Note: we have a 'dirty' test somewhere: I ended up with 4 new testbuilder-style itemtypes in my db. They should have been cleaned up by the test creating them.

3) Did some GUI testing:

* Checkouts with rental feeds
* Backdated checkouts
* Renewal
* Checkin
** Fines calculated
** Suspension days added

4) Went through the code, found a typo:

-C<$item> item hashref
+C<$item> patron object

Fixed in follow-up.

I added my sign-off lines to this, feel free to switch to PQA.
Comment 29 Jonathan Druart 2023-09-22 09:54:12 UTC 
Created attachment 156059 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 30 Jonathan Druart 2023-09-22 09:54:15 UTC 
Created attachment 156060 [details] [review]
Bug 32496: Some fixes

This can be squashed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 31 Jonathan Druart 2023-09-22 09:54:19 UTC 
Created attachment 156061 [details] [review]
Bug 32496: (QA follow-up) Fix POD and DateTime use

 FAIL	t/db_dependent/Koha/Object.t
   FAIL	  forbidden patterns
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 991)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Jonathan Druart 2023-09-22 09:55:04 UTC 
Really, still not pushed??
Comment 33 Marcel de Rooy 2023-09-22 12:25:22 UTC 
Looking here
Comment 34 Marcel de Rooy 2023-09-22 12:34:35 UTC 
Created attachment 156080 [details] [review]
Bug 32496: Reduce unnecessary unblessings of objects in Circulation.pm

Refactor the most performance critical subroutines in Circulation.pm
to take objects instead of unblessed ones to reduce unnecessary
unblessings and generally clean up the code.

To test:

1) Ensure the following tests all pass:
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Illrequests.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Template/Plugin/CirculationRules.t
  t/db_dependent/Letters/TemplateToolkit.t
  t/db_dependent/Members/GetAllIssues.t
  t/db_dependent/Members/IssueSlip.t
  t/db_dependent/Patron/Borrower_Discharge.t
  t/db_dependent/Patron/Borrower_PrevCheckout.t
  t/db_dependent/SIP/ILS.t
  t/db_dependent/Holds.t
  t/db_dependent/Holds/LocalHoldsPriority.t
  t/db_dependent/Holds/HoldFulfillmentPolicy.t
  t/db_dependent/Holds/HoldItemtypeLimit.t
  t/db_dependent/Reserves/GetReserveFee.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/Reserves.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Plugins/Recall_hooks.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.t
  t/db_dependent/Circulation/_CalculateAndUpdateFine.t

Sponsored-by: Gothenburg University Library

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 35 Marcel de Rooy 2023-09-22 12:34:38 UTC 
Created attachment 156081 [details] [review]
Bug 32496: Some fixes

This can be squashed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 36 Marcel de Rooy 2023-09-22 12:34:41 UTC 
Created attachment 156082 [details] [review]
Bug 32496: (QA follow-up) Fix POD and DateTime use

 FAIL	t/db_dependent/Koha/Object.t
   FAIL	  forbidden patterns
		forbidden pattern: Use of DateTime->now should certainly be replaced with dt_from_string (bug 24840) (line 991)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 37 Tomás Cohen Arazi (tcohen) 2023-09-22 13:55:50 UTC 
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 38 Tomás Cohen Arazi (tcohen) 2023-09-22 14:32:36 UTC 
Created attachment 156099 [details] [review]
Bug 32496: (follow-up) Fix t/db_dependent/SIP/Message.t
Comment 39 Jonathan Druart 2023-09-22 14:56:54 UTC 
Created attachment 156102 [details] [review]
Bug 32496: Fix t/db_dependent/SIP/ILS.t
Comment 40 Jonathan Druart 2023-09-22 15:03:17 UTC 
Created attachment 156103 [details] [review]
Bug 32496: Fix tests
Comment 41 Tomás Cohen Arazi (tcohen) 2023-09-22 15:43:25 UTC 
Follow-up pushed to master. Thanks, Jonathan.
Comment 42 Fridolin Somers 2023-09-22 19:15:55 UTC 
Enhancement not pushed to 23.05.x

Great job BTW ;)