| Summary: | longoverdue.pl should not set 'datereturned' when --mark-returned option is used. | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Barton Chittenden <barton> |
| Component: | Command-line Utilities | Assignee: | Bugs List <koha-bugs> |
| Status: | NEW --- | QA Contact: | Testopia <testopia> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | andrew, gmcharlt, 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: | ||
| Circulation function: | |||
| 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
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. |