[Bug 38926] New: POST /biblios returns 200 even if AddBiblio fails
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Bug ID: 38926 Summary: POST /biblios returns 200 even if AddBiblio fails Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@gmail.com QA Contact: testopia@bugs.koha-community.org CC: tomascohen@gmail.com Depends on: 31800 For some reasons I POSTed an invalid biblio to /api/v1/biblios it returned {id: null} The biblio record is not created, but the response is 200. 707 my ( $biblio_id ) = AddBiblio( $record, $frameworkcode, { record_source_id => $record_source_id } ); 708 709 $c->render( 710 status => 200, 711 openapi => { id => $biblio_id } 712 ); Obviously we should test $biblio_id Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31800 [Bug 31800] Add POST endpoint for Biblios -- 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=38926 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- 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=38926 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 176795 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176795&action=edit Bug 38926: Regression tests -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 176796 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176796&action=edit Bug 38926: Make POST /biblios return 400 if AddBiblio fails We don't have proper exceptions in the `C4::Biblio::AddBiblio` method, but we at least know `$biblio_id` will be `undef` in the even of an error processing the call. This patch makes the controller handle this situation so (at least) it is obvious that something bad happened. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t => FAIL: Tests fail! The endpoint returns 200 even on error! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The endpoint returns a 400 with a reasonable message! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |38929 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38929 [Bug 38929] POST endpoints not returning the Location header -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 --- Comment #3 from Jonathan Druart <jonathan.druart@gmail.com> --- should not we list the error_code in the spec? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176796|0 |1 is obsolete| | --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 176835 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176835&action=edit Bug 38926: Make POST /biblios return 400 if AddBiblio fails We don't have proper exceptions in the `C4::Biblio::AddBiblio` method, but we at least know `$biblio_id` will be `undef` in the even of an error processing the call. This patch makes the controller handle this situation so (at least) it is obvious that something bad happened. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t => FAIL: Tests fail! The endpoint returns 200 even on error! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The endpoint returns a 400 with a reasonable message! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #3)
should not we list the error_code in the spec?
Good catch! Fixed. Thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Jonathan Druart <jonathan.druart@gmail.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=38926 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176795|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176847 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176847&action=edit Bug 38926: Regression tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176835|0 |1 is obsolete| | --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 176848 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176848&action=edit Bug 38926: Make POST /biblios return 400 if AddBiblio fails We don't have proper exceptions in the `C4::Biblio::AddBiblio` method, but we at least know `$biblio_id` will be `undef` in the even of an error processing the call. This patch makes the controller handle this situation so (at least) it is obvious that something bad happened. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t => FAIL: Tests fail! The endpoint returns 200 even on error! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The endpoint returns a 400 with a reasonable message! 5. Sign off :-D Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176847|0 |1 is obsolete| | --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 176890 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176890&action=edit Bug 38926: Regression tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #176848|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 176891 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=176891&action=edit Bug 38926: Make POST /biblios return 400 if AddBiblio fails We don't have proper exceptions in the `C4::Biblio::AddBiblio` method, but we at least know `$biblio_id` will be `undef` in the even of an error processing the call. This patch makes the controller handle this situation so (at least) it is obvious that something bad happened. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/biblios.t => FAIL: Tests fail! The endpoint returns 200 even on error! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! The endpoint returns a 400 with a reasonable message! 5. Sign off :-D Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #10 from David Nind <david@davidnind.com> --- Have added my signoff, so maybe Jonathan's can count as QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.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=38926 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.05.00 |25.05.00,24.11.04 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 --- Comment #12 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 24.11.x for 24.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to stable |Needs documenting --- Comment #13 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Not backporting to 22.11.x as it wasn't backported to 24.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38926 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #14 from David Nind <david@davidnind.com> --- Bug fix, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org