Bug 31735 - Avoid re-fetching objects from database by passing them down instead of object ids
Summary: Avoid re-fetching objects from database by passing them down instead of objec...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Gustafsson
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 31676
Blocks: 30362 32496 34518
  Show dependency treegraph
 
Reported: 2022-10-11 13:25 UTC by David Gustafsson
Modified: 2023-08-31 11:39 UTC (History)
6 users (show)

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


Attachments
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (109.36 KB, patch)
2022-10-11 13:34 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid unesssary unblessing of objects (160.93 KB, patch)
2022-10-11 13:34 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Add caching for relatively expensive patron attributes (4.91 KB, patch)
2022-10-11 13:34 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache (4.07 KB, patch)
2022-10-11 13:34 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Match rules on effective item type instead of itype (897 bytes, patch)
2022-10-11 13:34 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (3.36 KB, patch)
2022-10-11 13:34 UTC, David Gustafsson
Details | Diff | Splinter Review
Benchmark before (772.21 KB, image/png)
2022-10-11 13:43 UTC, David Gustafsson
Details
Benchmark after (816.95 KB, image/png)
2022-10-11 13:43 UTC, David Gustafsson
Details
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (114.10 KB, patch)
2022-10-13 11:51 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid unesssary unblessing of objects (160.93 KB, patch)
2022-10-13 11:51 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Add caching for relatively expensive patron attributes (4.91 KB, patch)
2022-10-13 11:51 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache (4.07 KB, patch)
2022-10-13 11:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Match rules on effective item type instead of itype (897 bytes, patch)
2022-10-13 11:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (3.36 KB, patch)
2022-10-13 11:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache (4.07 KB, patch)
2022-10-13 12:55 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Match rules on effective item type instead of itype (897 bytes, patch)
2022-10-13 12:55 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (3.36 KB, patch)
2022-10-13 12:55 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (114.11 KB, patch)
2022-11-01 12:59 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid unesssary unblessing of objects (160.93 KB, patch)
2022-11-01 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Add caching for relatively expensive patron attributes (4.91 KB, patch)
2022-11-01 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache (4.07 KB, patch)
2022-11-01 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Match rules on effective item type instead of itype (897 bytes, patch)
2022-11-01 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (3.36 KB, patch)
2022-11-01 13:00 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (110.48 KB, patch)
2022-12-15 18:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid unesssary unblessing of objects (160.99 KB, patch)
2022-12-15 18:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache (4.25 KB, patch)
2022-12-15 18:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Match rules on effective item type instead of itype (897 bytes, patch)
2022-12-15 18:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (3.36 KB, patch)
2022-12-15 18:25 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (110.48 KB, patch)
2022-12-15 18:52 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (110.48 KB, patch)
2022-12-19 17:09 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.12 KB, patch)
2022-12-19 18:27 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (705 bytes, patch)
2022-12-21 17:12 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (113.68 KB, patch)
2022-12-21 18:20 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.12 KB, patch)
2022-12-21 18:21 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (705 bytes, patch)
2022-12-21 18:21 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (132.20 KB, patch)
2022-12-22 17:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.12 KB, patch)
2022-12-22 17:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (705 bytes, patch)
2022-12-22 17:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (132.39 KB, patch)
2022-12-22 17:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.12 KB, patch)
2022-12-22 17:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (705 bytes, patch)
2022-12-22 17:39 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.36 KB, patch)
2023-01-04 16:12 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.12 KB, patch)
2023-01-04 16:12 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (705 bytes, patch)
2023-01-04 16:12 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.01 KB, patch)
2023-01-04 16:35 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.01 KB, patch)
2023-01-04 16:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.12 KB, patch)
2023-01-04 16:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (705 bytes, patch)
2023-01-04 16:36 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.01 KB, patch)
2023-02-16 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.18 KB, patch)
2023-02-16 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (761 bytes, patch)
2023-02-16 14:56 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.13 KB, patch)
2023-02-22 11:53 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (707 bytes, patch)
2023-02-22 11:53 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (141.55 KB, patch)
2023-02-22 11:54 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.13 KB, patch)
2023-02-22 11:54 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (707 bytes, patch)
2023-02-22 11:54 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (141.54 KB, patch)
2023-02-22 15:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.18 KB, patch)
2023-02-22 15:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (761 bytes, patch)
2023-02-22 15:07 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.15 KB, patch)
2023-05-03 18:08 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.18 KB, patch)
2023-05-03 18:08 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (763 bytes, patch)
2023-05-03 18:08 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.14 KB, patch)
2023-05-04 15:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.23 KB, patch)
2023-05-04 15:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (816 bytes, patch)
2023-05-04 15:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines (142.19 KB, patch)
2023-05-05 00:25 UTC, David Nind
Details | Diff | Splinter Review
Bug 31735: Optimize OPAC checkouts view (4.28 KB, patch)
2023-05-05 00:25 UTC, David Nind
Details | Diff | Splinter Review
Bug 31735: Fix array that should be passed as reference (864 bytes, patch)
2023-05-05 00:25 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Gustafsson 2022-10-11 13:25:12 UTC
We recently had time-out issues and high server loads for patrons with high checkout counts (some patrons have several hundreds, up to about 400). One of the major bottlenecks turned out to be C4::Circulation::CanBookBeRenewed. The primary reason it performed so poorly was constant re-fetching of different database entities which could just as well be passed down directly instead of passing ids which requires re-fetching further down the call stack.

