Bug 29975 - (Bug 21729 followup) patron_expiration_date missing in API
Summary: (Bug 21729 followup) patron_expiration_date missing in API
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 21729
Blocks:
  Show dependency treegraph
 
Reported: 2022-01-29 00:19 UTC by Fridolin Somers
Modified: 2022-10-06 20:09 UTC (History)
3 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:
22.05.00


Attachments
Bug 29975: [DRAFT] fix API (1.95 KB, patch)
2022-01-29 00:21 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t (921 bytes, patch)
2022-01-31 10:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t (982 bytes, patch)
2022-02-02 06:41 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 29975: Add missing patron_expiration_date attribute to hold spec (1.25 KB, patch)
2022-02-02 12:14 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29975: (bug 21729 follow-up) Fix api/v1/holds.t (1.01 KB, patch)
2022-02-02 12:24 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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