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. To recreate the issue: - turn on recalls with UseRecalls and scaffold out the minimal circulation rules needed to use the feature - set the recall due date interval to 7 days or any other arbitrary period - 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) - as a second patron, place a recall on the item in question Actual behavior: the item's due date is extended to the current date plus the recalls due date interval. Desired behavior: the item's due date should stay the same so the recalling patron doesn't face an additional delay in obtaining it. The logic is that the new due date should be either the existing due date or the current date plus the recall due date interval, whichever is sooner.
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