Between the writing time of the endpoint (2017) and the time it was refactored and pushed, attribute mappings were introduced, and the query parameters didn't get enough attention so it still refers to budget_id. We need to fix it!
Created attachment 98562 [details] [review] Bug 24611: Regression test
Created attachment 98563 [details] [review] Bug 24611: Fix wrong budget_id query param in /acquisitions/orders This patch fixes the wrong query parameter that slipped on rewriting the patches for the voted RFC. To test: 1. Apply the tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests fail because fund_id is not a valid query parameter 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 98592 [details] [review] Bug 24611: Regression test Signed-off-by: David Nind <david@davidnind.com>
Created attachment 98593 [details] [review] Bug 24611: Fix wrong budget_id query param in /acquisitions/orders This patch fixes the wrong query parameter that slipped on rewriting the patches for the voted RFC. To test: 1. Apply the tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests fail because fund_id is not a valid query parameter 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>
Test is not happy on my system - leaving this Signed Off as I think they passed for David and Tomas: # Failed test 'SWAGGER3.2.2' # at t/db_dependent/api/v1/acquisitions_orders.t line 65. # got: '500' # expected: '200' # Failed test 'has value for JSON Pointer "/14/order_id"' # at t/db_dependent/api/v1/acquisitions_orders.t line 65. # Looks like you failed 2 tests of 8. t/db_dependent/api/v1/acquisitions_orders.t .. 1/5 # Failed test 'list() tests' # at t/db_dependent/api/v1/acquisitions_orders.t line 97. t/db_dependent/api/v1/acquisitions_orders.t .. 5/5 # Looks like you failed 1 test of 5. t/db_dependent/api/v1/acquisitions_orders.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/5 subtests Test Summary Report ------------------- t/db_dependent/api/v1/acquisitions_orders.t (Wstat: 256 Tests: 5 Failed: 1) Failed test: 1 Non-zero exit status: 1 Files=1, Tests=5, 16 wallclock secs ( 0.02 usr 0.02 sys + 11.47 cusr 1.89 csys = 13.40 CPU) Result: FAIL
Katirna, I don't manage to reproduce: (base) ✔ ~/git/koha [qa_24611 ↑·2|⚑ 13] 11:52 $ git reset --hard origin/master HEAD is now at 99e2fbd083 Bug 13193: (RM follow-up) Remove Cache::Memcached::Fast from PerlDependencies (base) ✔ ~/git/koha [qa_24611|⚑ 13] 11:53 $ git bz apply 24611 Bug 24611 - Wrong budget_id query parameter in /acquisitions/orders 98592 - Bug 24611: Regression test 98593 - Bug 24611: Fix wrong budget_id query param in /acquisitions/orders Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 24611: Regression test Applying: Bug 24611: Fix wrong budget_id query param in /acquisitions/orders (base) ✔ ~/git/koha [qa_24611 ↑·2|⚑ 13] 11:53 $ kshell root@d749cd762ad3:kohadevbox$ kshell kohadev-koha@d749cd762ad3:/kohadevbox/koha$ prove t/db_dependent/api/v1/acquisitions_orders.t t/db_dependent/api/v1/acquisitions_orders.t .. ok All tests successful. Files=1, Tests=5, 22 wallclock secs ( 0.05 usr 0.02 sys + 12.53 cusr 2.75 csys = 15.35 CPU) Result: PASS kohadev-koha@d749cd762ad3:/kohadevbox/koha$
Just what I thought - I think it's my environment.
(In reply to Katrin Fischer from comment #7) > Just what I thought - I think it's my environment. Katrina, it would be interesting to see the output of you add use Data::Printer colored => 1; p($t->tx->res); After the failed test. It might highlight something important (bad handling on error conditions).
Created attachment 98822 [details] [review] Bug 24611: Regression test Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 98823 [details] [review] Bug 24611: Fix wrong budget_id query param in /acquisitions/orders This patch fixes the wrong query parameter that slipped on rewriting the patches for the voted RFC. To test: 1. Apply the tests patch 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => FAIL: Tests fail because fund_id is not a valid query parameter 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: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
If you are still interested in me testing again, please ping me when you are back from vacation - but I believe it's probably fine!
Does not apply to 19.11.x branch. Please Rebase if needed.