Bug 11995

Summary: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: SerialsAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: colin.campbell, fridolin.somers, gmcharlt, jonathan.druart
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 7688    
Bug Blocks:    
Attachments: Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl
[SIGNED OFF] Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl
Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl

Description Julian Maurice 2014-03-25 12:51:16 UTC
Bug 7688 changes the prototype for GetNextDate. The serialsUpdate.pl
cronjob script has not been updated.
Comment 1 Julian Maurice 2014-03-25 12:52:15 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2014-04-07 17:19:04 UTC
Comment on attachment 26559 [details] [review]
Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl

Review of attachment 26559 [details] [review]:
-----------------------------------------------------------------

::: misc/cronjobs/serialsUpdate.pl
@@ +110,4 @@
>  while ( my $issue = $sth->fetchrow_hashref ) {
>  
>      my $subscription = &GetSubscription( $issue->{subscriptionid} );
> +    my $publisheddate  = $issue->{publisheddate};

What is the difference between publisheddate and planneddate? This is the only thing holding me back from signing off.

@@ +114,3 @@
>  
> +    if ( $subscription && $publisheddate && $publisheddate ne "0000-00-00" ) {
> +        my $nextpublisheddate = GetNextDate( $subscription, $publisheddate );

Yikes! Swapping parameter order is kind of important in non-hashref parameter lists. Good thing you are fixing this. :)
Comment 3 Jonathan Druart 2014-04-08 07:55:58 UTC
(In reply to M. Tompsett from comment #2)
> Comment on attachment 26559 [details] [review] [review]
> >      my $subscription = &GetSubscription( $issue->{subscriptionid} );
> > +    my $publisheddate  = $issue->{publisheddate};
> 
> What is the difference between publisheddate and planneddate? This is the
> only thing holding me back from signing off.

publisheddate is the date of the publication and planneddate is the planned date of the reception (and becomes the date of the reception, when it has been done).
Before bug 7688, the GetNextDate was called with planneddate sometimes and publisheddate other times. To get the next published date, it is logical to be based on the current published date (and not planneddate, which could be different for each library).
Comment 4 Mark Tompsett 2014-04-10 03:52:45 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2014-04-11 11:05:47 UTC
Created attachment 27008 [details] [review]
Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl

Bug 7688 changes the prototype for GetNextDate. The serialsUpdate.pl
cronjob script has not been updated.

Test plan:
Before applying the patch:
1/ Check that the following SQL query returns something:
SELECT serial.*
FROM serial
LEFT JOIN subscription ON (subscription.subscriptionid = serial.subscriptionid)
WHERE serial.status = 1
AND DATE_ADD(planneddate, INTERVAL CAST(graceperiod AS SIGNED) DAY) < NOW()
AND subscription.closed = 0;
2/ Run misc/cronjobs/serialsUpdate.pl -v
   It should die with an error message like this:
   Can't use string ("2011-03-05") as a HASH ref while "strict refs" in use
3/ Apply the patch
4/ Run misc/cronjobs/serialsUpdate.pl -v
   It should exit normally and print messages like this:
   Serial issue with id=XX updated
5/ Run the Koha QA test tools.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

NOTE: The only file modified is misc/cronjobs/serialsUpdate.pl
      The error is caused by switching the order of a hash and
      parameter. This also corrects the date used to be
      publisheddate instead of planneddate, because the latter
      does not logically predict the next one. See comment #3.
      The id=XX's listed in step 4 match the ids displayed by
      the SQL query in step 1.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Galen Charlton 2014-04-11 16:28:59 UTC
Pushed to master.  Thanks, Jonathan!
Comment 7 Fridolin Somers 2014-08-06 08:01:18 UTC
Pushed to 3.14.x, will be in 3.14.10