Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report
Summary: ExpireReservesMaxPickUpDelay works with hold(s) over report
Status: BLOCKED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Olli-Antti Kivilahti
QA Contact: Testopia
URL:
Keywords:
Depends on: 14536 13409 13906 13995
Blocks:
  Show dependency treegraph
 
Reported: 2013-08-16 19:35 UTC by Galen Charlton
Modified: 2023-11-02 21:16 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10744 - ExpireReservesMaxPickUpDelay has minor workflow conflicts with hold(s) over report (11.22 KB, patch)
2015-10-28 16:38 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 10744 - ExpireReservesMaxPickUpDelay has minor workflow conflicts with hold(s) over report (19.82 KB, patch)
2015-11-02 18:28 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report (25.84 KB, patch)
2015-11-04 15:17 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report (25.86 KB, patch)
2015-11-05 09:27 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report (21.30 KB, patch)
2020-07-01 07:03 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 10744: DO NOT PUSH! Schema change (2.79 KB, patch)
2020-09-14 06:40 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report (18.72 KB, patch)
2020-09-14 06:41 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report (18.62 KB, patch)
2021-03-31 07:32 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 10744: DO NOT PUSH! Schema changes (2.97 KB, patch)
2021-03-31 07:33 UTC, Emmi Takkinen
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Galen Charlton 2013-08-16 19:35:39 UTC
Turning on ExpireReservesMaxPickUpDelay means that the hold(s) over part of the holds awaiting pickup report (circ/waitingreserves.pl) can be confusing.

Specifically, if the hold cancellation cronjob runs before the library opens, the hold(s) over list will never contain any contents -- all of the holds that have hit the max pickup delay will have been cancelled.

Having a report that will never produce results can be (and has been for at least one of our customers) confusing.

I can think of a couple options for improving this:

- suppress the display of the hold(s) over tab if ExpireReservesMaxPickUpDelay is on.
- have the hold(s) over report display the list of waiting holds that were canceled 
  - today or
  - the last time cancel_expired_holds.pl was run
- as alternative or in conjunction with one of the other options, have cancel_expired_holds.pl create a report of cancelled waiting holds to assist with clearing the items from the holds shelf
Comment 1 Olli-Antti Kivilahti 2015-10-28 16:38:01 UTC Comment hidden (obsolete)
Comment 2 Olli-Antti Kivilahti 2015-11-02 18:28:21 UTC Comment hidden (obsolete)
Comment 3 Olli-Antti Kivilahti 2015-11-04 15:17:21 UTC Comment hidden (obsolete)
Comment 4 Olli-Antti Kivilahti 2015-11-05 09:27:47 UTC
Created attachment 44464 [details] [review]
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report
Comment 5 Katrin Fischer 2017-05-01 22:22:52 UTC
Applying: Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report
fatal: sha1 information is lacking or useless (C4/Reserves.pm).
error: could not build fake ancestor
Comment 6 Emmi Takkinen 2020-07-01 07:03:24 UTC
Created attachment 106446 [details] [review]
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report

Hold(s) over -report at circ/waitingreserves.pl doesn't work because
ExpireReservesMaxPickup removes reserves from the koha.reserves-table to the
koha.old_reserves-table.

This patch adds a new column koha.reserves.pickupexpired telling when/if the
reserve has got it's pickup duration expired.

This date is used to pull old reserves to the waitingreserves-view.

syspref 'PickupExpiredHoldsOverReportDuration' controls how many days after
expiration old reserves are displayed on the report, and the values respect
Koha::Calendar holidays

To test:
1) Apply patch.
2) Enable 'ExpireReservesMaxPickUpDelay'.
3) Set some value to 'ReservesMaxPickUpDelay' and 'PickupExpiredHoldsOverReportDuration' (e.g. 2 and 7).
4) Find or create hold(s) waiting pickup and set their expiration date so it exceeds 'ReservesMaxPickUpDelay'
but not 'PickupExpiredHoldsOverReportDuration'.
5) Confirm hold(s) are moved to 'Holds waiting over ...' list.
6) Run misc/cronjobs/holds/cancel_expired_holds.pl.
7) Confirm hold(s) are still displayed on the list.
8) Confirm from db that hold(s) have 'pickupexpired' set same as 'expirationdate'.
9) Set new value to 'PickupExpiredHoldsOverReportDuration' so it no longer covers hold(s) expiration date(s).
10) Confirm that hold(s) are no longer displayed in waiting list

Also prove t/db_depentent/Koha/Old.t
Comment 7 Emmi Takkinen 2020-07-01 07:05:05 UTC
Fixed patch to apply master version and removed dependencies to other patches.
Comment 8 Joonas Kylmälä 2020-09-10 13:23:45 UTC
The patch doesn't apply because the schema files have been changed. Please add the schema files to a separate patch and write e.g. DO NOT PUSH to it because the RM will handle updating it. For more info on database updates and schema changes please see https://wiki.koha-community.org/wiki/Database_updates
Comment 9 Emmi Takkinen 2020-09-14 06:40:20 UTC
Created attachment 110030 [details] [review]
Bug 10744: DO NOT PUSH! Schema change
Comment 10 Emmi Takkinen 2020-09-14 06:41:29 UTC
Created attachment 110031 [details] [review]
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report