After refactoring the most performance critical subroutines to take objects instead of object ids the calls to expensive DBIx-methods has been greatly reduced. In addition some other optimizations and bug fixes related to non working caching resulted in further speedups. The performance gain for a patron with around 400 checkouts is currently about 500%.

I made an effort to fix all tests affected by the changes, but I can not guarantee there are absolutely no regressions left in the test-suite. Since many tests seem to fail due to reasons not related to this patch, mainly due to the state of my local instance triggering tests-bugs (for example caused by preferences not being mocked correctly, or relying on database test-entries that are not guaranteed to exist), it was impractical and very time consuming to make sure all tests still run successfully. So subset of tests where the changed subroutines are called directly was selected and these should now pass.

The following tests should be effected by the changes and should be run as a part of the test-plan:

t/db_dependent/api/v1/biblios.t
t/db_dependent/api/v1/checkouts.t
t/db_dependent/api/v1/return_claims.t
t/db_dependent/Circulation/CalcDateDue.t
t/db_dependent/Circulation/CheckIfIssuedToPatron.t
t/db_dependent/Circulation/dateexpiry.t
t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
t/db_dependent/Circulation/GetTopIssues.t
t/db_dependent/Circulation_holdsqueue.t
t/db_dependent/Circulation/IsItemIssued.t
t/db_dependent/Circulation/issue.t
t/db_dependent/Circulation/MarkIssueReturned.t
t/db_dependent/Circulation/maxsuspensiondays.t
t/db_dependent/Circulation/ReturnClaims.t
t/db_dependent/Circulation/Returns.t
t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
t/db_dependent/Circulation.t
t/db_dependent/Circulation/TooMany.t
t/db_dependent/Circulation/transferbook.t
t/db_dependent/DecreaseLoanHighHolds.t
t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
t/db_dependent/HoldsQueue.t
t/db_dependent/Holds/RevertWaitingStatus.t
t/db_dependent/Illrequests.t
t/db_dependent/ILSDI_Services.t
t/db_dependent/Items.t
t/db_dependent/Koha/Account/Line.t
t/db_dependent/Koha/Acquisition/Order.t
t/db_dependent/Koha/Biblio.t
t/db_dependent/Koha/Holds.t
t/db_dependent/Koha/Items.t
t/db_dependent/Koha/Item.t
t/db_dependent/Koha/Object.t
t/db_dependent/Koha/Patrons.t
t/db_dependent/Koha/Plugins/Circulation_hooks.t
t/db_dependent/Koha/Pseudonymization.t
t/db_dependent/Koha/Recalls.t
t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
t/db_dependent/Reserves.t
t/db_dependent/rollingloans.t
t/db_dependent/selenium/regressions.t
t/db_dependent/SIP/ILS.t

