Bug 26604 - "Generate next" button gives error on serials-collection.pl
Summary: "Generate next" button gives error on serials-collection.pl
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-02 16:10 UTC by Kyle M Hall
Modified: 2020-11-17 09:34 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06, 19.11.12, 19.05.17


Attachments
Bug 26604: "Generate next" button gives error on serials-collection.pl (2.19 KB, patch)
2020-10-02 16:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26604: "Generate next" button gives error on serials-collection.pl (2.19 KB, patch)
2020-10-02 16:24 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 26604: "Generate next" button gives error on serials-collection.pl (2.24 KB, patch)
2020-10-02 17:46 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 26604: "Generate next" button gives error on serials-collection.pl (2.30 KB, patch)
2020-10-05 15:07 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26604: "Generate next" button gives error on serials-collection.pl (2.87 KB, patch)
2020-10-09 13:25 UTC, Jonathan Druart
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 2020-10-02 16:10:49 UTC
In serials-collection.pl, the line

$frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});

causes the error

Can't use string ("13") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Serials.pm line 2198.

This function call makes no sense, that subroutine always returns a list and doesn't take any parameters. I think it was meant to be GetSubscriptionFrequency which is in fact already called a few lines down.
Comment 1 Kyle M Hall 2020-10-02 16:12:30 UTC
Created attachment 111167 [details] [review]
Bug 26604: "Generate next" button gives error on serials-collection.pl

In serials-collection.pl, the line

$frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});

causes the error

Can't use string ("13") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Serials.pm line 2198.

This function call makes no sense, that subroutine always returns a list and doesn't take any parameters. I think it was meant to be GetSubscriptionFrequency which is in fact already called a few lines down.
Comment 2 Kelly McElligott 2020-10-02 16:20:33 UTC
Test Plan:
1. Create a serial 
2 Have a serial with a frequency set for this serial. 
3. Receive some serials.
4. Attempt to generate the next issue, get a software error.
Comment 3 Kyle M Hall 2020-10-02 16:24:25 UTC
Created attachment 111168 [details] [review]
Bug 26604: "Generate next" button gives error on serials-collection.pl

In serials-collection.pl, the line

$frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});

causes the error

Can't use string ("13") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Serials.pm line 2198.

This function call makes no sense, that subroutine always returns a list and doesn't take any parameters. I think it was meant to be GetSubscriptionFrequency which is in fact already called a few lines down.
Comment 4 Kelly McElligott 2020-10-02 16:37:48 UTC
Kyle, error while testing:

Could not compile /kohadevbox/koha/serials/serials-collection.pl: Global symbol "$frequency" requires explicit package name (did you forget to declare "my $frequency"?) at /kohadevbox/koha/serials/serials-collection.pl line 80.
Global symbol "$frequency" requires explicit package name (did you forget to declare "my $frequency"?) at /kohadevbox/koha/serials/serials-collection.pl line 85.
Global symbol "$frequency" requires explicit package name (did you forget to declare "my $frequency"?) at /kohadevbox/koha/serials/serials-collection.pl line 88.
BEGIN not safe after errors--compilation aborted at /kohadevbox/koha/serials/serials-collection.pl line 218.
 at /usr/share/perl5/CGI/Compile.pm line 132
Comment 5 Kyle M Hall 2020-10-02 17:04:17 UTC
(In reply to Kelly McElligott from comment #4)
> Kyle, error while testing:
> 
> Could not compile /kohadevbox/koha/serials/serials-collection.pl: Global
> symbol "$frequency" requires explicit package name (did you forget to
> declare "my $frequency"?) at /kohadevbox/koha/serials/serials-collection.pl
> line 80.
> Global symbol "$frequency" requires explicit package name (did you forget to
> declare "my $frequency"?) at /kohadevbox/koha/serials/serials-collection.pl
> line 85.
> Global symbol "$frequency" requires explicit package name (did you forget to
> declare "my $frequency"?) at /kohadevbox/koha/serials/serials-collection.pl
> line 88.
> BEGIN not safe after errors--compilation aborted at
> /kohadevbox/koha/serials/serials-collection.pl line 218.
>  at /usr/share/perl5/CGI/Compile.pm line 132

I caught that and uploaded a fresh version of the patch! Can you try again?
Comment 6 ByWater Sandboxes 2020-10-02 17:46:46 UTC
Created attachment 111171 [details] [review]
Bug 26604: "Generate next" button gives error on serials-collection.pl

In serials-collection.pl, the line

$frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});

