Bug 33992 - Only consider the date when labelling a waiting recall as problematic
Summary: Only consider the date when labelling a waiting recall as problematic
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Aleisha Amohia
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 19532 33990
Blocks:
  Show dependency treegraph
 
Reported: 2023-06-13 05:13 UTC by Aleisha Amohia
Modified: 2023-12-28 20:47 UTC (History)
5 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.03,22.11.09,22.05.16
Circulation function:


Attachments
Bug 33992: Only consider the date when labelling a waiting recall as problematic (2.76 KB, patch)
2023-06-13 05:21 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33992: Only consider the date when auto-expiring problematic recalls (3.47 KB, patch)
2023-06-13 05:32 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33992: Only consider the date when labelling a waiting recall as problematic (2.81 KB, patch)
2023-06-14 23:10 UTC, David Nind
Details | Diff | Splinter Review
Bug 33992: Only consider the date when auto-expiring problematic recalls (3.51 KB, patch)
2023-06-14 23:10 UTC, David Nind
Details | Diff | Splinter Review
Bug 33992: Only consider the date when auto-expiring problematic recalls (3.62 KB, patch)
2023-06-14 23:16 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 33992: Only consider the date when labelling a waiting recall as problematic (2.81 KB, patch)
2023-06-15 00:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 33992: Only consider the date when auto-expiring problematic recalls (3.67 KB, patch)
2023-06-15 00:22 UTC, David Nind
Details | Diff | Splinter Review
Bug 33992: Only consider the date when labelling a waiting recall as problematic (2.86 KB, patch)
2023-07-16 11:11 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33992: Only consider the date when auto-expiring problematic recalls (3.72 KB, patch)
2023-07-16 11:11 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2023-06-13 05:13:49 UTC
The recalls awaiting pickup report has two tabs - recalls waiting, and recalls waiting over X days. This number of days when a recall is labelled 'problematic' is defined by the RecallsMaxPickUpDelay system preference.

Sometimes recalls do not show here even when we would expect them to. This is because the waiting_date for recalls is a timestamp, and Koha wrongly considers the time when deciding if the recall has been waiting for a problematic number of days.

For example, consider this set up:

- RecallsMaxPickUpDelay = 1 day
- Recall has been waiting since 12 June
- Today is 13 June

Treating 12 June as 1 day, the recall has been awaiting pickup for more than 1 day, so I would expect it to show under the 'recalls waiting over X days' tab.

It doesn't, because when we look at the database timestamps:

- Recall has been waiting since 2023-06-12 19:06:58
- Time now is 2023-06-13 17:06:58
- Recall waiting_date plus 1 max pickup delay day equals 2023-06-13 19:06:58 - which is 2 hours after the cut off to be considered problematic.

Koha should not consider the timestamp in this case, it should only look at the date
Comment 1 Aleisha Amohia 2023-06-13 05:21:22 UTC
Created attachment 152288 [details] [review]
Bug 33992: Only consider the date when labelling a waiting recall as problematic

This patches makes sure Koha ignores the hours/minutes/seconds when comparing a recall waiting date to decide if it has been waiting too long for pickup.

To test:

1. Enable UseRecalls syspref and set the relevant circulation and fines rules to configure recalls
2. Set the RecallsMaxPickUpDelay to 1 day
3. Check out Item A to Patron A
4. Log into the OPAC as Patron B
5. Search for Item A and place a recall
6. Log back into the staff interface and check in Item A
7. Confirm the recall as waiting for Patron B
8. Go to Circulation -> Recalls awaiting pickup
9. Confirm your recall exists under the 'Recalls waiting' tab
10. Log into the database and get the time for now

sudo koha-mysql INSTANCE
select now();

11. Change the waiting_date in the database for this recall to yesterday so we would expect to see it under the 'Recalls waiting over 1 days' tab. Base the hours on the now() timestamp from the step before, add a few hours to this.

For example, if the above SQl query returns "2023-06-13 10:54:21", change waiting_date to be slightly less than 1 day/24 hours before this now() timestamp, but still what we would consider 'yesterday'

select * from recalls; # to get the recall_id
update recalls set waiting_date = "2023-06-12 14:54:21" where recall_id = X;

12. Refresh the Circulation -> Recalls awaiting pickup page. The recall will not show up under the 'Recalls waiting over 1 days' tab, even though we expect it to.

13. Apply the patch, restart services, refresh the page
14. Confirm the recall now shows under the 'Recalls waiting over 1 days' tab as expected.

Sponsored-by: Auckland University of Technology
Comment 2 Aleisha Amohia 2023-06-13 05:32:19 UTC
Created attachment 152289 [details] [review]
Bug 33992: Only consider the date when auto-expiring problematic recalls

This patch carries this fix into the misc/cronjobs/recalls/expire_recalls.pl cronjob so that recalls are automatically expired when they have been waiting a problematic number of days, not considering hours, as expected.

To test, follow the test plan from the first patch. This will set you up with a waiting problematic recall.

Run the cronjob manually

perl misc/cronjobs/recalls/expire_recalls.pl

