When the requested item doesn't exist it returns a 404. But it returns a 400 if a biblio was requested and is not present.
Created attachment 116216 [details] [review] Bug 27593: Regression tests This patch introduces tests for the behaviour change (400 => 404) and also adds tests for untested (error) behaviours. Notably, the 'Biblio not found' problematic return value couldn't be triggered, because the OpenAPI plugin returns a 500 because the response was malformed (expected { error => 'message' } and it was returning a string). For the above reason, running the regression tests will show a 500... instead of the expected 400. I covered the rest of the codepaths that trigger errors to have full coverage. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/clubs_holds.t => FAIL: Tests fail loudly Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 116217 [details] [review] Bug 27593: Consistent behavior for club holds route This patch: - Fixes a malformed response when biblio_id is invalid - Fixes the return status (400 => 404) when biblio_id is invalid - Adapts the error strings to be consistent with the Cities.pm file To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/clubs_holds.t => FAIL: Tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 117007 [details] [review] Bug 27593: Regression tests This patch introduces tests for the behaviour change (400 => 404) and also adds tests for untested (error) behaviours. Notably, the 'Biblio not found' problematic return value couldn't be triggered, because the OpenAPI plugin returns a 500 because the response was malformed (expected { error => 'message' } and it was returning a string). For the above reason, running the regression tests will show a 500... instead of the expected 400. I covered the rest of the codepaths that trigger errors to have full coverage. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/clubs_holds.t => FAIL: Tests fail loudly Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117008 [details] [review] Bug 27593: Consistent behavior for club holds route This patch: - Fixes a malformed response when biblio_id is invalid - Fixes the return status (400 => 404) when biblio_id is invalid - Adapts the error strings to be consistent with the Cities.pm file To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/clubs_holds.t => FAIL: Tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 117155 [details] [review] Bug 27593: Regression tests This patch introduces tests for the behaviour change (400 => 404) and also adds tests for untested (error) behaviours. Notably, the 'Biblio not found' problematic return value couldn't be triggered, because the OpenAPI plugin returns a 500 because the response was malformed (expected { error => 'message' } and it was returning a string). For the above reason, running the regression tests will show a 500... instead of the expected 400. I covered the rest of the codepaths that trigger errors to have full coverage. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/clubs_holds.t => FAIL: Tests fail loudly Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 117156 [details] [review] Bug 27593: Consistent behavior for club holds route This patch: - Fixes a malformed response when biblio_id is invalid - Fixes the return status (400 => 404) when biblio_id is invalid - Adapts the error strings to be consistent with the Cities.pm file To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/clubs_holds.t => FAIL: Tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected, a sensible improvement. QA Script passes, covered by tests. Passing QA
Pushed to master for 21.05, thanks to everybody involved!
Pushed to 20.11.x for 20.11.04
Pushed to 20.05.x for 20.05.10
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.