The Talking Tech outbound script adds the ReservesMaxPickUpDelay to the waitingdate of a reserve to calculate its last pickup day. However, this day may fall on a holiday. In this case, the last day to pick the item up should be pushed to the next open day.
Created attachment 13176 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date.
I perltidy'ed the file which has obscured my changes. The only non-formatting change is at lines 296-298. It just checks to see if the pickup date is a holiday, and if it is loops day by day until if finds the next open day for that library.
(In reply to comment #2) > I perltidy'ed the file which has obscured my changes. The only > non-formatting change is at lines 296-298. It just checks to see if the > pickup date is a holiday, and if it is loops day by day until if finds the > next open day for that library. Could you please break this into two patches? Either order is fine.
Created attachment 13212 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date - Perltidy script.
Created attachment 13213 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date - Add holidays check
> Could you please break this into two patches? Either order is fine. No problem! Again, the main change is the addition of the loop starting at line 295. I also switched from using arrays for the date parts to named variables, in the interest of self-documenting code.
How is this going to get tested?
I'm adding dependencies on two other bugs concerned with adjusting the max pickup date for holidays. The bugs are interlinked in a somewhat tangled way. As it would be awkward if the iTiva export, the hold expiration cronjob, and the hold display were to have different notions of exactly when a waiting hold has sat on the shelf too long, any final patches for all three bugs will almost certainly be pushed as a unit.
This bug depends on 8367 which is in status Patch doesn't apply
Setting status to BLOCKED since this patch depends on two patches that do not apply resp. failed qa.
(In reply to Kyle M Hall from comment #5) > Created attachment 13213 [details] [review] [review] > Bug 9004 - Talking Tech doesn't account for holidays when calculating a > holds last pickup date - Add holidays check Code looks good to me but I'm unable to test this.
Test plan: 0 - Set ReservesMaxPickUpDelay to some value greater than 0 1 - Place a hold for a patron 2 - Check in and confirm the hold to set status to waiting 3 - On patron screen there should be a message "Waiting at X until d/a/te" 4 - Run the itiva outbound cronjob script: perl /path/to/koha/misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl -o ~/itiva.tmp -w 0 --type=RESERVE 5 - Check the output file, the max waiting date seen above should be there 6 - Add a holiday (or several) on the max waiting date 7 - Run the script again 8 - Date should eb unchanged in file 9 - Apply patch 10 - Run the script again 11 - Date should now be moved past the holidays set in step 6
Created attachment 50042 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date - Perltidy script. Signed-off-by: Chris William <chrisw@pascolibraries.org>
Created attachment 50043 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date - Add holidays check Signed-off-by: Chris William <chrisw@pascolibraries.org>
FAIL misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl FAIL pod in file misc/cronjobs/thirdparty/TalkingTech_itiva_outbound.pl *** ERROR: =item without previous =over I cannot test this patch, but I think we can trust ByWater for the signoff. It would have been better to use DateTime and Koha::Calendar to do the date calculation.
Created attachment 50082 [details] [review] Bug 9004: (QA followup) Fix pod
No iTiva account is required for testing, this simply alters the data in the file generated by the script. Note:the output to screen option for this script doesn't appear to function, results must be saved to a file to view.
Created attachment 50085 [details] [review] Bug 9004: Use Koha::Calendar instead of C4::Calendar This patch tries to make the code more readable using Koha::Calendar instead of deprecated C4::Calendar and Date::Calc
Hi Nick, Any chances to get a signoff on this one? I have never used this script before so I may have missed something!
Created attachment 50086 [details] [review] Bug 9004: Use Koha::Calendar instead of C4::Calendar This patch tries to make the code more readable using Koha::Calendar instead of deprecated C4::Calendar and Date::Calc Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 50111 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date - Perltidy script. Signed-off-by: Chris William <chrisw@pascolibraries.org>
Created attachment 50112 [details] [review] Bug 9004 - Talking Tech doesn't account for holidays when calculating a holds last pickup date - Add holidays check Signed-off-by: Chris William <chrisw@pascolibraries.org>
Created attachment 50113 [details] [review] Bug 9004: (QA followup) Fix pod
Created attachment 50114 [details] [review] Bug 9004: Use Koha::Calendar instead of C4::Calendar This patch tries to make the code more readable using Koha::Calendar instead of deprecated C4::Calendar and Date::Calc
Pushed to master for the May 2016 release!