[Bug 30735] New: Filtering by patron attribute with AV does not work in overdues report
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Bug ID: 30735 Summary: Filtering by patron attribute with AV does not work in overdues report Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: fridolin.somers@biblibre.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|koha-bugs@lists.koha-commun |fridolin.somers@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 --- Comment #1 from Fridolin Somers <fridolin.somers@biblibre.com> --- This looks like very-old-buggy-code ^^ -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 --- Comment #2 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 134905 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134905&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134905|0 |1 is obsolete| | --- Comment #3 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 134971 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134971&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #134971|0 |1 is obsolete| | --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 134994 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=134994&action=edit 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> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |martin.renvoize@ptfs-europe | |.com --- Comment #5 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I would have loved to see some of this moved up into module code and unit tested.. but it does resolve the issue here and I feel asking for such a move at this time would be a bit above an beyond. So, working as expected, no regressions found and QA script is happy. Passing QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 --- Comment #6 from Fridolin Somers <fridolin.somers@biblibre.com> --- Indeed, the code is weired. Maybe should be completely revamped with ORM. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |22.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 --- Comment #7 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30735 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arthur.suzuki@biblibre.com --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Arthur, could you check this one for backporting please? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org