Summary: | Internal server error when receiving shipment with order with deleted biblio | ||
---|---|---|---|
Product: | Koha | Reporter: | Aleisha Amohia <aleisha> |
Component: | Acquisitions | Assignee: | Aleisha Amohia <aleisha> |
Status: | RESOLVED DUPLICATE | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | jonathan.druart, lari.taskula |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | Sponsored | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 19293: Prevent error when receiving shipment with order with deleted biblio
Bug 19293: Prevent error when receiving shipment with order with deleted biblio [SIGNED-OFF] Bug 19293: Prevent error when receiving shipment with order with deleted biblio [SIGNED-OFF] Bug 19293: Prevent error when receiving shipment with order with deleted biblio Bug 19293: Prevent error when receiving shipment with order with deleted biblio |
Description
Aleisha Amohia
2017-09-12 05:15:22 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 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 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. (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 :) 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> 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> (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. 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> |