https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #1 from Nick Clemens (kidclamp) <nick@bywatersolutions.com> --- Aspen bug report: https://aspen-discovery.atlassian.net/browse/DIS-537 -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Piia Semenoff <piia.semenoff@ouka.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |piia.semenoff@ouka.fi -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Esther Melander <esther.melander@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |esther.melander@bywatersolu | |tions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #2 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 185568 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=185568&action=edit Bug 39419: Enable Holds API to set expiration_date and patron_expiration_date independently This patch adds patron_expiration_date as a new parameter to the Holds API for the add and edit methods. To test: 1. Apply patch and restart_all 2. Make some requests to add holds like POST /api/v1/holds example body: { "biblio_id": "22", "patron_id": "42", "pickup_library_id": "FFL", "expiration_date": "2099-12-31", "patron_expiration_date": "3099-01-01" } 3. Verify that you can set the expiration_date and patron_expiration_date indepdently 4. Confirm that patron_expiration_date is a new field in the API response 5. Make note of the hold_id in the response for the next step 6. Make some requests to edit the hold like PATCH /api/v1/holds/{hold_id} example body: { "patron_expiration_date": "2999-09-19", "expiration_date": "2031-09-19" } 7. Verify that you can set the expiration_date and patron_expiration_date indepdently 8. Make sure making and modifying holds isn't broken in the staff intterface or opac -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #4 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 186176 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186176&action=edit Bug 39419: (follow-up) Unit tests perl t/db_dependent/Reserves.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #6 from Brendan Lawlor <blawlor@clamsnet.org> --- I added a working test for AddReserve but I had trouble getting the test to work for ModReserve -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #8 from Brendan Lawlor <blawlor@clamsnet.org> --- Added unit tests for AddReserve(), ModReserve() and for API calls to add() and edit() Thanks Nick! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186240|0 |1 is obsolete| | --- Comment #11 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 190442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190442&action=edit Bug 39419: (follow-up) Unit tests perl t/db_dependent/Reserves.t perl t/db_dependent/api/v1/holds.t -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #13 from David Nind <david@davidnind.com> --- Hi Brendan. I've tested, but the tests fail for me for t/db_dependent/Reserves.t [1] Everything with the API worked as expected (I used https://www.usebruno.com/) - I could set expirationdate and patron_exiration_date independently and together. However, I couldn't work out how to get a date in patron_expiration_date using either the staff interface or OPAC (which all seemed to work fine for placing holds). I thought it might have been this system preference, but dates went into expirationdate: DefaultHoldExpirationdate DefaultHoldExpirationdatePeriod DefaultHoldExpirationdateUnitOfTime [1] Failed tests prove t/db_dependent/Reserves.t t/db_dependent/Reserves.t .. 68/71 # Looks like you planned 71 tests but ran 72. t/db_dependent/Reserves.t .. Dubious, test returned 255 (wstat 65280, 0xff00) All 71 subtests passed Test Summary Report ------------------- t/db_dependent/Reserves.t (Wstat: 65280 (exited 255) Tests: 72 Failed: 1) Failed test: 72 Non-zero exit status: 255 Parse errors: Bad plan. You planned 71 tests but ran 72. Files=1, Tests=72, 8 wallclock secs ( 0.03 usr 0.01 sys + 5.71 cusr 2.00 csys = 7.75 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190441|0 |1 is obsolete| | --- Comment #14 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 190443 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190443&action=edit Bug 39419: Enable Holds API to set expiration_date and patron_expiration_date independently This patch adds patron_expiration_date as a new parameter to the Holds API for the add and edit methods. To test: 1. Apply patch and restart_all 2. Make some requests to add holds like POST /api/v1/holds example body: { "biblio_id": "22", "patron_id": "42", "pickup_library_id": "FFL", "expiration_date": "2099-12-31", "patron_expiration_date": "3099-01-01" } 3. Verify that you can set the expiration_date and patron_expiration_date indepdently 4. Confirm that patron_expiration_date is a new field in the API response 5. Make note of the hold_id in the response for the next step 6. Make some requests to edit the hold like PATCH /api/v1/holds/{hold_id} example body: { "patron_expiration_date": "2999-09-19", "expiration_date": "2031-09-19" } 7. Verify that you can set the expiration_date and patron_expiration_date indepdently 8. Make sure making and modifying holds isn't broken in the staff intterface or opac Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #16 from Brendan Lawlor <blawlor@clamsnet.org> --- (In reply to David Nind from comment #13)

