Summary: | Recalls can inadvertently extend the due date | ||
---|---|---|---|
Product: | Koha | Reporter: | Eric Phetteplace <ephetteplace> |
Component: | Circulation | Assignee: | Janusz Kaczmarek <januszop> |
Status: | RESOLVED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | minor | ||
Priority: | P5 - low | CC: | aleisha, fridolin.somers, gmcharlt, januszop, kelly, kyle.m.hall, lucas, m.de.rooy, wainuiwitikapark |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36209 | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Trivial patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
24.11.00,24.05.04,23.11.10
|
Circulation function: | |
Attachments: |
Bug 32696: Recalls can inadvertently extend the due date
Bug 32696: Unit tests Bug 32696: Recalls can inadvertently extend the due date Bug 32696: Unit tests Bug 32696: Recalls can inadvertently extend the due date Bug 32696: Unit tests |
Description
Eric Phetteplace
2023-01-20 23:08:28 UTC
I am changing this from an enhancement bug to a minor bug, as this is an issue with the due dates of recalls. The issue with adjusting the checkout due date when placing recall is even more complicated and weird. Consider the following scenario: 1. the issue is already overdue when placing the recall and has fine accruing; 2. adjusting due date, as it is now, stop accruing (since there is no longer overdue - cf. fines.pl); 3. when in turn, for such a case, a recall overdues, the previous fine will be overwritten by a new sum calculated from recall overdue fines. But, if we didn't update the checkout due date, the new fine would be calculated as a product of total overdue days and recall overdue fine. In case when the recall overdue fine were different (like e.g. 5 x the regular overdue fine), the newly calculated fine would be surprisingly high. This leads to the conclusion that the issue is not as simple. The checkout due date should definitely not be moved forward, but the method of fine calculation should probably be modified. This may mean that we should record the recall due date separately (in the recalls table?). Created attachment 168640 [details] [review] Bug 32696: Recalls can inadvertently extend the due date If an item is due sooner than the recall due date interval then placing a recall on it can inadvertently extend the book's due date, possibly causing the recalling patron to receive the item later than they would otherwise. Test plan: ========= 1. Turn on recalls with UseRecalls. 2. In circulation rules, set the recall due date interval to 7 days or any other arbitrary period. 3. Check out an item to one patron & set the due date to be before the recall due date interval will have elapsed (e.g. the next day) 4. As a second patron, place a recall on the item in question. 5. See that the item's due date is extended to the current date plus the recalls due date interval. 6. Apply the patch, restart_all; 7. Repeat steps 3. and 4. See that the due date has not been extended. BTW, the calculation of $due_interval has been changed, because with the current code and empty 'Recall due date interval' $due_interval is undefined, despite the intention of the author of the code. (after calling get_effective_rule $recall_due_date_interval is defined, but $recall_due_date_interval->rule_value is undefined; the patron gets a message: '... return the item within days, by ...' - no days count). Sponsored-by: Ignatianum University in Cracow Created attachment 168641 [details] [review] Bug 32696: Unit tests Added unit tests. Created attachment 168764 [details] [review] Bug 32696: Recalls can inadvertently extend the due date If an item is due sooner than the recall due date interval then placing a recall on it can inadvertently extend the book's due date, possibly causing the recalling patron to receive the item later than they would otherwise. Test plan: ========= 1. Turn on recalls with UseRecalls. 2. In circulation rules, set the recall due date interval to 7 days or any other arbitrary period. 3. Check out an item to one patron & set the due date to be before the recall due date interval will have elapsed (e.g. the next day) 4. As a second patron, place a recall on the item in question. 5. See that the item's due date is extended to the current date plus the recalls due date interval. 6. Apply the patch, restart_all; 7. Repeat steps 3. and 4. See that the due date has not been extended. BTW, the calculation of $due_interval has been changed, because with the current code and empty 'Recall due date interval' $due_interval is undefined, despite the intention of the author of the code. (after calling get_effective_rule $recall_due_date_interval is defined, but $recall_due_date_interval->rule_value is undefined; the patron gets a message: '... return the item within days, by ...' - no days count). Sponsored-by: Ignatianum University in Cracow Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 168765 [details] [review] Bug 32696: Unit tests Added unit tests. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Created attachment 170438 [details] [review] Bug 32696: Recalls can inadvertently extend the due date If an item is due sooner than the recall due date interval then placing a recall on it can inadvertently extend the book's due date, possibly causing the recalling patron to receive the item later than they would otherwise. Test plan: ========= 1. Turn on recalls with UseRecalls. 2. In circulation rules, set the recall due date interval to 7 days or any other arbitrary period. 3. Check out an item to one patron & set the due date to be before the recall due date interval will have elapsed (e.g. the next day) 4. As a second patron, place a recall on the item in question. 5. See that the item's due date is extended to the current date plus the recalls due date interval. 6. Apply the patch, restart_all; 7. Repeat steps 3. and 4. See that the due date has not been extended. BTW, the calculation of $due_interval has been changed, because with the current code and empty 'Recall due date interval' $due_interval is undefined, despite the intention of the author of the code. (after calling get_effective_rule $recall_due_date_interval is defined, but $recall_due_date_interval->rule_value is undefined; the patron gets a message: '... return the item within days, by ...' - no days count). Sponsored-by: Ignatianum University in Cracow Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 170439 [details] [review] Bug 32696: Unit tests Added unit tests. Signed-off-by: Roman Dolny <roman.dolny@jezuici.pl> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Makes sense to me. Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.x for upcoming 24.05.04 Pushed to 23.11.x for 23.11.10 Not backporting to 23.05.x unless requested Not backporting to 22.11 |