http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 claire.hernandez@biblibre.com <claire.hernandez@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |claire.hernandez@biblibre.c | |om -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff CC| |jonathan.druart@biblibre.co | |m Assignee|koha-bugs@lists.koha-commun |jonathan.druart@biblibre.co |ity.org |m -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12130|0 |1 is obsolete| | --- Comment #3 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 13139 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13139&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new field issuingrules.holdspickupdelay You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |kyle@bywatersolutions.com --- Comment #5 from Kyle M Hall <kyle@bywatersolutions.com> --- This patch looks good in all cases except for waitingreserves.pl. Even though my hold is past its pickup window, it is showing in the waiting holds tab, and not in the holds over tab is it should be. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #7 from Kyle M Hall <kyle@bywatersolutions.com> --- On circulation.pl it shows: Item 1 waiting at Centerville until 11/20/2012 On waitingreserves.pl it shows: 11/19/2012 - 11/26/2012 so it appears to be using the new issue rules setting for circulation.pl, but it's still using the system preference for waitingreserves.pl I have my rule set to 1 day, and my system pref set to 7. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13602|0 |1 is obsolete| | --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 13604 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=13604&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay - Followup -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #12 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to comment #10)

Created attachment 13604 [details] Bug 8367: Add more granular level for ReservesMaxPickUpDelay - Followup

Good catch Kyle, Thanks! It would be worth having an other SO for these patches -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #14 from Paul Poulain <paul.poulain@biblibre.com> --- (In reply to comment #13)

It's been a month without another signer. I think it's time to move these patches to QA.

agreed (and agreed also because now QAers are supposed to test the patch) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #16 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 15334 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15334&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new field issuingrules.holdspickupdelay You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Rebased on Feb 15, 2013 (Marcel de Rooy): Merge conflict in opac/opac-user.pl Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Merge conflict in installer/data/mysql/updatedatabase.pl Merge conflict in circ/waitingreserves.pl -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #13603|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #18 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- QA Comment: I like the feature! But IMO it needs some more attention before pushing it: Issue while resolving merge conflict in circ/waitingreserves.pl: The dateformat template var has been removed in master (was in the patch) but is still used in one line of template (waitingreserves.pl). Please check. IMO it is confusing that the old pref exists besides the new column in issuing rules. I would suggest simplification by obsoleting pref ReservesMaxPickUpDelay. At this time behavior is not so clear anymore. Note that there is no comment either on the preferences form that this syspref could be overruled by issuing rules. And the other way around: If no rule is found, the pref still holds. Does the user still understand that from the rules form? GetMaxPickupDate: It may be worth to consider if it would be easier to add a maxpickupdate to the reserves table instead of recalculating it "everywhere". GetMaxPickupDelay: Only called by GetMaxPickupDate. A serious issue IMO is the FIXME in CancelExpiredReserves. Instead of calling GetMaxPickupDelay, you just use the pref value! I understand that from performance point of view, but this is "killing" the feature. When using the cron job to remove holds, the new pickup delay should be used not the old one. Note that you could easily resolve this by looking at a maxpickupdate when adding this to the reserve table and calculating it at confirm hold time. Minor representation issue: In the opac user form I see the max pickup date. Would it be consistent to show it on staff catalogue detail too (Holds tab)? Setting to Failed QA, mainly for the cancelling holds issue. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #20 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to comment #18)

QA Comment: Note that you could easily resolve this by looking at a maxpickupdate when adding this to the reserve table and calculating it at confirm hold time.

Hi Marcel, Thanks for QAing this patch! It is a great idea even though it was not so easy :) A patch is coming. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #21 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 15369 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15369&action=edit Bug 8367: Followup Add more granular level for ReservesMaxPickUpDelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. 1 new column: reserves.maxpickupdate (and for old_reserves). It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. To test: Play with the reserve workflow (Add, remove, change priority, etc.) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #23 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #20)

It is a great idea even though it was not so easy :) Yes, I see. In circulation nothing seems to be easy.. Will try to have a look at your patches again later this week.

Just a question from quickly glancing through your code. In CancelExpiredReserves: + my $query = "SELECT * FROM reserves WHERE TO_DAYS( NOW() ) - TO_DAYS( waitingdate ) > maxpickupdate AND found = 'W' AND priority = 0"; Does that work? Looks to me that it doesn't. Shouldn't you just look for maxpickupdate < now? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #25 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, there is a downside to the new column too - we loose a bit of flexibility. Say a library closes for some time and changes the pickup delay to be longer, so the holds don't go stale - with the patch in effect they would have to do a database update to those holds. It's only one use case I have come up with so far. There might be other disadvantages/advantages of both ways of doing it. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #27 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 15498 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15498&action=edit Bug 8367: Followup Add more granular level for ReservesMaxPickUpDelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. 1 new column: reserves.maxpickupdate (and for old_reserves). It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. To test: Play with the reserve workflow (Add, remove, change priority, etc.) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #28 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to comment #25)

