Bugzilla – Attachment 156336 Details for
Bug 9525
Add option to define float groups and rules for float
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 9525: (QA follow-up) Tidy
Bug-9525-QA-follow-up-Tidy.patch (text/plain), 11.56 KB, created by
Nick Clemens (kidclamp)
on 2023-09-28 14:12:18 UTC
(
hide
)
Description:
Bug 9525: (QA follow-up) Tidy
Filename:
MIME Type:
Creator:
Nick Clemens (kidclamp)
Created:
2023-09-28 14:12:18 UTC
Size:
11.56 KB
patch
obsolete
>From 93f46ae25410bf18e00251046b37a19600610420 Mon Sep 17 00:00:00 2001 >From: Nick Clemens <nick@bywatersolutions.com> >Date: Thu, 28 Sep 2023 13:59:24 +0000 >Subject: [PATCH] Bug 9525: (QA follow-up) Tidy > >--- > 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 222ff1274a..3c38eadb7f 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.30.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=156336">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 9525
:
40077
|
40102
|
40103
|
44754
|
44756
|
44760
|
48310
|
100420
|
100421
|
100422
|
100741
|
100742
|
100743
|
103828
|
103829
|
103830
|
103831
|
112550
|
112551
|
112552
|
112553
|
124825
|
124826
|
124827
|
133071
|
133072
|
133073
|
139024
|
139025
|
139026
|
143740
|
143741
|
143742
|
143743
|
150241
|
150242
|
150243
|
150244
|
150245
|
150246
|
156330
|
156331
|
156332
|
156333
|
156334
|
156335
| 156336