Attachment 153424 Details for Bug 31147 – Bug 31147: Recalls should not adjust due time for related checkouts

[patch] Bug 31147: Recalls should not adjust due time for related checkouts

Bug-31147-Recalls-should-not-adjust-due-time-for-r.patch (text/plain), 4.96 KB, created by Tomás Cohen Arazi (tcohen) on 2023-07-13 17:18:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2023-07-13 17:18:03 UTC

Size: 4.96 KB

patch

obsolete

>From b750dd9448e6030df186542f6939360fba5ff7b5 Mon Sep 17 00:00:00 2001
>From: Aleisha Amohia <aleishaamohia@hotmail.com>
>Date: Mon, 10 Jul 2023 03:45:52 +0000
>Subject: [PATCH] Bug 31147: Recalls should not adjust due time for related
> checkouts
>
>The current recalls behaviour adjusts the due date of the most appropriate checkout based on the 'recall due date interval' circulation rule. It also adjusts the due time, which is buggy behaviour. The due date should be adjusted based on the circulation rule, but the due time should remain the same.
>
>To test:
>
>1. Enable the UseRecalls system preference and configure recalls-related circulation and fines rules
>2. Check out Item A to Patron A
>3. Log into the database and check the due date for that checked out item, e.g.
>
>SELECT date_due FROM issues WHERE itemnumber = XXXX;
>
>--> It should be YYYY-MM-DD 23:59:00
>
>4. Log into the OPAC as Patron B and place a recall on Item A. This will force a change to the recall's due date. Check the due date in the database
>
>SELECT date_due FROM issues WHERE itemnumber = XXXX;
>
>--> It should be today + the number of days in your recall due date interval circulation rule, at the exact time you placed the recall. This is buggy behaviour.
>
>5. Apply the patch and restart services
>6. Check in Item A, do NOT fulfill the recall
>7. Cancel the recall
>
>8. Check out Item A to Patron A
>9. Log into the database and check the due date for that checked out item, e.g.
>
>SELECT date_due FROM issues WHERE itemnumber = XXXX;
>
>--> It should be YYYY-MM-DD 23:59:00
>
>10. Log into the OPAC as Patron B and place a recall on Item A. This will force a change to the recall's due date. Check the due date in the database
>
>SELECT date_due FROM issues WHERE itemnumber = XXXX;
>
>--> It should be today + the number of days in your recall due date interval circulation rule, with time of 23:59:00.
>
>11. Check in Item A, do NOT fulfill the recall
>12. Cancel the recall
>
>13. Check out Item A to Patron A. Specify a due date and change the due time so it isn't 23:59.
>14. Log into the OPAC as Patron B and place a recall on Item A. This will force a change to the recall's due date. Check the due date in the database
>
>SELECT date_due FROM issues WHERE itemnumber = XXXX;
>
>--> It should be today + the number of days in your recall due date interval circulation rule, with time you set in Step 13.
>
>15. Confirm tests pass t/db_dependent/Koha/Recalls.t
>
>Sponsored-by: Catalyst IT
>Signed-off-by: Sam Lau <samalau@gmail.com>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
>---
> Koha/Recalls.pm               | 10 ++++++++--
> t/db_dependent/Koha/Recalls.t | 10 ++++++++--
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
>diff --git a/Koha/Recalls.pm b/Koha/Recalls.pm
>index 5526f7d5299..29be68ce05a 100644
>--- a/Koha/Recalls.pm
>+++ b/Koha/Recalls.pm
>@@ -142,8 +142,14 @@ sub add_recall {
>         });
>         my $due_interval = defined $recall_due_date_interval ? $recall_due_date_interval->rule_value : 5;
>         my $timestamp = dt_from_string( $recall->timestamp );
>-        my $due_date = $timestamp->add( days => $due_interval );
>-        $checkout->update({ date_due => $due_date });
>+        my $checkout_timestamp = dt_from_string( $checkout->date_due );
>+        my $due_date = $timestamp->set(
>+            {
>+                hour   => $checkout_timestamp->hour, minute => $checkout_timestamp->minute,
>+                second => $checkout_timestamp->second
>+            }
>+        )->add( days => $due_interval );
>+        $checkout->update( { date_due => $due_date } );
> 
>         # get itemnumber of most relevant checkout if a biblio-level recall
>         unless ( $recall->item_level ) { $itemnumber = $checkout->itemnumber; }
>diff --git a/t/db_dependent/Koha/Recalls.t b/t/db_dependent/Koha/Recalls.t
>index 63d4c0dd055..a32ed768cb4 100755
>--- a/t/db_dependent/Koha/Recalls.t
>+++ b/t/db_dependent/Koha/Recalls.t
>@@ -136,8 +136,14 @@ is( $due_interval, 3, "Recall due date interval is based on circulation rules" )
> });
> is( $recall->item_level, 0, "No item provided so recall not flagged as item-level" );
> 
>-my $expected_due_date = dt_from_string->add( days => 3 );
>-is( t::lib::Dates::compare( $recall->checkout->date_due, $expected_due_date ), 0, "Checkout due date has correctly been extended by recall_due_date_interval days" );
>+my $checkout_timestamp = dt_from_string( $recall->checkout->date_due );
>+my $expected_due_date  = dt_from_string->set(
>+    { hour => $checkout_timestamp->hour, minute => $checkout_timestamp->minute, second => $checkout_timestamp->second }
>+)->add( days => 3 );
>+is(
>+    t::lib::Dates::compare( $recall->checkout->date_due, $expected_due_date ), 0,
>+    "Checkout due date has correctly been extended by recall_due_date_interval days"
>+);
> is( t::lib::Dates::compare( $due_date, $expected_due_date ), 0, "Due date correctly returned" );
> 
> my $messages_count = Koha::Notice::Messages->search({ borrowernumber => $patron3->borrowernumber, letter_code => 'RETURN_RECALLED_ITEM' })->count;
>-- 
>2.41.0

Actions: View | Diff | Splinter Review

Attachments on bug 31147: 153247 | 153283 | 153339 | 153424