In C4/Members.pm: 2379 sub IssueSlip { 2380 my ($branch, $borrowernumber, $quickslip) = @_; 2381 2382 # return unless ( C4::Context->boolean_preference('printcirculationslips') ); 2383 2384 my $now = POSIX::strftime("%Y-%m-%d", localtime); 2385 2386 my $issueslist = GetPendingIssues($borrowernumber); 2387 foreach my $it (@$issueslist){ 2388 if ((substr $it->{'issuedate'}, 0, 10) eq $now || (substr $it->{'lastreneweddate'}, 0, 10) eq $now) { 2389 $it->{'now'} = 1; 2390 } 2391 elsif ((substr $it->{'date_due'}, 0, 10) le $now) { 2392 $it->{'overdue'} = 1; 2393 } 2394 my $dt = dt_from_string( $it->{'date_due'} ); 2395 $it->{'date_due'} = output_pref( $dt );; 2396 } At line 2384, $now is being defined with the format "%Y-%m-%d", however this is being compared against 'issuedate', 'lastrenewdate' and 'date_due'. 'date_due' is formatted '09/04/2014 23:59' The comparison only uses the first 10 characters of 'date_due', so we can ignore the timestamp at the end, but it's trying to compare 'YYYY-MM-DD' against 'MM/YY/DDDD', e.g. 2014-08-28 against 09/04/2014 -- This is a string comparison; so the initial '0' in '09/04/2014' sorts before the '2' in '2014-08-28'... since the date due is less than $now, the 'overdue' flag is set for this item. I believe that all of the dates involved (issuedate, lastreneweddate, and date_due) are Koha::DateUtils objects; as such it should be possible to define $now the same way, and use Koha::DateUtils methods directly for date comparisons, rather than doing string comparisons.
All items checked out by a patron will show between <overdue></overdue> tags in ISSUESLIP. To replicate: go to cgi-bin/koha/tools/letter.pl?op=add_form&module=circulation&code=ISSUESLIP Add the following section <h4>Overdues</h4> <overdue> <p> <<biblio.title>> <br /> Date due: <<issues.date_due>> at <<issues.branchcode>><br /> </p> </overdue> Check out an item. to a sample patron. Print the issue slip for that patron. Even though the item is not overdue, it will show in the 'Overdues' section.
Created attachment 31280 [details] [review] Bug 12847 - Date comparisons in C4::Members::IssueSlip are broken Test Plan: 1) Ensure you have the default Koha issue slip 2) Check out an item 3) Print the issue slip for that patron 4) Even though the item is not overdue, it will show in the 'Overdues' section 5) Apply this patch 6) Re-print the issue slip 7) The item should no longer be in the overdues section
Created attachment 31281 [details] [review] Bug 12847 [Follwup] - Remove code that calculates "overdue" key as C4::Members::GetPendingIssues already creates it
Created attachment 31286 [details] [review] Bug 12847 [Followup] - Remove code that calculates "overdue" key as C4::Members::GetPendingIssues already creates it
Impossible to reproduce this bug. Following the test plan, at step 4, I don't have last issue printed in the 'overdue' section.
(In reply to Frédéric Demians from comment #5) > Impossible to reproduce this bug. Following the test plan, at step 4, I > don't have last issue printed in the 'overdue' section. I think it will depend on the date, also the syspref you have set. eg if you have it dd/mm/yyyy 28/10/2014 > 2014-10-28 .. so i think you need to make sure you have it set to mm/dd/yyyy (american format)
(In reply to Chris Cormack from comment #6) > 28/10/2014 > 2014-10-28 .. so i think you need to make sure you have it set > to mm/dd/yyyy (american format) I already tested switching dateformat do dd/mm/yyyy. It doesn't help. Sorry.
(In reply to Frédéric Demians from comment #7) > (In reply to Chris Cormack from comment #6) > > > 28/10/2014 > 2014-10-28 .. so i think you need to make sure you have it set > > to mm/dd/yyyy (american format) > > I already tested switching dateformat do dd/mm/yyyy. It doesn't help. Sorry. Frédéric: did you test dd/mm/yyyy or mm/dd/yyyy? The latter is the one we're having trouble with. Your reply above suggests that you tested the former, but I can't tell if you mis-typed your answer or mis-tested. No big deal either way, I'm just looking for clarification.
(In reply to Barton Chittenden from comment #8) > Frédéric: did you test dd/mm/yyyy or mm/dd/yyyy? The latter is the one we're > having trouble with. Your reply above suggests that you tested the former, > but I can't tell if you mis-typed your answer or mis-tested. > > No big deal either way, I'm just looking for clarification. Sorry. I've tried all values for dateformat syspref, and it always works on master. I can't understand how it wouldn't work, indeed. You said: 'date_due' is formatted '09/04/2014 23:59' But this is not what I have. I have yyyy/mm/dd hh:mm, and so the code in master seems valid. There is a mystery...
Is this really 3.14, or should it be master?
I think the other problem blocking this is that people haven't been able to see the issue, could we get a more detailed test plan? Maybe with an example date?
Created attachment 35969 [details] [review] Bug 12847 [Follwup] - Remove code that calculates "overdue" key as C4::Members::GetPendingIssues already creates it Signed-off-by: Laurie McKee <lmckee@littleelm.org>
(In reply to Katrin Fischer from comment #11) > I think the other problem blocking this is that people haven't been able to > see the issue, could we get a more detailed test plan? > Maybe with an example date? Could we get an answer? Please add the syspref value + the dates to reproduce the issue. Confirm the issue is still on master. Back to assigned.
Taking a guess at problems I had in bug 13679 I was able to recreate the issue today. Materials needed to be issued prior to today, but due today. I checked out an item as due today (February 19, 2015) , updated the issuedate to set it to 8 days prior (February 11, 2015), then printed a due slip: item showed as overdue Applied patch: Item no longer showed as overdue.
Created attachment 36048 [details] [review] [SIGNED OFF] Bug 12847 - Date comparisons in C4::Members::IssueSlip are broken Test Plan: 1) Ensure you have the default Koha issue slip 2) Check out an item 3) Print the issue slip for that patron 4) Even though the item is not overdue, it will show in the 'Overdues' section 5) Apply this patch 6) Re-print the issue slip 7) The item should no longer be in the overdues section Signed-off-by: Nick <Nick@quechelibrary.org>
Created attachment 36049 [details] [review] [SIGNED OFF] Bug 12847 [Follwup] - Remove code that calculates "overdue" key as C4::Members::GetPendingIssues already creates it Signed-off-by: Laurie McKee <lmckee@littleelm.org> Signed-off-by: Nick <Nick@quechelibrary.org>
Kyle (or someone else), Could you add some tests to highlight the problem?
*** Bug 8813 has been marked as a duplicate of this bug. ***
Failing QA for missing tests.
I can no longer reproduce this issue. It appears to be fixed in master.
I just got a screenshot proof from a library on 3.18.5.1 - it's still happening. Item is due today, 23:59 Dateformat is set to metric. ISSUSLIP <checkedout> <p> <<biblio.title>> <br /> Signatur: <<items.itemcallnumber>><br/> Barcode: <<items.barcode>><br /> Fällig am: <<issues.date_due>><br /> </p> </checkedout> <br /> <h4>Überfällig</h4> <overdue> <p> <<biblio.title>> <br /> Signatur: <<items.itemcallnumber>><br/> Barcode: <<items.barcode>><br /> Fällig am: <<issues.date_due>><br /> </p> </overdue>
Created attachment 37908 [details] screenshot of the wrong display note date on top and overdues (Überfällig) due date
Kyle, I am not able to reproduce the issue, and looking at the cait's data: issuedate: '2015-04-15 16:16:14' lastreneweddate: NULL I really don't understand how this code (C4::Members::IssueSlip) 2443 if ((substr $it->{'issuedate'}, 0, 10) eq $now || (substr $it->{'lastreneweddate'}, 0, 10) eq $now) { 2444 $it->{'now'} = 1; 2445 } 2446 elsif ((substr $it->{'date_due'}, 0, 10) le $now) { 2447 $it->{'overdue'} = 1; 2448 } could set overdue :) All what I know is that in 3.18.05.1, GetPendingIssues does some stuff with the date_due using DateTime::Format::DateParse, which has been removed from master. Anyway, the overdue flag is set in GetPendingIssues, why don't you remove all the code around the date manipulations?
Even if we weren't able to figure it out yet, this: 2446 elsif ((substr $it->{'date_due'}, 0, 10) le $now) { 2447 $it->{'overdue'} = 1; ... is wrong, if you think of hourly loans, I think? So we shouldn't keep that alive. I can see that we need the flag now for the Quick slip, but why do we need to mangle with the overdue status here...?
I think I managed to create a test case: 1) Check out an item today, due date is today 23.59 2) Check out an item yesterday (change issue date in db), due date is today 23:59 ... and the ISSUESLIP is reliably wrong it appears.
So, confirmed on master. make sure you have: item1 => date_due = today, issuedate = yesterday item2 => date_due = today, issuedate = today
Kyle, would you be ok squashing the patches?
Kyle, some questions have been asked, please have a look. Also tests are missing.
I will try to provide a patch today.
Created attachment 37946 [details] [review] Bug 12847: Items issued today is considered as overdue The date comparisons in C4::Members::IssueSlip does not work as expected. Is an item is issue yesterday and due today (23:59), it should not be considered as an overdue yet. Test plan: Define a valid issue slip (code ISSUESLIP) Check 2 items out and update the issuedate value for one of them as yesterday (using the mariadb/mysql cli or similar) Print the slip Before this patch the item marked as issued yesterday is considered as overdue. Special cases: - hourly loans - Quick slip is impacted too
Created attachment 37992 [details] [review] [SIGNED OFF] Bug 12847: Items issued today is considered as overdue The date comparisons in C4::Members::IssueSlip does not work as expected. Is an item is issue yesterday and due today (23:59), it should not be considered as an overdue yet. Test plan: Define a valid issue slip (code ISSUESLIP) Check 2 items out and update the issuedate value for one of them as yesterday (using the mariadb/mysql cli or similar) Print the slip Before this patch the item marked as issued yesterday is considered as overdue. Special cases: - hourly loans - Quick slip is impacted too Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 38020 [details] [review] [PPASSED QA] Bug 12847: Items issued today is considered as overdue The date comparisons in C4::Members::IssueSlip does not work as expected. Is an item is issue yesterday and due today (23:59), it should not be considered as an overdue yet. Test plan: Define a valid issue slip (code ISSUESLIP) Check 2 items out and update the issuedate value for one of them as yesterday (using the mariadb/mysql cli or similar) Print the slip Before this patch the item marked as issued yesterday is considered as overdue. Special cases: - hourly loans - Quick slip is impacted too Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Patch pushed to master. Thanks Jonathan!
Doesn't apply to 3.18.x please rebase if it is needed
It's needed - library found this in 3.18.5.1
Right, well you've (collective you) have a few hours to get it in a state to apply on 3.18.x (until Bernardo sends the po files and I do the release that is)
Trying to see if I can fix it
Created attachment 38302 [details] [review] [3.18.x] Bug 12847: Items issued today is considered as overdue The date comparisons in C4::Members::IssueSlip does not work as expected. Is an item is issue yesterday and due today (23:59), it should not be considered as an overdue yet. Test plan: Define a valid issue slip (code ISSUESLIP) Check 2 items out and update the issuedate value for one of them as yesterday (using the mariadb/mysql cli or similar) Print the slip Before this patch the item marked as issued yesterday is considered as overdue. Special cases: - hourly loans - Quick slip is impacted too Signed-off-by: Nick Clemens <nick@quecheelibrary.org> Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de> Bug 13601 is not in 3.18.x, so I removed the change connected to that and it still tested ok.
Pushed to 3.18.x will be in 3.18.6