Bug 18467 - Error calling count on undefined bib in basket.pl if order cancelled and record deleted
Summary: Error calling count on undefined bib in basket.pl if order cancelled and reco...
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 18256
Blocks:
  Show dependency treegraph
 
Reported: 2017-04-21 01:11 UTC by Nick Clemens
Modified: 2017-05-13 12:02 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 18467 - Error calling count on undefined bib in basket.pl if order cancelled and record deleted (1.32 KB, patch)
2017-04-21 01:13 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 18467 - Error calling count on undefined bib in basket.pl if order cancelled and record deleted (1.42 KB, patch)
2017-04-21 04:08 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18467: Handle orders with deleted biblio when viewing a basket (4.08 KB, patch)
2017-04-21 16:10 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18467: Handle orders with deleted biblio when viewing a basket (4.08 KB, patch)
2017-04-22 13:09 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18467: Handle orders with deleted biblio when viewing a basket (4.17 KB, patch)
2017-04-22 13:10 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 18467: Handle orders with deleted biblio when viewing a basket (4.23 KB, patch)
2017-04-25 10:07 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2017-04-21 01:11:04 UTC
To recreate:
Create a basket
Add an order
Cancel the order and delete the record
Error loading basket!
Comment 1 Nick Clemens 2017-04-21 01:13:35 UTC
Created attachment 62511 [details] [review]
Bug 18467 - Error calling count on undefined bib in basket.pl if order cancelled and record deleted

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted
Comment 2 Alex Buckley 2017-04-21 04:08:57 UTC
Created attachment 62514 [details] [review]
Bug 18467 - Error calling count on undefined bib in basket.pl if order cancelled and record deleted

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted

Followed test plan and it works as specified
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 3 Jonathan Druart 2017-04-21 16:03:23 UTC
The error is: Can't call method "items" on an undefined value at /home/vagrant/kohaclone/acqui/basket.pl line 470.
Comment 4 Jonathan Druart 2017-04-21 16:08:05 UTC
I think I have a counter-patch.
Comment 5 Jonathan Druart 2017-04-21 16:10:34 UTC
Created attachment 62537 [details] [review]
Bug 18467: Handle orders with deleted biblio when viewing a basket

If the bibliographic record of an order has been removed, $order->{bibionumber}
is undefined. We need to handle this specific case correctly.

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted
Comment 6 Jonathan Druart 2017-04-21 16:12:12 UTC
Nick,
Reading the code I think it's important to clearly highlight this specific case.
Moreover if the bibliographic record does no longer exist, we can skip the whole block.
Let me know if you agree.
Comment 7 Nick Clemens 2017-04-21 16:15:21 UTC
Looks right to me, much cleaner.
Comment 8 Alex Buckley 2017-04-22 13:09:01 UTC
Created attachment 62570 [details] [review]
Bug 18467: Handle orders with deleted biblio when viewing a basket

If the bibliographic record of an order has been removed, $order->{bibionumber}
is undefined. We need to handle this specific case correctly.

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted
Comment 9 Alex Buckley 2017-04-22 13:10:57 UTC
Created attachment 62571 [details] [review]
Bug 18467: Handle orders with deleted biblio when viewing a basket

If the bibliographic record of an order has been removed, $order->{bibionumber}
is undefined. We need to handle this specific case correctly.

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted

Followed test plan, works as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 10 Nick Clemens 2017-04-25 10:07:26 UTC
Created attachment 62654 [details] [review]
Bug 18467: Handle orders with deleted biblio when viewing a basket

If the bibliographic record of an order has been removed, $order->{bibionumber}
is undefined. We need to handle this specific case correctly.

To test:
1 - Create a basket
2 - Order a bib
3 - Cancel order and delete record
4 - You cannot view basket
5 - Apply patch
6 - View basket
7 - There should not be an error
r calling count on undefined bib in basket.pl if order cancelled and
record deleted

Followed test plan, works as intended

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Kyle M Hall 2017-04-28 10:44:23 UTC
Pushed to master for 17.05, thanks Jonathan!
Comment 12 Katrin Fischer 2017-05-13 12:02:18 UTC
This doesn't apply cleanly on 16.11.x, please rebase/comment if it's needed there.