Bug 31160 - Required fields in the Patrons API are a bit random
Summary: Required fields in the Patrons API are a bit random
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Matt Blenkinsop
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-15 08:56 UTC by Martin Renvoize
Modified: 2023-06-08 22:32 UTC (History)
5 users (show)

See Also:
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


Attachments
Bug 31160: Required Fields in Patron API (1.36 KB, patch)
2022-11-24 16:27 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 31160: Required Fields in Patron API (1.39 KB, patch)
2022-11-28 22:17 UTC, David Nind
Details | Diff | Splinter Review
Bug 31160: Required fields in patron api (1.41 KB, patch)
2022-11-29 10:45 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 31160: Required fields in patron api (1.49 KB, patch)
2022-12-09 10:47 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 2022-12-13 12:27:22 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 9 Martin Renvoize 2022-12-15 13:33:06 UTC
Nice work everyone!

Pushed to 22.11.x for the next release