Making a call to /api/v1/patrons/<patron_id>/account where the patron has an accountline with manager_id = NULL causes the request to fail with error code 500 because the swagger spec defines only integer as user_id/manager_id type. It should also accept null types because fines.pl cron seems to create accountlines with manager_id=NULL.
Created attachment 108820 [details] [review] Bug 26271: Add failing test to reveal issue with patrons API endpoint When manager_id is null/undef the API returns error code 500. To test: 1) Notice failure when running prove t/db_dependent/api/v1/patrons_accounts.t
Created attachment 108821 [details] [review] Bug 26271: Add null to the list of accepted account_line data types The database schema for accountlines table allows the manager_id/user_id column to be NULL. If request to /api/v1/patrons/<patron_id>/account returns such an accountline where it is NULL we get 500 error as response. Adding NULL to allowed data types fixes this issue. To test: 1) Run prove t/db_dependent/api/v1/patrons_accounts.t and notice it doesn't fail
Created attachment 109583 [details] [review] Bug 26271: Add null to the list of accepted account_line data types The database schema for accountlines table allows the manager_id/user_id column to be NULL. If request to /api/v1/patrons/<patron_id>/account returns such an accountline where it is NULL we get 500 error as response. Adding NULL to allowed data types fixes this issue. To test: 1) Run prove t/db_dependent/api/v1/patrons_accounts.t and notice it doesn't fail Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 109584 [details] [review] Bug 26271: Add failing test to reveal issue with patrons API endpoint When manager_id is null/undef the API returns error code 500. To test: 1) Notice failure when running prove t/db_dependent/api/v1/patrons_accounts.t Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 109585 [details] [review] Bug 26271: Add null to the list of accepted account_line data types The database schema for accountlines table allows the manager_id/user_id column to be NULL. If request to /api/v1/patrons/<patron_id>/account returns such an accountline where it is NULL we get 500 error as response. Adding NULL to allowed data types fixes this issue. To test: 1) Run prove t/db_dependent/api/v1/patrons_accounts.t and notice it doesn't fail Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Looks perfect ;)
Created attachment 109591 [details] [review] Bug 26271: Add failing test to reveal issue with patrons API endpoint When manager_id is null/undef the API returns error code 500. To test: 1) Notice failure when running prove t/db_dependent/api/v1/patrons_accounts.t Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 109592 [details] [review] Bug 26271: Add null to the list of accepted account_line data types The database schema for accountlines table allows the manager_id/user_id column to be NULL. If request to /api/v1/patrons/<patron_id>/account returns such an accountline where it is NULL we get 500 error as response. Adding NULL to allowed data types fixes this issue. To test: 1) Run prove t/db_dependent/api/v1/patrons_accounts.t and notice it doesn't fail Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected, well spotted. Passing QA
That is something I was expected for a while. We are going to face this problem again and again. We need a test to compare the API definition and the DB schema (datatype and nullable).
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.04
backported to 19.11.x for 19.11.10
Not backported to oldoldstable (19.05.x). Feel free to ask if it's needed.