Bug 17996

Summary: longoverdue.pl should not set 'datereturned' when --mark-returned option is used.
Product: Koha Reporter: Barton Chittenden <barton>
Component: Command-line UtilitiesAssignee: Bugs List <koha-bugs>
Status: NEW --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: andrewfh, gmcharlt, katrin.fischer, kyle.m.hall, kyle, nick, robin, tmisilo
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 17996 - longoverdue.pl should not set 'datereturned' when --mark-returned option is used
Bug 17996 - longoverdue.pl should not set 'datereturned' when --mark-returned option is used

Description Barton Chittenden 2017-01-25 19:45:27 UTC
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.
Comment 1 Kyle M Hall 2017-01-25 19:57:05 UTC
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
Comment 2 Barton Chittenden 2017-01-31 19:57:53 UTC
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.
Comment 3 Katrin Fischer 2017-10-15 11:30:33 UTC
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
Comment 4 Nick Clemens (kidclamp) 2020-03-06 14:21:22 UTC
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
Comment 5 Nick Clemens (kidclamp) 2020-03-06 14:22:58 UTC
Rebased, but this needs test coverage - the pod is explicit that we set the date, so maybe we have moved on from here
Comment 6 Andrew Fuerste-Henry 2021-09-23 11:38:11 UTC
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.