Prestashop: Undefined index Notice error in debug mode when deleting a custom order status

[roja45]

Describe the bug

This is issue #16657, which has already been raised, and closed, though I think there is an issue here.
The problem comes if an order status has been removed (or as in the original bug report the mapping between order status has not been completed correctly).

I think you can also recreate this by creating an order using paypal for example, and then uninstalling the paypal module. The order remains, but the status does not, this then causes the error.

As explained in the original bug report, in these circumstances, $val['id_order_state'] below is not guaranteed to have a value, can be null, and then this code generates the Notice error in debug mode, and it's not possible to load the page even to cancel the order.

foreach ($res as $key => $val) { $res[$key]['order_state'] = $indexedOrderStates[$val['id_order_state']]['name']; $res[$key]['invoice'] = $indexedOrderStates[$val['id_order_state']]['invoice']; $res[$key]['order_state_color'] = $indexedOrderStates[$val['id_order_state']]['color']; }

Expected behavior

Customer page should open without error.

Steps to Reproduce

Steps to reproduce the behavior:

1. Enable debug mode
2. Create a new order status
3. Raise a new order, and assign the order the status you created.
4. Delete the order status
5. Try to load the customer screen - Error
6. Assign the order a different order status
7. Load the customer screen again - Now it loads.

Screenshots

If applicable, add screenshots or screen recordings to help explain your problem.

Additional information

• PrestaShop version: 1.7.6.5
• PHP version: 7.2

[prestashop-issue-bot[bot]]

Thanks for opening this issue! We will help you to keep its state consistent

[Matt75]

I think you can also recreate this by creating an order using paypal for example, and then uninstalling the paypal module. The order remains, but the status does not, this then causes the error.

This is a bug from module PayPal in this case, a module should never delete an OrderState, on module uninstall, OrderState should be marked as deleted and not removed from the database.
There a deleted field on table ps_order_state for this to preserve integrity of passed orders

This issue should be sent to 202 e-commerce, developer of PayPal module.

[roja45]

Read the steps to reproduce, you can recreate this error using just Prestashop. If what you are saying regarding order states is true, then deleting an order state should presumably set this flag, and not delete itself.

[Matt75]

Native OrderState have a hidden field unremovable in ps_order_state table to unsure merchant cannot delete this OrderState.
Customized OrderState should be marked as deleted if orders passed with it instead of removed from database 👍
Can you modify your bug report with this scenario instead of using a module ? Thanks

[roja45]

Bug report uses the prestashop only scenario, you can see this in the steps to reproduce section.

Prestashop allows the the order state to be deleted, irrespective of whether someone should or shouldn't delete a custom order state, they can.

The problem then is one of three things;
1) Prestashop shouldn't allow the order state to be delete (if in use),
2) Prestashop should mark it as deleted and not remove from the database (your suggestion)
3) (and lets face it, this is the issue), the code highlighted above is dodgy, and making an assumption that should not be made. The id_order_state can be null.

To reenforce this point, if you delete an order state (because it is permitted), and open the orders page, no error, the order state is simply blanked. If you open the customers page, symfony error page, no more fun to be had. Why are there two different results, because the code for the customer page is incorrect.

[colinegin]

Hello @roja45 ,

indeed i manage to reproduce this issue by creating a custom order status.
I think the best solution is number 2 as suggested by @Matt75, so that merchant can still "remove" status that they no longer use.

About your third point, what do you think @PrestaShop/prestashop-core-developers about @roja45 's comment ?

[colinegin]

I've also noted that after changing the order status (with a native one) from the order listing, if I try to go to the order page, I get this error :

Is it related ?
ping @matthieu-rolland

[matthieu-rolland]

Yes it's probably related @colinegin , it seems like a good idea to mark the status as deleted instead of removing it from DB as @Matt75 said, actually there's a field "deleted" in ps_order_state.
@roja45 is also right when he says that the code is dodgy, a verification should be made on the existence of id_order_state in the array.

I'll have a look at it soon, thank you !

[Matt75]

Can be reproduced with module Payment Example v2.0.0

1. Install module Payment Example v2.0.0
2. Make an Order with PaymentOption Pay Offline
3. Order is created with a custom OrderState from module
4. Go to Module Manager and reset module
5. Custom OrderState previously created by module is marked as deleted
6. Go on FO > Account > Order History
7. See error

[matthieu-rolland]

So, to sum up:

The bug breaks (at least) 4 pages:

• the order history page in FO (exception)
• the Order detail page in BO (exception)
• the order listing page in BO (no exception, but the status is not displayed).
• the customer page

To fix that, the idea is to change the order status hard delete into a soft delete, setting the "deleted" value to 1. So that the status can be retrieved in DB even after it has been deleted, preventing orders having this status to break.

There's just one thing that I'm not sure how to handle: in the order detail page(BO), the order status list appears in a selector, the current status being the one selected. This selector is both used to indicate what is the current status, and to change the status of the order, so nothing prevents the user from setting a deleted order in the order detail page... how should we deal with that?

I'm thinking that maybe we should not display deleted order status in the select of the order detail page, except when the deleted status is the one currently assigned, and it would then disappear from the select if we assign another status.

[colinegin]

Hello @arouiadib and @Matt75 ,

after discussing with @matthieu-rolland we think that the best way to fix this would be to prevent merchants from deleting an order status which is used in at least one order.
If there is at least one order with the order status, when the merchant will try to delete it, he will get an error message.

We think this is easier than applying a soft delete we can cause some issues on the order detail interface (for instance in the order status dropdowns).

Wdyt ?

[Matt75]

@colinegin I don't understand, you means not use flag deleted ? Or not display OrderState with flag deleted in OrderState dropdowns ?

For example with PrestaShop Checkout, when a merchant uninstall the module, OrderState created by the module are flaged as deleted so they should not be in OrderState dropdowns but Order passed should work as before. Goal is to not let Merchant to use a flagged as deleted OrderState.

[colinegin]

Alright, thanks for your feedbacks.

After further discussion with @matthieu-rolland and @MatShir, we actually changed our mind :) the best solution we have found is :

• flag the order status as deleted
• leave it visible in the order status droplist (with (deleted) displayed) the status deleted until another one is selected
• once another one is selected, remove it from the droplist so that it cannot be chosen anymore

[colinegin]

FYI @matthieu-rolland and @MatShir you can find detailed specifications of the improvement here : https://github.com/PrestaShop/prestashop-specs/blob/master/back-office/shop-parameters/order-settings/Order%20Setting%20%3E%20Statuses%20Page.md

[matks]

Might be fixed by https://github.com/PrestaShop/PrestaShop/pull/19581

[matks]

I put this issue back in "WIP" because the discussion is going on on the PR

[matks]

PR going to QA https://github.com/PrestaShop/PrestaShop/pull/19581

[Progi1984]

Fixed by #19581