Bug 8769 - Allow SIP2 return backdating
Summary: Allow SIP2 return backdating
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact:
URL:
Keywords:
Depends on: 10694
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-13 15:04 UTC by Kyle M Hall
Modified: 2014-12-07 20:07 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 8769 - WIP - Allow SIP2 return backdating (2.56 KB, patch)
2012-09-13 15:05 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (6.06 KB, patch)
2012-10-15 12:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (2.23 KB, patch)
2012-10-15 12:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (2.17 KB, patch)
2012-10-15 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (2.53 KB, patch)
2013-02-11 16:10 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (2.58 KB, patch)
2013-07-31 09:43 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (3.18 KB, patch)
2013-07-31 09:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (3.67 KB, patch)
2013-10-10 11:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (2.84 KB, patch)
2013-12-19 17:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - Allow SIP2 return backdating (3.21 KB, patch)
2014-04-02 09:44 UTC, Benjamin Rokseth
Details | Diff | Splinter Review
Bug 8769 [QA Followup] - Remove unnecessary module import (729 bytes, patch)
2014-04-23 11:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 8769 - Allow SIP2 return backdating (3.30 KB, patch)
2014-04-23 12:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 8769 - (QA Followup) Remove unnecessary module import (789 bytes, patch)
2014-04-23 12:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 8769 - (QA Followup) Rename $today to $return_date_dt (3.05 KB, patch)
2014-05-05 11:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - (QA Followup) Rename $today to $return_date_dt (3.13 KB, patch)
2014-05-05 11:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 8769 - (QA Followup) Undo change to value of $today (761 bytes, patch)
2014-05-05 11:55 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2012-09-13 15:04:32 UTC
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.
Comment 1 Kyle M Hall 2012-09-13 15:05:20 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2012-10-15 12:43:29 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-10-15 12:48:35 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2012-10-15 12:52:01 UTC Comment hidden (obsolete)
Comment 5 Frédéric Demians 2013-02-10 07:39:09 UTC
It works. Could you edit C4::Circulation::AddReturn POD to document the new return_date parameter?
Comment 6 Kyle M Hall 2013-02-11 16:10:46 UTC Comment hidden (obsolete)
Comment 7 Kyle M Hall 2013-07-31 09:43:14 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2013-07-31 09:47:31 UTC Comment hidden (obsolete)
Comment 9 I'm just a bot 2013-09-29 05:57:37 UTC
Patch applied cleanly, go forth and signoff
Comment 10 Kyle M Hall 2013-10-10 11:15:46 UTC Comment hidden (obsolete)
Comment 11 I'm just a bot 2013-10-30 07:05:57 UTC
Patch applied cleanly, go forth and signoff
Comment 12 Kyle M Hall 2013-12-19 17:38:51 UTC Comment hidden (obsolete)
Comment 13 Petter Goksøyr Åsen 2014-01-29 12:29:33 UTC
I'd like to test this, can you rebase?

Thanks!
Comment 14 Dobrica Pavlinusic 2014-03-12 13:55:10 UTC
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
Comment 15 Benjamin Rokseth 2014-04-02 09:44:38 UTC Comment hidden (obsolete)
Comment 16 Colin Campbell 2014-04-22 13:44:53 UTC
A minor issue with the patch as stands. It shouldnt unnecessarily import Koha::DateUtils into ILS/Transaction/CheckIn.pm.
Comment 17 Kyle M Hall 2014-04-23 11:45:05 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2014-04-23 12:34:36 UTC
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>
Comment 19 Martin Renvoize 2014-04-23 12:35:18 UTC
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>
Comment 20 Martin Renvoize 2014-04-23 12:36:07 UTC
This patch is good now, happy to pass qa.
Comment 21 Galen Charlton 2014-05-02 23:05:51 UTC
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?
Comment 22 Kyle M Hall 2014-05-05 11:42:27 UTC Comment hidden (obsolete)
Comment 23 Kyle M Hall 2014-05-05 11:45:44 UTC Comment hidden (obsolete)
Comment 24 Kyle M Hall 2014-05-05 11:55:21 UTC
Created attachment 27946 [details] [review]
Bug 8769 - (QA Followup) Undo change to value of $today
Comment 25 Kyle M Hall 2014-05-05 11:58:20 UTC
(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.
Comment 26 Galen Charlton 2014-05-05 15:06:59 UTC
Pushed to master.  Thanks, Kyle!