Bug 28248 - Exception when getting all orders
Summary: Exception when getting all orders
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: 20.11
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on: 18731
Blocks:
  Show dependency treegraph
 
Reported: 2021-04-28 14:27 UTC by Stefan Norinder
Modified: 2021-05-25 14:45 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.06,20.05.12


Attachments
Bug 28248: aqorders.created_by is nullable (1.38 KB, patch)
2021-04-29 18:47 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 28248: aqorders.created_by is nullable (1.46 KB, patch)
2021-04-30 22:01 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 28248: aqorders.created_by is nullable (1.54 KB, patch)
2021-05-07 11:05 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Norinder 2021-04-28 14:27:52 UTC
Making a get request against /api/v1/acquisitions/orders/ generates the following exception when aqorders.created_by is null i database

[2021-04-28 14:10:21.48828] [600131] [warn] OpenAPI >>> GET api/v1/acquisitions/orders/ [{"message":"Expected integer - got null.","path":"\/0\/created_by"},{"message":"Expected integer - got null.","path":"\/1\/created_by"},{"message":"Expected integer - got null.","path":"\/2\/created_by"},{"message":"Expected integer - got null.","path":"\/3\/created_by"},{"message":"Expected integer - got null.","path":"\/4\/created_by"},{"message":"Expected integer - got null."
Comment 1 Jonathan Druart 2021-04-28 14:31:27 UTC
Can be NULL in DB
 638   `created_by` int(11) DEFAULT NULL COMMENT 'the borrowernumber of order line''s creator',

But not in the API specs.
Comment 2 Tomás Cohen Arazi 2021-04-28 14:54:07 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2021-04-28 15:18:13 UTC
They are using 20.05.09
Comment 4 Tomás Cohen Arazi 2021-04-29 18:45:47 UTC
This is fixed in master/21.05+ by bug 20212, so making it a target for 20.11 as well.
Comment 5 Tomás Cohen Arazi 2021-04-29 18:47:50 UTC
Created attachment 120318 [details] [review]
Bug 28248: aqorders.created_by is nullable

This patch introduces a change to the spec to acknowledge that the field
is nullable. This is fixed in 21.05+ as part of bug 20212.

To test:
1. Have an order with created_by = NULL
2. Use your favourite REST tool (Postman?) to fetch the order
=> FAIL: It throws a nasty error about non-nullable field being null
3. Apply this patch
4. Reload all
5. Repeat 2
=> SUCCESS: It returns things, created_by is NULL, as it should
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Jonathan Druart 2021-04-30 08:46:28 UTC
Stefan, can you test this patch and confirm that it fixes the issue you reported?
Comment 7 Victor Grousset/tuxayo 2021-04-30 22:01:22 UTC
Created attachment 120349 [details] [review]
Bug 28248: aqorders.created_by is nullable

This patch introduces a change to the spec to acknowledge that the field
is nullable. This is fixed in 21.05+ as part of bug 20212.

To test:
1. Have an order with created_by = NULL
2. Use your favourite REST tool (Advanced REST Client(libre), Postman?) to fetch the order
=> FAIL: It throws a nasty error about non-nullable field being null
3. Apply this patch
4. Reload all
5. Repeat 2
=> SUCCESS: It returns things, created_by is NULL, as it should
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 8 Victor Grousset/tuxayo 2021-04-30 22:02:46 UTC
Works :)

Testing notes:
- don't forget it's for 20.11.x, not master.
- Testing without REST client can be done via http://localhost:8081/api/v1/acquisitions/orders/1
Comment 9 Victor Grousset/tuxayo 2021-04-30 22:06:19 UTC
Code looks good, QA script happy. Count my QA stamp in :)

Signoff still needed if you can Stefan.
Comment 10 Kyle M Hall 2021-05-07 10:52:34 UTC
It's been over a week. I think it's safe to move forward on this one!
Comment 11 Kyle M Hall 2021-05-07 11:05:41 UTC
Created attachment 120675 [details] [review]
Bug 28248: aqorders.created_by is nullable

This patch introduces a change to the spec to acknowledge that the field
is nullable. This is fixed in 21.05+ as part of bug 20212.

To test:
1. Have an order with created_by = NULL
2. Use your favourite REST tool (Advanced REST Client(libre), Postman?) to fetch the order
=> FAIL: It throws a nasty error about non-nullable field being null
3. Apply this patch
4. Reload all
5. Repeat 2
=> SUCCESS: It returns things, created_by is NULL, as it should
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Jonathan Druart 2021-05-07 11:43:46 UTC
Sending to 20.11 queue.
Comment 13 Fridolin Somers 2021-05-12 15:31:34 UTC
Pushed to 20.11.x for 20.11.06
Comment 14 Andrew Fuerste-Henry 2021-05-25 13:33:25 UTC
Pushed to 20.05.x for 20.05.12
Comment 15 Victor Grousset/tuxayo 2021-05-25 14:45:00 UTC
Missing dependencies for 19.11.x, it shouldn't be affected, no backport.