The following still fails for me but should either work on a fresh Koha installation, or fail for unrelated reasons (that is they also fail in Koha master):

t/db_dependent/Koha/Plugins/Circulation_hooks.t
t/db_dependent/Koha/Items.t
t/db_dependent/SIP/ILS.t

To be able to run t/db_dependent/Circulation.t in my Koha instance there where some existing bugs that needed fixing, so this patch have a dependency on Bug 31676.
Comment 1 David Gustafsson 2022-10-11 13:34:33 UTC
Created attachment 141603 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 2 David Gustafsson 2022-10-11 13:34:38 UTC
Created attachment 141604 [details] [review]
Bug 31735: Avoid unesssary unblessing of objects
Comment 3 David Gustafsson 2022-10-11 13:34:43 UTC
Created attachment 141605 [details] [review]
Bug 31735: Add caching for relatively expensive patron attributes
Comment 4 David Gustafsson 2022-10-11 13:34:46 UTC
Created attachment 141606 [details] [review]
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache

get_yaml_pref_hash also allows invalid YAML and only parses a limited
subset so remove this method to avoid future issues.
Comment 5 David Gustafsson 2022-10-11 13:34:51 UTC
Created attachment 141607 [details] [review]
Bug 31735: Match rules on effective item type instead of itype
Comment 6 David Gustafsson 2022-10-11 13:34:56 UTC
Created attachment 141608 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 7 David Gustafsson 2022-10-11 13:43:31 UTC
Created attachment 141609 [details]
Benchmark before
Comment 8 David Gustafsson 2022-10-11 13:43:59 UTC
Created attachment 141610 [details]
Benchmark after
Comment 9 David Gustafsson 2022-10-13 11:51:47 UTC
Created attachment 141795 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 10 David Gustafsson 2022-10-13 11:51:52 UTC
Created attachment 141796 [details] [review]
Bug 31735: Avoid unesssary unblessing of objects
Comment 11 David Gustafsson 2022-10-13 11:51:56 UTC
Created attachment 141797 [details] [review]
Bug 31735: Add caching for relatively expensive patron attributes
Comment 12 David Gustafsson 2022-10-13 11:52:01 UTC
Created attachment 141798 [details] [review]
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache

get_yaml_pref_hash also allows invalid YAML and only parses a limited
subset so remove this method to avoid future issues.
Comment 13 David Gustafsson 2022-10-13 11:52:06 UTC
Created attachment 141799 [details] [review]
Bug 31735: Match rules on effective item type instead of itype
Comment 14 David Gustafsson 2022-10-13 11:52:11 UTC
Created attachment 141800 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 15 David Gustafsson 2022-10-13 12:55:19 UTC
Created attachment 141819 [details] [review]
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache

get_yaml_pref_hash also allows invalid YAML and only parses a limited
subset so remove this method to avoid future issues.
Comment 16 David Gustafsson 2022-10-13 12:55:24 UTC
Created attachment 141820 [details] [review]
Bug 31735: Match rules on effective item type instead of itype
Comment 17 David Gustafsson 2022-10-13 12:55:29 UTC
Created attachment 141821 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 18 David Gustafsson 2022-11-01 12:59:57 UTC
Created attachment 142865 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 19 David Gustafsson 2022-11-01 13:00:03 UTC
Created attachment 142866 [details] [review]
Bug 31735: Avoid unesssary unblessing of objects
Comment 20 David Gustafsson 2022-11-01 13:00:09 UTC
Created attachment 142867 [details] [review]
Bug 31735: Add caching for relatively expensive patron attributes
Comment 21 David Gustafsson 2022-11-01 13:00:14 UTC
Created attachment 142868 [details] [review]
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache

