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
To show what it should like, try to add "&close=1" at the end of the URL of an order. Mathieu
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?
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.
Sorry my mistake, I think I failed to apply the patch properly. I'll test again now!
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>
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>
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>
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.05
backported to 19.11.x for 19.11.11
Not backported to oldoldstable (19.05.x). Feel free to ask if it's needed.