Bug 10921 - You can edit an order even when it is in a closed basket
Summary: You can edit an order even when it is in a closed basket
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-20 10:33 UTC by mathieu saby
Modified: 2020-10-20 21:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Manual merge request:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.05, 19.11.11


Attachments
Bug 10921: Prevent an order from a closed basket to be edited (3.38 KB, patch)
2020-07-31 09:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10921: Prevent an order from a closed basket to be edited (3.45 KB, patch)
2020-09-03 20:29 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 10921: Prevent an order from a closed basket to be edited (3.51 KB, patch)
2020-09-11 13:37 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 mathieu saby 2013-09-20 10:33:12 UTC
You can edit an order even when it is in a closed basket.

I think it should be only allowed to some librarians.
In fact, there is a piece of code supposed to make a control, but it is broken :
In neworderempty.tt, I think the [% IF ( close ) %] condition is always false, or is not true when it need to be, because in neworderempty.pl, "close" parameter is not passed to the template if you try to look at an order in a closed basket (by tweaking the url of an open order for ex)

in perl file : 

my $close           = $input->param('close');
[...]
if ($close) {
    $budget_id      =  $data->{'budget_id'};
    $budget_name    =   $budget->{'budget_name'};

}
[...]
# fill template
$template->param(
    close        => $close,
    budget_id    => $budget_id,
    budget_name  => $budget_name
) if ($close);


M. Saby
Comment 1 mathieu saby 2013-09-20 10:51:06 UTC
To show what it should like, try to add "&close=1" at the end of the URL of an order.

Mathieu
Comment 2 Jonathan Druart 2020-07-31 09:23:55 UTC
Created attachment 107624 [details] [review]
Bug 10921: Prevent an order from a closed basket to be edited

We don't allow editing of orders that are part of a closed basket, but
we don't enforce the rule in the controller file.

This patch use output_and_exit to stop the script and display an error
to the end user.

Test plan:
Create a basket, add an order
On the basket view you see the "Modify" link, open it in a separate tab
=> You can edit the basket
Keep this tab open, get back to the other one and close the basket
Reload the tab with the order edition form
=> You cannot longer edit the basket

QA: Do we need a check in addorder.pl as well?
Comment 3 Henry Bolshaw 2020-09-03 20:25:57 UTC
Sorry Jonathan, I've tried testing this but I can't get the error to display. Where should I see it?

These were the steps I followed:

- Create a basket
- Add an order
- Open the modify order link in a new tab: neworderempty.pl?ordernumber=[ordernumber]&booksellerid=[booksellerid]&basketno=[basketnumber] (Is this the right link?)
- Close the basket
- Refresh the modify order tab
- I can't edit the quantity  and get "You can't add a new item, please create a new order line" (but this is the same behaviour with/without the patch)
- However, I can still change the fund and edit the currency, price, notes etc.

I also tried the same method with the 'edit basket' link - basketheader.pl?booksellerid=[booksellerid]&basketno=[basketnumber]&op=add_form - and was also still able to edit the basket name and other details when the basket was closed.
Comment 4 Henry Bolshaw 2020-09-03 20:28:37 UTC
Sorry my mistake, I think I failed to apply the patch properly. I'll test again now!
Comment 5 PTFS Europe Sandboxes 2020-09-03 20:29:53 UTC
Created attachment 109639 [details] [review]
Bug 10921: Prevent an order from a closed basket to be edited

We don't allow editing of orders that are part of a closed basket, but
we don't enforce the rule in the controller file.

This patch use output_and_exit to stop the script and display an error
to the end user.

Test plan:
Create a basket, add an order
On the basket view you see the "Modify" link, open it in a separate tab
=> You can edit the basket
Keep this tab open, get back to the other one and close the basket
Reload the tab with the order edition form
=> You cannot longer edit the basket

QA: Do we need a check in addorder.pl as well?

Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Comment 6 Henry Bolshaw 2020-09-03 20:33:41 UTC
Now that I've applied the patch (sorry about that!) I have followed the test plan and can no longer modify an order in a closed basket.

I can still edit the basket details but if that's a problem then it should be a separate bug.

Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Comment 7 Nick Clemens 2020-09-11 13:37:46 UTC
Created attachment 109945 [details] [review]
Bug 10921: Prevent an order from a closed basket to be edited

We don't allow editing of orders that are part of a closed basket, but
we don't enforce the rule in the controller file.

This patch use output_and_exit to stop the script and display an error
to the end user.

Test plan:
Create a basket, add an order
On the basket view you see the "Modify" link, open it in a separate tab
=> You can edit the basket
Keep this tab open, get back to the other one and close the basket
Reload the tab with the order edition form
=> You cannot longer edit the basket

QA: Do we need a check in addorder.pl as well?

Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Jonathan Druart 2020-09-18 09:53:09 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Lucas Gass 2020-10-16 15:01:41 UTC
backported to 20.05.x for 20.05.05
Comment 10 Aleisha Amohia 2020-10-20 01:57:10 UTC
backported to 19.11.x for 19.11.11
Comment 11 Victor Grousset/tuxayo 2020-10-20 21:02:39 UTC
Not backported to oldoldstable (19.05.x). Feel free to ask if it's needed.