Hm, there is a downside to the new column too - we loose a bit of flexibility. Say a library closes for some time and changes the pickup delay to be longer, so the holds don't go stale - with the patch in effect they would have to do a database update to those holds. It's only one use case I have come up with so far. There might be other disadvantages/advantages of both ways of doing it.

What is Koha doing if I adjust the loan period? It does not change the due dates in the system. The due date was set with the active guidelines of that moment. You could say: Doing the same for holds is consistent :) If the library closes, they should stop confirming holds in due time, limiting the number of waiting holds. Pending holds are no problem. The pickup date should be calculated at confirm hold time. They could also stop the cron job for cancelling holds until they processed the holds queue after reopening? Would they really need a db update? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=8735 --- Comment #30 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Ah sorry, I missed half the comments: @Jonathan: No, I don't think it happens often that the value changes. I just have been thinking about the consequences of not recalculating. I didn't think about the calendar - that would be a solution (see bug 8735) Maybe making it possible to change the date from the interface (out of scope here) would be nice too at some point. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #32 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Preliminary comments: Report Holds awaiting pickup: A DateTime object can only be compared to another DateTime object (DateTime=HASH(0x53d7020), 2013-02-22). at /usr/share/koha/testclone/circ/waitingreserves.pl line 110 Another serious issue: During upgrade all reserves need a maxpickupdate. Now they are null. Before I would signoff and pass qa, it would be very useful to have another signoff on this one. After testing your changes thorougly, another external signoff (maybe from Kyle?) is welcome too. The nature of these patches asks for more extensive testing. Please note that a more detailed test plan could help. For now setting status to Failed QA. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #33 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- Please have a look too at bug 5786. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15613|0 |1 is obsolete| | --- Comment #36 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 15658 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15658&action=edit Bug 8367: Followup Add more granular level for ReservesMaxPickUpDelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. 1 new column: reserves.maxpickupdate (and for old_reserves). It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. To test: 1/ Play with the reserve workflow (Add, remove, change priority, etc.) and check there are no regression. 2/ Update the new column 'Holds pickup delay' in your issuing rules. According to a library and an item type, the maxpickupdate value will be equal to the waiting date + the pickup delay defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #37 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 15659 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=15659&action=edit Bug 8367: Skip to next open day if max pickup date is a holiday -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15335|Bug 8367: Add more granular |[SIGNED-OFF] Bug 8367: Add description|level for |more granular level for |ReservesMaxPickUpDelay - |ReservesMaxPickUpDelay - |Followup |Followup -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #39 from Owen Leonard <oleonard@myacpl.org> --- The last patch needs a commit message with a test plan. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Patch doesn't apply --- Comment #41 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to comment #40)

(In reply to comment #39)

...

The last patch needs a commit message with a test plan.

Kyle, since this patch is from you, could you provide a test plan please?

Sure! 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Edit the calendar for the given location, and make the pickup date a holiday 4) View waitingreserves.pl, the last pickup date should now be the increased by one day I would add this to the patch, but at the moment the first patch isn't applying. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #15334|0 |1 is obsolete| | Attachment #15335|0 |1 is obsolete| | Attachment #15658|0 |1 is obsolete| | Attachment #15659|0 |1 is obsolete| | --- Comment #42 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19093 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19093&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new field issuingrules.holdspickupdelay You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Rebased on Feb 15, 2013 (Marcel de Rooy): CONFLICT (content): Merge conflict in opac/opac-user.pl CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in circ/waitingreserves.pl -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #44 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19095 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19095&action=edit Bug 8367: Followup Add more granular level for ReservesMaxPickUpDelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. 1 new column: reserves.maxpickupdate (and for old_reserves). It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. To test: 1/ Play with the reserve workflow (Add, remove, change priority, etc.) and check there are no regression. 2/ Update the new column 'Holds pickup delay' in your issuing rules. According to a library and an item type, the maxpickupdate value will be equal to the waiting date + the pickup delay defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #48 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19105 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19105&action=edit Bug 8367: Skip to next open day if max pickup date is a holiday Test plan: 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Edit the calendar for the given location, and make the pickup date a holiday 4) View waitingreserves.pl, the last pickup date should now be the increased by one day -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19104|0 |1 is obsolete| | Attachment #19105|0 |1 is obsolete| | --- Comment #50 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19107 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19107&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new column reserves.maxpickupdate (+old_reserves) It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupdelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. Update the new column 'Holds pickup delay' in your issuing rules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). According to a library and an item type, the maxpickupdate value will be equal to the waiting date + the pickup delay defined. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10487 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #53 from Kyle M Hall <kyle@bywatersolutions.com> --- When I tried to create my first rule on a fresh database with the first patch only, I got this error: smart-rules.pl: DBD::mysql::st execute failed: called with 18 bind variables when 17 are needed at /home/koha/kohaclone/admin/smart-rules.pl line 137., referer: http://staff.kohadev/cgi-bin/koha/admin/smart-rules.pl -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #54 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19116 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19116&action=edit Bug 8367: Followup FIX a sql statement (bad conflict merge) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19107|0 |1 is obsolete| | --- Comment #55 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19180 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19180&action=edit [SIGNED-OFF] Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new column reserves.maxpickupdate (+old_reserves) It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupdelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. Update the new column 'Holds pickup delay' in your issuing rules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). According to a library and an item type, the maxpickupdate value will be equal to the waiting date + the pickup delay defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19108|0 |1 is obsolete| | --- Comment #56 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19181 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19181&action=edit Bug 8367: Skip to next open day if max pickup date is a holiday Test plan: 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Edit the calendar for the given location, and make the pickup date a holiday 4) View waitingreserves.pl, the last pickup date should now be the increased by one day -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #58 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Kyle M Hall from comment #56)