causes the error

Can't use string ("13") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Serials.pm line 2198.

This function call makes no sense, that subroutine always returns a list and doesn't take any parameters. I think it was meant to be GetSubscriptionFrequency which is in fact already called a few lines down.

Signed-off-by: kelly mcelligott <kelly@bywatersolutions.com>
Comment 7 Martin Renvoize 2020-10-05 15:05:02 UTC
Odd.. has this really not been working since 19.05!  It looks like it was introduced with bug 17047
Comment 8 Martin Renvoize 2020-10-05 15:07:19 UTC
Created attachment 111242 [details] [review]
Bug 26604: "Generate next" button gives error on serials-collection.pl

In serials-collection.pl, the line

$frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});

causes the error

Can't use string ("13") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Serials.pm line 2198.

This function call makes no sense, that subroutine always returns a list and doesn't take any parameters. I think it was meant to be GetSubscriptionFrequency which is in fact already called a few lines down.

Signed-off-by: kelly mcelligott <kelly@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-10-05 15:07:42 UTC
Works as expected, passing QA
Comment 10 Jonathan Druart 2020-10-08 07:43:25 UTC
I don't recreate the problem, can you provide me a more detailed test plan please?
Comment 11 Jonathan Druart 2020-10-08 07:46:15 UTC
(In reply to Kelly McElligott from comment #2)
> Test Plan:

> 2 Have a serial with a frequency set for this serial. 

=> This is mandatory when you create a new subscription. Do I have a set something else?

> 3. Receive some serials.

How? One by one, multi receiving? Which status?

> 4. Attempt to generate the next issue, get a software error.

https://snipboard.io/mih40L.jpg
It generates ok for me
Comment 12 Kelly McElligott 2020-10-09 11:25:11 UTC
Jonathan,

1. Receive serial in any fashion.
2. Attempt to generate the next issue- error.


Upon looking at this further, the library's serial was not generating issue numbers.  They had the frequency and starting issue in the subscription ,but Koha was not filling in this issue number- I re-edited the subscription and saved it, then the issue numbers started and the generate next worked.
Comment 13 Jonathan Druart 2020-10-09 13:23:21 UTC
Ok, I managed to recreate it. This is very tricky.

Test plan:
1. Create a new subscription
  * When there is an irregular issue: Skip issue number 
  * Select a frequency (not irregular)
  * Test prediction pattern and mark some "not published" (to fill the subscription.irregular DB field
2. Receive a first serial (mark it arrived, an "expected" one is created)
3. Claim the expected one
=> You have 1 arrived and 1 claimed.
It's the only way to not have an expected one and hit the else block in serials/serials-collection.pl
4. Generate next => boom
Comment 14 Jonathan Druart 2020-10-09 13:25:30 UTC
Created attachment 111399 [details] [review]
Bug 26604: "Generate next" button gives error on serials-collection.pl

In serials-collection.pl, the line

$frequency = C4::Serials::Frequency::GetSubscriptionFrequency($subscription->{periodicity});

causes the error

Can't use string ("13") as a HASH ref while "strict refs" in use at /usr/share/koha/lib/C4/Serials.pm line 2198.

This function call makes no sense, that subroutine always returns a list and doesn't take any parameters. I think it was meant to be GetSubscriptionFrequency which is in fact already called a few lines down.

Signed-off-by: kelly mcelligott <kelly@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

JD

Test plan:
1. Create a new subscription
  * When there is an irregular issue: Skip issue number
  * Select a frequency (not irregular)
  * Test prediction pattern and mark some "not published" (to fill the subscription.irregular DB field
2. Receive a first serial (mark it arrived, an "expected" one is created)
3. Claim the expected one
=> You have 1 arrived and 1 claimed.
It's the only way to not have an expected one and hit the else block in serials/serials-collection.pl
4. Generate next => boom

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2020-10-12 09:31:19 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 16 Lucas Gass 2020-11-10 22:36:56 UTC
backported to 20.05.x for 20.05.06
Comment 17 Aleisha Amohia 2020-11-17 03:55:25 UTC
backported to 19.11.x for 19.11.12
Comment 18 Victor Grousset/tuxayo 2020-11-17 09:34:16 UTC
Backported to 19.05.x branch for 19.05.17