get_yaml_pref_hash also allows invalid YAML and only parses a limited
subset so remove this method to avoid future issues.
Comment 22 David Gustafsson 2022-11-01 13:00:19 UTC
Created attachment 142869 [details] [review]
Bug 31735: Match rules on effective item type instead of itype
Comment 23 David Gustafsson 2022-11-01 13:00:24 UTC
Created attachment 142870 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 24 Katrin Fischer 2022-11-14 21:09:50 UTC
Optimizations are always good, but reading a bug title as general like this, always makes me suspicious. 

I feel like this should be an omnibus with separate dependent bugs for the identified issues. If they need to be done in a certain sequence (which seems likely for conflicts) they can also be linked to each other using depends.

I'd strongly recommend 'small steps' over big massive patch sets, as they tend to get stuck in queues because it's too hard to find the time to test them in one session and they are also most of the time scary.
Comment 25 David Gustafsson 2022-12-15 18:14:11 UTC
Sorry about the late reply. Yes I should probably have split this one up into several issues and fully understand this is would be very cumbersome to to review all at once. I anticipated this could be the case so each commit should already be pretty isolated and even though there are some dependencies breaking them into separate issues is doable. Have begun in doing so and changing status of this one until ready to review again.
Comment 26 David Gustafsson 2022-12-15 18:25:19 UTC
Created attachment 144628 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 27 David Gustafsson 2022-12-15 18:25:25 UTC
Created attachment 144629 [details] [review]
Bug 31735: Avoid unesssary unblessing of objects
Comment 28 David Gustafsson 2022-12-15 18:25:30 UTC
Created attachment 144630 [details] [review]
Bug 31735: Remove Koha::Config::SysPref->find since bypasses cache

get_yaml_pref_hash also allows invalid YAML and only parses a limited
subset so remove this method to avoid future issues.
Comment 29 David Gustafsson 2022-12-15 18:25:34 UTC
Created attachment 144631 [details] [review]
Bug 31735: Match rules on effective item type instead of itype
Comment 30 David Gustafsson 2022-12-15 18:25:39 UTC
Created attachment 144632 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 31 David Gustafsson 2022-12-15 18:52:08 UTC
Created attachment 144635 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 32 David Gustafsson 2022-12-15 19:01:42 UTC
Have moved two of the commits to bug 32476 and bug 32478 which has no dependencies. What is left to do is create an issue depending on this one with the changes regarding uncalled-for unblessings of Koha objects. This one should now be ready for review.
Comment 33 David Gustafsson 2022-12-19 17:09:41 UTC
Created attachment 144721 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 34 David Gustafsson 2022-12-19 18:27:17 UTC
Created attachment 144724 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 35 David Nind 2022-12-21 00:49:35 UTC
I had a go at testing. Failed QA because tests failed and there were error traces when renewing items from the OPAC.

Tested using KTD (koha-testing-docker) with the sample data.

Test failures
=============

These tests failed - note that I didn't check whether these failed before the patch was applied:

prove t/db_dependent/api/v1/biblios.t
prove t/db_dependent/api/v1/checkouts.t
prove t/db_dependent/api/v1/return_claims.t
prove t/db_dependent/Circulation/Returns.t
prove t/db_dependent/Circulation.t
prove t/db_dependent/Circulation/transferbook.t
prove t/db_dependent/Illrequests.t
prove t/db_dependent/Koha/Account/Line.t
prove t/db_dependent/Koha/Items.t
prove t/db_dependent/Koha/Object.t
prove t/db_dependent/Koha/Patrons.t
prove t/db_dependent/Koha/Recalls.t
prove t/db_dependent/Koha/Recall.t
prove t/db_dependent/Letters/TemplateToolkit.t
prove t/db_dependent/Reserves.t
prove t/db_dependent/selenium/regressions.t
prove t/db_dependent/SIP/ILS.t

Renewing from the OPAC - errors
===============================

I checked out three different items to one patron, making one of them already overdue. The default circulation rule in KTD allows 5 renewals.