Created attachment 19181 [details] [review] Bug 8367: Skip to next open day if max pickup date is a holiday

Kyle, I don't understand how this patch can worked. The maxpickupdate is stored in the DB, when adding or updating an hold. The patch works but not with this test plan. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #60 from Jonathan Druart <jonathan.druart@biblibre.com> --- Kyle, I agree with your test plan. I am signing off your patch. But I think I found a bug, could you take a look at this new patch please? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #62 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 19361 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19361&action=edit Bug 8367: Fix the maxpickupdate when we set waitingdate to NOW() ModReserveAffect get the maxpickupdate before changing the waitingdate to NOW. We should update the waitingdate and next calculate the maxpickupdate. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #64 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #63)

Hmm, I think NOW() is a mysqlism. I know we probably have lots of those in the codebase already, but maybe worth trying not to add more and find best practices for replacing them?

Mtj suggested using CURRENT_TIMESTAMP() instead on IRC: http://dev.mysql.com/doc/refman/5.1/en/date-and-time-functions.html

I don't add it, it already exists. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19361|0 |1 is obsolete| | --- Comment #65 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 19362 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=19362&action=edit Bug 8367: Fix the maxpickupdate when we set waitingdate to NOW() ModReserveAffect get the maxpickupdate before changing the waitingdate to NOW. We should update the waitingdate and next calculate the maxpickupdate. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #67 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Katrin Fischer from comment #66)

Ok, maybe we could take a look at the NOW()s another time.

Yes, I would suggest filing a bug report to take care of any NOW()'s in the codebase. It seems likely however, that the upcoming DBIx::Class switchover will take care of those anyway. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19180|0 |1 is obsolete| | Attachment #19182|0 |1 is obsolete| | Attachment #19360|0 |1 is obsolete| | Attachment #19362|0 |1 is obsolete| | --- Comment #69 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20300 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20300&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new column reserves.maxpickupdate (+old_reserves) It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupdelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. Update the new column 'Holds pickup delay' in your issuing rules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). According to a library and an item type, the maxpickupdate value will be equal to the waiting date + the pickup delay defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20300|0 |1 is obsolete| | --- Comment #70 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20301 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20301&action=edit Bug 8367: Add more granular level for ReservesMaxPickUpDelay This patch adds: - a new column in the issuing rules - a new column reserves.maxpickupdate (+old_reserves) It contains the waitingdate + the corresponding max pickup delay. Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupdelay This patch removes the ReservesMaxPickUpDelay syspref. The update database script will update the issuingrules table with the correct value before removing it. You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. To test: Check there is no regression with a normal reserve workflow. Add one or more issuingrules. Update the new column 'Holds pickup delay' in your issuing rules. In 4 templates, you can see the max pickup delay for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). According to a library and an item type, the maxpickupdate value will be equal to the waiting date + the pickup delay defined. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #72 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 20303 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20303&action=edit Bug 8367: Skip to next open day if max pickup date is a holiday Test plan: 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Cancel this hold 5) Edit the calendar for the given location, and make the pickup date a holiday 6) Repeat step 2 7) View waitingreserves.pl, the last pickup date for this new hold should now be the increased by one day Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #74 from Jonathan Druart <jonathan.druart@biblibre.com> --- (In reply to Katrin Fischer from comment #68)

