Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fine accordingly.
Created attachment 12193 [details] [review] Bug 8769 - WIP - Allow SIP2 return backdating
Created attachment 12811 [details] [review] Bug 8769 - Allow SIP2 return backdating Patch based on code by Scott Kushner <skushner@mplmain.mtpl.org>
Created attachment 12812 [details] [review] Bug 8769 - Allow SIP2 return backdating Patch based on code by Scott Kushner <skushner@mplmain.mtpl.org>
Created attachment 12813 [details] [review] Bug 8769 - Allow SIP2 return backdating
It works. Could you edit C4::Circulation::AddReturn POD to document the new return_date parameter?
Created attachment 15248 [details] [review] Bug 8769 - Allow SIP2 return backdating
Created attachment 19999 [details] [review] Bug 8769 - Allow SIP2 return backdating Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fines accordingly.
Created attachment 20000 [details] [review] Bug 8769 - Allow SIP2 return backdating Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fines accordingly.
Patch applied cleanly, go forth and signoff
Created attachment 21942 [details] [review] Bug 8769 - Allow SIP2 return backdating Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fines accordingly.
Created attachment 23698 [details] [review] Bug 8769 - Allow SIP2 return backdating Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fines accordingly.
I'd like to test this, can you rebase? Thanks!
Applying: Bug 8769 - Allow SIP2 return backdating fatal: sha1 information is lacking or useless (C4/Circulation.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 8769 - Allow SIP2 return backdating
Created attachment 26753 [details] [review] Bug 8769 - Allow SIP2 return backdating Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fines accordingly. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Works as notified, second date field in SIP checkin (return date) is used as return date. Return shows up in history with correct date. Comments: - patron is fined if return date is before issue date, but that is largely irrelevant anyway. - dependent bug 10694 is marked as Failed QA but seems to have passed?
A minor issue with the patch as stands. It shouldnt unnecessarily import Koha::DateUtils into ILS/Transaction/CheckIn.pm.
Created attachment 27465 [details] [review] Bug 8769 [QA Followup] - Remove unnecessary module import
Created attachment 27466 [details] [review] [PASSED QA] Bug 8769 - Allow SIP2 return backdating Currently Koha's SIP server ignores the return date part of an 09 ( aka CHECKIN ) message. Koha should backdate a return, and remove fines accordingly. Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no> Works as notified, second date field in SIP checkin (return date) is used as return date. Return shows up in history with correct date. Comments: - patron is fined if return date is before issue date, but that is largely irrelevant anyway. - dependent bug 10694 is marked as Failed QA but seems to have passed? Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 27467 [details] [review] [PASSED QA] Bug 8769 - (QA Followup) Remove unnecessary module import Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This patch is good now, happy to pass qa.
Comment on attachment 27466 [details] [review] [PASSED QA] Bug 8769 - Allow SIP2 return backdating Review of attachment 27466 [details] [review]: ----------------------------------------------------------------- ::: C4/Circulation.pm @@ +1780,4 @@ > } > > # case of a return of document (deal with issues and holdingbranch) > + my $today = dt_from_string( $return_date, 'iso' ); This makes me nervous. Besides the comments that Colin made about this change in bug 10694, a variable named $today that doesn't necessarily contain today's date is ... confusing. Also, it is not clear to be why this would be part of the SIP2 change. Can you explain the purpose of this line?
Created attachment 27944 [details] [review] Bug 8769 - (QA Followup) Rename $today to $return_date_dt
Created attachment 27945 [details] [review] Bug 8769 - (QA Followup) Rename $today to $return_date_dt
Created attachment 27946 [details] [review] Bug 8769 - (QA Followup) Undo change to value of $today
(In reply to Galen Charlton from comment #21) > Comment on attachment 27466 [details] [review] [review] > [PASSED QA] Bug 8769 - Allow SIP2 return backdating > > Review of attachment 27466 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: C4/Circulation.pm > @@ +1780,4 @@ > > } > > > > # case of a return of document (deal with issues and holdingbranch) > > + my $today = dt_from_string( $return_date, 'iso' ); > > This makes me nervous. Besides the comments that Colin made about this > change in bug 10694, a variable named $today that doesn't necessarily > contain today's date is ... confusing. Also, it is not clear to be why this > would be part of the SIP2 change. > > Can you explain the purpose of this line? After some examination, I have concluded that this change serves no useful purpose and only causes code obfuscation. I have attached a followup to revert that line of code. I believe the change is vestigial from a time before this feature was dependent on bug 10694.
Pushed to master. Thanks, Kyle!