From 42c4089fad79461f5aaea3c3354e902d192af543 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 21 Dec 2020 15:50:13 -0300 Subject: [PATCH] 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 Signed-off-by: Jonathan Druart --- 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 40f629599e..06571e528c 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -52,9 +52,15 @@ sub filter_by_for_loan { =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 @@ -62,6 +68,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; @@ -69,21 +102,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 25d3ff053b..b96d1927e0 100755 --- 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 Time::Fake; @@ -1321,10 +1323,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 @@ -1391,6 +1400,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 ); @@ -1400,6 +1412,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, @@ -1407,6 +1425,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, @@ -1442,3 +1466,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.39.5