| Summary: | Successful resource deletion should return 204 | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | 1joynelson, jonathan.druart, kyle, martin.renvoize |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| URL: | https://wiki.koha-community.org/wiki/Coding_Guidelines_-_API#SWAGGER3.2.4_DELETE | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
20.05.00
|
|
| Circulation function: | |||
| Bug Depends on: | 25032 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 25048: Regression tests
Bug 25048: Make successful resource deletion return 204 Bug 25048: Regression tests Bug 25048: Make successful resource deletion return 204 Bug 25048: Regression tests Bug 25048: Make successful resource deletion return 204 |
||
|
Description
Tomás Cohen Arazi (tcohen)
2020-04-02 18:25:16 UTC
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'. Created attachment 102332 [details] [review] 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 Created attachment 102333 [details] [review] 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 + ->status_is(204, 'SWAGGER3.2.4')
+ ->content_is('', 'SWAGGER3.3.4');
Can you explain that?
(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. Created attachment 102403 [details] [review] 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> Created attachment 102404 [details] [review] 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> Dependency is not signed off yet - moving to BLOCKED. Please unlock when bug 25032 is ready. (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. Created attachment 103943 [details] [review] 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> Created attachment 103944 [details] [review] 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> Nice work everyone! Pushed to master for 20.05 missing dependencies - not backported |