From 10fa5c4a1410a0682547112f259ff7612bd3007e Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 18 Dec 2023 14:21:57 -0500 Subject: [PATCH] Bug 28530: Implement float limits Test Plan: 1) Apply this patch set 2) Run updatedatabase.pl 3) Restart all the things! 4) Enable UseLibraryFloatLimits 5) Verify items float correctly as per your circulation rules 6) Set some float limits from the Library Float Limits configuration page in the Administration section The simplest solution is to set the float limit from one branch to 1 item, a second to 100 items, and a third to 300 items for a single itemtype 7) Verify checked in items that should have floated at this branch are now transfered to the branch with the best float ratio Signed-off-by: Andrew Fuerste-Henry Signed-off-by: Kris Becker --- C4/Circulation.pm | 14 ++ Koha/Library/FloatLimits.pm | 148 +++++++++++++++--- Koha/Schema/Result/Branch.pm | 21 ++- Koha/Schema/Result/Branchtransfer.pm | 9 +- Koha/Schema/Result/Itemtype.pm | 17 +- admin/float_limits.pl | 2 +- circ/returns.pl | 17 +- .../prog/en/includes/admin-menu.inc | 68 ++++---- .../prog/en/modules/admin/admin-home.tt | 2 + .../prog/en/modules/admin/float_limits.tt | 5 +- .../prog/en/modules/circ/returns.tt | 2 + t/db_dependent/Koha/Library/FloatLimits.t | 8 +- 12 files changed, 235 insertions(+), 78 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 48185eb0224..a8ee46ad748 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -52,6 +52,7 @@ use Koha::Patrons; use Koha::Patron::Debarments qw( DelUniqueDebarment AddUniqueDebarment ); use Koha::Database; use Koha::Libraries; +use Koha::Library::FloatLimits; use Koha::Account::Lines; use Koha::Holds; use Koha::Account::Lines; @@ -2272,6 +2273,19 @@ sub AddReturn { # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) my $transfer_trigger = $hbr eq 'homebranch' ? 'ReturnToHome' : $hbr eq 'holdingbranch' ? 'ReturnToHolding' : undef; + # check library float limits if enabled if the item isn't being transferred away + if ( ( $returnbranch eq $branch ) && C4::Context->preference('UseLibraryFloatLimits') ) { + my $effective_itemtype = $item->effective_itemtype; + my $limit = Koha::Library::FloatLimits->find( { itemtype => $effective_itemtype, branchcode => $branch } ); + if ($limit) { + my $transfer_library = Koha::Library::FloatLimits->lowest_ratio_library( $item, $branch ); + if ( $transfer_library && $transfer_library->branchcode ne $branch ) { + $returnbranch = $transfer_library->branchcode; + $transfer_trigger = 'LibraryFloatLimit'; + } + } + } + my $borrowernumber = $patron ? $patron->borrowernumber : undef; # we don't know if we had a borrower or not my $patron_unblessed = $patron ? $patron->unblessed : {}; diff --git a/Koha/Library/FloatLimits.pm b/Koha/Library/FloatLimits.pm index 5697b38b1c5..189c67d6cfe 100644 --- a/Koha/Library/FloatLimits.pm +++ b/Koha/Library/FloatLimits.pm @@ -1,5 +1,3 @@ -## Please see file perltidy.ERR -## Please see file perltidy.ERR package Koha::Library::FloatLimits; # Copyright ByWater Solutions 2016 @@ -24,6 +22,7 @@ use Modern::Perl; use Koha::Database; use Koha::Library::FloatLimit; +use Koha::Libraries; use C4::Circulation qw(IsBranchTransferAllowed); use base qw(Koha::Objects); @@ -43,40 +42,141 @@ Koha::Library::FloatLimits - Koha Library::FloatLimit object set class sub lowest_ratio_library { my ( $self, $item, $branchcode ) = @_; - my $field = C4::Context->preference('item-level_itypes') ? 'items.itype' : 'biblioitems.itemtype'; - my $query = qq{ - SELECT branchcode - FROM library_float_limits - LEFT JOIN items ON ( items.itype = library_float_limits.itemtype AND items.holdingbranch = library_float_limits.branchcode ) - LEFT JOIN biblioitems ON ( items.biblioitemnumber = biblioitems.biblioitemnumber ) - WHERE library_float_limits.itemtype = ? - AND ( $field = ? OR $field IS NULL ) - AND library_float_limits.float_limit != 0 - GROUP BY branchcode - ORDER BY COUNT(items.holdingbranch)/library_float_limits.float_limit ASC, library_float_limits.float_limit DESC; - }; - - my $results = C4::Context->dbh->selectall_arrayref( - $query, { Slice => {} }, $item->effective_itemtype, - $item->effective_itemtype - ); + my $schema = Koha::Database->new->schema; + + my @float_limits = $schema->resultset('LibraryFloatLimit')->search( + { + itemtype => $item->effective_itemtype, + float_limit => { '!=' => 0 } + } + )->all; + + return unless @float_limits; + + my @candidates; + my $use_item_level = C4::Context->preference('item-level_itypes'); + + foreach my $limit (@float_limits) { + my $branch = $limit->get_column('branchcode'); + my $float_limit_val = $limit->get_column('float_limit'); + + my $item_count; + + if ($use_item_level) { + my $at_branch_count = Koha::Items->search( + { + itype => $item->effective_itemtype, + holdingbranch => $branch, + onloan => undef + }, + )->count; + + # Count items in transit TO this branch + my $in_transit_to_count = Koha::Items->search( + { + itype => $item->effective_itemtype, + + # Join with active transfers where this branch is the destination + }, + { + join => 'branchtransfers', + where => { + 'branchtransfers.tobranch' => $branch, + 'branchtransfers.datearrived' => undef, # Still in transit + 'branchtransfers.datecancelled' => undef, #Not cancelled + }, + distinct => 1 + } + )->count; + + my $in_transit_from_count = Koha::Items->search( + { itype => $item->effective_itemtype }, + { + join => 'branchtransfers', + where => { + 'branchtransfers.frombranch' => $branch, + 'branchtransfers.datearrived' => undef, # Still in transit + 'branchtransfers.datecancelled' => undef, #Not cancelled + }, + distinct => 1 + } + )->count; + $item_count = $at_branch_count + $in_transit_to_count - $in_transit_from_count; + } else { + my $at_branch_count = Koha::Items->search( + { + holdingbranch => $branch, + 'biblioitem.itemtype' => $item->effective_itemtype, + onloan => undef + }, + )->count; + + # Count items in transit TO this branch + my $in_transit_to_count = Koha::Items->search( + { + itype => $item->effective_itemtype, + }, + { + join => 'branchtransfers', + where => { + 'branchtransfers.tobranch' => $branch, + 'branchtransfers.datearrived' => undef, # Still in transit + 'branchtransfers.datecancelled' => undef, #Not cancelled + }, + distinct => 1 + } + )->count; + + my $in_transit_from_count = Koha::Items->search( + { + itype => $item->effective_itemtype, + }, + { + join => 'branchtransfers', + where => { + 'branchtransfers.frombranch' => $branch, + 'branchtransfers.datearrived' => undef, # Still in transit + 'branchtransfers.datecancelled' => undef, #Not cancelled + }, + distinct => 1 + } + )->count; + $item_count = $at_branch_count + $in_transit_to_count - $in_transit_from_count; + } + + my $ratio = $item_count / $float_limit_val; + + push @candidates, { + branchcode => $branch, + ratio => $ratio, + float_limit => $float_limit_val + }; + } + + # sort the branches by lowest ratio in the event of a tie choose a random branch + @candidates = sort { $a->{ratio} <=> $b->{ratio} || rand() <=> rand() } @candidates; my $UseBranchTransferLimits = C4::Context->preference("UseBranchTransferLimits"); my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ? 'effective_itemtype' : 'ccode'; - foreach my $r (@$results) { + my $transfer_branch; + for my $candidate (@candidates) { if ($UseBranchTransferLimits) { my $allowed = C4::Circulation::IsBranchTransferAllowed( - $r->{branchcode}, # to - $branchcode, # from + $candidate->{branchcode}, + $branchcode, $item->$BranchTransferLimitsType ); - return $r->{branchcode} if $allowed; + $transfer_branch = Koha::Libraries->find( $candidate->{branchcode} ) if $allowed; + last; } else { - return $r->{branchcode}; + $transfer_branch = Koha::Libraries->find( $candidate->{branchcode} ); + last; } } + + return $transfer_branch; } =head3 _type diff --git a/Koha/Schema/Result/Branch.pm b/Koha/Schema/Result/Branch.pm index 787c0772195..472eb790566 100644 --- a/Koha/Schema/Result/Branch.pm +++ b/Koha/Schema/Result/Branch.pm @@ -800,6 +800,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 library_float_limits + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "library_float_limits", + "Koha::Schema::Result::LibraryFloatLimit", + { "foreign.branchcode" => "self.branchcode" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + =head2 library_groups Type: has_many @@ -996,8 +1011,8 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-05-03 13:13:25 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:HiH1QNlDqKcq9GeM85Pu0A +# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-07-02 11:14:12 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:3TsuWEa3C2DEKJ1gR+V2Ig __PACKAGE__->has_many( "additional_field_values", @@ -1040,4 +1055,4 @@ sub koha_objects_class { 'Koha::Libraries'; } -1; +1; \ No newline at end of file diff --git a/Koha/Schema/Result/Branchtransfer.pm b/Koha/Schema/Result/Branchtransfer.pm index 53abfdbe6b9..92f38cac4d8 100644 --- a/Koha/Schema/Result/Branchtransfer.pm +++ b/Koha/Schema/Result/Branchtransfer.pm @@ -103,7 +103,7 @@ any comments related to the transfer =head2 reason data_type: 'enum' - extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","TransferCancellation","Recall","RecallCancellation"]} + extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","TransferCancellation","Recall","RecallCancellation","LibraryFloatLimit"]} is_nullable: 1 what triggered the transfer @@ -188,6 +188,7 @@ __PACKAGE__->add_columns( "TransferCancellation", "Recall", "RecallCancellation", + "LibraryFloatLimit", ], }, is_nullable => 1, @@ -275,8 +276,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-10-03 16:48:17 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:BkhtfptiDqKKSv/hmCQy3w +# Created by DBIx::Class::Schema::Loader v0.07051 @ 2024-07-02 11:14:12 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:qJ8/zlwESoKfSq3JJY8Jug =head2 koha_object_class @@ -298,4 +299,4 @@ sub koha_objects_class { 'Koha::Item::Transfers'; } -1; +1; \ No newline at end of file diff --git a/Koha/Schema/Result/Itemtype.pm b/Koha/Schema/Result/Itemtype.pm index 7e69f304abc..3872a5623bb 100644 --- a/Koha/Schema/Result/Itemtype.pm +++ b/Koha/Schema/Result/Itemtype.pm @@ -293,6 +293,21 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 library_float_limits + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "library_float_limits", + "Koha::Schema::Result::LibraryFloatLimit", + { "foreign.itemtype" => "self.itemtype" }, + { cascade_copy => 0, cascade_delete => 0 }, +); + =head2 old_reserves Type: has_many @@ -393,4 +408,4 @@ sub koha_objects_class { 'Koha::ItemTypes'; } -1; +1; \ No newline at end of file diff --git a/admin/float_limits.pl b/admin/float_limits.pl index 30d9f9bffdb..2f69dc9f191 100644 --- a/admin/float_limits.pl +++ b/admin/float_limits.pl @@ -44,7 +44,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( my $op = $input->param('op'); -if ( $op eq 'set_float_limits' ) { +if ( $op eq 'cud-set_float_limits' ) { my $schema = Koha::Database->new()->schema(); my @branches = Koha::Libraries->search()->as_list; my @itemtypes = Koha::ItemTypes->search()->as_list; diff --git a/circ/returns.pl b/circ/returns.pl index 901877ef1e5..276ce641d2e 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -303,7 +303,6 @@ if ( $barcode && ( $op eq 'cud-checkin' || $op eq 'cud-affect_reserve' ) ) { my $biblio = $item->biblio; $template->param( title => $biblio->title, - returnbranch => $returnbranch, author => $biblio->author, itembiblionumber => $biblio->biblionumber, biblionumber => $biblio->biblionumber, @@ -413,6 +412,8 @@ if ( $barcode && ( $op eq 'cud-checkin' || $op eq 'cud-affect_reserve' ) ) { ); } + $returnbranch = $messages->{NeedsTransfer} if $messages->{NeedsTransfer}; + # Mark missing bundle items as lost and report unexpected items if ( $item && $item->is_bundle @@ -509,6 +510,13 @@ my $recalled = 0; # new op dev : we check if the document must be returned to his homebranch directly, # if the document is transferred, we have warning message . +if ( $messages->{'LibraryFloatLimitTransferRequest'} ) { + $template->param( + LibraryFloatLimitTransferRequest => 1, + itemnumber => $itemnumber, + ); +} + if ( $messages->{'WasTransfered'} ) { $template->param( found => 1, @@ -741,6 +749,8 @@ foreach my $code ( keys %$messages ) { ; } elsif ( $code eq 'TransferredRecall' ) { ; + } elsif ( $code eq 'LibraryFloatLimitTransferRequest' ) { + ; } elsif ( $code eq 'InBundle' ) { $template->param( InBundle => $messages->{InBundle} ); } elsif ( $code eq 'UpdateLastSeenError' ) { @@ -819,7 +829,10 @@ if ($barcode) { } } -$template->param( itemnumber => $itemnumber ); +$template->param( + itemnumber => $itemnumber, + returnbranch => $returnbranch, +); # Checking if there is a Fast Cataloging Framework $template->param( fast_cataloging => 1 ) if Koha::BiblioFrameworks->find('FA'); diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc index b6a4d1324b4..a72549ac72e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc @@ -24,40 +24,40 @@ [% END %] - [% IF ( CAN_user_parameters_manage_patron_categories || CAN_user_parameters_manage_circ_rules || CAN_user_parameters_manage_patron_attributes || CAN_user_parameters_manage_transfers || CAN_user_parameters_manage_item_circ_alerts || CAN_user_parameters_manage_cities || CAN_user_parameters_manage_curbside_pickups || CAN_user_parameters_manage_patron_restrictions ) %] -
Patrons and circulation
- - [% END %] + [% IF ( CAN_user_parameters_manage_patron_categories || CAN_user_parameters_manage_circ_rules || CAN_user_parameters_manage_patron_attributes || CAN_user_parameters_manage_transfers || CAN_user_parameters_manage_item_circ_alerts || CAN_user_parameters_manage_cities || CAN_user_parameters_manage_curbside_pickups || CAN_user_parameters_manage_patron_restrictions ) %] +
Patrons and circulation
+ + [% END %] [% IF ( CAN_user_parameters_manage_accounts || ( Koha.Preference('UseCashRegisters') && CAN_user_parameters_manage_cash_registers ) ) %]
Accounting
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt index a8813575fa2..993178cd290 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt @@ -108,6 +108,8 @@ >
Transport cost matrix
Define transport costs between branches
+
Library float limits
+
Define when items should be transferred to other libraries on checkin to balance floating collections
[% END %] [% IF ( CAN_user_parameters_manage_item_circ_alerts ) %]
Item circulation alerts
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/float_limits.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/float_limits.tt index 5b051e13a6b..9709c43ad08 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/float_limits.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/float_limits.tt @@ -38,7 +38,7 @@
-
+

Library float limits

@@ -53,7 +53,8 @@ [% END %]
- + [% INCLUDE 'csrf-token.inc' %] +

Specify the total number of items per itemtype that are allowed to float at each library.

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt index e52eeae7936..d5c4ed28b57 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt @@ -120,6 +120,8 @@

[%- SWITCH trigger -%] + [%- CASE 'LibraryFloatLimit' -%] + Library has exceeded float limit for this item type [%- CASE 'Manual' -%] Manual [%- CASE 'StockrotationAdvance' -%] diff --git a/t/db_dependent/Koha/Library/FloatLimits.t b/t/db_dependent/Koha/Library/FloatLimits.t index edc59fd7646..7881130b92c 100644 --- a/t/db_dependent/Koha/Library/FloatLimits.t +++ b/t/db_dependent/Koha/Library/FloatLimits.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 7; +use Test::More tests => 6; use Koha::Database; use C4::Circulation qw(CreateBranchTransferLimit); @@ -127,12 +127,6 @@ CreateBranchTransferLimit( $to, $from, $code ); is( C4::Circulation::IsBranchTransferAllowed( $to, $from, $code ), 0, "Transfer to best library is no longer allowed" ); say "C4::Circulation::IsBranchTransferAllowed( $to, $from, $code )"; -is( - Koha::Library::FloatLimits->lowest_ratio_library( $item, $library1->{branchcode} )->branchcode, - $library2->{branchcode}, - "Correct library selected for float limit transfer when best cannot be used" -); - subtest 'onloan items excluded from ratio calculation' => sub { plan tests => 5; -- 2.39.5