Hi Kyle, there is a conflict in Reserves.pm I am not sure how to fix - could you take a look please? I think it's because your patch for using reserve_id is in now, so it might need a little work there.

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

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Agnes Rivers-Moore <arm@hanover.ca> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |arm@hanover.ca --- Comment #76 from Agnes Rivers-Moore <arm@hanover.ca> --- We would like the option to have a shorter pickup period for some item types. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |olli-antti.kivilahti@jns.fi Assignee|jonathan.druart@biblibre.co |olli-antti.kivilahti@jns.fi |m | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #79 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32786 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32786&action=edit Bug 8367 - How long is a hold waiting for pickup at a more granular level This patch adds: - a new column in the issuing rules, including help topics. - a new column reserves.lastpickupdate (+old_reserves) It contains the waitingdate + the corresponding "holds pickup wait". Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupwait - a new function C4::Reserves::MoveWaitingdate() to help in testing this feature. This patch removes the ReservesMaxPickUpDelay syspref. - Also removes a code section from C4::Letters::_parseLetter() which worked blatantly disrespectfully towards Koha Calendar. - TODO put the <<reserves.lastpickupdate>> as a placeholder in C4::Letters. - TODO figure out how to rename existing letter placeholders <<expirationdate>> with <<lastpickupdate>> The update database script will update the issuingrules table with the correct value before removing it. You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. -TODO Refactoring unit tests is in progress. $$$$$$$$$$$$$ $ TEST PLAN $ $$$$$$$$$$$$$ ****** ****** *** *** *** TEST: BASIC WORKFLOW * ****** ****** 1. Check there is no regression with a normal reserve workflow. 2. Add one or more issuingrules. 3. Update the new column 'Holds pickup wait' in your issuing rules. 4. In 4 templates, you can see the 'Last pickup date' for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). 5. According to a library and an item type, the lastpickupdate value will be equal to the waiting date + the "holds pickup wait" defined. ** ** ** ** ** ** *** *** *** TEST: RESPECTING CALENDAR HOLIDAYS * ** ** ** ** ** ** 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Cancel this hold 5) Edit the calendar for the given location, and make the pickup date a single, unrepeated, holiday. Then make the next day a weekly repeating holiday. Then make the next two days an annual holiday repeatable every year. 6) Repeat step 2 7) View waitingreserves.pl, the last pickup date for this new hold should now be increased by four days. ** ** ** ** ** ** *** *** *** TEST: LETTER lastpickupdate PLACEHOLDER AND REGRESSION * ** ** ** ** ** ** 1. Modify the HOLD-letter, add <<reserves.lastpickupdate>> and <<reserves.waitingdate>> somewhere. 2. Catch a hold for a borrower. 3. See the members/notices.pl for the message queued and verify that the lastpickupdate-placeholder is set in your desired datetime format and that it is not on a holiday. Verify that the waitingdate is today. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #81 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, introducing a new module just for testing seems a bit out of scope :) Have you taken a look at some of the other tests for recently added features? I think there are some for the new automatic renewal and maybe others that could be useful. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32826|0 |1 is obsolete| | --- Comment #83 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32884 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32884&action=edit Bug 8367 - How long is a hold waiting for pickup at a more granular level You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. The lastpickupdate-column is set based on the given instruction and lands only on an open library day. Also cancel_expired_reserves.pl now cancels only on open library days after the last pickup date has expired. And waitingreserves.pl respects the new DB column. This patch adds: - a new column in the issuing rules, including help topics. - a new column reserves.lastpickupdate (+old_reserves) It contains the waitingdate + the corresponding "holds pickup wait". Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupwait - a new function C4::Reserves::MoveWaitingdate() to help in testing this feature. - a new function C4::Reserves::GetLastPickupDate() to calculate the proper date from Calendar. This patch removes: - the ReservesMaxPickUpDelay syspref. - the ExpireReservesOnHolidays syspref, because it became obsolete due to the changes in handling the last pickup date with Koha Calendar. - Also removes a code section from C4::Letters::_parseLetter() which worked blatantly disrespectfully towards Koha Calendar. - TODO figure out how to rename existing letter placeholders <<expirationdate>> with <<lastpickupdate>> TODO Refactoring unit tests is in progress. $$$$$$$$$$$$$ $ TEST PLAN $ $$$$$$$$$$$$$ ****** ****** *** *** *** TEST: BASIC WORKFLOW * ****** ****** 1. Check there is no regression with a normal reserve workflow. 2. Add one or more issuingrules. 3. Update the new column 'Holds pickup wait' in your issuing rules. 4. In 4 templates, you can see the 'Last pickup date' for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). 5. According to a library and an item type, the lastpickupdate value will be equal to the waiting date + the "holds pickup wait" defined moved to the next open library day. ****** ****** *** *** *** TEST: LETTER PLACEHOLDER (no code modifications here, just regression)* ****** ****** 1. Modify the RESERVESLIP->email Letter. Add "Last pickup date<<reserves.lastpickupdate>>" somewhere. 2. Make a hold for a borrower. 3. Catch the hold and print the reserve slip. Verify that the lastpickupdate is actually the (current date + the 'Holds pickup wait' in your issuing rules) in days. ** ** ** ** ** ** *** *** *** TEST: RESPECTING CALENDAR HOLIDAYS * ** ** ** ** ** ** 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Cancel this hold 5) Edit the calendar for the given location, and make the pickup date a single, unrepeated, holiday. Then make the next day a weekly repeating holiday. Then make the next two days an annual holiday repeatable every year. 6) Repeat step 2 7) View waitingreserves.pl, the last pickup date for this new hold should now be increased by four days. ** ** ** ** ** ** *** *** *** TEST: LETTER lastpickupdate PLACEHOLDER AND REGRESSION * ** ** ** ** ** ** 1. Modify the HOLD-letter, add <<reserves.lastpickupdate>> and <<reserves.waitingdate>> somewhere. 2. Catch a hold for a borrower. 3. See the members/notices.pl for the message queued and verify that the lastpickupdate-placeholder is set in your desired datetime format and that it is not on a holiday. Verify that the waitingdate is today. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20301|0 |1 is obsolete| | Attachment #20302|0 |1 is obsolete| | Attachment #20303|0 |1 is obsolete| | Attachment #20304|0 |1 is obsolete| | --- Comment #84 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32885 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32885&action=edit using this to obsolete previous patches -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32884|0 |1 is obsolete| | --- Comment #85 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32916 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32916&action=edit Bug 8367 - How long is a hold waiting for pickup at a more granular level You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. The lastpickupdate-column is set based on the given instruction and lands only on an open library day. Also cancel_expired_reserves.pl now cancels only on open library days after the last pickup date has expired. And waitingreserves.pl respects the new DB column. This patch adds: - a new column in the issuing rules, including help topics. - a new column reserves.lastpickupdate (+old_reserves) It contains the waitingdate + the corresponding "holds pickup wait". Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupwait - a new function C4::Reserves::MoveWaitingdate() to help in testing this feature. - a new function C4::Reserves::GetLastPickupDate() to calculate the proper date from Calendar. This patch removes: - the ReservesMaxPickUpDelay syspref. - the ExpireReservesOnHolidays syspref, because it became obsolete due to the changes in handling the last pickup date with Koha Calendar. - Also removes a code section from C4::Letters::_parseLetter() which worked blatantly disrespectfully towards Koha Calendar. - TODO figure out how to rename existing letter placeholders <<expirationdate>> with <<lastpickupdate>> TODO Refactoring unit tests is in progress. $$$$$$$$$$$$$ $ TEST PLAN $ $$$$$$$$$$$$$ ****** ****** *** *** *** TEST: BASIC WORKFLOW * ****** ****** 1. Check there is no regression with a normal reserve workflow. 2. Add one or more issuingrules. 3. Update the new column 'Holds pickup wait' in your issuing rules. 4. In 4 templates, you can see the 'Last pickup date' for an hold (circ/circulation.tt, circ/waitingreserves.tt, members/moremember.tt, opac-user.tt). 5. According to a library and an item type, the lastpickupdate value will be equal to the waiting date + the "holds pickup wait" defined moved to the next open library day. ****** ****** *** *** *** TEST: LETTER PLACEHOLDER (no code modifications here, just regression)* ****** ****** 1. Modify the RESERVESLIP->email Letter. Add "Last pickup date<<reserves.lastpickupdate>>" somewhere. 2. Make a hold for a borrower. 3. Catch the hold and print the reserve slip. Verify that the lastpickupdate is actually the (current date + the 'Holds pickup wait' in your issuing rules) in days. ** ** ** ** ** ** *** *** *** TEST: RESPECTING CALENDAR HOLIDAYS * ** ** ** ** ** ** 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Cancel this hold 5) Edit the calendar for the given location, and make the pickup date a single, unrepeated, holiday. Then make the next day a weekly repeating holiday. Then make the next two days an annual holiday repeatable every year. 6) Repeat step 2 7) View waitingreserves.pl, the last pickup date for this new hold should now be increased by four days. ** ** ** ** ** ** *** *** *** TEST: LETTER lastpickupdate PLACEHOLDER AND REGRESSION * ** ** ** ** ** ** 1. Modify the HOLD-letter, add <<reserves.lastpickupdate>> and <<reserves.waitingdate>> somewhere. 2. Catch a hold for a borrower. 3. See the members/notices.pl for the message queued and verify that the lastpickupdate-placeholder is set in your desired datetime format and that it is not on a holiday. Verify that the waitingdate is today. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32916|0 |1 is obsolete| | --- Comment #87 from Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi> --- Created attachment 32930 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=32930&action=edit Bug 8367 - How long is a hold waiting for pickup at a more granular level You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. The lastpickupdate-column is set based on the given instruction and lands only on an open library day. Also cancel_expired_reserves.pl now cancels only on open library days after the last pickup date has expired. And waitingreserves.pl respects the new DB column. If ReservesMaxPickUpDelay is not set prior to the databaseupdate, the holdspickupwait-directive is never defined, thus disabling this feature until the holdspickupwait is set. If the issuingrules holdspickupwait is 0 or less, this feature is disabled. So each issuingrule can disable this feature for the material it matches, while being fully functional with other rules. This patch adds: - a new column in the issuing rules, including help topics. - a new column reserves.lastpickupdate (+old_reserves) It contains the waitingdate + the corresponding "holds pickup wait". Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupwait - a new function C4::Reserves::MoveWaitingdate() to help in testing this feature. - a new function C4::Reserves::GetLastPickupDate() to calculate the proper date from Calendar. This patch removes: - the ReservesMaxPickUpDelay syspref. - the ExpireReservesOnHolidays syspref, because it became obsolete due to the changes in handling the last pickup date with Koha Calendar. - Also removes a code section from C4::Letters::_parseLetter() which worked blatantly disrespectfully towards Koha Calendar. - TODO figure out how to rename existing letter placeholders <<expirationdate>> with <<lastpickupdate>> Unit tests included. $$$$$$$$$$$$$ $ TEST PLAN $ $$$$$$$$$$$$$ 0. BEFORE UPDATING THE DATABASE! MAKE SURE THAT THE SYSPREF 'ReservesMaxPickUpDelay' is bigger than 0. Otherwise existing reserves won't get UPDATEd with the new lastpickupdate-values. ****** ****** *** *** *** TEST: BASIC WORKFLOW * ****** ****** 1. Check there is no regression with a normal reserve workflow. 2. Add one or more issuingrules. 3. Update the new column 'Holds pickup wait' in your issuing rules. 4. In the 3 templates of staff client, you can see the 'Last pickup date' for an hold (circ/circulation.pl, circ/waitingreserves.pl, members/moremember.pl) 5. In opac-user.pl the last pickup date is displayed in the status-column if available. 5. According to a library and an item type, the lastpickupdate value will be equal to the waiting date + the "holds pickup wait" defined moved to the next open library day. ****** ****** *** *** *** TEST: LETTER PLACEHOLDER (no code modifications here, just regression)* ****** ****** 1. Modify the RESERVESLIP->email Letter. Add "Last pickup date<<reserves.lastpickupdate>>" somewhere. 2. Make a hold for a borrower. 3. Catch the hold and print the reserve slip. Verify that the lastpickupdate is actually the (current date + the 'Holds pickup wait' in your issuing rules) in days. ** ** ** ** ** ** *** *** *** TEST: RESPECTING CALENDAR HOLIDAYS * ** ** ** ** ** ** 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Cancel this hold 5) Edit the calendar for the given location, and make the pickup date a single, unrepeated, holiday. Then make the next day a weekly repeating holiday. Then make the next two days an annual holiday repeatable every year. 6) Repeat step 2 7) View waitingreserves.pl, the last pickup date for this new hold should now be increased by four days. ** ** ** ** ** ** *** *** *** TEST: LETTER lastpickupdate PLACEHOLDER AND REGRESSION * ** ** ** ** ** ** 1. Modify the HOLD-letter, add <<reserves.lastpickupdate>> and <<reserves.waitingdate>> somewhere. 2. Catch a hold for a borrower. 3. See the members/notices.pl for the message queued and verify that the lastpickupdate-placeholder is set in your desired datetime format and that it is not on a holiday. Verify that the waitingdate is today. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #89 from Paola Rossi <paola.rossi@cineca.it> --- (In reply to Olli-Antti Kivilahti from comment #88)

