Bug 19293 - Internal server error when receiving shipment with order with deleted biblio
Summary: Internal server error when receiving shipment with order with deleted biblio
Status: RESOLVED DUPLICATE of bug 19596
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Aleisha Amohia
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-09-12 05:15 UTC by Aleisha Amohia
Modified: 2017-12-21 13:29 UTC (History)
2 users (show)

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


Attachments
Bug 19293: Prevent error when receiving shipment with order with deleted biblio (6.49 KB, patch)
2017-09-12 05:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 19293: Prevent error when receiving shipment with order with deleted biblio (5.89 KB, patch)
2017-09-14 01:34 UTC, Dilan Johnpullé
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 19293: Prevent error when receiving shipment with order with deleted biblio (6.71 KB, patch)
2017-10-05 04:17 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 19293: Prevent error when receiving shipment with order with deleted biblio (6.70 KB, patch)
2017-10-05 04:18 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 19293: Prevent error when receiving shipment with order with deleted biblio (6.74 KB, patch)
2017-10-09 12:04 UTC, Lari Taskula
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Aleisha Amohia 2017-09-12 05:15:22 UTC
To test:
1) Find or create a basket
2) Add an item to it
3) Find the record for that item in a separate tab, delete the record
4) Notice the order in the basket now says 'deleted bibliographic record'
5) close the basket and receive the shipment
6) After entering an invoice number and clicking next, you'll see the error:
Can't call method "subscriptions" on an undefined value at /home/vagrant/kohaclone/acqui/parcel.pl line 245.

Note that if you cancel the order, this error is not triggered and everything works as expected.
Comment 1 Aleisha Amohia 2017-09-12 05:18:05 UTC
Created attachment 67070 [details] [review]
Bug 19293: Prevent error when receiving shipment with order with deleted biblio

This patch checks if the biblio exists when receiving a shipment, and
will skip the order if the biblio does not exist.

To test:
1) Find or create a basket
2) Add TWO items to it
3) Find the record for one item in a separate tab, delete the record
4) Notice the order in the basket for that item now says 'deleted bibliographic
record'
5) close the basket and receive the shipment
6) After entering an invoice number and clicking next, you'll see the
error:
Can't call method "subscriptions" on an undefined value at
/home/vagrant/kohaclone/acqui/parcel.pl line 245.
7) Apply the patch and go back to the basket to receive the shipment
8) Put in an invoice number and click next
9) This should now work as expected, skipping the deleted biblio, and
the other item should show as ready to receive

Sponsored-by: Catalyst IT
Comment 2 Dilan Johnpullé 2017-09-14 01:34:35 UTC
Created attachment 67127 [details] [review]
Bug 19293: Prevent error when receiving shipment with order with deleted biblio

Patch applies and works as described when test plan is followed.

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Comment 3 Lari Taskula 2017-10-03 12:00:40 UTC
Comment on attachment 67070 [details] [review]
Bug 19293: Prevent error when receiving shipment with order with deleted biblio

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

Test plan works, just some small thoughts for the patch. Also I noticed test plan was dropped from the commit message after the sign-off.

::: acqui/parcel.pl
@@ +225,4 @@
>      for (my $i = 0 ; $i < $countpendings ; $i++) {
>          my $order = $pendingorders->[$i];
>  
> +        unless( !defined $order->{biblionumber} ){ # if this biblio has been deleted and the orderline hasn't been cancelled

if ( defined $order->{biblionumber} ) {

Or perhaps even
next unless defined $order->{biblionumber};
to avoid having to fix indendation of 80+ lines below.
Comment 4 Lari Taskula 2017-10-03 12:03:51 UTC
(In reply to Lari Taskula from comment #3)
> Comment on attachment 67070 [details] [review] [review]
> to avoid having to fix indendation of 80+ lines below.
40+ lines, sorry :)
Comment 5 Aleisha Amohia 2017-10-05 04:17:42 UTC
Created attachment 67624 [details] [review]
[SIGNED-OFF] Bug 19293: Prevent error when receiving shipment with order with deleted biblio

This patch checks if the biblio exists when receiving a shipment, and
will skip the order if the biblio does not exist.

To test:
1) Find or create a basket
2) Add TWO items to it
3) Find the record for one item in a separate tab, delete the record
4) Notice the order in the basket for that item now says 'deleted
bibliographic
record'
5) close the basket and receive the shipment
6) After entering an invoice number and clicking next, you'll see the
error:
Can't call method "subscriptions" on an undefined value at
/home/vagrant/kohaclone/acqui/parcel.pl line 245.
7) Apply the patch and go back to the basket to receive the shipment
8) Put in an invoice number and click next
9) This should now work as expected, skipping the deleted biblio, and
the other item should show as ready to receive

