From 151298709afcac0376f9a4b8c3e3b13cd3b9d207 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Thu, 28 Sep 2023 13:59:24 +0000 Subject: [PATCH] Bug 9525: (QA follow-up) Tidy Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 25 ++++++++++++--------- Koha/Library.pm | 14 ++++++------ admin/library_groups.pl | 24 ++++++++++---------- circ/returns.pl | 12 ++++++++-- t/db_dependent/Koha/Libraries.t | 40 +++++++++++++++++++++++---------- 5 files changed, 72 insertions(+), 43 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 93656d9665..386738a098 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2126,22 +2126,27 @@ sub AddReturn { } - # full item data, but no borrowernumber or checkout info (no issue) + # full item data, but no borrowernumber or checkout info (no issue) my $hbr = Koha::CirculationRules->get_return_branch_policy($item); - # check if returnbranch and homebranch belong to the same float group - my $validate_float = Koha::Libraries->find( $item->homebranch )->validate_float_sibling({ branchcode => $branch }); - # get the proper branch to which to return the item + + # check if returnbranch and homebranch belong to the same float group + my $validate_float = + Koha::Libraries->find( $item->homebranch )->validate_float_sibling( { branchcode => $branch } ); + + # get the proper branch to which to return the item my $returnbranch; - if($hbr eq 'noreturn'){ + if ( $hbr eq 'noreturn' ) { $returnbranch = $branch; - }elsif($hbr eq 'returnbylibrarygroup'){ - # if library isn't in same the float group, transfer item to homebranch - $hbr = 'homebranch'; + } elsif ( $hbr eq 'returnbylibrarygroup' ) { + + # if library isn't in same the float group, transfer item to homebranch + $hbr = 'homebranch'; $returnbranch = $validate_float ? $branch : $item->$hbr; - }else{ + } else { $returnbranch = $item->$hbr; } - # if $hbr was "noreturn" or any other non-item table value, then it should 'float' (i.e. stay at this branch) + + # 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; my $borrowernumber = $patron ? $patron->borrowernumber : undef; # we don't know if we had a borrower or not diff --git a/Koha/Library.pm b/Koha/Library.pm index ff93d8a8da..0cad18364f 100644 --- a/Koha/Library.pm +++ b/Koha/Library.pm @@ -376,6 +376,7 @@ sub opac_info { }); } + =head3 get_float_libraries Return all libraries belonging to the same float group @@ -383,23 +384,23 @@ Return all libraries belonging to the same float group =cut sub get_float_libraries { - my ( $self ) = @_; + my ($self) = @_; my $library_groups = $self->library_groups; my @float_libraries; while ( my $library_group = $library_groups->next ) { - my $root = Koha::Library::Groups->get_root_ancestor({id => $library_group->id}); - if($root->ft_local_float_group) { + my $root = Koha::Library::Groups->get_root_ancestor( { id => $library_group->id } ); + if ( $root->ft_local_float_group ) { push @float_libraries, $root->all_libraries; } } my %seen; @float_libraries = - grep { !$seen{ $_->id }++ } @float_libraries; + grep { !$seen{ $_->id }++ } @float_libraries; - return Koha::Libraries->search({ branchcode => { '-in' => [ keys %seen ] } }); + return Koha::Libraries->search( { branchcode => { '-in' => [ keys %seen ] } } ); } =head3 validate_float_sibling @@ -414,8 +415,7 @@ sub validate_float_sibling { return 1 if $params->{branchcode} eq $self->id; my $branchcode = $params->{branchcode}; - return $self->get_float_libraries->search( { branchcode => $branchcode } ) - ->count > 0; + return $self->get_float_libraries->search( { branchcode => $branchcode } )->count > 0; } =head2 Internal methods diff --git a/admin/library_groups.pl b/admin/library_groups.pl index e37ee339a7..239f3ead76 100755 --- a/admin/library_groups.pl +++ b/admin/library_groups.pl @@ -50,8 +50,8 @@ if ( $action eq 'add' ) { my $ft_limit_item_editing = $cgi->param('ft_limit_item_editing') || 0; my $ft_search_groups_opac = $cgi->param('ft_search_groups_opac') || 0; my $ft_search_groups_staff = $cgi->param('ft_search_groups_staff') || 0; - my $ft_local_hold_group = $cgi->param('ft_local_hold_group') || 0; - my $ft_local_float_group = $cgi->param('ft_local_float_group') || 0; + my $ft_local_hold_group = $cgi->param('ft_local_hold_group') || 0; + my $ft_local_float_group = $cgi->param('ft_local_float_group') || 0; if ( !$branchcode && Koha::Library::Groups->search( { title => $title } )->count() ) { $template->param( error_duplicate_title => $title ); @@ -89,22 +89,22 @@ elsif ( $action eq 'edit' ) { my $ft_limit_item_editing = $cgi->param('ft_limit_item_editing') || 0; my $ft_search_groups_opac = $cgi->param('ft_search_groups_opac') || 0; my $ft_search_groups_staff = $cgi->param('ft_search_groups_staff') || 0; - my $ft_local_hold_group = $cgi->param('ft_local_hold_group') || 0; - my $ft_local_float_group = $cgi->param('ft_local_float_group') || 0; + my $ft_local_hold_group = $cgi->param('ft_local_hold_group') || 0; + my $ft_local_float_group = $cgi->param('ft_local_float_group') || 0; if ($id) { my $group = Koha::Library::Groups->find($id); $group->set( { - title => $title, - description => $description, - ft_hide_patron_info => $ft_hide_patron_info, - ft_limit_item_editing => $ft_limit_item_editing, - ft_search_groups_opac => $ft_search_groups_opac, - ft_search_groups_staff => $ft_search_groups_staff, - ft_local_hold_group => $ft_local_hold_group, - ft_local_float_group => $ft_local_float_group, + title => $title, + description => $description, + ft_hide_patron_info => $ft_hide_patron_info, + ft_limit_item_editing => $ft_limit_item_editing, + ft_search_groups_opac => $ft_search_groups_opac, + ft_search_groups_staff => $ft_search_groups_staff, + ft_local_hold_group => $ft_local_hold_group, + ft_local_float_group => $ft_local_float_group, } )->store(); diff --git a/circ/returns.pl b/circ/returns.pl index 9b6efed481..8a0cb1a41d 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -298,10 +298,18 @@ if ($barcode) { # make sure return branch respects home branch circulation rules, default to homebranch my $hbr = Koha::CirculationRules->get_return_branch_policy($item); - my $validate_float = Koha::Libraries->find( $item->homebranch )->validate_float_sibling({ branchcode => $userenv_branch }); + my $validate_float = + Koha::Libraries->find( $item->homebranch )->validate_float_sibling( { branchcode => $userenv_branch } ); + # get the proper branch to which to return the item # if library isn't in same the float group, transfer item to homelibrary - $returnbranch = $hbr eq 'noreturn' ? $userenv_branch : $hbr eq 'returnbylibrarygroup' ? $validate_float ? $userenv_branch : $item->homebranch : $item->$hbr; + $returnbranch = + $hbr eq 'noreturn' + ? $userenv_branch + : $hbr eq 'returnbylibrarygroup' ? $validate_float + ? $userenv_branch + : $item->homebranch + : $item->$hbr; my $materials = $item->materials; my $descriptions = Koha::AuthorisedValues->get_description_by_koha_field({frameworkcode => '', kohafield =>'items.materials', authorised_value => $materials }); $materials = $descriptions->{lib} // $materials; diff --git a/t/db_dependent/Koha/Libraries.t b/t/db_dependent/Koha/Libraries.t index c2477c2f97..5c746d679d 100755 --- a/t/db_dependent/Koha/Libraries.t +++ b/t/db_dependent/Koha/Libraries.t @@ -337,28 +337,44 @@ subtest 'get_float_libraries and validate_float_sibling' => sub { $schema->storage->txn_begin; - my $library1 = $builder->build_object({ class => 'Koha::Libraries' }); - my $library2 = $builder->build_object({ class => 'Koha::Libraries' }); - my $library3 = $builder->build_object({ class => 'Koha::Libraries' }); - my $library4 = $builder->build_object({ class => 'Koha::Libraries' }); + my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $library2 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $library3 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $library4 = $builder->build_object( { class => 'Koha::Libraries' } ); my $root1 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_float_group => 1 } } ); my $root2 = $builder->build_object( { class => 'Koha::Library::Groups', value => { ft_local_float_group => 1 } } ); + # Float group 1 - $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library1->branchcode } } ); - $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library2->branchcode } } ); + $builder->build_object( + { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library1->branchcode } } + ); + $builder->build_object( + { class => 'Koha::Library::Groups', value => { parent_id => $root1->id, branchcode => $library2->branchcode } } + ); + # Float group 2 - $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library3->branchcode } } ); - $builder->build_object( { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library4->branchcode } } ); + $builder->build_object( + { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library3->branchcode } } + ); + $builder->build_object( + { class => 'Koha::Library::Groups', value => { parent_id => $root2->id, branchcode => $library4->branchcode } } + ); my @libraries1 = $library1->get_float_libraries()->as_list; - is(scalar @libraries1, '2', '1st float group contains 2 libraries'); + is( scalar @libraries1, '2', '1st float group contains 2 libraries' ); my @libraries2 = $library3->get_float_libraries()->as_list; - is(scalar @libraries2, '2', '2nd float group also contains 2 libraries'); + is( scalar @libraries2, '2', '2nd float group also contains 2 libraries' ); - ok($library1->validate_float_sibling({ branchcode => $library2->branchcode }), "Library1 and library2 belong in to the same float group."); - ok($library3->validate_float_sibling({ branchcode => $library4->branchcode }), "Library3 and library5 belong in to the same float group."); + ok( + $library1->validate_float_sibling( { branchcode => $library2->branchcode } ), + "Library1 and library2 belong in to the same float group." + ); + ok( + $library3->validate_float_sibling( { branchcode => $library4->branchcode } ), + "Library3 and library5 belong in to the same float group." + ); $schema->storage->txn_rollback; }; -- 2.39.5