Error 1: Renew a single overdue item from the OPAC (Summary > Overdue tab)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

SQL::Abstract::puke(): [SQL::Abstract::__ANON__] Fatal: Argument passed to the 'IN' operator can not be undefined at /kohadevbox/koha/Koha/Objects.pm line 312
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

    in DBIx::Class::Exception::throw at /usr/share/perl5/DBIx/Class/Exception.pm line 77
......

Note that it renewed fine from Summary > Checkout tab.

Error 2: Renew multiple items (two items individually selected) from the Summary > Checked out tab (where one is overdue)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

SQL::Abstract::puke(): [SQL::Abstract::__ANON__] Fatal: unexpected operator '715' with undef operand at /kohadevbox/koha/Koha/Objects.pm line 312
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

    in DBIx::Class::Exception::throw at /usr/share/perl5/DBIx/Class/Exception.pm line 77
....


Error 3: Renew multiple items (renew all button selected) from the Summary > Checked out tab where one is overdue:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

SQL::Abstract::puke(): [SQL::Abstract::__ANON__] Fatal: Argument passed to the 'IN' operator can not be undefined at /kohadevbox/koha/Koha/Objects.pm line 312
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77

    in DBIx::Class::Exception::throw at /usr/share/perl5/DBIx/Class/Exception.pm line 77
....
Comment 36 David Gustafsson 2022-12-21 17:12:00 UTC
Created attachment 144784 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 37 David Gustafsson 2022-12-21 18:20:59 UTC
Created attachment 144790 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 38 David Gustafsson 2022-12-21 18:21:04 UTC
Created attachment 144791 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 39 David Gustafsson 2022-12-21 18:21:10 UTC
Created attachment 144792 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 40 David Gustafsson 2022-12-21 18:37:19 UTC
Thanks for the review. I obviously failed to manually test the opac-view, this is now fixed (the array for the in-condition should have been passed by reference).

Regarding the failing tests, it's surprising so many are failing. I pretty sure I originally ran through them successfully, so it could be that those test regressions have been introduced after the rebase was performed, even though it seems odd regarding how many are now failing. I ran through those that failed for you on the current master, and got the following results:

Passing:

t/db_dependent/api/v1/biblios.t
t/db_dependent/api/v1/return_claims.t
t/db_dependent/Circulation/transferbook.t

Failing:

t/db_dependent/api/v1/checkouts.t
t/db_dependent/Circulation/Returns.t
t/db_dependent/Circulation.t
t/db_dependent/Illrequests.t
t/db_dependent/Koha/Account/Line.t (foreign key constraint, could work on clean Koha with only test data)
t/db_dependent/Koha/Items.t
t/db_dependent/Koha/Object.t
t/db_dependent/Koha/Patrons.t
t/db_dependent/Koha/Recalls.t
t/db_dependent/Koha/Recall.t
t/db_dependent/Letters/TemplateToolkit.t (Duplicate entry 'a_t_barcode', could work with test data)
t/db_dependent/Reserves.t
t/db_dependent/SIP/ILS.t (foreign key constraint, might work with test data)

The ones passing also pass after applying the patches (though there were some changes in bug 32496 that should have been included here which I have now moved for transferbook.t to pass).

I haven't used koha-testing-docker before, but could try to run the tests there and see if I get the same result (for koha-master). Feel free to do the same if you have the time I don't know how easy it will be to get it up and running. It could be that the current test suite is pretty broken, even though it seems odd this has gone under the radar regarding the number of failing tests.
Comment 41 David Nind 2022-12-21 19:29:40 UTC
Hi David.

Thanks for the updates!

I'll have another go at testing in the next day and let you know how I get on - I'll run the tests before applying the patch to see if any have "problems".

