Attachment 114540 Details for Bug 24254 – Bug 24254: Read the OpacHiddenItems preference internally

[patch] Bug 24254: Read the OpacHiddenItems preference internally

Bug-24254-Read-the-OpacHiddenItems-preference-inte.patch (text/plain), 4.57 KB, created by Tomás Cohen Arazi (tcohen) on 2020-12-19 13:27:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2020-12-19 13:27:48 UTC

Size: 4.57 KB

patch

obsolete

>From b2fca1045dd35fec91e65931bf336f53c2890fe7 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>
>---
> 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 85ab92788d5..65ad80c7d64 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 baba4f3be4e..642fab7b833 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.29.2

Actions: View | Diff | Splinter Review

Attachments on bug 24254: 96373 | 114159 | 114160 | 114206 | 114207 | 114278 | 114279 | 114539 | 114540 | 114548 | 114549 | 114550 | 114551 | 114574 | 114594 | 114595 | 114596 | 114597 | 114598 | 114599 | 114664 | 114665 | 114666 | 114667 | 114668 | 115131 | 115132 | 115147 | 115148 | 115149 | 115150 | 115151 | 115152 | 115153 | 116070 | 116071 | 116072 | 116073 | 116074 | 116075