Bug 11995 - Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl
Summary: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on: 7688
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-25 12:51 UTC by Julian Maurice
Modified: 2015-06-04 23:23 UTC (History)
4 users (show)

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


Attachments
Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl (1.95 KB, patch)
2014-03-25 12:52 UTC, Julian Maurice
Details | Diff | Splinter Review
[SIGNED OFF] Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl (2.43 KB, patch)
2014-04-10 03:52 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 11995: Followup for Bug 7688 - FIX GetNextDate in serialsUpdate.pl (2.48 KB, patch)
2014-04-11 11:05 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 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