Sponsored-by: Catalyst IT

Patch applies and works as described when test plan is followed.

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Comment 6 Aleisha Amohia 2017-10-05 04:18:43 UTC
Created attachment 67625 [details] [review]
[SIGNED-OFF] Bug 19293: Prevent error when receiving shipment with order with deleted biblio

This patch checks if the biblio exists when receiving a shipment, and
will skip the order if the biblio does not exist.

To test:
1) Find or create a basket
2) Add TWO items to it
3) Find the record for one item in a separate tab, delete the record
4) Notice the order in the basket for that item now says 'deleted
bibliographic
record'
5) close the basket and receive the shipment
6) After entering an invoice number and clicking next, you'll see the
error:
Can't call method "subscriptions" on an undefined value at
/home/vagrant/kohaclone/acqui/parcel.pl line 245.
7) Apply the patch and go back to the basket to receive the shipment
8) Put in an invoice number and click next
9) This should now work as expected, skipping the deleted biblio, and
the other item should show as ready to receive

Sponsored-by: Catalyst IT

Patch applies and works as described when test plan is followed.

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>
Comment 7 Aleisha Amohia 2017-10-05 04:19:17 UTC
(In reply to Lari Taskula from comment #3)
> Comment on attachment 67070 [details] [review] [review]
> Bug 19293: Prevent error when receiving shipment with order with deleted
> biblio
> 
> Review of attachment 67070 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> Test plan works, just some small thoughts for the patch. Also I noticed test
> plan was dropped from the commit message after the sign-off.
> 
> ::: acqui/parcel.pl
> @@ +225,4 @@
> >      for (my $i = 0 ; $i < $countpendings ; $i++) {
> >          my $order = $pendingorders->[$i];
> >  
> > +        unless( !defined $order->{biblionumber} ){ # if this biblio has been deleted and the orderline hasn't been cancelled
> 
> if ( defined $order->{biblionumber} ) {
> 
> Or perhaps even
> next unless defined $order->{biblionumber};
> to avoid having to fix indendation of 80+ lines below.

There we go, changed the line to 
if ( defined $order->{biblionumber} ) {

and put test plan back in commit message.
Comment 8 Lari Taskula 2017-10-09 12:04:51 UTC
Created attachment 67839 [details] [review]
Bug 19293: Prevent error when receiving shipment with order with deleted biblio

This patch checks if the biblio exists when receiving a shipment, and
will skip the order if the biblio does not exist.

To test:
1) Find or create a basket
2) Add TWO items to it
3) Find the record for one item in a separate tab, delete the record
4) Notice the order in the basket for that item now says 'deleted
bibliographic
record'
5) close the basket and receive the shipment
6) After entering an invoice number and clicking next, you'll see the
error:
Can't call method "subscriptions" on an undefined value at
/home/vagrant/kohaclone/acqui/parcel.pl line 245.
7) Apply the patch and go back to the basket to receive the shipment
8) Put in an invoice number and click next
9) This should now work as expected, skipping the deleted biblio, and
the other item should show as ready to receive

Sponsored-by: Catalyst IT

Patch applies and works as described when test plan is followed.

Signed-off-by: Dilan Johnpullé <dilan@calyx.net.au>

Signed-off-by: Lari Taskula <lari.taskula@jns.fi>
Comment 9 Jonathan Druart 2017-12-21 13:29:13 UTC
Fixed already by bug 19596.

*** This bug has been marked as a duplicate of bug 19596 ***