Attachment 134905 Details for Bug 30735 – Bug 30735: Fix filtering by patron attribute with AV in overdues report

[patch] Bug 30735: Fix filtering by patron attribute with AV in overdues report

Bug-30735-Fix-filtering-by-patron-attribute-with-A.patch (text/plain), 3.90 KB, created by Fridolin Somers on 2022-05-11 21:06:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Fridolin Somers

Created: 2022-05-11 21:06:19 UTC

Size: 3.90 KB

patch

obsolete

>From 11f0762cddc05e6964c682c7de0439fe33998e7c 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
>---
> 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.35.3

Actions: View | Diff | Splinter Review

Attachments on bug 30735: 134905 | 134971 | 134994