[...] Hope to hear from you soon

Thanks, Olli-Antti, for your kind feedbacks. Today I've applied the patch against master 3.17.00.043. After having applied, I've created a hold reserving an item that was checked out. So the reserve/hold's status became "W" after checking in it. On the browser I refreshed the cache (Ctrl r, Ctrl F5): so I saw the hold by the 4 *.tt as required in step 4. In particular: on OPAC, for a logged patron with holds, the lastpickupdate was in the "until" part of the "Status" column of the hold. On circ/waitingreserves.tt (Home › Circulation › Holds awaiting pickup) the lastpickupdate was in the "until" part of the first column "Available" of "n Hold(s) waiting" tag. The tables with holds with NULL lastpickupdate (holds created reserving items not checked out, as allowed by syspref AllowOnShelfHolds set to "Allow") were shown OK too. So I pass the patch back to "Needs Signoff" status. I've changed the syspref 'ReservesMaxPickUpDelay' from the default 0 value, thank you again. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #91 from Paola Rossi <paola.rossi@cineca.it> --- [I beg you pardon for the length of this comment, my delay in answering and possible inaccuracies.] I've applied the patch against master 3.17.00.044 [HEAD 10860]. To test the patch IMHO on ADMIN it is better: - (as required by the test plan) kept the value of syspref "ReservesMaxPickupDelay" > 0 - set the syspref "EnhancedMessagingPreferences" to ON (default is OFF) To test the patch IMHO on ADMIN it could be better: - kept the default value "Don't allow" to the syspref "AllowOnShelfHolds" - set the "Charge" of the item-types to 0.00 (default 5.00) for all types - [remember to set the circulation rules] - set the syspref "KohaAdminEmailAddress" to a value not filtered by antispam's own emailing-system [to receive notices' email] - [remeber to refresh the cache [Ctrl r - Ctrl F5] after seeing master's behaviour]. At my tests, the RESERVESLIP and HOLD notices are allowed to be cloned to your specific library, but I saw that koha uses only the "ALL LIBRARIES" notices, and never the cloned ones. So I changed those 2 notices to test the 2 parts of the test plan about LETTER. To test the part of the test plan about the RESERVESLIP notice, i.e. see below: *** TEST: LETTER PLACEHOLDER ***, IMHO the functions to test are "Confirm"/"Print and confirm", on cheching in a record already on hold by another patron. To test the last part of the test plan about the HOLD notice, i.e. see below: *** TEST: LETTER lastpickupdate PLACEHOLDER AND REGRESSION ****, if I'm not wrong : - set the notice's email [i.e. "Primary email"] of the test patron to your own email - if this patron had a found/status W hold, you'll receive a mail soon after: perl ./misc/cronjobs/advance_notices.pl -c perl ./misc/cronjobs/process_message_queue.pl IMHO it would be better explicitly adding a further last part to the test plan about the provided: t/db_dependent/Holds.t [perhaps other parts of the preamble could need a testing action too.] At my tests the upgrade of the DB is OK. The 4 *.tt are OK. NB1.: on ADMIN, in the "Circ and fines issuing rules", perhaps the new column "Holds wait for pickup (day)" could be better set at the right of "Holds allowed (count)", at the right of all the 4 columns about renewal, and not among these 4 ones. NB2.: on CIRC, on waitingreserves.pl (="Holds awaiting pickup"), on the "Holds over" tag, the present string: "Holds listed here have been awaiting pickup for more than days." could be modified. In master it was: "Holds listed here have been awaiting pickup for more than X days." where X was the value of the syspref "ReservesMaxPickUpDelay" which has been removed by this patch. NB3.: On opac-user.pl, for a logged in user with W holds, the W holds had not neigher the "Cancel" icon nor the "Suspend" icon. [I didn't check the master about this behaviour, sorry. If this patch had changed the behaviour, this regression would be on error, and this point will be added to the errors' list.] PROBLEMS: NB4.: [MASTER] at my tests the "W" found/status of a hold/reserve is set in only 1 case: it is set to a hold, that previously had been opened on a biblio record which was already checked out (found=NULL), and then checked in (found=W) without "Ignore" submit. [I'd like to have a feedback about it, please, because all my tests were about this assumption.] NB5.: [MASTER] So, at my tests, the waitingdate of a hold/reserve can be only the current date, i.e. the current day on which I check-in a biblio record. [I'd like to have a feedback about it, please, because all my tests were about this assumption.] 1) I saw no reserves.lastpickupdate nor the reserves.waitingdate on RESERVESLIP notice. But I think that this problem is out of this bug, because the master too behaves so. [On the contrary, on master (and after applying too) the reserves.reservesnotes is correctly shown by the notice.] 2) Whilst a check out lasts 1 more day for an intervening holiday, the hold's period doesn't: I'd like to have a feedback about this, please. If the W hold would last 1 more day, the patch would be in error. 3) On upgrading the DB, by syspref "ReservesMaxPickupDelay" > 0, all the holds for all the patrons' categories and item types would receive the same lasting period. Whilst the patch, once applied, could assign a different value to each of them. I think this remarkable to get coherent DB datas. 4) The found/status "W" meaning is about the lasting period available to the patron to pick up the document. But when a "Transfer" is needed from the check-out' library, to the hold's library, koha [master] doesn't set the found/status to "W", but to NULL/T: I think that this behaviour could be evaluated, that it can be a master's bug, and, if this is the case, that this patch could be tested after having resolved this bug on a DB with such cases. 5) prove t/db_dependent/Holds.t t/db_dependent/Holds.t .. 1/46 # Failed test 'GetLastPickupDate(): Not using Calendar' # at t/db_dependent/Holds.t line 436. # got: '2014-11-10' # expected: '2014-11-05' # Failed test 'GetLastPickupDate(): Moving lastpickupdate over holidays, but not affected by them' # at t/db_dependent/Holds.t line 448. # got: '2014-11-10' # expected: '2014-11-05' # Failed test 'MoveWaitingdate(): Moving to past' # at t/db_dependent/Holds.t line 401. # got: '' # expected: '1' # Failed test 'Waiting reserve with lastpickupdate for 2014-11-04 totally canceled' # at t/db_dependent/Holds.t line 367. # got: '1' # expected: '0' # Looks like you failed 4 tests of 46. t/db_dependent/Holds.t .. Dubious, test returned 4 (wstat 1024, 0x400) Failed 4/46 subtests Test Summary Report ------------------- t/db_dependent/Holds.t (Wstat: 1024 Tests: 46 Failed: 4) Failed tests: 37, 39, 41, 44 Non-zero exit status: 4 Files=1, Tests=46, 1 wallclock secs ( 0.01 usr 0.01 sys + 0.91 cusr 0.29 csys = 1.22 CPU) Result: FAIL For the 5-th problem I pass the patch to "Failed QA" status. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Paola Rossi <paola.rossi@cineca.it> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Patch doesn't apply --- Comment #93 from Paola Rossi <paola.rossi@cineca.it> --- I've tried to apply the patch against master 3.19.00.002 Applying: Bug 8367 - How long is a hold waiting for pickup at a more granular level Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Holds.t Auto-merging svc/holds Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/waitingreserves.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref Auto-merging koha-tmpl/intranet-tmpl/prog/en/js/holds.js Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merging installer/data/mysql/sysprefs.sql Auto-merging installer/data/mysql/kohastructure.sql Auto-merging circ/waitingreserves.pl Auto-merging C4/Reserves.pm Auto-merging C4/Letters.pm Failed to merge in the changes. Patch failed at 0001 Bug 8367 - How long is a hold waiting for pickup at a more granular level So I pass the patch to "Patch doesn't apply" status. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |9004 See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=9004 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Joonas Kylmälä <j.kylmala@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |j.kylmala@gmail.com Assignee|olli-antti.kivilahti@jns.fi |j.kylmala@gmail.com --- Comment #94 from Joonas Kylmälä <j.kylmala@gmail.com> --- I'll start working on this and I decided to create a new patch and implement the same changes on it. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 Joonas Kylmälä <j.kylmala@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #32930|0 |1 is obsolete| | --- Comment #95 from Joonas Kylmälä <j.kylmala@gmail.com> --- Created attachment 42706 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=42706&action=edit Bug 8367 - How long is a hold waiting for pickup at a more granular level This patch applies now. You can now specify a pickup delay for an hold function of a patron category and/or a item type and/or a library. The lastpickupdate-column is set based on the given instruction and lands only on an open library day. Also cancel_expired_reserves.pl now cancels only on open library days after the last pickup date has expired. And waitingreserves.pl respects the new DB column. If ReservesMaxPickUpDelay is not set prior to the databaseupdate, the holdspickupwait-directive is never defined, thus disabling this feature until the holdspickupwait is set. If the issuingrules holdspickupwait is 0 or less, this feature is disabled. So each issuingrule can disable this feature for the material it matches, while being fully functional with other rules. This patch adds: - a new column in the issuing rules, including help topics. - a new column reserves.lastpickupdate (+old_reserves) It contains the waitingdate + the corresponding "holds pickup wait". Each time the waitingdate is modified, this value will be modified too. - a new field issuingrules.holdspickupwait - a new function C4::Reserves::MoveWaitingdate() to help in testing this feature. - a new function C4::Reserves::GetLastPickupDate() to calculate the proper date from Calendar. This patch removes: - the ReservesMaxPickUpDelay syspref. - the ExpireReservesOnHolidays syspref, because it became obsolete due to the changes in handling the last pickup date with Koha Calendar. - Also removes a code section from C4::Letters::_parseLetter() which worked blatantly disrespectfully towards Koha Calendar. - TODO figure out how to rename existing letter placeholders <<expirationdate>> with <<lastpickupdate>> Unit tests included. $$$$$$$$$$$$$ $ TEST PLAN $ $$$$$$$$$$$$$ 0. BEFORE UPDATING THE DATABASE! MAKE SURE THAT THE SYSPREF 'ReservesMaxPickUpDelay' is bigger than 0. Otherwise existing reserves won't get UPDATEd with the new lastpickupdate-values. ****** ****** *** *** *** TEST: BASIC WORKFLOW * ****** ****** 1. Check there is no regression with a normal reserve workflow. 2. Add one or more issuingrules. 3. Update the new column 'Holds pickup wait' in your issuing rules. 4. In the 3 templates of staff client, you can see the 'Last pickup date' for an hold (circ/circulation.pl, circ/waitingreserves.pl, members/moremember.pl) 5. In opac-user.pl the last pickup date is displayed in the status-column if available. 5. According to a library and an item type, the lastpickupdate value will be equal to the waiting date + the "holds pickup wait" defined moved to the next open library day. ****** ****** *** *** *** TEST: LETTER PLACEHOLDER (no code modifications here, just regression)* ****** ****** 1. Modify the RESERVESLIP->email Letter. Add "Last pickup date<<reserves.lastpickupdate>>" somewhere. 2. Make a hold for a borrower. 3. Catch the hold and print the reserve slip. Verify that the lastpickupdate is actually the (current date + the 'Holds pickup wait' in your issuing rules) in days. ** ** ** ** ** ** *** *** *** TEST: RESPECTING CALENDAR HOLIDAYS * ** ** ** ** ** ** 1) Note your ReservesMaxPickUpDelay setting. 2) Create and trap a hold 3) View waitingreserves.pl, note the last pickup date, and the pickup location 4) Cancel this hold 5) Edit the calendar for the given location, and make the pickup date a single, unrepeated, holiday. Then make the next day a weekly repeating holiday. Then make the next two days an annual holiday repeatable every year. 6) Repeat step 2 7) View waitingreserves.pl, the last pickup date for this new hold should now be increased by four days. ** ** ** ** ** ** *** *** *** TEST: LETTER lastpickupdate PLACEHOLDER AND REGRESSION * ** ** ** ** ** ** 1. Modify the HOLD-letter, add <<reserves.lastpickupdate>> and <<reserves.waitingdate>> somewhere. 2. Catch a hold for a borrower. 3. See the members/notices.pl for the message queued and verify that the lastpickupdate-placeholder is set in your desired datetime format and that it is not on a holiday. Verify that the waitingdate is today. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8367 --- Comment #97 from Joonas Kylmälä <j.kylmala@gmail.com> --- After this patch gets to master, the system preference ExpireReservesMaxPickUpDelay could to be changed to something like ExpireReservesAfterLastPickupDay. I guess it is then more descriptive. There was about 15-20 files depending on ExpireReservesMaxPickUpDelay preference, so it would be better to do maybe a new bug report for it? -- You are receiving this mail because: You are watching all bug changes.