[Bug 39419] New: Holds API treats 'expiration_date' as 'patron_expiration_date'
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Bug ID: 39419 Summary: Holds API treats 'expiration_date' as 'patron_expiration_date' Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com This is going to have a related issue in Aspen, but I think the confusion starts here 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 The API takes "expiration_date" and maps it to "patron_expiration_date" This can be unclear for outside systems. Aspen has a 'Default Not Needed After Days' setting which is assumed to be setting the automatic expiraton date, but in fact sets the patron date We should allow either field to be set via the API -- 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 #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 Christine Lee <chlee@pascolibraries.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chlee@pascolibraries.org -- You are receiving this mail because: You are watching all bug changes.
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 Michelle Spinney <mspinney@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mspinney@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 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 Rebecca Coert <rcoert@arlingtonva.us> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |rcoert@arlingtonva.us -- 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 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Patch complexity|--- |Small patch -- 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 #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 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |blawlor@clamsnet.org |ity.org | --- Comment #3 from Brendan Lawlor <blawlor@clamsnet.org> --- Maybe the changes to Reserves.pm and Hold.pm need tests? There is a bit of extra logic that goes on with expirationdate and patron_expiration_date that is explained on bug 29975. I'm not clear on exactly how the API should work with relation to that though -- 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 #5 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 186177 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186177&action=edit Bug 39419: (follow-up) Unit tests -- 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 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #186176|0 |1 is obsolete| | Attachment #186177|0 |1 is obsolete| | --- Comment #7 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 186240 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=186240&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 --- 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 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com Status|Needs Signoff |Patch doesn't apply --- Comment #9 from David Nind <david@davidnind.com> --- The patch no longer applies 8-(... git bz apply 39419 Bug 39419 - Holds API treats 'expiration_date' as 'patron_expiration_date' 185568 - Bug 39419: Enable Holds API to set expiration_date and patron_expiration_date independently 186240 - Bug 39419: (follow-up) Unit tests Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 39419: Enable Holds API to set expiration_date and patron_expiration_date independently Using index info to reconstruct a base tree... M C4/Reserves.pm M Koha/Hold.pm M Koha/REST/V1/Holds.pm M api/v1/swagger/definitions/hold.yaml M api/v1/swagger/paths/holds.yaml Falling back to patching base and 3-way merge... Auto-merging api/v1/swagger/paths/holds.yaml Auto-merging api/v1/swagger/definitions/hold.yaml Auto-merging Koha/REST/V1/Holds.pm CONFLICT (content): Merge conflict in Koha/REST/V1/Holds.pm Auto-merging Koha/Hold.pm Auto-merging C4/Reserves.pm error: Failed to merge in the changes. Patch failed at 0001 Bug -- 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 #185568|0 |1 is obsolete| | --- Comment #10 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 190441 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190441&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 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 --- Comment #12 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks for testing David! One other note for testing you may get an API error 400 "Properties not allowed: patron_expiration_date." It seems you need both restart_all && yarn build for the API to be updated. -- 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 ---------------------------------------------------------------------------- Status|Failed QA |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 #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 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190442|0 |1 is obsolete| | --- Comment #15 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 190444 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190444&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 --- 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 #17 from David Nind <david@davidnind.com> --- Thanks Brendan. The tests all now pass. I still couldn't work out how to add a date to the patron_expiration_date field, using either the staff interface or OPAC (the API worked fine). When placing a hold using either the staff interface or OPAC, I selected a date: - staff interface: "Holds expires on date" field - OPAC: show more options, "Hold not needed after" field In both cases, it only updated the expirationdate field in the database. Will leave for someone else more knowledgeable to test. David -- 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 ---------------------------------------------------------------------------- Sponsorship status|--- |Sponsored Comma delimited| |CLAMS list of Sponsors| | --- Comment #19 from Brendan Lawlor <blawlor@clamsnet.org> --- rebased for current main -- 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 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #190444|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 #21 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 197361 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197361&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 --- 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 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com Status|Needs Signoff |Failed QA --- Comment #23 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- As David has pointed out, with these patches the Koha staff interface and OPAC no longer put values into patron_expiration_date. Without these patches, patron_expiration_date and expirationdate both get populated when either staff or patron enter an expiration date when placing a hold via either the staff interface or OPAC. With these patches, the same actions only put a value in expirationdate. -- 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 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |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 --- 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 ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- 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 #197361|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 #26 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 197463 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197463&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 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 #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 #28 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 197465 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=197465&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 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 ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |jonathan.druart@gmail.com |y.org | -- 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 #197464|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 #30 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 198736 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198736&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 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 #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 --- Comment #32 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 198738 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198738&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 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 ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA --- Comment #33 from Brendan Lawlor <blawlor@clamsnet.org> --- rebased for bug 37762 -- 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 #198737|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 #34 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 199262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199262&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 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 #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 #36 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 199264 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199264&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 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 --- Comment #38 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- * Proving /kohadevbox/koha/t/db_dependent/Reserves.t FAIL # Looks like you planned 71 tests but ran 72. /kohadevbox/koha/t/db_dependent/Reserves.t .. Dubious, test returned 255 (wstat 65280, 0xff00) All 71 subtests passed Test Summary Report ------------------- /kohadevbox/koha/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, 17 wallclock secs ( 0.04 usr 0.01 sys + 11.05 cusr 3.67 csys = 14.77 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 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.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #39 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 203811 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=203811&action=edit Bug 39419: (follow-up) Fix test plan and tidy -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|jonathan.druart@gmail.com |jacob.omara@openfifth.co.uk -- 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 ---------------------------------------------------------------------------- QA Contact|jacob.omara@openfifth.co.uk |jonathan.druart@gmail.com Status|Signed Off |Patch doesn't apply CC| |jonathan.druart@gmail.com -- 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 ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #40 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 206145 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206145&action=edit Bug 39419: QA follow-up - tidy and number of tests 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 #41 from Jonathan Druart <jonathan.druart@gmail.com> --- Sorry I totally forgot about this one. When RM fails QA you are allowed to send back to the RM queue, especially when they are trivial patches. I had to rewrite it, I got "sha1 information is lacking or useless" -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply CC| |pedro.amorim@openfifth.co.u | |k -- 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 ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- 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 #199262|0 |1 is obsolete| | Attachment #199263|0 |1 is obsolete| | Attachment #199264|0 |1 is obsolete| | Attachment #203811|0 |1 is obsolete| | Attachment #206145|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 #42 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 206471 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206471&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 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 #43 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 206472 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206472&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 #44 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 206473 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206473&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 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 #45 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 206474 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206474&action=edit Bug 39419: QA follow-up - tidy and number of tests 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 #46 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks Joubu! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #47 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Something is wrong here: 1) In KTD, run: curl -s -u koha:koha -X POST http://localhost:8081/api/v1/holds \ -H "Content-Type: application/json" \ -d '{"biblio_id": 1, "patron_id": 20, "pickup_library_id": "CPL", "expiration_date": "2099-01-01", "patron_expiration_date": "2099-01-01"}' 2) Grab the hold_id from the response above, then run (use the hold_id): curl -s -u koha:koha -X PATCH http://localhost:8081/api/v1/holds/<hold_id> \ -H "Content-Type: application/json" \ -d '{"expiration_date": null, "patron_expiration_date": null}' 3) Check the PATCH response body. expiration_date is correctly null, but patron_expiration_date still shows 2099-01-01, unchanged. The added unit tests only cover setting patron_expiration_date to non-null values, never the explicit-null/clearing case, so this went uncaught. Updating a hold's patron_expiration_date to null through the REST API isn't possible, even though the swagger spec says it should be. -- 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|Failed QA |Signed Off -- 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 #206471|0 |1 is obsolete| | Attachment #206472|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 #48 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 206814 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206814&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 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 #49 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 206815 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206815&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 #50 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 206816 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206816&action=edit Bug 39419: (follow-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 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 #51 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 206817 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206817&action=edit Bug 39419: (QA follow-up) Tidy and number of tests 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 #52 from Brendan Lawlor <blawlor@clamsnet.org> --- Created attachment 206818 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206818&action=edit Bug 39419: (QA follow-up) Allow clearing patron_expiration_date via PATCH The Holds API edit method used '//' to fall back to the existing patron_expiration_date, so an explicit null in the request body was ignored and the date could not be cleared. This patch uses 'exists', consistent with expiration_date. It also fixes a stray comma and semi colons in the AddReserve POD and removes an unused patron_expiration_date variable in ModReserve. Test plan: 1. In KTD, create a hold: curl -s -u koha:koha -X POST http://localhost:8081/api/v1/holds \ -H "Content-Type: application/json" \ -d '{"biblio_id": 1, "patron_id": 20, "pickup_library_id": "CPL", "expiration_date": "2099-01-01", "patron_expiration_date": "2099-01-01"}' 2. Using the hold_id from the response, clear both dates: curl -s -u koha:koha -X PATCH http://localhost:8081/api/v1/holds/<hold_id> \ -H "Content-Type: application/json" \ -d '{"expiration_date": null, "patron_expiration_date": null}' 3. Note patron_expiration_date is still 2099-01-01 4. Apply patch and restart_all 5. Repeat steps 1 and 2 6. Note both expiration_date and patron_expiration_date are now null 7. Run: prove t/db_dependent/api/v1/holds.t t/db_dependent/Reserves.t Sponsored-by: CLAMS Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Brendan Lawlor <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 --- Comment #53 from Brendan Lawlor <blawlor@clamsnet.org> --- Thanks for catching that Pedro. The issue was in Koha/REST/V1/Holds.pm in edit(). Before it used $body->{patron_expiration_date} // $hold->patron_expiration_date, so a null in the PATCH body fell back to the stored date. Now it uses exists $body->{patron_expiration_date} ? $body->{patron_expiration_date} : $hold->patron_expiration_date; so null in the PATCH body clears the date, which is consistent with how expiration_date, hold_date and item_id work. The tests in t/db_dependent/api/v1/holds.t in edit() are for both bib and item holds. They cover a PATCH that only sends expiration_date checking that patron_expiration_date does not change and a Patch that sends null for both dates, checking that the dates are cleared. A test in t/db_dependent/Reserves.t in ModReserve() was added to check calling ModReserve with patron_expiration_date => undef I also made a couple fixes to a couple commit messages that had the wrong format for (follow-up) and (QA-follow up) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 --- Comment #54 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi Brendan, this should go back in the queue (PQA) correct? -- 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|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- 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 #206473|0 |1 is obsolete| | --- Comment #55 from Brendan Lawlor <blawlor@clamsnet.org> --- Comment on attachment 206473 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206473 Bug 39419: (folow-up) Set both expiration dates when placing holds in opac and staff interface
From 1b31bd70ebd10f6da9f06c040da2d219ff63c625 Mon Sep 17 00:00:00 2001 From: Brendan Lawlor <blawlor@clamsnet.org> Date: Wed, 15 Apr 2026 17:35:15 +0000 Subject: [PATCH] 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 Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- opac/opac-reserve.pl | 25 ++++++------- reserve/placerequest.pl | 77 +++++++++++++++++++++-------------------- 2 files changed, 53 insertions(+), 49 deletions(-)
diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl index a1f7d26f5bd..507485fdaf9 100755 --- a/opac/opac-reserve.pl +++ b/opac/opac-reserve.pl @@ -295,18 +295,19 @@ if ( $op eq 'cud-place_reserve' ) { if ($canreserve) { my $reserve_id = AddReserve( { - branchcode => $branch, - borrowernumber => $borrowernumber, - biblionumber => $biblioNum, - priority => $rank, - reservation_date => $startdate, - expiration_date => $patron_expiration_date, - notes => $notes, - title => $biblio->title, - itemnumber => $itemNum, - found => undef, - itemtype => $itemtype, - item_group_id => $item_group_id, + branchcode => $branch, + borrowernumber => $borrowernumber, + biblionumber => $biblioNum, + priority => $rank, + reservation_date => $startdate, + expiration_date => $patron_expiration_date, + patron_expiration_date => $patron_expiration_date, + notes => $notes, + title => $biblio->title, + itemnumber => $itemNum, + found => undef, + itemtype => $itemtype, + item_group_id => $item_group_id, } ); if ($reserve_id) { diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl index 166775dc97b..63328eda3bf 100755 --- a/reserve/placerequest.pl +++ b/reserve/placerequest.pl @@ -106,18 +106,19 @@ if ( $op eq 'cud-placerequest' && $patron ) { { my $reserve_id = AddReserve( { - branchcode => $item_pickup_location, - borrowernumber => $patron->borrowernumber, - biblionumber => $biblionumber, - priority => $hold_priority, - reservation_date => $startdate, - expiration_date => $expirationdate, - notes => $notes, - title => $title, - itemnumber => $checkitem, - found => undef, - itemtype => $itemtype, - non_priority => $non_priority, + branchcode => $item_pickup_location, + borrowernumber => $patron->borrowernumber, + biblionumber => $biblionumber, + priority => $hold_priority, + reservation_date => $startdate, + expiration_date => $expirationdate, + patron_expiration_date => $expirationdate, + notes => $notes, + title => $title, + itemnumber => $checkitem, + found => undef, + itemtype => $itemtype, + non_priority => $non_priority, supplyill => $supplyill, iso18626_payload => \%iso18626_payload, } @@ -138,18 +139,19 @@ if ( $op eq 'cud-placerequest' && $patron ) { if ( $can_override || CanBookBeReserved( $patron->borrowernumber, $biblionumber )->{status} eq 'OK' ) { my $reserve_id = AddReserve( { - branchcode => $bibinfo->{pickup}, - borrowernumber => $patron->borrowernumber, - biblionumber => $biblionumber, - priority => $bibinfo->{rank}, - reservation_date => $startdate, - expiration_date => $expirationdate, - notes => $notes, - title => $bibinfo->{title}, - itemnumber => undef, - found => undef, - itemtype => $itemtype, - non_priority => $non_priority, + branchcode => $bibinfo->{pickup}, + borrowernumber => $patron->borrowernumber, + biblionumber => $biblionumber, + priority => $bibinfo->{rank}, + reservation_date => $startdate, + expiration_date => $expirationdate, + patron_expiration_date => $expirationdate, + notes => $notes, + title => $bibinfo->{title}, + itemnumber => undef, + found => undef, + itemtype => $itemtype, + non_priority => $non_priority,
# supplyill => $supplyill, FIXME: How does supplyill work with multi-hold? # iso18626_payload => \%iso18626_payload, @@ -165,19 +167,20 @@ if ( $op eq 'cud-placerequest' && $patron ) { if ( $can_override || CanBookBeReserved( $patron->borrowernumber, $biblionumber )->{status} eq 'OK' ) { my $reserve_id = AddReserve( { - branchcode => $branch, - borrowernumber => $patron->borrowernumber, - biblionumber => $biblionumber, - priority => $rank[0], - reservation_date => $startdate, - expiration_date => $expirationdate, - notes => $notes, - title => $title, - itemnumber => undef, - found => undef, - itemtype => $itemtype, - non_priority => $non_priority, - item_group_id => $item_group_id, + branchcode => $branch, + borrowernumber => $patron->borrowernumber, + biblionumber => $biblionumber, + priority => $rank[0], + reservation_date => $startdate, + expiration_date => $expirationdate, + patron_expiration_date => $expirationdate, + notes => $notes, + title => $title, + itemnumber => undef, + found => undef, + itemtype => $itemtype, + non_priority => $non_priority, + item_group_id => $item_group_id, supplyill => $supplyill, iso18626_payload => \%iso18626_payload, } -- 2.43.0
-- 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 #206474|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 ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA --- Comment #56 from Brendan Lawlor <blawlor@clamsnet.org> --- The patch didn't apply because I needed to obsolete the two old patches where I fixed the commit message format. Sorry for messing that up :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39419 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Failed QA --- Comment #57 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Hi Brendan, it appears there is a regression here, it's causing the following tests file to fail: prove t/db_dependent/Koha/Holds.t -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org