David
Comment 42 David Gustafsson 2022-12-22 15:02:34 UTC
Ok! Thanks. Got the test-suite working and most tests that failed in my local docker setup now works, so must be some issue with missing test-data or some other difference in environment. You don't have to run the tests, I will have a look at the tests that passes on master in KTD but fails after applying the patch.
Comment 43 David Gustafsson 2022-12-22 17:36:08 UTC
Created attachment 144798 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.t
  t/db_dependent/SIP/ILS.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 44 David Gustafsson 2022-12-22 17:36:13 UTC
Created attachment 144799 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 45 David Gustafsson 2022-12-22 17:36:18 UTC
Created attachment 144800 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 46 David Gustafsson 2022-12-22 17:39:19 UTC
Created attachment 144801 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 47 David Gustafsson 2022-12-22 17:39:24 UTC
Created attachment 144802 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 48 David Gustafsson 2022-12-22 17:39:29 UTC
Created attachment 144803 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 49 David Gustafsson 2022-12-22 17:41:51 UTC
The tests should now be passing, must have previously failed to run all relevant tests (added a few), and found some issues that now should be fixed.
Comment 50 David Gustafsson 2022-12-22 17:48:51 UTC
Running the entire test-suite and will check the results tomorrow, in case there are any other tests affected that also failed to check.
Comment 51 David Gustafsson 2023-01-04 15:28:55 UTC
Koha testing docker is really not working all that well for me, it keeps crashing erratically, some times starts up successfully, very often crashes for a host of different reasons. Thus it's very hard for me to currently judge if there are more tests then those listed affected by the changes. Feel free to run the test suite with and without the patch set, for me it seems to fail at various points in both cases, but it's possible the reason is my ktd-environment is screwed up someway.
Comment 52 David Gustafsson 2023-01-04 16:12:47 UTC
Created attachment 145028 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 53 David Gustafsson 2023-01-04 16:12:51 UTC
Created attachment 145029 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 54 David Gustafsson 2023-01-04 16:12:56 UTC
Created attachment 145030 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 55 David Gustafsson 2023-01-04 16:35:28 UTC
Created attachment 145032 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 56 David Gustafsson 2023-01-04 16:36:43 UTC
Created attachment 145033 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 57 David Gustafsson 2023-01-04 16:36:48 UTC
Created attachment 145034 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 58 David Gustafsson 2023-01-04 16:36:53 UTC
Created attachment 145035 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 59 Nick Clemens 2023-02-16 14:56:03 UTC
Created attachment 146735 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 60 Nick Clemens 2023-02-16 14:56:06 UTC
Created attachment 146736 [details] [review]
Bug 31735: Optimize OPAC checkouts view

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 61 Nick Clemens 2023-02-16 14:56:09 UTC
Created attachment 146737 [details] [review]
Bug 31735: Fix array that should be passed as reference

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 62 Kyle M Hall 2023-02-17 17:57:21 UTC
Getting one unit test failure:

root@kohadevbox:koha(bug31735-qa)$ prove t/db_dependent/Koha/CirculationRules.t
t/db_dependent/Koha/CirculationRules.t .. 1/7
    #   Failed test 'No rule for branch, no default rule, fallback default (refund)'
    #   at t/db_dependent/Koha/CirculationRules.t line 856.
    #     Structures begin differing at:
    #          $got->{lostreturn} = '0'
    #     $expected->{lostreturn} = 'refund'
    # Looks like you failed 1 test of 7.

#   Failed test 'get_lostreturn_policy() tests'
#   at t/db_dependent/Koha/CirculationRules.t line 865.
# Looks like you failed 1 test of 7.
t/db_dependent/Koha/CirculationRules.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/7 subtests

