From 212b15cd2b8ba535935cb1c4457c9e0a18e8b43f Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 21 Dec 2020 15:50:13 -0300 Subject: [PATCH] [20.05.x] Bug 24254: Implement separate filters and add patron param This patch introduces two new methods for stacking filters on Koha::Items: - filter_out_lost _ filter_out_opachiddenitems This two filters are what actually happened inside the filter_by_visible_in_opac. Everything is covered by tests. In the process I added a Koha::Patron param to the method that is internally used to decide if the OPACHiddenItems syspref needs to be honoured or not. This *could* be better done with a fallback to C4::Context->userenv if no param is passed. I decided to leave that part for later, if we really find it would help (e.g. if bug 10589 gets some action and we really need something here to handle that). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Items.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall Signed-off-by: Martin Renvoize --- Koha/Items.pm | 65 ++++++++++++---- t/db_dependent/Koha/Items.t | 147 +++++++++++++++++++++++++++++++++++- 2 files changed, 195 insertions(+), 17 deletions(-) diff --git a/Koha/Items.pm b/Koha/Items.pm index be7889b13a7..7d07f08ad10 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -39,9 +39,15 @@ Koha::Items - Koha Item object set class =head3 filter_by_visible_in_opac - my $filered_items = $items->filter_by_visible_in_opac; + my $filered_items = $items->filter_by_visible_in_opac( + { + [ patron => $patron ] + } + ); + +Returns a new resultset, containing those items that are not expected to be hidden in OPAC +for the passed I object that is passed. -Returns a new resultset, containing those items that are not expected to be hidden in OPAC. The I and I system preferences are honoured. =cut @@ -49,6 +55,33 @@ The I and I system preferences are honoured. sub filter_by_visible_in_opac { my ($self, $params) = @_; + my $patron = $params->{patron}; + + my $result = $self; + + unless ( $patron and $patron->category->override_hidden_items ) { + $result = $result->filter_out_opachiddenitems; + } + + if (C4::Context->preference('hidelostitems')) { + $result = $result->filter_out_lost; + } + + return $result; +} + +=head3 filter_out_opachiddenitems + + my $filered_items = $items->filter_out_opachiddenitems; + +Returns a new resultset, containing those items that are not expected to be hidden in OPAC. +The I system preference is honoured. + +=cut + +sub filter_out_opachiddenitems { + my ($self) = @_; + my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {}; my $rules_params; @@ -56,21 +89,23 @@ sub filter_by_visible_in_opac { $rules_params->{$field}->{'-not_in'} = $rules->{$field}; } - my $itemlost_params; - $itemlost_params = { itemlost => 0 } - if C4::Context->preference('hidelostitems'); + return $self->search( $rules_params ); +} - my $search_params; - if ( $rules_params and $itemlost_params ) { - $search_params = { - '-and' => [ $rules_params, $itemlost_params ] - }; - } - else { - $search_params = $rules_params // $itemlost_params; - } +=head3 filter_out_lost + + my $filered_items = $items->filter_out_lost; + +Returns a new resultset, containing those items that are not marked as lost. + +=cut + +sub filter_out_lost { + my ($self) = @_; + + my $params = { itemlost => 0 }; - return $self->search( $search_params ); + return $self->search( $params ); } =head2 Internal methods diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t index 26480c4302f..c886e05b199 100644 --- a/t/db_dependent/Koha/Items.t +++ b/t/db_dependent/Koha/Items.t @@ -19,7 +19,9 @@ use Modern::Perl; -use Test::More tests => 13; +use Test::More tests => 15; + +use Test::MockModule; use Test::Exception; use C4::Circulation; @@ -244,10 +246,17 @@ $schema->storage->txn_rollback; subtest 'filter_by_visible_in_opac() tests' => sub { - plan tests => 8; + plan tests => 11; $schema->storage->txn_begin; + my $patron = $builder->build_object({ class => 'Koha::Patrons' }); + my $mocked_category = Test::MockModule->new('Koha::Patron::Category'); + my $exception = 1; + $mocked_category->mock( 'override_hidden_items', sub { + return $exception; + }); + # have a fresh biblio my $biblio = $builder->build_sample_biblio; # have two itemtypes @@ -314,6 +323,9 @@ subtest 'filter_by_visible_in_opac() tests' => sub { is( $biblio->items->filter_by_visible_in_opac->count, 6, 'No rules passed, hidelostitems unset' ); + is( $biblio->items->filter_by_visible_in_opac({ patron => $patron })->count, + 6, 'No rules passed, hidelostitems unset, patron exception changes nothing' ); + $rules = {}; t::lib::Mocks::mock_preference( 'hidelostitems', 1 ); @@ -323,6 +335,12 @@ subtest 'filter_by_visible_in_opac() tests' => sub { 'No rules passed, hidelostitems set' ); + is( + $biblio->items->filter_by_visible_in_opac({ patron => $patron })->count, + 3, + 'No rules passed, hidelostitems set, patron exception changes nothing' + ); + $rules = { withdrawn => [ 1, 2 ] }; is( $biblio->items->filter_by_visible_in_opac->count, @@ -330,6 +348,12 @@ subtest 'filter_by_visible_in_opac() tests' => sub { 'Rules on withdrawn, hidelostitems set' ); + is( + $biblio->items->filter_by_visible_in_opac({ patron => $patron })->count, + 3, + 'hidelostitems set, rules on withdrawn but patron override passed' + ); + $rules = { itype => [ $itype_1->itemtype ] }; is( $biblio->items->filter_by_visible_in_opac->count, @@ -365,3 +389,122 @@ subtest 'filter_by_visible_in_opac() tests' => sub { $schema->storage->txn_rollback; }; + +subtest 'filter_out_lost() tests' => sub { + + plan tests => 2; + + $schema->storage->txn_begin; + + # have a fresh biblio + my $biblio = $builder->build_sample_biblio; + # have 3 items on that biblio + my $item_1 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + itemlost => -1, + } + ); + my $item_2 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + itemlost => 0, + } + ); + my $item_3 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + itemlost => 1, + } + ); + + 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' ); + + $schema->storage->txn_rollback; +}; + +subtest 'filter_out_opachiddenitems() tests' => sub { + + plan tests => 6; + + $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 + my $item_1 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + itype => $itype_1->itemtype, + withdrawn => 1 + } + ); + my $item_2 = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + itype => $itype_2->itemtype, + withdrawn => 2 + } + ); + 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 + } + ); + + 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' ); + + $schema->storage->txn_rollback; +}; -- 2.30.0