Attachment 114599 Details for Bug 24254 – Bug 24254: Implement separate filters and add patron param

[patch] Bug 24254: Implement separate filters and add patron param

Bug-24254-Implement-separate-filters-and-add-patro.patch (text/plain), 9.62 KB, created by Kyle M Hall (khall) on 2020-12-22 11:41:41 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2020-12-22 11:41:41 UTC

Size: 9.62 KB

patch

obsolete

>From b2df0d4868ed0d7915751ef2eba2eb3d40ab9b61 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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 <kyle@bywatersolutions.com>
>---
> 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<Koha::Patron> object that is passed.
> 
>-Returns a new resultset, containing those items that are not expected to be hidden in OPAC.
> The I<OpacHiddenItems> and I<hidelostitems> system preferences are honoured.
> 
> =cut
>@@ -62,6 +68,33 @@ The I<OpacHiddenItems> and I<hidelostitems> 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<OpacHiddenItems> 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.24.1 (Apple Git-126)

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