When longoverdue.pl returns items, it sets old_issues.returndate to the current date. This means that when staff look at the borrowers' circulation history, they see a returndate displayed, which is confusing. We don't lose information by not setting old_issues.returndate, because items.itemlost_on will contain the date that the item was lost.
Created attachment 59560 [details] [review] Bug 17996 - longoverdue.pl should not set 'datereturned' when --mark-returned option is used When longoverdue.pl returns items, it sets old_issues.returndate to the current date. This means that when staff look at the borrowers' circulation history, they see a returndate displayed, which is confusing. We don't lose information by not setting old_issues.returndate, because items.itemlost_on will contain the date that the item was lost. Test Plan: 1) Apply this patch 2) Mark an item lost using longoverdue.pl with --mark-returned 3) Note the item's old_issues line does not have a returndate
In my initial testing, I've found that longoverdue.pl --lost 1=2 --charge=2 --mark-returned --confirm Does indeed check the item in and set old_issues.returndate to NULL, but the item still shows as 'Checked out' on the patron and item circ history. See koha-tmpl/intranet-tmpl/prog/en/modules/members/readingrec.tt line 125 125 [% IF issue.returndate %] 126 <span title="[% issue.returndate %]">[% issue.returndate |$KohaDates with_hours => 1 %]</span> 127 [% ELSE %] 128 <span title="Checked out"><small>Checked out</small></span> 129 [% END %] koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/issuehistory.tt line 86 86 <td>[% IF ( issue.returndate ) %] 87 <span title="[% issue.returndate %]">[% issue.returndate |$KohaDates with_hours => 1 %]</span> 88 [% ELSE %] 89 <span title="Checked out"><small>Checked out</small></span> 90 [% END %]</td> ... unfortunately I don't think that it's as simple as adding a test for lost lost status, because it *is* possible for items to be checked out and lost. the underlying problem is that there's an implicit assumption that an item is checked out if either isssues.returndate or old_issues.returndate is NULL.
Changes in the patch are small and testing should be easy. Can you please rebase? Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 17996 - longoverdue.pl should not set 'datereturned' when --mark-returned option is used Using index info to reconstruct a base tree... M C4/Circulation.pm Falling back to patching base and 3-way merge... Auto-merging C4/Circulation.pm CONFLICT (content): Merge conflict in C4/Circulation.pm Failed to merge in the changes. Patch failed at 0001 Bug 17996 - longoverdue.pl should not set 'datereturned' when --mark-returned option is used The copy of the patch that failed is found in: /home/vagrant/kohaclone/.git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-17996---longoverduepl-should-not-set-dateretur-IwbaAP.patch
Created attachment 100272 [details] [review] Bug 17996 - longoverdue.pl should not set 'datereturned' when --mark-returned option is used When longoverdue.pl returns items, it sets old_issues.returndate to the current date. This means that when staff look at the borrowers' circulation history, they see a returndate displayed, which is confusing. We don't lose information by not setting old_issues.returndate, because items.itemlost_on will contain the date that the item was lost. Test Plan: 1) Apply this patch 2) Mark an item lost using longoverdue.pl with --mark-returned 3) Note the item's old_issues line does not have a returndate
Rebased, but this needs test coverage - the pod is explicit that we set the date, so maybe we have moved on from here
I'm not sure about this patch. If I manually mark an item lost while I have MarkLostItemsAsReturned on, that *also* sets the returndate. And I'm not sure that's wrong? After all, we've elected to mark it as returned.