| Summary: | Sticky due date calendar unexpected behaviour | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | Circulation | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | major | ||
| Priority: | P5 - low | CC: | director, eivin, f.demians, gmcharlt, jonathan.druart, jschmidt, julian.maurice, kyle.m.hall, kyle |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18885 | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | 10067 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 16527: Restore sticky due date behavior
[SIGNED-OFF] Bug 16527: Restore sticky due date behavior Bug 16527: Restore sticky due date behavior [PASSED QA] Bug 16527: Restore sticky due date behavior |
||
|
Description
Nick Clemens (kidclamp)
2016-05-16 12:56:16 UTC
It seems that it is a regression introduced by bug 10067. I'd say that the following diff will fix it: diff --git a/circ/circulation.pl b/circ/circulation.pl index 66b3dcf..d7fcfbb 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -166,7 +166,7 @@ my $stickyduedate = $query->param('stickyduedate') || $session->param('stickydu my $duedatespec = $query->param('duedatespec') || $session->param('stickyduedate'); $duedatespec = eval { output_pref( { dt => dt_from_string( $duedatespec ), dateformat => 'iso' }); } if ( $duedatespec ); -my $restoreduedatespec = $query->param('restoreduedatespec') || $session->param('stickyduedate') || $duedatespec; +my $restoreduedatespec = $query->param('restoreduedatespec') || $duedatespec || $session->param('stickyduedate'); if ($restoreduedatespec eq "highholds_empty") { undef $restoreduedatespec; } But I have no idea if it won't break bug 10067. Eivin, could you have a look please? This should be fixed before the next release (soon!) Created attachment 51673 [details] [review] Bug 16527: Restore sticky due date behavior Bug 10067 has introduced a regression on the sticky due date behavior: If "remember for session" is checked and the due date is changed from 1 checkout to another, the old due date will be used. The code added by bug 10067 is really confusing and it's hard to say this patch won't introduce another regression. The manipulation of $restoreduedatespec, $duedatespec and $stickyduedate is not easy to follow. Test plan: Confirm that the regression is fixed and that this patch will not introduced a regression on bug 10067 (good luck!) I am without a koha dev env because my laptop died.. Installing another computer today. The stickyduedate behaviour and 10067 is confusing because there is some jquery code that intercepts the request and I had to send that data to serverside somehow to be able to manipulate. (from the top of my head..) If I am able to get a dev env up and running I will look into it. Created attachment 51701 [details] [review] [SIGNED-OFF] Bug 16527: Restore sticky due date behavior Bug 10067 has introduced a regression on the sticky due date behavior: If "remember for session" is checked and the due date is changed from 1 checkout to another, the old due date will be used. The code added by bug 10067 is really confusing and it's hard to say this patch won't introduce another regression. The manipulation of $restoreduedatespec, $duedatespec and $stickyduedate is not easy to follow. Test plan: Confirm that the regression is fixed and that this patch will not introduced a regression on bug 10067 (good luck!) Signed-off-by: Eivin Giske Skaaren <eivin@sysmystic.com> This change seems to fix the issues raised by Nick but I also noticed when testing and trying to understand 10067 again that there has been other changes to the parts of Koha we are dealing with here.. (And the mix of TT, jQuery and Perl to handle this together with the highholds is pretty confusing.) Another regression might be that the decreaseLoanHighHolds does not work anymore. There was several changes to that since last I looked into this and when testing on a new kohadevbox the checkout length was not decreased, even when the new checkbox "Don't decrease checkout length based on holds" was not selected. That has nothing to do with this patch however so I signed off. Created attachment 52508 [details] [review] Bug 16527: Restore sticky due date behavior Bug 10067 has introduced a regression on the sticky due date behavior: If "remember for session" is checked and the due date is changed from 1 checkout to another, the old due date will be used. The code added by bug 10067 is really confusing and it's hard to say this patch won't introduce another regression. The manipulation of $restoreduedatespec, $duedatespec and $stickyduedate is not easy to follow. Test plan: Confirm that the regression is fixed and that this patch will not introduced a regression on bug 10067 (good luck!) Signed-off-by: Eivin Giske Skaaren <eivin@sysmystic.com> Created attachment 52550 [details] [review] [PASSED QA] Bug 16527: Restore sticky due date behavior Bug 10067 has introduced a regression on the sticky due date behavior: If "remember for session" is checked and the due date is changed from 1 checkout to another, the old due date will be used. The code added by bug 10067 is really confusing and it's hard to say this patch won't introduce another regression. The manipulation of $restoreduedatespec, $duedatespec and $stickyduedate is not easy to follow. Test plan: Confirm that the regression is fixed and that this patch will not introduced a regression on bug 10067 (good luck!) Signed-off-by: Eivin Giske Skaaren <eivin@sysmystic.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 16.11, thanks Jonathan! Can we have this fix backported to 3.22? Pushed in 16.05. Will be in 16.05.02. Patch pushed to 3.22.x, will be in 3.22.10 |