Refresh your 'Recalls awaiting pickup' page. Your problematic recall should be gone/expired.

Sponsored-by: Auckland University of Technology
Comment 3 David Nind 2023-06-14 23:10:37 UTC
Created attachment 152363 [details] [review]
Bug 33992: Only consider the date when labelling a waiting recall as problematic

This patches makes sure Koha ignores the hours/minutes/seconds when comparing a recall waiting date to decide if it has been waiting too long for pickup.

To test:

1. Enable UseRecalls syspref and set the relevant circulation and fines rules to configure recalls
2. Set the RecallsMaxPickUpDelay to 1 day
3. Check out Item A to Patron A
4. Log into the OPAC as Patron B
5. Search for Item A and place a recall
6. Log back into the staff interface and check in Item A
7. Confirm the recall as waiting for Patron B
8. Go to Circulation -> Recalls awaiting pickup
9. Confirm your recall exists under the 'Recalls waiting' tab
10. Log into the database and get the time for now

sudo koha-mysql INSTANCE
select now();

11. Change the waiting_date in the database for this recall to yesterday so we would expect to see it under the 'Recalls waiting over 1 days' tab. Base the hours on the now() timestamp from the step before, add a few hours to this.

For example, if the above SQl query returns "2023-06-13 10:54:21", change waiting_date to be slightly less than 1 day/24 hours before this now() timestamp, but still what we would consider 'yesterday'

select * from recalls; # to get the recall_id
update recalls set waiting_date = "2023-06-12 14:54:21" where recall_id = X;

12. Refresh the Circulation -> Recalls awaiting pickup page. The recall will not show up under the 'Recalls waiting over 1 days' tab, even though we expect it to.

13. Apply the patch, restart services, refresh the page
14. Confirm the recall now shows under the 'Recalls waiting over 1 days' tab as expected.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2023-06-14 23:10:40 UTC
Created attachment 152364 [details] [review]
Bug 33992: Only consider the date when auto-expiring problematic recalls

This patch carries this fix into the misc/cronjobs/recalls/expire_recalls.pl cronjob so that recalls are automatically expired when they have been waiting a problematic number of days, not considering hours, as expected.

To test, follow the test plan from the first patch. This will set you up with a waiting problematic recall.

Run the cronjob manually

perl misc/cronjobs/recalls/expire_recalls.pl

Refresh your 'Recalls awaiting pickup' page. Your problematic recall should be gone/expired.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Comment 5 Aleisha Amohia 2023-06-14 23:16:11 UTC
Created attachment 152365 [details] [review]
Bug 33992: Only consider the date when auto-expiring problematic recalls

This patch carries this fix into the misc/cronjobs/recalls/expire_recalls.pl cronjob so that recalls are automatically expired when they have been waiting a problematic number of days, not considering hours, as expected.

To test, follow the test plan from the first patch. This will set you up with a waiting problematic recall.

Run the cronjob manually

perl misc/cronjobs/recalls/expire_recalls.pl

Refresh your 'Recalls awaiting pickup' page. Your problematic recall should be gone/expired.

Expiration dates will apply when expiring any 'unfulfilled' recall i.e. newly requested, overdue to be returned, and awaiting pickup.

Sponsored-by: Auckland University of Technology
Comment 6 Aleisha Amohia 2023-06-14 23:17:13 UTC
Hi David,

Sorry I didn't realise this was already Signed off, I've just amended the second patch - this was the only change from the patch you had already tested.

