From 1a8bb74f705598b0dbe61ceccda81c3fc2a7ea6a Mon Sep 17 00:00:00 2001
From: Fridolin Somers <fridolin.somers@biblibre.com>
Date: Wed, 11 May 2022 10:51:20 -1000
Subject: [PATCH] Bug 30735: Fix filtering by patron attribute with AV in
 overdues report

In Circulation > Overdues, if there are seachable patron attributes
they can be used in left filters. Search does not work if this attribute
is linked to an authorized values category.

The problem is that AV code is compared to the AV desciption in :
  if (grep { $attrval eq lc($_->[1]) } @{ $pattrs->{$code} }) {

Looking at a Data::Dumper of the var $pattrs you see for example :
$VAR1 = {
          'ATT1' => [
                      [
                        'afr',
                        'Afrikaans'
                      ]
                    ]
        };
First row is attribute code and second row is desciption.
It works for an attribute without AV because in this case
'avdescription' is undefined so code is stored as description :
          push @{ $pattrs->{$row->{attrcode}} }, [
            $row->{attrval},
            defined $row->{avdescription} ? $row->{avdescription} : $row->{attrval},
        ];

This patch fixes the code and removes commented line.
Also reviews the SQL query :
  - avcategory is useless
  - use alias attrcode for borrower_attributes.code to be more explicit

Test plan :
1) Create a patron A with overdues
2) Go to Circulation > Overdues
3) Check you see the patron A
4) Create a patron attribute, searchable, with an authorized value
   category, ie LANG
5) Edit patron A to set a value in this attribute
6) Go to Circulation > Overdues
7) Select attribute value and apply filter
8) Check you see the patron A
9) Redo test plan with a patron attribute, searchable, without an
   authorized value category

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
---
 circ/overdue.pl | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/circ/overdue.pl b/circ/overdue.pl
index 178ec03041..33f2d1b3cd 100755
--- a/circ/overdue.pl
+++ b/circ/overdue.pl
@@ -157,17 +157,19 @@ if (@patron_attr_filter_loop) {
     #              too resource intensive, MySQL can be used to do the filtering, i.e. rewire the
     #              SQL below to select only those attribute values that match the filters.
 
-    my $sql = q(SELECT borrowernumber AS bn, b.code, attribute AS val, category AS avcategory, lib AS avdescription
+    my $sql = q{
+        SELECT b.borrowernumber AS bn, b.code AS attrcode, b.attribute AS attrval, a.lib AS avdescription
         FROM borrower_attributes b
         JOIN borrower_attribute_types bt ON (b.code = bt.code)
-        LEFT JOIN authorised_values a ON (a.category = bt.authorised_value_category AND a.authorised_value = b.attribute));
+        LEFT JOIN authorised_values a ON (a.category = bt.authorised_value_category AND a.authorised_value = b.attribute)
+    };
     my $sth = $dbh->prepare($sql);
     $sth->execute();
     while (my $row = $sth->fetchrow_hashref) {
         my $pattrs = $borrowernumber_to_attributes{$row->{bn}} ||= { };
-        push @{ $pattrs->{$row->{code}} }, [
-            $row->{val},
-            defined $row->{avdescription} ? $row->{avdescription} : $row->{val},
+        push @{ $pattrs->{$row->{attrcode}} }, [
+            $row->{attrval},
+            defined $row->{avdescription} ? $row->{avdescription} : $row->{attrval},
         ];
     }
 
@@ -178,8 +180,7 @@ if (@patron_attr_filter_loop) {
             # discard patrons that do not match (case insensitive) at least one of each attribute filter value
             my $discard = 1;
             for my $attrval (map { lc $_ } @{ $cgi_attrcode_to_attrvalues{$code} }) {
-                ## if (grep { $attrval eq lc($_->[0]) } @{ $pattrs->{$code} })
-                if (grep { $attrval eq lc($_->[1]) } @{ $pattrs->{$code} }) {
+                if (grep { $attrval eq lc($_->[0]) } @{ $pattrs->{$code} }) {
                     $discard = 0;
                     last;
                 }
-- 
2.20.1