However, I couldn't work out how to get a date in patron_expiration_date using either the staff interface or OPAC (which all seemed to work fine for placing holds).

The expiration date is mostly set on holds as an automatic process based on system preferences, upon checking the hold into the hold shelf. When it becomes waiting it gets an expiration date accordingly. The patron_expiration_date is set in Koha when the patron places a hold in the OPAC, clicks Show more options and enters a Hold not needed after date. Or in the staff interface when staff places a hold and enters a Hold expires on date. These actions actually set both expirationdate and patron_expiration_date. As far as I can tell in the Koha OPAC and staff interface you can only set the patron_expirtion_date at the time of placing the hold, but there's no way to update it once the hold is placed. I could be wrong about this though :) The terminology and labels are not consistent so it is rather confusing, but you can confirm what field is getting set using ktd --dbshell with a query like: select biblionumber, expirationdate, patron_expiration_date from reserves where borrowernumber=38; Or even better you can confirm using the API with a call like GET /api/v1/holds?patron_id=38 We're using Aspen for our OPAC and this bug affects us because Aspen is currently setting the expiration_date when it should be setting the patron_expiration_date.

[1] Failed tests I fixed the failing test. Not sure but I think I may have made this mistake when rebasing the first time.

Thanks for testing again! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #18 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks for testing again, David. I could be wrong, but I thought that the expiration date is calculated based on system preferences when the hold is checked into the shelf and the patron expiration date is set when the patron requests the hold with a not wanted after date. That's the intent of those fields at least according to the schema: https://schema.koha-community.org/25_05/tables/reserves.html expirationdate the date the hold expires (calculated value) patron_expiration_date the date the hold expires - usually the date entered by the patron to say they don’t need the hold after a certain date I'll have to take a closer look again -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190443|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #20 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 197360 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197360&action=edit Bug 39419: Enable Holds API to set expiration_date and patron_expiration_date independently This patch adds patron_expiration_date as a new parameter to the Holds API for the add and edit methods. To test: 1. Apply patch and restart_all 2. Make some requests to add holds like POST /api/v1/holds example body: { "biblio_id": "22", "patron_id": "42", "pickup_library_id": "FFL", "expiration_date": "2099-12-31", "patron_expiration_date": "3099-01-01" } 3. Verify that you can set the expiration_date and patron_expiration_date indepdently 4. Confirm that patron_expiration_date is a new field in the API response 5. Make note of the hold_id in the response for the next step 6. Make some requests to edit the hold like PATCH /api/v1/holds/{hold_id} example body: { "patron_expiration_date": "2999-09-19", "expiration_date": "2031-09-19" } 7. Verify that you can set the expiration_date and patron_expiration_date indepdently 8. Make sure making and modifying holds isn't broken in the staff intterface or opac Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #22 from David Nind <david@davidnind.com> --- As per the test plan, the API works to update the database with both the: - expiration_date - patron_expiration_date expiration_date = date shown in holds "Expiration" column in the patron's holds table and holds table for a record, and the "Expires on" column in OPAC holds summary page. However, using the staff interface or OPAC I can only seem to update the expiration_date field (that is, I could not get any value to appear in the patron_expiration_date column in the database using the staff interface or OPAC): 1. Staff interface: holds table for patron, shows in the Expiration column 2. Staff interface: placing a hold for the record: . When placing the hold, in the "Hold details section": "Hold expires on date" field . Once hold is place, can edit the date in the Expiration column (this updates the expiration_date column in the database) 3. OPAC: when placing a hold, expand "Show more options", enter a date in "Hold not needed after" field (this updates the expiration_date column in the database, and shows in OPAC summary under holds in the "Expires on" column) 4. If I set DefaultHoldExpirationdate system preference, that date is set for the expiration_date column in the database (if I choose a different date in the OPAC or staff interface, that date is used for the expiration_date column) Also, the tests no longer pass for me, which is a bit strange - both using qa and prove commands. Note: is a yarn api:bundle or yarn build required after apply the patch? I did both. I think I'm probably misunderstanding something, or don't have the right settings, because nothing I do in the staff interface or OPAC updates the patron_expiration_date column in the database (as per discussions in comments 13, 16, 17, and 18). Maybe the statement in the bug description is wrong? (Not that I'd ever say that Nick is wrong about something 8-))

