Attachment 115152 Details for Bug 24254 – Bug 24254: (QA follow-up) Inlines opachiddenitems handling

[patch] Bug 24254: (QA follow-up) Inlines opachiddenitems handling

Bug-24254-QA-follow-up-Inlines-opachiddenitems-han.patch (text/plain), 7.58 KB, created by Martin Renvoize (ashimema) on 2021-01-14 11:47:38 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2021-01-14 11:47:38 UTC

Size: 7.58 KB

patch

obsolete

>From 6cc5d00b6de894461e61d6bd0f670f430397f98d Mon Sep 17 00:00:00 2001
>From: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>Date: Thu, 14 Jan 2021 11:28:35 +0000
>Subject: [PATCH] Bug 24254: (QA follow-up) Inlines opachiddenitems handling
>
>We decided to inline the opachiddenitems filter as we don't believe it
>will be used exclusively.
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> Koha/Items.pm               | 46 ++++++++++++----------
> t/db_dependent/Koha/Items.t | 77 +++++++------------------------------
> 2 files changed, 40 insertions(+), 83 deletions(-)
>
>diff --git a/Koha/Items.pm b/Koha/Items.pm
>index 06571e528c..4d19c51e0a 100644
>--- a/Koha/Items.pm
>+++ b/Koha/Items.pm
>@@ -39,9 +39,9 @@ Koha::Items - Koha Item object set class
> 
> =head3 filter_by_for_loan
> 
>-$items->filter_by_not_for_loan;
>+    my $filtered_items = $items->filter_by_for_loan;
> 
>-Return the items of the set that are not for loan
>+Return the items of the set that are loanable
> 
> =cut
> 
>@@ -61,7 +61,8 @@ sub filter_by_for_loan {
> 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.
> 
>-The I<OpacHiddenItems> and I<hidelostitems> system preferences are honoured.
>+The I<OpacHiddenItems>, I<hidelostitems> and I<OpacHiddenItemsExceptions> system preferences
>+are honoured.
> 
> =cut
> 
>@@ -72,48 +73,51 @@ sub filter_by_visible_in_opac {
> 
>     my $result = $self;
> 
>+    # Filter out OpacHiddenItems unless disabled by OpacHiddenItemsExceptions
>     unless ( $patron and $patron->category->override_hidden_items ) {
>-        $result = $result->filter_out_opachiddenitems;
>+        my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {};
>+
>+        my $rules_params;
>+        foreach my $field (keys %$rules){
>+            $rules_params->{$field}->{'-not_in'} = $rules->{$field};
>+        }
>+
>+        $result = $result->search( $rules_params );
>     }
> 
>     if (C4::Context->preference('hidelostitems')) {
>-        $result = $result->filter_out_lost;
>+        $result = $result->filter_by_not_lost;
>     }
> 
>     return $result;
> }
> 
>-=head3 filter_out_opachiddenitems
>+=head3 filter_by_lost
> 
>-    my $filered_items = $items->filter_out_opachiddenitems;
>+    my $filered_items = $items->filter_by_lost;
> 
>-Returns a new resultset, containing those items that are not expected to be hidden in OPAC.
>-The I<OpacHiddenItems> system preference is honoured.
>+Returns a new resultset, containing those items that are marked as lost.
> 
> =cut
> 
>-sub filter_out_opachiddenitems {
>+sub filter_by_lost {
>     my ($self) = @_;
> 
>-    my $rules = C4::Context->yaml_preference('OpacHiddenItems') // {};
>+    my $params = { itemlost => { '!=' => 0 } };
> 
>-    my $rules_params;
>-    foreach my $field (keys %$rules){
>-        $rules_params->{$field}->{'-not_in'} = $rules->{$field};
>-    }
>-
>-    return $self->search( $rules_params );
>+    return $self->search( $params );
> }
> 
>-=head3 filter_out_lost
> 
>-    my $filered_items = $items->filter_out_lost;
>+=head3 filter_by_not_lost
>+
>+    my $filered_items = $items->filter_by_not_lost;
> 
> Returns a new resultset, containing those items that are not marked as lost.
> 
> =cut
> 
>-sub filter_out_lost {
>+sub filter_by_not_lost {
>     my ($self) = @_;
> 
>     my $params = { itemlost => 0 };
>@@ -142,6 +146,8 @@ sub object_class {
> =head1 AUTHOR
> 
> Kyle M Hall <kyle@bywatersolutions.com>
>+Tomas Cohen Arazi <tomascohen@theke.io>
>+Martin Renvoize <martin.renvoize@ptfs-europe.com>
> 
> =cut
> 
>diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
>index b96d1927e0..89dd0e16fe 100755
>--- a/t/db_dependent/Koha/Items.t
>+++ b/t/db_dependent/Koha/Items.t
>@@ -1467,9 +1467,9 @@ subtest 'filter_by_visible_in_opac() tests' => sub {
>     $schema->storage->txn_rollback;
> };
> 
>-subtest 'filter_out_lost() tests' => sub {
>+subtest 'filter_by_lost() tests' => sub {
> 
>-    plan tests => 2;
>+    plan tests => 3;
> 
>     $schema->storage->txn_begin;
> 
>@@ -1495,93 +1495,44 @@ subtest 'filter_out_lost() tests' => sub {
>         }
>     );
> 
>-    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' );
>+    my $results = $biblio->items->filter_by_lost;
>+    is( $results->count, 2, 'Two items are not lost' );
>+    isnt( $results->next->itemnumber, $item_2->itemnumber, 'Right first item returned' );
>+    isnt( $results->next->itemnumber, $item_2->itemnumber, 'Right second item returned' );
> 
>     $schema->storage->txn_rollback;
> };
> 
>-subtest 'filter_out_opachiddenitems() tests' => sub {
>+subtest 'filter_by_not_lost() tests' => sub {
> 
>-    plan tests => 6;
>+    plan tests => 2;
> 
>     $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
>+    # have 3 items on that biblio
>     my $item_1 = $builder->build_sample_item(
>         {
>             biblionumber => $biblio->biblionumber,
>-            itype        => $itype_1->itemtype,
>-            withdrawn    => 1
>+            itemlost     => -1,
>         }
>     );
>     my $item_2 = $builder->build_sample_item(
>         {
>             biblionumber => $biblio->biblionumber,
>-            itype        => $itype_2->itemtype,
>-            withdrawn    => 2
>+            itemlost     => 0,
>         }
>     );
>     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
>+            itemlost     => 1,
>         }
>     );
> 
>-    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' );
>+    is( $biblio->items->filter_by_not_lost->next->itemnumber, $item_2->itemnumber, 'Right item returned' );
>+    is( $biblio->items->filter_by_not_lost->count, 1, 'Only one item is not lost' );
> 
>     $schema->storage->txn_rollback;
> };
>-- 
>2.20.1

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