Summary: | Holds history errors 500 if old_reserves.biblionumber is NULL | ||
---|---|---|---|
Product: | Koha | Reporter: | Pedro Amorim <pedro.amorim> |
Component: | Hold requests | Assignee: | Pedro Amorim <pedro.amorim> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P2 | CC: | fridolin.somers, george, gmcharlt, jonathan.druart, kebliss, lucas, martin.renvoize, nick, r.delahunty, rkuiper, tomascohen, wizzyrea |
Version: | Main | Keywords: | rel_22_11_candidate, rel_23_05_candidate |
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.04,22.11.10
|
|
Circulation function: | |||
Bug Depends on: | 32894 | ||
Bug Blocks: | |||
Attachments: |
Bug 34609: Return empty if no biblionumber
Bug 34609: Return empty if no biblionumber Bug 34609: Add biblio method to Old::Hold Bug 34609: Add tests Bug 34609: Add biblio method to Old::Hold Bug 34609: Add tests Bug 34609: Add biblio method to Old::Hold Bug 34609: Add missing test for Koha::Hold->biblio |
Description
Pedro Amorim
2023-08-24 13:52:36 UTC
Created attachment 154738 [details] [review] Bug 34609: Return empty if no biblionumber Test plan, clean k-t-d: 1) Add a reserve to koha/koha user, mysql run: insert into reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, biblionumber) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 76); 2) Add an old_reserve to koha/koha user, mysql run: insert into old_reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, reserve_id) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 1); 3) Visit holdshistory on either OPAC (requires OPACHoldsHistory sys pref) or STAFF STAFF/cgi-bin/koha/members/holdshistory.pl?borrowernumber=1 OPAC//cgi-bin/koha/opac-holdshistory.pl 4) Notice it blows up with a 500 error 5) Repeat step 3 *** Bug 34603 has been marked as a duplicate of this bug. *** Thanks Pedro, this works! Do you think we need unit tests for this change? There's some prior art for tests for this [U+1F642]. I hate writing them as they feel like over repetition... but they would have caught this regression.. so... (In reply to Lucas Gass from comment #3) > Thanks Pedro, this works! Do you think we need unit tests for this change? Thanks Lucas, I had not realized there was already a bug filed for this. What I think happened is, the biblio method in Hold.pm returned undef if it did not have a biblionumber, but after this patch: https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=32894&attachment=152086 It started returning an empty DBix::ResultSource(?) instead of undef leading to holdhistory.tt to explode. A test making sure biblio in Hold.pm returns either a biblio object or undef would certainly been helpful before bug 32894, and certainly does not hurt now either! Created attachment 154901 [details] [review] Bug 34609: Return empty if no biblionumber Test plan, clean k-t-d: 1) Add a reserve to koha/koha user, mysql run: insert into reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, biblionumber) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 76); 2) Add an old_reserve to koha/koha user, mysql run: insert into old_reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, reserve_id) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 1); 3) Visit holdshistory on either OPAC (requires OPACHoldsHistory sys pref) or STAFF STAFF/cgi-bin/koha/members/holdshistory.pl?borrowernumber=1 OPAC//cgi-bin/koha/opac-holdshistory.pl 4) Notice it blows up with a 500 error 5) Repeat step 3 Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Raising this to major given the nature of the issue and attention it's gotten. No, this must be in Koha::Old::Hold, Koha::Hold reflect the 'reserves' table, and we have a NOT NULL there `biblionumber` int(11) NOT NULL Also, tests are missing. Created attachment 154952 [details] [review] Bug 34609: Add biblio method to Old::Hold Created attachment 154953 [details] [review] Bug 34609: Add tests Created attachment 154954 [details] [review] Bug 34609: Add biblio method to Old::Hold Test plan, clean k-t-d: 1) Add a reserve to koha/koha user, mysql run: insert into reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, biblionumber) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 76); 2) Add an old_reserve to koha/koha user, mysql run: insert into old_reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, reserve_id) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 1); 3) Visit holdshistory on either OPAC (requires OPACHoldsHistory sys pref) or STAFF STAFF/cgi-bin/koha/members/holdshistory.pl?borrowernumber=1 OPAC//cgi-bin/koha/opac-holdshistory.pl 4) Notice it blows up with a 500 error 5) Repeat step 3 Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Created attachment 154993 [details] [review] Bug 34609: Add tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 154994 [details] [review] Bug 34609: Add biblio method to Old::Hold Test plan, clean k-t-d: 1) Add a reserve to koha/koha user, mysql run: insert into reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, biblionumber) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 76); 2) Add an old_reserve to koha/koha user, mysql run: insert into old_reserves(borrowernumber, reservedate, branchcode, cancellationdate, timestamp, reserve_id) VALUES (51, '2022-09-23', 'CPL', '2022-09-23', '2022-09-23 15:46:21', 1); 3) Visit holdshistory on either OPAC (requires OPACHoldsHistory sys pref) or STAFF STAFF/cgi-bin/koha/members/holdshistory.pl?borrowernumber=1 OPAC//cgi-bin/koha/opac-holdshistory.pl 4) Notice it blows up with a 500 error 5) Repeat step 3 Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 154995 [details] [review] Bug 34609: Add missing test for Koha::Hold->biblio Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 23.11. Nice work everyone, thanks! This can also cause a 500 error when partners go to their Cataloging > Export page. Log will say something like Template process failed: undef error - DBIC result _type isn't of the _type Biblio at /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/modules/members/holdshistory.tt line 73 (In reply to Liz Rea from comment #16) > This can also cause a 500 error when partners go to their Cataloging > > Export page. Log will say something like Template process failed: undef > error - DBIC result _type isn't of the _type Biblio at > /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/modules/members/ > holdshistory.tt line 73 With this patchset applied that error is also fixed. Hi may we have a backport to oldstable and oldoldstable please? (In reply to Liz Rea from comment #18) > Hi may we have a backport to oldstable and oldoldstable please? +1 Yes, backport please! Pushed to 23.05.x for 23.05.04 Nice work everyone! Pushed to oldstable for 22.11.x |