| Summary: | Database update for bug 12063 incorrectly calculates expirationdate for holds | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Hold requests | Assignee: | Nick Clemens (kidclamp) <nick> |
| Status: | CLOSED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | gmcharlt, m.de.rooy |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Crowdfunding goal: | 0 |
| Patch complexity: | Trivial patch | Documentation contact: | |
| Documentation submission: | Text to go in the release notes: | ||
| Version(s) released in: | Circulation function: | ||
| Bug Depends on: | 12063 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 19626 - Calculate expiration date for existing holds based on waitingdate
Bug 19626 - Calculate expiration date for existing holds based on waitingdate Bug 19626 - Calculate expiration date for existing holds based on waitingdate Bug 19626: (QA follow-up) Small optimization |
||
Created attachment 69152 [details] [review] Bug 19626 - Calculate expiration date for existing holds based on waitingdate To test: 1 - Have/create a 16.11 instance with some waiting holds 2 - Those holds should not have an expirationdate 3 - Make sure some of the holds are waiting longer than ReservesMaxPickupDelay BACKUP THE DB 4 - Upgrade to 17.05 (or later) 5 - Check the expirationdate for the holds 6 - The date wil be in the future (curdate + delay) 7 - Restore DB 8 - Apply patch 9 - Run the upgrade again 10 - expirationdate should now be based on waitingdate You dont catch:
if ( C4::Context->preference("ExcludeHolidaysFromMaxPickUpDelay") ) {
$expirationdate = $calendar->days_forward( dt_from_string(), $max_pickup_delay );
}
Created attachment 69183 [details] [review] Bug 19626 - Calculate expiration date for existing holds based on waitingdate To test: 1 - Have/create a 16.11 instance with some waiting holds 2 - Those holds should not have an expirationdate 3 - Make sure some of the holds are waiting longer than ReservesMaxPickupDelay BACKUP THE DB 4 - Upgrade to 17.05 (or later) 5 - Check the expirationdate for the holds 6 - The date wil be in the future (curdate + delay) 7 - Restore DB 8 - Apply patch 9 - Run the upgrade again 10 - expirationdate should now be based on waitingdate Created attachment 69188 [details] [review] Bug 19626 - Calculate expiration date for existing holds based on waitingdate To test: 1 - Have/create a 16.11 instance with some waiting holds 2 - Those holds should not have an expirationdate 3 - Make sure some of the holds are waiting longer than ReservesMaxPickupDelay BACKUP THE DB 4 - Upgrade to 17.05 (or later) 5 - Check the expirationdate for the holds 6 - The date wil be in the future (curdate + delay) 7 - Restore DB 8 - Apply patch 9 - Run the upgrade again 10 - expirationdate should now be based on waitingdate Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 69189 [details] [review] Bug 19626: (QA follow-up) Small optimization We are only using $calendar when the pref is set. Date calculation can be moved in if-else structure. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Not sure if this really is a major btw. Combining SO and QA in one step. Pushed to master for 17.11, thanks to everybody involved! |
Logic in short seems to be: 1 - Does hold already have an expirationdate? a- Keep it if before calculated expirationdate 2 - If not 1 then calculate expiration as curdate + delay (+ holidays if pref) The problem is, before 12063 waiting holds didn't have an expirationdate, so a hold that has been waiting for 2 months will now get a waiting date after the upgrade. I think line 14374 should be: my $expirationdate = dt_from_string($hold->waitingdate);