avstudnitz / AvS_ScopeHint2

Magento 2 module for displaying additional information in configuration
157 stars 33 forks source link

If a product attribute value can't be casted to a string, try to repr… #16

Closed hostep closed 5 years ago

hostep commented 5 years ago

…esent it as json data. This fixes crashing the product edit page when using different tier prices over multiple websites for example.

This fixes https://github.com/avstudnitz/AvS_ScopeHint2/issues/13 and is a replacement for another proposed PR: https://github.com/avstudnitz/AvS_ScopeHint2/pull/15

Steps to reproduce the original reported problem:

The reason why it crashes, is because the tier prices are returned as an array instead of the expected string. We've seen a similar problem with a third party module, which also returns an array, but a multi dimensional one. The original attempt to fix this tried to implode the value if it was an array, but that won't work on a multi dimensional array.

This PR attempt to fix all of these possible scenario's.

I've chosen a slightly different approach here, because I think there might be more than only array types not being able to get casted to a string. Here I'm trying to cast the value to a string in a try-catch statement, if this fails, the \Throwable will get catched and the error won't appear on the frontend. If this happens, I'm trying to json_encode the value, to get a string representation of the value. If that still fails, we silently ignore the problem and won't output anything.

In the case of the tier prices problem, there is another bug in that the tooltip doesn't show up for some reason. I haven't looked why that that happens yet, but that should be considered a different bug. This PR is trying make the product edit page more stable, I prefer to have it not crashing over showing the correct tooltip. Fixing the tooltip can be done at a later stage in my opinion.

If there are remarks, let me know! 🙂

avstudnitz commented 5 years ago

Thanks a lot!

hostep commented 5 years ago

Awesome, thanks for merging so quickly!

Any chance you can also tag a new version? 🙂

(and PR https://github.com/avstudnitz/AvS_ScopeHint2/pull/15 can be closed now I believe)