Bug 24611

Summary: Wrong budget_id query parameter in /acquisitions/orders
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact:
Severity: trivial    
Priority: P5 - low CC: 1joynelson, ere.maijala, katrin.fischer, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00
Bug Depends on: 18731    
Bug Blocks:    
Attachments: Bug 24611: Regression test
Bug 24611: Fix wrong budget_id query param in /acquisitions/orders
Bug 24611: Regression test
Bug 24611: Fix wrong budget_id query param in /acquisitions/orders
Bug 24611: Regression test
Bug 24611: Fix wrong budget_id query param in /acquisitions/orders

Description Tomás Cohen Arazi 2020-02-07 12:36:44 UTC
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!
Comment 1 Tomás Cohen Arazi 2020-02-07 12:54:31 UTC
Created attachment 98562 [details] [review]
Bug 24611: Regression test
Comment 2 Tomás Cohen Arazi 2020-02-07 12:54:34 UTC
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>
Comment 3 David Nind 2020-02-07 21:01:44 UTC
Created attachment 98592 [details] [review]
Bug 24611: Regression test

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2020-02-07 21:01:47 UTC
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>
Comment 5 Katrin Fischer 2020-02-09 10:28:54 UTC
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
Comment 6 Tomás Cohen Arazi 2020-02-09 14:55:49 UTC
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$
Comment 7 Katrin Fischer 2020-02-09 15:44:32 UTC
Just what I thought - I think it's my environment.
Comment 8 Tomás Cohen Arazi 2020-02-09 17:01:12 UTC
(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).
Comment 9 Jonathan Druart 2020-02-13 13:09:31 UTC
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>
Comment 10 Jonathan Druart 2020-02-13 13:09:34 UTC
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>
Comment 11 Martin Renvoize 2020-02-13 16:02:04 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Katrin Fischer 2020-02-14 15:14:25 UTC
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!
Comment 13 Joy Nelson 2020-03-05 00:59:17 UTC
Does not apply to 19.11.x branch.  Please Rebase if needed.