From 6cc5d00b6de894461e61d6bd0f670f430397f98d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 14 Jan 2021 11:28:35 +0000 Subject: [PATCH] Bug 24254: (QA follow-up) Inlines opachiddenitems handling We decided to inline the opachiddenitems filter as we don't believe it will be used exclusively. Signed-off-by: Martin Renvoize --- Koha/Items.pm | 46 ++++++++++++---------- t/db_dependent/Koha/Items.t | 77 +++++++------------------------------ 2 files changed, 40 insertions(+), 83 deletions(-) diff --git a/Koha/Items.pm b/Koha/Items.pm index 06571e528c..4d19c51e0a 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -39,9 +39,9 @@ Koha::Items - Koha Item object set class =head3 filter_by_for_loan -$items->filter_by_not_for_loan; + my $filtered_items = $items->filter_by_for_loan; -Return the items of the set that are not for loan +Return the items of the set that are loanable =cut @@ -61,7 +61,8 @@ sub filter_by_for_loan { Returns a new resultset, containing those items that are not expected to be hidden in OPAC for the passed I object that is passed. -The I and I system preferences are honoured. +The I, I and I system preferences +are honoured. =cut @@ -72,48 +73,51 @@ sub filter_by_visible_in_opac { my $result = $self; + # Filter out OpacHiddenItems unless disabled by OpacHiddenItemsExceptions unless ( $patron and $patron->category->override_hidden_items ) { - $result = $result->filter_out_opachiddenitems; + my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {}; + + my $rules_params; + foreach my $field (keys %$rules){ + $rules_params->{$field}->{'-not_in'} = $rules->{$field}; + } + + $result = $result->search( $rules_params ); } if (C4::Context->preference('hidelostitems')) { - $result = $result->filter_out_lost; + $result = $result->filter_by_not_lost; } return $result; } -=head3 filter_out_opachiddenitems +=head3 filter_by_lost - my $filered_items = $items->filter_out_opachiddenitems; + my $filered_items = $items->filter_by_lost; -Returns a new resultset, containing those items that are not expected to be hidden in OPAC. -The I system preference is honoured. +Returns a new resultset, containing those items that are marked as lost. =cut -sub filter_out_opachiddenitems { +sub filter_by_lost { my ($self) = @_; - my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {}; + my $params = { itemlost => { '!=' => 0 } }; - my $rules_params; - foreach my $field (keys %$rules){ - $rules_params->{$field}->{'-not_in'} = $rules->{$field}; - } - - return $self->search( $rules_params ); + return $self->search( $params ); } -=head3 filter_out_lost - my $filered_items = $items->filter_out_lost; +=head3 filter_by_not_lost + + my $filered_items = $items->filter_by_not_lost; Returns a new resultset, containing those items that are not marked as lost. =cut -sub filter_out_lost { +sub filter_by_not_lost { my ($self) = @_; my $params = { itemlost => 0 }; @@ -142,6 +146,8 @@ sub object_class { =head1 AUTHOR Kyle M Hall +Tomas Cohen Arazi +Martin Renvoize =cut diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index b96d1927e0..89dd0e16fe 100755 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -1467,9 +1467,9 @@ subtest 'filter_by_visible_in_opac() tests' => sub { $schema->storage->txn_rollback; }; -subtest 'filter_out_lost() tests' => sub { +subtest 'filter_by_lost() tests' => sub { - plan tests => 2; + plan tests => 3; $schema->storage->txn_begin; @@ -1495,93 +1495,44 @@ subtest 'filter_out_lost() tests' => sub { } ); - is( $biblio->items->filter_out_lost->next->itemnumber, $item_2->itemnumber, 'Right item returned' ); - is( $biblio->items->filter_out_lost->count, 1, 'Only one item is not lost' ); + my $results = $biblio->items->filter_by_lost; + is( $results->count, 2, 'Two items are not lost' ); + isnt( $results->next->itemnumber, $item_2->itemnumber, 'Right first item returned' ); + isnt( $results->next->itemnumber, $item_2->itemnumber, 'Right second item returned' ); $schema->storage->txn_rollback; }; -subtest 'filter_out_opachiddenitems() tests' => sub { +subtest 'filter_by_not_lost() tests' => sub { - plan tests => 6; + plan tests => 2; $schema->storage->txn_begin; # have a fresh biblio my $biblio = $builder->build_sample_biblio; - # have two itemtypes - my $itype_1 = $builder->build_object({ class => 'Koha::ItemTypes' }); - my $itype_2 = $builder->build_object({ class => 'Koha::ItemTypes' }); - # have 5 items on that biblio + # have 3 items on that biblio my $item_1 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, - itype => $itype_1->itemtype, - withdrawn => 1 + itemlost => -1, } ); my $item_2 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, - itype => $itype_2->itemtype, - withdrawn => 2 + itemlost => 0, } ); my $item_3 = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, - itype => $itype_1->itemtype, - withdrawn => 3 - } - ); - my $item_4 = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - itype => $itype_2->itemtype, - withdrawn => 4 - } - ); - my $item_5 = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - itype => $itype_1->itemtype, - withdrawn => 5 - } - ); - my $item_6 = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - itype => $itype_1->itemtype, - withdrawn => 5 + itemlost => 1, } ); - my $rules = undef; - - my $mocked_context = Test::MockModule->new('C4::Context'); - $mocked_context->mock( 'yaml_preference', sub { - return $rules; - }); - - is( $biblio->items->filter_out_opachiddenitems->count, 6, 'No rules passed' ); - - $rules = {}; - - $rules = { withdrawn => [ 1, 2 ] }; - is( $biblio->items->filter_out_opachiddenitems->count, 4, 'Rules on withdrawn' ); - - $rules = { itype => [ $itype_1->itemtype ] }; - is( $biblio->items->filter_out_opachiddenitems->count, 2, 'Rules on itype' ); - - $rules = { withdrawn => [ 1, 2 ], itype => [ $itype_1->itemtype ] }; - is( $biblio->items->filter_out_opachiddenitems->count, 1, 'Rules on itype and withdrawn' ); - is( $biblio->items->filter_out_opachiddenitems->next->itemnumber, - $item_4->itemnumber, - 'The right item is returned' - ); - - $rules = { withdrawn => [ 1, 2 ], itype => [ $itype_2->itemtype ] }; - is( $biblio->items->filter_out_opachiddenitems->count, 3, 'Rules on itype and withdrawn' ); + is( $biblio->items->filter_by_not_lost->next->itemnumber, $item_2->itemnumber, 'Right item returned' ); + is( $biblio->items->filter_by_not_lost->count, 1, 'Only one item is not lost' ); $schema->storage->txn_rollback; }; -- 2.20.1