Bug 31160

Summary:	Required fields in the Patrons API are a bit random
Product:	Koha	Reporter:	Martin Renvoize (ashimema) <martin.renvoize>
Component:	REST API	Assignee:	Matt Blenkinsop <matt.blenkinsop>
Status:	CLOSED FIXED	QA Contact:	Marcel de Rooy <m.de.rooy>
Severity:	minor
Priority:	P5 - low	CC:	david, m.de.rooy, nick, tomascohen
Version:	Main
Hardware:	All
OS:	All
Change sponsored?:	---	Patch complexity:	Trivial patch
Documentation contact:		Documentation submission:
Text to go in the release notes:		Version(s) released in:	23.05.00,22.11.01
Circulation function:
Attachments:	Bug 31160: Required Fields in Patron API Bug 31160: Required Fields in Patron API Bug 31160: Required fields in patron api Bug 31160: Required fields in patron api

Description Martin Renvoize (ashimema) 2022-07-15 08:56:11 UTC

Currently, the required fields in the patron API do not match the required fields in the CSV imports.. and they don't entirely make sense.. city and address are required for example.. is there some reasoning behind those choices?

Comment 1 Martin Renvoize (ashimema) 2022-11-24 11:54:49 UTC

First spec for this is that I feel we should make sure the required fields match those listed in the csv import page.

Comment 2 Matt Blenkinsop 2022-11-24 16:27:55 UTC

Created attachment 144218 [details] [review]
Bug 31160: Required Fields in Patron API

The Patron API PUT request required the fields "surname","address","city","library_id","category_id". Meanwhile the CSV import page for patrons only requires "surname","library_id","category_id". The fields for "address" and "city" have been removed from the "required" section of the api>v1>swagger>definitions>patron.yaml file to match the CSV import page.

Test Plan:
1) Send a PUT request to http://localhost:8081/api/v1/patrons/19 with an empty body
2) Error message should ask for 5 missing fields as listed above
3) Apply patch
4) Restart Apache server using "restart_all" in your ktd shell
5) Resubmit your request
6) Error message should now only request "surname","library_id","category_id"

Comment 3 Martin Renvoize (ashimema) 2022-11-28 08:28:43 UTC

Remember to set 'Needs Signoff' Matt ;)

Comment 4 David Nind 2022-11-28 22:17:11 UTC

Created attachment 144308 [details] [review]
Bug 31160: Required Fields in Patron API

The Patron API PUT request required the fields "surname","address","city","library_id","category_id". Meanwhile the CSV import page for patrons only requires "surname","library_id","category_id". The fields for "address" and "city" have been removed from the "required" section of the api>v1>swagger>definitions>patron.yaml file to match the CSV import page.

Test Plan:
1) Send a PUT request to http://localhost:8081/api/v1/patrons/19 with an empty body
2) Error message should ask for 5 missing fields as listed above
3) Apply patch
4) Restart Apache server using "restart_all" in your ktd shell
5) Resubmit your request
6) Error message should now only request "surname","library_id","category_id"

Signed-off-by: David Nind <david@davidnind.com>

Comment 5 David Nind 2022-11-28 22:20:39 UTC

Hi Matt.

I'll let you fix up the title for the patch - it should be in sentence case.

David

Testing notes using KTD (koha-testing-docker):

1. I used Postman to make the request.

2. I had to put empty curly brackets in the body ({}) and select JSON as the format to get the error message asking for the empty fields.

Comment 6 Matt Blenkinsop 2022-11-29 10:45:28 UTC

Created attachment 144313 [details] [review]
Bug 31160: Required fields in patron api

The Patron API PUT request required the fields "surname","address","city","library_id","category_id". Meanwhile the CSV import page for patrons only requires "surname","library_id","category_id". The fields for "address" and "city" have been removed from the "required" section of the api>v1>swagger>definitions>patron.yaml file to match the CSV import page.

Test Plan:
1) Send a PUT request to http://localhost:8081/api/v1/patrons/19 with an empty body
2) Error message should ask for 5 missing fields as listed above
3) Apply patch
4) Restart Apache server using "restart_all" in your ktd shell
5) Resubmit your request
6) Error message should now only request "surname","library_id","category_id"

Signed-off-by: David Nind <david@davidnind.com>

Comment 7 Marcel de Rooy 2022-12-09 10:47:43 UTC

Created attachment 144525 [details] [review]
Bug 31160: Required fields in patron api

The Patron API PUT request required the fields "surname","address","city","library_id","category_id". Meanwhile the CSV import page for patrons only requires "surname","library_id","category_id". The fields for "address" and "city" have been removed from the "required" section of the api>v1>swagger>definitions>patron.yaml file to match the CSV import page.

Test Plan:
1) Send a PUT request to http://localhost:8081/api/v1/patrons/19 with an empty body
2) Error message should ask for 5 missing fields as listed above
3) Apply patch
4) Restart Apache server using "restart_all" in your ktd shell
5) Resubmit your request
6) Error message should now only request "surname","library_id","category_id"

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Comment 8 Tomás Cohen Arazi (tcohen) 2022-12-13 12:27:22 UTC

Pushed to master for 23.05.

Nice work everyone, thanks!

Comment 9 Martin Renvoize (ashimema) 2022-12-15 13:33:06 UTC

Nice work everyone!

Pushed to 22.11.x for the next release