[Bug 31381] New: [Intranet/REST-API] Searching patrons by letter broken when using non-mandatory extended attributes
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Bug ID: 31381 Summary: [Intranet/REST-API] Searching patrons by letter broken when using non-mandatory extended attributes Change sponsored?: --- Product: Koha Version: 22.05 Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: oliver.behnke@aei.mpg.de CC: tomascohen@gmail.com Hi, searching for patrons by letter like this returns an empty list: /cgi-bin/koha/members/members-home.pl?firstletter=A I find the following in intranet-error.log: [cgi:error] AH01215: [warn] OpenAPI >>> GET api/v1/patrons [{"message":"Expected string - got null.","path":"\\/body\\/10\\/extended_attributes\\/0\\/value"},{"message":"Expected string - got null.","path":"\\/body\\/14\\/extended_attributes\\/0\\/value"}]: /usr/share/koha/api/v1/app.pl, referer: /cgi-bin/koha/members/members-home.pl?firstletter=A We have defined a single extended attribute which isn't mandatory, so it might not exist for some patrons. If I interpret "10" and "14" as representing borrower_attributes.borrowernumber (table.attribute) values then "10" does indeed lack an entry, but "14" has one. Not a smoking gun yet... Maybe this is a regression due to this? https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27857 Cheers -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #1 from Oliver Behnke <oliver.behnke@aei.mpg.de> --- Note: the empty list isn't returned for all letters, so I presume that some patron DB entries break the list creation, probably those with the extended attribute "missing". I'll try to confirm that. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #2 from Oliver Behnke <oliver.behnke@aei.mpg.de> --- Gotcha! It's not the borrowers with the extended attribute "missing" but rather those which (existing) borrower_attributes.attribute is NULL. I'd argue that a NULL value should be treated like a missing attribute or like an empty string. I doubt it's intended to cause the error above and the subsequent patron list issue. Hope this helps -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|22.05 |master --- Comment #3 from Owen Leonard <oleonard@myacpl.org> --- I can reproduce this problem in master by setting a patron attribute on the patron edit screen and then editing the values in the database to be NULL. -- 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=31381 --- Comment #4 from Oliver Behnke <oliver.behnke@aei.mpg.de> --- I can also confirm that editing/saving an affected patron's NULL attribute via the intranet effectively removes it from borrower_attributes. Doing so for all affected patrons of a given letter (here "A") fixes the patron list issue for that letter. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- The REST API spec is not expecting NULL/undef for an attribute's value. As the UI is not allowing undef I am not sure this is a bug. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- If we need to fix this, we could apply this: diff --git a/api/v1/swagger/definitions/patron_extended_attribute.yaml b/api/v1/swagger/definitions/patron_extended_attribute.yaml index 84343d258b1..adb42944b80 100644 --- a/api/v1/swagger/definitions/patron_extended_attribute.yaml +++ b/api/v1/swagger/definitions/patron_extended_attribute.yaml @@ -6,7 +6,9 @@ properties: type: integer type: description: Extended attribute type - type: string + type: + - string + - "null" value: description: Extended attribute value type: And adjust the tests. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- And certainly as well api/v1/swagger/paths/patrons_extended_attributes.yaml -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #8 from Oliver Behnke <oliver.behnke@aei.mpg.de> --- If you decide against allowing/preparing for NULL values then the DB schema should probably be changed to define borrower_attributes.attribute as not nullable. This way whatever tries to do that would be caught right away, without causing subsequent errors down the line. Ideally this would also allow to find the root cause. -- 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=31381 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion CC| |nick@bywatersolutions.com --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Yes, Oliver, I think it's the way to go. Asking for more opinions, Nick what do you think? -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #9)
Yes, Oliver, I think it's the way to go.
Asking for more opinions, Nick what do you think?
Asking Tomas :-) -- 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=31381 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- 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=31381 --- Comment #11 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144173 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144173&action=edit Bug 31381: Handle null attributes in list -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.com |ity.org | --- Comment #12 from Nick Clemens <nick@bywatersolutions.com> --- This is affecting stables and preventing patron searching, we can handle it via data as well, but we should patch this to prevent failures and then forbid null attributes at the DB level on another bug -- 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=31381 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Jonathan Druart from comment #6)
And adjust the tests.
(In reply to Jonathan Druart from comment #7)
And certainly as well api/v1/swagger/paths/patrons_extended_attributes.yaml
What about that, Nick? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Jonathan Druart from comment #13)
(In reply to Jonathan Druart from comment #6)
And adjust the tests.
(In reply to Jonathan Druart from comment #7)
And certainly as well api/v1/swagger/paths/patrons_extended_attributes.yaml
What about that, Nick?
The tests pass - I can add a test that a null value doesn't fail For the second, I don't think we should add support to set attributes as null - we should prevent it wholly on another bug -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32331 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32331 [Bug 32331] Remove NULLABLE on borrower_attributes.attribute -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 144188 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144188&action=edit Bug 31381: Add tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Nick Clemens <nick@bywatersolutions.com> 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=31381 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144173|0 |1 is obsolete| | Attachment #144188|0 |1 is obsolete| | --- Comment #16 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144196 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144196&action=edit Bug 31381: Handle null attributes in list This patch allows null values to be returned in patron attributes to prevent a crash when searching patrons To test: 1 - Add a NULL attribute to a borrower, in sample data, Edna Acosta sudo koha-mysql kohadev INSERT INTO borrower_attributes (borrowernumber,code,attribute) VALUES (5,'SHOW_BCODE',NULL); 2 - Browse to 'Home->Patrons' http://localhost:8081/cgi-bin/koha/members/members-home.pl 3 - Click 'Browse by last name: A' 4 - Datatables error: Expected string - got null 5 - Apply patch 6 - Restart all 7 - Browse by last name: A 8 - Success! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #17 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 144197 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144197&action=edit Bug 31381: Add tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|[Intranet/REST-API] |Searching patrons by letter |Searching patrons by letter |broken when using |broken when using |non-mandatory extended |non-mandatory extended |attributes |attributes | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #18 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Works, makes sense, QA script happy. PQA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144196|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #144197|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #19 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144351 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144351&action=edit Bug 31381: Handle null attributes in list This patch allows null values to be returned in patron attributes to prevent a crash when searching patrons To test: 1 - Add a NULL attribute to a borrower, in sample data, Edna Acosta sudo koha-mysql kohadev INSERT INTO borrower_attributes (borrowernumber,code,attribute) VALUES (5,'SHOW_BCODE',NULL); 2 - Browse to 'Home->Patrons' http://localhost:8081/cgi-bin/koha/members/members-home.pl 3 - Click 'Browse by last name: A' 4 - Datatables error: Expected string - got null 5 - Apply patch 6 - Restart all 7 - Browse by last name: A 8 - Success! Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 --- Comment #20 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 144352 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=144352&action=edit Bug 31381: Add tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.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=31381 --- Comment #21 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 22.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 Jacob O'Mara <jacob.omara@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jacob.omara@ptfs-europe.com Status|Pushed to master |Pushed to stable Version(s)|23.05.00 |23.05.00, 22.11.01 released in| | --- Comment #22 from Jacob O'Mara <jacob.omara@ptfs-europe.com> --- Nice work! Pushed to 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31381 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org