--- a/misc/cronjobs/recalls/expire_recalls.pl
+++ b/misc/cronjobs/recalls/expire_recalls.pl
@@ -42,7 +42,7 @@ cronlogaction({ info => $command_line_options });
 my $recalls = Koha::Recalls->search({ completed => 0 });
 my $today = dt_from_string()->truncate( to  => 'day' );
 while( my $recall = $recalls->next ) {
-    if ( ( $recall->requested or $recall->overdue ) and $recall->expiration_date ) {
+    if ( ( $recall->requested or $recall->overdue or $recall->waiting ) and $recall->expiration_date ) {

Thanks
Aleisha
Comment 7 David Nind 2023-06-15 00:22:56 UTC
Created attachment 152368 [details] [review]
Bug 33992: Only consider the date when labelling a waiting recall as problematic

This patches makes sure Koha ignores the hours/minutes/seconds when comparing a recall waiting date to decide if it has been waiting too long for pickup.

To test:

1. Enable UseRecalls syspref and set the relevant circulation and fines rules to configure recalls
2. Set the RecallsMaxPickUpDelay to 1 day
3. Check out Item A to Patron A
4. Log into the OPAC as Patron B
5. Search for Item A and place a recall
6. Log back into the staff interface and check in Item A
7. Confirm the recall as waiting for Patron B
8. Go to Circulation -> Recalls awaiting pickup
9. Confirm your recall exists under the 'Recalls waiting' tab
10. Log into the database and get the time for now

sudo koha-mysql INSTANCE
select now();

11. Change the waiting_date in the database for this recall to yesterday so we would expect to see it under the 'Recalls waiting over 1 days' tab. Base the hours on the now() timestamp from the step before, add a few hours to this.

For example, if the above SQl query returns "2023-06-13 10:54:21", change waiting_date to be slightly less than 1 day/24 hours before this now() timestamp, but still what we would consider 'yesterday'

select * from recalls; # to get the recall_id
update recalls set waiting_date = "2023-06-12 14:54:21" where recall_id = X;

12. Refresh the Circulation -> Recalls awaiting pickup page. The recall will not show up under the 'Recalls waiting over 1 days' tab, even though we expect it to.

13. Apply the patch, restart services, refresh the page
14. Confirm the recall now shows under the 'Recalls waiting over 1 days' tab as expected.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2023-06-15 00:22:59 UTC
Created attachment 152369 [details] [review]
Bug 33992: Only consider the date when auto-expiring problematic recalls

This patch carries this fix into the misc/cronjobs/recalls/expire_recalls.pl cronjob so that recalls are automatically expired when they have been waiting a problematic number of days, not considering hours, as expected.

To test, follow the test plan from the first patch. This will set you up with a waiting problematic recall.

Run the cronjob manually

perl misc/cronjobs/recalls/expire_recalls.pl

Refresh your 'Recalls awaiting pickup' page. Your problematic recall should be gone/expired.

Expiration dates will apply when expiring any 'unfulfilled' recall i.e. newly requested, overdue to be returned, and awaiting pickup.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2023-06-15 00:24:36 UTC
(In reply to Aleisha Amohia from comment #6)
> Hi David,
> 
> Sorry I didn't realise this was already Signed off, I've just amended the
> second patch - this was the only change from the patch you had already
> tested.
> 
..
> Thanks
> Aleisha

No problem! I have retested (I think I tested correctly for the second patch, so have signed off.)

David
Comment 10 Katrin Fischer 2023-07-16 11:11:19 UTC
There were some warns when running the expire_recalls.pl script, but the lines seem unrelated to this change:

Argument "" isn't numeric in numeric ge (>=) at misc/cronjobs/recalls/expire_recalls.pl line 61.
Argument "" isn't numeric in numeric gt (>) at misc/cronjobs/recalls/expire_recalls.pl line 62.

File separately?
Comment 11 Katrin Fischer 2023-07-16 11:11:55 UTC
Created attachment 153512 [details] [review]
Bug 33992: Only consider the date when labelling a waiting recall as problematic

This patches makes sure Koha ignores the hours/minutes/seconds when comparing a recall waiting date to decide if it has been waiting too long for pickup.

To test:

1. Enable UseRecalls syspref and set the relevant circulation and fines rules to configure recalls
2. Set the RecallsMaxPickUpDelay to 1 day
3. Check out Item A to Patron A
4. Log into the OPAC as Patron B
5. Search for Item A and place a recall
6. Log back into the staff interface and check in Item A
7. Confirm the recall as waiting for Patron B
8. Go to Circulation -> Recalls awaiting pickup
9. Confirm your recall exists under the 'Recalls waiting' tab
10. Log into the database and get the time for now

sudo koha-mysql INSTANCE
select now();

11. Change the waiting_date in the database for this recall to yesterday so we would expect to see it under the 'Recalls waiting over 1 days' tab. Base the hours on the now() timestamp from the step before, add a few hours to this.

For example, if the above SQl query returns "2023-06-13 10:54:21", change waiting_date to be slightly less than 1 day/24 hours before this now() timestamp, but still what we would consider 'yesterday'

select * from recalls; # to get the recall_id
update recalls set waiting_date = "2023-06-12 14:54:21" where recall_id = X;

12. Refresh the Circulation -> Recalls awaiting pickup page. The recall will not show up under the 'Recalls waiting over 1 days' tab, even though we expect it to.

13. Apply the patch, restart services, refresh the page
14. Confirm the recall now shows under the 'Recalls waiting over 1 days' tab as expected.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2023-07-16 11:11:58 UTC
Created attachment 153513 [details] [review]
Bug 33992: Only consider the date when auto-expiring problematic recalls

This patch carries this fix into the misc/cronjobs/recalls/expire_recalls.pl cronjob so that recalls are automatically expired when they have been waiting a problematic number of days, not considering hours, as expected.

To test, follow the test plan from the first patch. This will set you up with a waiting problematic recall.

Run the cronjob manually

perl misc/cronjobs/recalls/expire_recalls.pl

Refresh your 'Recalls awaiting pickup' page. Your problematic recall should be gone/expired.

Expiration dates will apply when expiring any 'unfulfilled' recall i.e. newly requested, overdue to be returned, and awaiting pickup.

Sponsored-by: Auckland University of Technology
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Tomás Cohen Arazi (tcohen) 2023-07-19 15:08:40 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 14 Fridolin Somers 2023-08-04 01:35:49 UTC
Pushed to 23.05.x for 23.05.03
Comment 15 Pedro Amorim 2023-08-18 10:53:34 UTC
Nice work everyone!

Pushed to 22.11.x for next release
Comment 16 Lucas Gass (lukeg) 2023-09-13 20:55:23 UTC
Backported to 22.05.x for upcoming 22.05.16