[Bug 25048] New: Successful resource deletion should return 204
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Bug ID: 25048 Summary: Successful resource deletion should return 204 Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com As stated in our coding guidelines for the API [1], when deleting a resource, the API should return a 204 instead of the 200 some do now. [1] https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.4_DE... -- 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=25048 Tomás Cohen Arazi <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 watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- References for people reviewing this: - RESTful DELETE: https://restfulapi.net/http-methods/#delete - OpenAPI handling on 204: https://swagger.io/docs/specification/2-0/describing-responses/ In the latter search for 'Empty Response Body'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Tomás Cohen Arazi <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=25048 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |25032 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25032 [Bug 25032] Generic unhandled exception handling -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com URL| |https://wiki.koha-community | |.org/wiki/Coding_Guidelines | |_-_API#SWAGGER3.2.4_DELETE 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=25048 --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 102332 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102332&action=edit Bug 25048: Regression tests This patch adds regression tests for the response bodies and statuses on DELETE actions against existing API routes. This is just enforcing the existing (voted) Coding guidelines for the API (tm). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => FAIL: Several routes have problems -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 102333 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102333&action=edit Bug 25048: Make successful resource deletion return 204 This patch adapts the spec and the controllers so existing routes return 204 and an empty response body when a successful deletion happens. Right now we have a coding guideline but haven't adapted the existing routes. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => FAIL: Some tests fail 3. Apply this patch 4. Repeat 2. => SUCCESS: Tests pass! 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=25048 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- + ->status_is(204, 'SWAGGER3.2.4') + ->content_is('', 'SWAGGER3.3.4'); Can you explain that? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #4)
+ ->status_is(204, 'SWAGGER3.2.4') + ->content_is('', 'SWAGGER3.3.4');
Can you explain that?
That refers to the API coding guidelines pointed by the bug. This is something already present in some tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 David Nind <david@davidnind.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=25048 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102332|0 |1 is obsolete| | --- Comment #6 from David Nind <david@davidnind.com> --- Created attachment 102403 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102403&action=edit Bug 25048: Regression tests This patch adds regression tests for the response bodies and statuses on DELETE actions against existing API routes. This is just enforcing the existing (voted) Coding guidelines for the API (tm). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => FAIL: Several routes have problems 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=25048 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102333|0 |1 is obsolete| | --- Comment #7 from David Nind <david@davidnind.com> --- Created attachment 102404 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=102404&action=edit Bug 25048: Make successful resource deletion return 204 This patch adapts the spec and the controllers so existing routes return 204 and an empty response body when a successful deletion happens. Right now we have a coding guideline but haven't adapted the existing routes. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => FAIL: Some tests fail 3. Apply this patch 4. Repeat 2. => SUCCESS: Tests pass! 5. Sign off :-D 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=25048 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de Status|Signed Off |BLOCKED --- Comment #8 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Dependency is not signed off yet - moving to BLOCKED. Please unlock when bug 25032 is ready. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|BLOCKED |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #8)
Dependency is not signed off yet - moving to BLOCKED. Please unlock when bug 25032 is ready.
Dependency unlocked. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #102403|0 |1 is obsolete| | Attachment #102404|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103943 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103943&action=edit Bug 25048: Regression tests This patch adds regression tests for the response bodies and statuses on DELETE actions against existing API routes. This is just enforcing the existing (voted) Coding guidelines for the API (tm). To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => FAIL: Several routes have problems Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103944 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103944&action=edit Bug 25048: Make successful resource deletion return 204 This patch adapts the spec and the controllers so existing routes return 204 and an empty response body when a successful deletion happens. Right now we have a coding guideline but haven't adapted the existing routes. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/*.t => FAIL: Some tests fail 3. Apply this patch 4. Repeat 2. => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.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=25048 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25048 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #13 from Joy Nelson <joy@bywatersolutions.com> --- missing dependencies - not backported -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org