From 21728830955fa43bad5e4c249bbe0c13a74b4d71 Mon Sep 17 00:00:00 2001
From: Tomas Cohen Arazi <tomascohen@theke.io>
Date: Sat, 19 Dec 2020 10:05:16 -0300
Subject: [PATCH] Bug 24354: Compare itemlost with Perl's false values

On C4::Search and C4::Circulation the uses of the items.itemlost field
highlight the fact that the comparisson itemlost <= 0 was wrong, as it
is evaluated as a Perl boolean.

As authorised values can take many values that can be casted to 'false',
we need to compare with them explicitly.

This patch changes the tests to reflect this, and adjust the
Koha::Items->filter_by_visible_in_opac implementation to adapt to this.

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>

https://bugs.koha-community.org/show_bug.cgi?id=24254
---
 Koha/Items.pm               | 20 ++++++++++++++++----
 t/db_dependent/Koha/Items.t | 12 ++++++++++--
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/Koha/Items.pm b/Koha/Items.pm
index 78663f0dc81..85ab92788d5 100644
--- a/Koha/Items.pm
+++ b/Koha/Items.pm
@@ -65,13 +65,25 @@ sub filter_by_visible_in_opac {
 
     my $rules = $params->{rules} // {};
 
-    my $search_params;
+    my $rules_params;
     foreach my $field (keys %$rules){
-        $search_params->{$field}->{'-not_in'} = $rules->{$field};
+        $rules_params->{$field}->{'-not_in'} = $rules->{$field};
     }
 
-    $search_params->{itemlost}->{'<='} = 0
-        if C4::Context->preference('hidelostitems');
+    my $itemlost_params;
+    $itemlost_params = {
+        itemlost => [ 0, undef, '' ] # things Perl evaluates to false
+    } if C4::Context->preference('hidelostitems');
+
+    my $search_params;
+    if ( $rules_params and $itemlost_params ) {
+        $search_params = {
+            '-and' => [ $rules_params, $itemlost_params ]
+        };
+    }
+    else {
+        $search_params = $rules_params // $itemlost_params;
+    }
 
     return $self->search( $search_params );
 }
diff --git a/t/db_dependent/Koha/Items.t b/t/db_dependent/Koha/Items.t
index eca3909853f..baba4f3be4e 100755
--- a/t/db_dependent/Koha/Items.t
+++ b/t/db_dependent/Koha/Items.t
@@ -1371,17 +1371,25 @@ subtest 'filter_by_visible_in_opac() tests' => sub {
             withdrawn    => 5
         }
     );
+    my $item_6 = $builder->build_sample_item(
+        {
+            biblionumber => $biblio->biblionumber,
+            itemlost     => 2,
+            itype        => $itype_1->itemtype,
+            withdrawn    => 5
+        }
+    );
 
     my $rules = {};
 
     t::lib::Mocks::mock_preference( 'hidelostitems', 0 );
     is( $biblio->items->filter_by_visible_in_opac->count,
-        5, 'No rules passed, hidelostitems unset' );
+        6, 'No rules passed, hidelostitems unset' );
 
     t::lib::Mocks::mock_preference( 'hidelostitems', 1 );
     is(
         $biblio->items->filter_by_visible_in_opac( { rules => $rules } )->count,
-        4,
+        3,
         'No rules passed, hidelostitems set'
     );
 
-- 
2.29.2