Bug 29658 - Crash on cancelling cancelled order
Summary: Crash on cancelling cancelled order
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Marcel de Rooy
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-08 13:45 UTC by Marcel de Rooy
Modified: 2022-11-20 00:34 UTC (History)
10 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:
22.11.00, 22.05.07, 21.11.14


Attachments
Bug 29658: Fix crash on cancelling cancelled order (3.39 KB, patch)
2021-12-08 13:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29658: Fix crash on cancelling cancelled order (3.51 KB, patch)
2022-08-15 12:26 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 29658: Fix crash on cancelling cancelled order (3.61 KB, patch)
2022-08-28 17:22 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 29658: Fix crash on cancelling cancelled order (3.67 KB, patch)
2022-09-12 15:58 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2021-12-08 13:45:41 UTC
See bug 29823.
Comment 1 Marcel de Rooy 2021-12-08 13:55:03 UTC
Created attachment 128364 [details] [review]
Bug 29658: Fix crash on cancelling cancelled order

Found this crash in our 20.11 logs:
Cannot insert order: Mandatory parameter biblionumber is missing at /usr/share/koha/acqui/cancelorder.pl line 60.
 at /usr/share/perl/5.28/Carp.pm line 289
        Carp::croak('Cannot insert order: Mandatory parameter biblionumber is missing') called at /usr/share/koha/Koha/Acquisition/Order.pm line 79
        Koha::Acquisition::Order::store('Koha::Acquisition::Order=HASH(0x55f3760e2860)') called at /usr/share/koha/Koha/Acquisition/Order.pm line 189
        Koha::Acquisition::Order::cancel('Koha::Acquisition::Order=HASH(0x55f3760e2860)', 'HASH(0x55f375a17ec0)') called at /usr/share/koha/acqui/cancelorder.pl line 60

Not sure how to reproduce this one as it happened. But might be related to repeated clicking, backspacing etc.

Test plan:
Create a new basket and order.
Open this same basket in two browser tabs.
Cancel the order line (delete catalog record) in tab 1.
Go to second tab, try again.
Without this patch, it will crash. With this patch, an error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2021-12-08 14:10:26 UTC
Hmm, this even opens up new possibilities like:

Can't call method "edi_order" on an undefined value at /usr/share/koha/acqui/basket.pl line 381

