Bug 39267 - Specified due date remains for checkouts even when removing unchecking remember for session
Summary: Specified due date remains for checkouts even when removing unchecking rememb...
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: David Gustafsson
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-03-06 17:31 UTC by David Gustafsson
Modified: 2025-03-07 18:12 UTC (History)
3 users (show)

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


Attachments
Bug 39267: Specified due date remains for checkouts even when removing unchecking remember for session (4.69 KB, patch)
2025-03-07 17:40 UTC, David Gustafsson
Details | Diff | Splinter Review
Bug 39267: Specified due date remains for checkouts even when removing unchecking remember for session (2.50 KB, patch)
2025-03-07 17:50 UTC, David Gustafsson
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Gustafsson 2025-03-06 17:31:56 UTC
When checking out an item for a borrower, choosing a specific due date and checking the "remember for session". If another checkout is done, removing the date and unchecking "remember for session", after the checkout has been performed, the previously set due date is still there. It's difficult to see how the correct behaviour shouldn't be that the date is cleared from the session as well.
Comment 1 David Gustafsson 2025-03-06 17:59:48 UTC
Have a patch but realized needs some more work, there are some unclear dependencies between how templates params are set from cgi and session params that needs to be straightened out. More or less got it to work but believe it's still very hard to follow and ensure correctness.
Comment 2 Gretchen Maxeiner 2025-03-06 19:33:27 UTC
We would also like to see this setting be less sticky. One almost has to log out/log back in to clear that manual due date once it's been remembered.
Comment 3 David Gustafsson 2025-03-07 17:40:07 UTC
Created attachment 179088 [details] [review]
Bug 39267: Specified due date remains for checkouts even when removing unchecking remember for session

To test:
1) Checkout an item for a patron, specify a due date and check remember
   for session.
2) Remove the due date, uncheck remember for session and checkout
   another item.
3) The previously specified due date is filled in again, while remember
   for session remains unchecked.
4) Apply patch
5) Repeat the steps above, in step 3) due date should now remain empty.

Sponsored-by: Gothenburg University Libarary
Comment 4 David Gustafsson 2025-03-07 17:50:07 UTC
Created attachment 179089 [details] [review]
Bug 39267: Specified due date remains for checkouts even when removing unchecking remember for session

To test:
1) Checkout an item for a patron, specify a due date and check remember
   for session.
2) Remove the due date, uncheck remember for session and checkout
   another item.
3) The previously specified due date is filled in again, while remember
   for session remains unchecked.
4) Apply patch
5) Repeat the steps above, in step 3) due date should now remain empty.

Sponsored-by: Gothenburg University Libarary
Comment 5 David Gustafsson 2025-03-07 18:10:08 UTC
In the first patch I moved the block where the stickyduedate session value is set if stickyduedate param or $restoreduedatespec is set (around line 737, at "# Restore adate if changed by holds and/or save stickyduedate to session) further up where the related template variable are set for clarity when reading the code. The only variable affected by this change is $duedatespec  which is validated on line 217, and that validation should not fail if doesn't come from user input. So thought perhaps better to leave that block where it is for a more minimal patch and less risk of introducing new behavior.

The reason for the current behavior is that even if $duedatespec is set to an empty string if the stickyduedate param is not present, it will still be reassigned the previous date in the block mentioned above, as $restoredduedate is assigned as:

my $restoreduedatespec = $query->param('restoreduedatespec') || $duedatespec || $session->param('stickyduedate');

Before the session is cleared. I don't see how this makes any sense and as far as I is not correct.

I originally felt the urge to try to clean up all the different assignments in a way that is easier to follow, but the current code relies on a lot of very subltle and opaque consequences of how these assignments are currently made and to do this without unintended consequences just to fix this isolated issue seems way out of scope for the current patch.
Comment 6 David Gustafsson 2025-03-07 18:12:34 UTC
To clarify, in the assignment above $session->param('stickyduedate') will have the value the previous due date. Clearing the session before this is made seems to resolve the issue, and have not been able to find any unintended consequences of doing so even if it's hard to be completely sure.