Test Summary Report
-------------------
t/db_dependent/Koha/CirculationRules.t (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  7
  Non-zero exit status: 1
Files=1, Tests=7,  3 wallclock secs ( 0.01 usr  0.00 sys +  1.90 cusr  0.13 csys =  2.04 CPU)
Result: FAIL
Comment 63 David Gustafsson 2023-02-22 11:53:06 UTC
Created attachment 147124 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 64 David Gustafsson 2023-02-22 11:53:10 UTC
Created attachment 147125 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 65 David Gustafsson 2023-02-22 11:54:43 UTC
Created attachment 147126 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Comment 66 David Gustafsson 2023-02-22 11:54:46 UTC
Created attachment 147127 [details] [review]
Bug 31735: Optimize OPAC checkouts view
Comment 67 David Gustafsson 2023-02-22 11:54:49 UTC
Created attachment 147128 [details] [review]
Bug 31735: Fix array that should be passed as reference
Comment 68 David Gustafsson 2023-02-22 12:00:51 UTC
(In reply to Kyle M Hall from comment #62)
> Getting one unit test failure:
> 
> root@kohadevbox:koha(bug31735-qa)$ prove
> t/db_dependent/Koha/CirculationRules.t
> t/db_dependent/Koha/CirculationRules.t .. 1/7
>     #   Failed test 'No rule for branch, no default rule, fallback default
> (refund)'
>     #   at t/db_dependent/Koha/CirculationRules.t line 856.
>     #     Structures begin differing at:
>     #          $got->{lostreturn} = '0'
>     #     $expected->{lostreturn} = 'refund'
>     # Looks like you failed 1 test of 7.
> 
> #   Failed test 'get_lostreturn_policy() tests'
> #   at t/db_dependent/Koha/CirculationRules.t line 865.
> # Looks like you failed 1 test of 7.
> t/db_dependent/Koha/CirculationRules.t .. Dubious, test returned 1 (wstat
> 256, 0x100)
> Failed 1/7 subtests
> 
> Test Summary Report
> -------------------
> t/db_dependent/Koha/CirculationRules.t (Wstat: 256 Tests: 7 Failed: 1)
>   Failed test:  7
>   Non-zero exit status: 1
> Files=1, Tests=7,  3 wallclock secs ( 0.01 usr  0.00 sys +  1.90 cusr  0.13
> csys =  2.04 CPU)
> Result: FAIL

Ok! This should now be fixed. Was actually a bug in the test (not clearing cache after changing rule ) exposed by fixing caching of falsy values. That fix however should not be in this bug but in Bug 32092, so removed the change and the test should now pass.
Comment 69 Nick Clemens 2023-02-22 15:07:18 UTC
Created attachment 147164 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 70 Nick Clemens 2023-02-22 15:07:21 UTC
Created attachment 147165 [details] [review]
Bug 31735: Optimize OPAC checkouts view

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 71 Nick Clemens 2023-02-22 15:07:24 UTC
Created attachment 147166 [details] [review]
Bug 31735: Fix array that should be passed as reference

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 72 Jonathan Druart 2023-03-23 14:17:06 UTC
I am getting a conflict here that I managed to resolve, but then a "sha1 lacking or useless" on bug 32496. Could you rebase and eventually push both to a remote branch?
Comment 73 David Gustafsson 2023-05-03 18:08:14 UTC
Created attachment 150611 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 74 David Gustafsson 2023-05-03 18:08:17 UTC
Created attachment 150612 [details] [review]
Bug 31735: Optimize OPAC checkouts view

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 75 David Gustafsson 2023-05-03 18:08:20 UTC
Created attachment 150613 [details] [review]
Bug 31735: Fix array that should be passed as reference

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 76 David Gustafsson 2023-05-03 18:08:46 UTC
Rebased against master.
Comment 77 Jonathan Druart 2023-05-04 06:32:52 UTC
(In reply to Jonathan Druart from comment #72)
> I am getting a conflict here that I managed to resolve, but then a "sha1
> lacking or useless" on bug 32496. Could you rebase and eventually push both
> to a remote branch?

I pushed the patches to https://gitlab.com/joubu/Koha/-/commits/bug_32496 in case we need it later.
Comment 78 Kyle M Hall 2023-05-04 15:01:36 UTC
Created attachment 150676 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 79 Kyle M Hall 2023-05-04 15:01:44 UTC
Created attachment 150677 [details] [review]
Bug 31735: Optimize OPAC checkouts view

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 80 Kyle M Hall 2023-05-04 15:01:46 UTC
Created attachment 150678 [details] [review]
Bug 31735: Fix array that should be passed as reference

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 81 David Nind 2023-05-05 00:25:50 UTC
Created attachment 150699 [details] [review]
Bug 31735: Avoid re-fetcing objects from database by passing them directly instead of ids to various subroutines

To test:

1) Run the following test and make sure all pass:
  t/db_dependent/api/v1/biblios.t
  t/db_dependent/api/v1/checkouts.t
  t/db_dependent/api/v1/return_claims.t
  t/db_dependent/Circulation/CalcDateDue.t
  t/db_dependent/Circulation/CheckIfIssuedToPatron.t
  t/db_dependent/Circulation/dateexpiry.t
  t/db_dependent/Circulation/GetPendingOnSiteCheckouts.t
  t/db_dependent/Circulation/GetTopIssues.t
  t/db_dependent/Circulation_holdsqueue.t
  t/db_dependent/Circulation/IsItemIssued.t
  t/db_dependent/Circulation/issue.t
  t/db_dependent/Circulation/MarkIssueReturned.t
  t/db_dependent/Circulation/maxsuspensiondays.t
  t/db_dependent/Circulation/ReturnClaims.t
  t/db_dependent/Circulation/Returns.t
  t/db_dependent/Circulation/SwitchOnSiteCheckouts.t
  t/db_dependent/Circulation.t
  t/db_dependent/Circulation/TooMany.t
  t/db_dependent/Circulation/transferbook.t
  t/db_dependent/DecreaseLoanHighHolds.t
  t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t
  t/db_dependent/HoldsQueue.t
  t/db_dependent/Holds/RevertWaitingStatus.t
  t/db_dependent/Illrequests.t
  t/db_dependent/ILSDI_Services.t
  t/db_dependent/Items.t
  t/db_dependent/Koha/Account/Line.t
  t/db_dependent/Koha/Acquisition/Order.t
  t/db_dependent/Koha/Biblio.t
  t/db_dependent/Koha/Holds.t
  t/db_dependent/Koha/Items.t
  t/db_dependent/Koha/Item.t
  t/db_dependent/Koha/Object.t
  t/db_dependent/Koha/Patrons.t
  t/db_dependent/Koha/Plugins/Circulation_hooks.t
  t/db_dependent/Koha/Pseudonymization.t
  t/db_dependent/Koha/Recalls.t
  t/db_dependent/Koha/Recall.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/Reserves/GetReserveFee.t
  t/db_dependent/Reserves.t
  t/db_dependent/rollingloans.t
  t/db_dependent/selenium/regressions.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/Circulation/transferbook.t
2) Performe one or more checkouts for a patron, making sure
  that the circulation rules allows for renewals (for example by
  setting an earlier due-date).
3) Log in as this patron in OPAC and make sure the list of
  checkouts is displayed correctly, and that renewing an issue
  still works.

Sponsored-by: Gothenburg University Library
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 82 David Nind 2023-05-05 00:25:53 UTC
Created attachment 150700 [details] [review]
Bug 31735: Optimize OPAC checkouts view

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 83 David Nind 2023-05-05 00:25:57 UTC
Created attachment 150701 [details] [review]
Bug 31735: Fix array that should be passed as reference

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: David Nind <david@davidnind.com>
Comment 84 David Nind 2023-05-05 00:32:15 UTC
I've tested and added my sign-off (status was set to needs signoff - maybe Kyle or Nick didn't change the status?).

Testing notes (using KTD):
- All tests pass before after the patch is applied (ran prove -v t/db_dependent/)
- Everything works as per the test plan
- I also checked out an item with a due date before today and checked the automatic renewal, then ran the automatic renewal script misc/cronjobs/automatic_renwal.pl -v -c
Comment 85 Tomás Cohen Arazi 2023-05-12 15:40:56 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 86 Matt Blenkinsop 2023-05-15 13:03:13 UTC
Enhancement - not backporting to 22.11.x