Probably by trying to delete a deleted basket?
Comment 3 Marcel de Rooy 2021-12-08 14:12:38 UTC
(In reply to Marcel de Rooy from comment #2)
> Hmm, this even opens up new possibilities like:
> 
> Can't call method "edi_order" on an undefined value at
> /usr/share/koha/acqui/basket.pl line 381
> 
> Probably by trying to delete a deleted basket?

Not sure about that. Cant exactly reproduce. Lets resolve that somewhere else..
Comment 4 Séverine Queune 2022-05-18 08:50:56 UTC
Hi Marcel,
I couldn't reproduce bug on master (devbox).
When I follow test plan, I don't crash on cancelling order a second time.
I checked data on aqorders table and saw that informations saved with the first cancel action are updated after the second cancel.
I pass my turn on that patch, sorry :/
Comment 5 Joonas Kylmälä 2022-05-28 11:07:27 UTC
(In reply to Séverine Queune from comment #4)
> Hi Marcel,
> I couldn't reproduce bug on master (devbox).
> When I follow test plan, I don't crash on cancelling order a second time.
> I checked data on aqorders table and saw that informations saved with the
> first cancel action are updated after the second cancel.
> I pass my turn on that patch, sorry :/

To reproduce you need to add the order to a biblio with 0 items and click twice  "Cancel order and delete catalog record".
Comment 6 David Nind 2022-05-28 18:31:26 UTC
I was able to reproduce using the tip from Joonas is comment #5 - in koha-testing-docker I added an order to the basket for record 5 (Perl best practices) as it has no items.

However, the patch no longer applies 8-(..

git bz apply 29658

Bug 29658 - Crash on cancelling cancelled order

128364 - Bug 29658: Fix crash on cancelling cancelled order

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 29658: Fix crash on cancelling cancelled order
Using index info to reconstruct a base tree...
M	acqui/cancelorder.pl
M	koha-tmpl/intranet-tmpl/prog/en/modules/acqui/cancelorder.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/cancelorder.tt
Auto-merging acqui/cancelorder.pl
CONFLICT (content): Merge conflict in acqui/cancelorder.pl
error: Failed to merge in the changes.
Patch failed at 0001 Bug 29658: Fix crash on cancelling cancelled order
Comment 7 Ray Delahunty 2022-07-27 13:29:43 UTC
I’m not sure I understand what ‘cancelling a cancelled order’ actually means here. We are having a fiscal period close clean-up, and I too receive an internal server error when I try to delete a basket when all the orders on it have been cancelled and one or more (or all) of the biblios in the orders have been deleted. Is this the same thing as is being reported here, or should I raise a separate bug report? The error I see in our production Koha is simply ‘Internal server error’ but in our test Koha I see: ‘Cannot insert order: Mandatory parameter biblionumber is missing at /home/koha/kohaclone/acqui/basket.pl line 141’.
Comment 8 Marcel de Rooy 2022-07-27 13:32:07 UTC
(In reply to Ray Delahunty from comment #7)
> I’m not sure I understand what ‘cancelling a cancelled order’ actually means
> here. We are having a fiscal period close clean-up, and I too receive an
> internal server error when I try to delete a basket when all the orders on
> it have been cancelled and one or more (or all) of the biblios in the orders
> have been deleted. Is this the same thing as is being reported here, or
> should I raise a separate bug report? The error I see in our production Koha
> is simply ‘Internal server error’ but in our test Koha I see: ‘Cannot insert
> order: Mandatory parameter biblionumber is missing at
> /home/koha/kohaclone/acqui/basket.pl line 141’.

I should still have a closer look to get this going again but at first sight I believe that we are talking about the same bug here.
Comment 9 Marcel de Rooy 2022-08-15 12:26:56 UTC
Created attachment 139126 [details] [review]
Bug 29658: Fix crash on cancelling cancelled order

Found this crash in our 20.11 logs:
Cannot insert order: Mandatory parameter biblionumber is missing at /usr/share/koha/acqui/cancelorder.pl line 60.
 at /usr/share/perl/5.28/Carp.pm line 289
        Carp::croak('Cannot insert order: Mandatory parameter biblionumber is missing') called at /usr/share/koha/Koha/Acquisition/Order.pm line 79
        Koha::Acquisition::Order::store('Koha::Acquisition::Order=HASH(0x55f3760e2860)') called at /usr/share/koha/Koha/Acquisition/Order.pm line 189
        Koha::Acquisition::Order::cancel('Koha::Acquisition::Order=HASH(0x55f3760e2860)', 'HASH(0x55f375a17ec0)') called at /usr/share/koha/acqui/cancelorder.pl line 60

Not sure how to reproduce this one as it happened. But might be related to repeated clicking, backspacing etc.

Test plan:
Create a new basket and order.
Open this same basket in two browser tabs.
Cancel the order line (delete catalog record) in tab 1.
Go to second tab, try again.
Without this patch, it will crash. With this patch, an error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2022-08-15 12:27:40 UTC
(In reply to Ray Delahunty from comment #7)
> I’m not sure I understand what ‘cancelling a cancelled order’ actually means
> here. We are having a fiscal period close clean-up, and I too receive an
> internal server error when I try to delete a basket when all the orders on
> it have been cancelled and one or more (or all) of the biblios in the orders
> have been deleted. Is this the same thing as is being reported here, or
> should I raise a separate bug report? The error I see in our production Koha
> is simply ‘Internal server error’ but in our test Koha I see: ‘Cannot insert
> order: Mandatory parameter biblionumber is missing at
> /home/koha/kohaclone/acqui/basket.pl line 141’.

Could you test the rebase please ?
Comment 11 Ray Delahunty 2022-08-16 07:16:20 UTC
I tested using a sandbox provisioned at 22.06 and have had no problems deleting baskets where the order has been cancelled and the biblio has been deleted. I tried routine workflow and also following the test plan. I did not receive any errors- the basket was just deleted without any fuss. When I run the same workflow in our 21.11 production system I receive the 'Internal server error' as before. I will ask that our support company apply the patch on our test server and will try again once that is done.
Comment 12 Joonas Kylmälä 2022-08-28 17:22:23 UTC
Created attachment 139930 [details] [review]
Bug 29658: Fix crash on cancelling cancelled order

Found this crash in our 20.11 logs:
Cannot insert order: Mandatory parameter biblionumber is missing at /usr/share/koha/acqui/cancelorder.pl line 60.
 at /usr/share/perl/5.28/Carp.pm line 289
        Carp::croak('Cannot insert order: Mandatory parameter biblionumber is missing') called at /usr/share/koha/Koha/Acquisition/Order.pm line 79
        Koha::Acquisition::Order::store('Koha::Acquisition::Order=HASH(0x55f3760e2860)') called at /usr/share/koha/Koha/Acquisition/Order.pm line 189
        Koha::Acquisition::Order::cancel('Koha::Acquisition::Order=HASH(0x55f3760e2860)', 'HASH(0x55f375a17ec0)') called at /usr/share/koha/acqui/cancelorder.pl line 60

Not sure how to reproduce this one as it happened. But might be related to repeated clicking, backspacing etc.

Test plan:
Create a new basket and order.
Open this same basket in two browser tabs.
Cancel the order line (delete catalog record) in tab 1.
Go to second tab, try again.
Without this patch, it will crash. With this patch, an error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 13 Marcel de Rooy 2022-08-29 05:58:56 UTC
(In reply to Joonas Kylmälä from comment #12)
> Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>

Thx Joonas!
Comment 14 Martin Renvoize 2022-09-12 15:58:21 UTC
Created attachment 140472 [details] [review]
Bug 29658: Fix crash on cancelling cancelled order

Found this crash in our 20.11 logs:
Cannot insert order: Mandatory parameter biblionumber is missing at /usr/share/koha/acqui/cancelorder.pl line 60.
 at /usr/share/perl/5.28/Carp.pm line 289
        Carp::croak('Cannot insert order: Mandatory parameter biblionumber is missing') called at /usr/share/koha/Koha/Acquisition/Order.pm line 79
        Koha::Acquisition::Order::store('Koha::Acquisition::Order=HASH(0x55f3760e2860)') called at /usr/share/koha/Koha/Acquisition/Order.pm line 189
        Koha::Acquisition::Order::cancel('Koha::Acquisition::Order=HASH(0x55f3760e2860)', 'HASH(0x55f375a17ec0)') called at /usr/share/koha/acqui/cancelorder.pl line 60

Not sure how to reproduce this one as it happened. But might be related to repeated clicking, backspacing etc.

Test plan:
Create a new basket and order.
Open this same basket in two browser tabs.
Cancel the order line (delete catalog record) in tab 1.
Go to second tab, try again.
Without this patch, it will crash. With this patch, an error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2022-09-12 15:58:46 UTC
Simple fix, no regressions. Passing QA
Comment 16 Tomás Cohen Arazi 2022-09-12 19:42:17 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 17 Marcel de Rooy 2022-09-13 11:13:53 UTC
(In reply to Martin Renvoize from comment #15)
> Simple fix, no regressions. Passing QA

Thanks. Even simple fixes can be long on the way ;)
Comment 18 Lucas Gass 2022-10-31 20:56:40 UTC
Backported to 22.05.x for upcoming 22.05.07 release
Comment 19 Arthur Suzuki 2022-11-14 14:12:03 UTC
applied to 21.11 for 21.11.14
Comment 20 Victor Grousset/tuxayo 2022-11-20 00:34:36 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.