Hold(s) over -report at circ/waitingreserves.pl doesn't work because
ExpireReservesMaxPickup removes reserves from the koha.reserves-table to the
koha.old_reserves-table.

This patch adds a new column koha.reserves.pickupexpired telling when/if the
reserve has got it's pickup duration expired.

This date is used to pull old reserves to the waitingreserves-view.

syspref 'PickupExpiredHoldsOverReportDuration' controls how many days after
expiration old reserves are displayed on the report, and the values respect
Koha::Calendar holidays

To test:
1) Apply patch.
2) Enable 'ExpireReservesMaxPickUpDelay'.
3) Set some value to 'ReservesMaxPickUpDelay' and 'PickupExpiredHoldsOverReportDuration' (e.g. 2 and 7).
4) Find or create hold(s) waiting pickup and set their expiration date so it exceeds 'ReservesMaxPickUpDelay'
but not 'PickupExpiredHoldsOverReportDuration'.
5) Confirm hold(s) are moved to 'Holds waiting over ...' list.
6) Run misc/cronjobs/holds/cancel_expired_holds.pl.
7) Confirm hold(s) are still displayed on the list.
8) Confirm from db that hold(s) have 'pickupexpired' set same as 'expirationdate'.
9) Set new value to 'PickupExpiredHoldsOverReportDuration' so it no longer covers hold(s) expiration date(s).
10) Confirm that hold(s) are no longer displayed in waiting list

Also prove t/db_depentent/Koha/Old.t
Comment 11 Mazen Khallaf 2021-01-19 20:34:08 UTC
Couldn't apply patch during signoff.


$ git bz apply 10744

Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report

110030 - Bug 10744: DO NOT PUSH! Schema change
110031 - Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report

Apply? [(y)es, (n)o, (i)nteractive] y

Applying: Bug 10744: DO NOT PUSH! Schema changes
Using index info to reconstruct a base tree...
M	Koha/Schema/Result/OldReserve.pm
M	Koha/Schema/Result/Reserve.pm
Falling back to patching base and 3-way merge...
Auto-merging Koha/Schema/Result/Reserve.pm
CONFLICT (content): Merge conflict in Koha/Schema/Result/Reserve.pm
Auto-merging Koha/Schema/Result/OldReserve.pm
CONFLICT (content): Merge conflict in Koha/Schema/Result/OldReserve.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 10744: DO NOT PUSH! Schema changes
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-10744-DO-NOT-PUSH-Schema-change-RxpQb0.patch
Comment 12 Emmi Takkinen 2021-03-31 07:32:32 UTC
Created attachment 119029 [details] [review]
Bug 10744 - ExpireReservesMaxPickUpDelay works with hold(s) over report

Hold(s) over -report at circ/waitingreserves.pl doesn't work because
ExpireReservesMaxPickup removes reserves from the koha.reserves-table to the
koha.old_reserves-table.

This patch adds a new column koha.reserves.pickupexpired telling when/if the
reserve has got it's pickup duration expired.

This date is used to pull old reserves to the waitingreserves-view.

syspref 'PickupExpiredHoldsOverReportDuration' controls how many days after
expiration old reserves are displayed on the report, and the values respect
Koha::Calendar holidays

To test:
1) Apply patch.
2) Enable 'ExpireReservesMaxPickUpDelay'.
3) Set some value to 'ReservesMaxPickUpDelay' and 'PickupExpiredHoldsOverReportDuration' (e.g. 2 and 7).
4) Find or create hold(s) waiting pickup and set their expiration date so it exceeds 'ReservesMaxPickUpDelay'
but not 'PickupExpiredHoldsOverReportDuration'.
5) Confirm hold(s) are moved to 'Holds waiting over ...' list.
6) Run misc/cronjobs/holds/cancel_expired_holds.pl.
7) Confirm hold(s) are still displayed on the list.
8) Confirm from db that hold(s) have 'pickupexpired' set same as 'expirationdate'.
9) Set new value to 'PickupExpiredHoldsOverReportDuration' so it no longer covers hold(s) expiration date(s).
10) Confirm that hold(s) are no longer displayed in waiting list

Also prove t/db_depentent/Koha/Old.t
Comment 13 Emmi Takkinen 2021-03-31 07:33:00 UTC
Created attachment 119030 [details] [review]
Bug 10744: DO NOT PUSH! Schema changes
Comment 14 Lucas Gass 2023-11-02 21:16:27 UTC
Marking this as BLOCKED until its dependencies can be resolved.