From 4458c2653707ee865e194335907b8f5d6966aba4 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Sat, 19 Dec 2020 10:21:18 -0300
Subject: [PATCH] Bug 24254: Read the OpacHiddenItems preference internally

After discussing the 'rules' parameter usefulness we decided it was not
the best idea, and the gains in terms of 'performance' would me
meaningless (in-memory caching of sysprefs). This patch makes a really
minor tweak to the tests so they mock the C4::Context->yaml_preference
method, but keeping the same original rules to highlight no behaviour
change takes place.

Then the rules parameter is removed from the calls, and the tests should
keep passing.

A minor change to make $rules = undef is made to highlight the // {}
behaviour when reading the syspref..

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: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 Koha/Items.pm               |  7 +++----
 t/db_dependent/Koha/Items.t | 23 +++++++++++++++--------
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/Koha/Items.pm b/Koha/Items.pm
index f123a614ba..40f629599e 100644
--- a/Koha/Items.pm
+++ b/Koha/Items.pm
@@ -52,18 +52,17 @@ sub filter_by_for_loan {
 
 =head3 filter_by_visible_in_opac
 
-    my $filered_items = $items->filter_by_visible_in_opac({ rules => $rules });
+    my $filered_items = $items->filter_by_visible_in_opac;
 
 Returns a new resultset, containing those items that are not expected to be hidden in OPAC.
-If no I<rules> are passed, it returns the whole resultset, with the only caveat that the
-I<hidelostitems> system preference is honoured.
+The I<OpacHiddenItems> and I<hidelostitems> system preferences are honoured.
 
 =cut
 
 sub filter_by_visible_in_opac {
     my ($self, $params) = @_;
 
-    my $rules = $params->{rules} // {};
+    my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {};
 
     my $rules_params;
     foreach my $field (keys %$rules){
diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
index ca328ba747..25d3ff053b 100755
--- a/t/db_dependent/Koha/Items.t
+++ b/t/db_dependent/Koha/Items.t
@@ -1380,41 +1380,48 @@ subtest 'filter_by_visible_in_opac() tests' => sub {
         }
     );
 
-    my $rules = {};
+    my $rules = undef;
+
+    my $mocked_context = Test::MockModule->new('C4::Context');
+    $mocked_context->mock( 'yaml_preference', sub {
+        return $rules;
+    });
 
     t::lib::Mocks::mock_preference( 'hidelostitems', 0 );
     is( $biblio->items->filter_by_visible_in_opac->count,
         6, 'No rules passed, hidelostitems unset' );
 
+    $rules = {};
+
     t::lib::Mocks::mock_preference( 'hidelostitems', 1 );
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )->count,
+        $biblio->items->filter_by_visible_in_opac->count,
         3,
         'No rules passed, hidelostitems set'
     );
 
     $rules = { withdrawn => [ 1, 2 ] };
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )->count,
+        $biblio->items->filter_by_visible_in_opac->count,
         2,
         'Rules on withdrawn, hidelostitems set'
     );
 
     $rules = { itype => [ $itype_1->itemtype ] };
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )->count,
+        $biblio->items->filter_by_visible_in_opac->count,
         2,
         'Rules on itype, hidelostitems set'
     );
 
     $rules = { withdrawn => [ 1, 2 ], itype => [ $itype_1->itemtype ] };
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )->count,
+        $biblio->items->filter_by_visible_in_opac->count,
         1,
         'Rules on itype and withdrawn, hidelostitems set'
     );
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )
+        $biblio->items->filter_by_visible_in_opac
           ->next->itemnumber,
         $item_4->itemnumber,
         'The right item is returned'
@@ -1422,12 +1429,12 @@ subtest 'filter_by_visible_in_opac() tests' => sub {
 
     $rules = { withdrawn => [ 1, 2 ], itype => [ $itype_2->itemtype ] };
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )->count,
+        $biblio->items->filter_by_visible_in_opac->count,
         1,
         'Rules on itype and withdrawn, hidelostitems set'
     );
     is(
-        $biblio->items->filter_by_visible_in_opac( { rules => $rules } )
+        $biblio->items->filter_by_visible_in_opac
           ->next->itemnumber,
         $item_5->itemnumber,
         'The right item is returned'
-- 
2.20.1