Bug 29975

Summary: (Bug 21729 followup) patron_expiration_date missing in API
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: REST APIAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: normal    
Priority: P5 - low CC: jonathan.druart, nick, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29976
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00
Bug Depends on: 21729    
Bug Blocks:    
Attachments: Bug 29975: [DRAFT] fix API
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t
Bug 29975: Add missing patron_expiration_date attribute to hold spec
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t

Description Fridolin Somers 2022-01-29 00:19:38 UTC
Bug 21729 added to holds table the column patron_expiration_date.

This must impact the REST API.
Currently test suite is failing :
https://jenkins.koha-community.org/view/master/job/Koha_Master_D10/516/testReport/
Comment 1 Fridolin Somers 2022-01-29 00:21:23 UTC
Created attachment 129961 [details] [review]
Bug 29975: [DRAFT] fix API
Comment 2 Fridolin Somers 2022-01-29 00:22:43 UTC
(In reply to Fridolin Somers from comment #1)
> Created attachment 129961 [details] [review] [review]
> Bug 29975: [DRAFT] fix API

This draft patch makes api test suite happy.
But I don't know if it is the correct solution.
Comment 3 Jonathan Druart 2022-01-31 10:00:15 UTC
Created attachment 129988 [details] [review]
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t

Unknown column 'patron_expiration_date' in 'field list'
The REST API routes don't need to be aware of the new column patron_expiration_date
Comment 4 Fridolin Somers 2022-02-01 00:47:09 UTC
Thanks a lot for the real patch Jonathan :D
Comment 5 Fridolin Somers 2022-02-02 06:41:36 UTC
Created attachment 130058 [details] [review]
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t

Unknown column 'patron_expiration_date' in 'field list'
The REST API routes don't need to be aware of the new column patron_expiration_date

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 6 Tomás Cohen Arazi 2022-02-02 12:14:33 UTC
Created attachment 130090 [details] [review]
Bug 29975: Add missing patron_expiration_date attribute to hold spec

On bug 21729 a new attribute was added to the holds objects, but we
forgot to add it to the spec. This broke the holds routes.

To test:
1. Run:
   $ kshell
  k$ prove t/db_dependent/api/v1/holds.t
=> FAIL: Tests fail, boo.
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2022-02-02 12:23:30 UTC
I revert what I did because Jonathan is right, this attribute is not strictly required on the API (yet?) so we should delay exposing it until there's a real need for it.
Comment 8 Tomás Cohen Arazi 2022-02-02 12:24:59 UTC
Created attachment 130092 [details] [review]
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t

Unknown column 'patron_expiration_date' in 'field list'
The REST API routes don't need to be aware of the new column patron_expiration_date

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 9 Fridolin Somers 2022-02-02 19:48:03 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 10 Nick Clemens 2022-10-06 20:09:47 UTC
Just for clarification here:
When creating a hold if an expirationdate is passed then it will be set as the patron_expiration_date

When the patron_expiration_date is set, this is copied to the expiration_date

When Koha sets an expiration date (when hold filled) it will use the lesser of the patron_expiration_date or the calculated date

i.e. the expiration_date field in the api will represent the patron_expiration_date or the Koha expiration_date as appropriate

i.e. The field is not so much hidden, as not needed externally - I didn't fully understand and leave this for others as muddled as me