Koha has two fields for hold expiration: expirationdate - (usually) auto generated date set based no when holds on the shelf expire patron_exiration_date - the day the patron says they don't need the hold after

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #24 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks for your patience and testing David and Andrew! Thanks for noting yarn api:bundle needs to be run for testing. For the tests failing, I think sometimes that can be caused by weird data from tests that failed to run completely, and I think you may need to do a reset_all. I'm still learning how to write a test like this, so it's also possibly something I did wrong This issue is a little confusing to unravel, but your comments were very helpful. I realize now that in addition to changes to the API, there needs to be some changes to the requests that create and modify the holds. Currently the patron_expiration_date can be set 3 different ways: When placing a hold: a) in the staff interface with setting a 'Hold expires on date' b) in the opac with setting a 'Hold not needed after' Or from the bib record's holds tab: c) by editing the date in the Expiration column The current behavior is that this sets both expirationdate and patron_expiration_date a) can be fixed in placerequest.pl b) can be fixed in opac-reserve.pl I'm not sure if this is the 'proper' way to do it, but since both fields should be set the same as expirationdate, it could be done by adding patron_expiration_date => $expirationdate, to the calls to AddReserve() c) is a little different because it is part of the datatable and ajax calls. I have to look into how to resolve that one. Thanks again! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #25 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 197442 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197442&action=edit Bug 39419: (folow-up) Set both expiration dates when placing holds in opac and staff interface This patch sets both the expirationdate and patron_expiration date when placing holds in the opac and staff interface as is consistent with current behavior. Test plan: 1. Place some holds: a) in the staff interface with setting a 'Hold expires on date' b) in the opac with setting a 'Hold not needed after' 2. Run this query in ktd --dbshell select borrowernumber, expirationdate, patron_expiration_date from reserves; 3. Note both expirationdate and patron_expiration_date are set in the holds Note: Editing the date in the Expiration column on the records' Holds tab only updates the expirationdate, which is consistent with current behavior. Sponsored-by: CLAMS -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197360|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197442|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #27 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 197464 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197464&action=edit Bug 39419: (follow-up) Unit tests perl t/db_dependent/Reserves.t perl t/db_dependent/api/v1/holds.t Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #29 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Silly bug I noticed while testing this: Bug 42392 - patron_expiration_date on holds is always enforced but never seen or updated -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197463|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #197465|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #31 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 198737 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198737&action=edit Bug 39419: (follow-up) Unit tests perl t/db_dependent/Reserves.t perl t/db_dependent/api/v1/holds.t Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Status|Passed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198736|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #198738|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #35 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 199263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199263&action=edit Bug 39419: (follow-up) Unit tests perl t/db_dependent/Reserves.t perl t/db_dependent/api/v1/holds.t Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #37 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- [FAIL] reserve/placerequest.pl FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl reserve/placerequest.pl` -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA -- You are receiving this mail because: You are watching all bug changes.