Prestashop: Some configuration pages are not extensible

[mickaelandrieu]

Describe the bug

I've noticed that some configuration pages are not extensible.

Expected behavior

Every form of the back office should be extensible

Steps to Reproduce

Steps to reproduce the behavior:

1. Create a module that try to add new fields to the Product Preferences form
2. On the submit, an "UndefinedOption" exception will be thrown by the OptionsResolver Component
3. Then, cry

Additional information

• PrestaShop version: 1.7.6.4
• PHP version: N/A

Note

Because I'm a good guy, this is what you need to check and do in order to fix this bug.

Removes every call to the OptionsResolver in **Configuration classes like this one and replaces it by isset statments.

Shame on the developer who have contributed this shiit (it was me :disappointed: ) !

This won't fix another (big) issue you have on your back office: it's almost impossible to remove something and expect the page to continue to work. For this one, I don't have an easy solution because your new architecture ensure every expected field to be there ("hello CQRS").

[matks]

This won't fix another (big) issue you have on your back office: it's almost impossible to remove something and expect the page to continue to work. For this one, I don't have an easy solution because your new architecture ensure every expected field to be there ("hello CQRS").

Interesting topic indeed. We always thought about adding/modifying things in form (mainly fields) but never about removing fields.

I guess an easy workaround is to hardcode a value to provide something.

CQRS is not the real issue here. Actually you can modify the Command and the CommandHandler as all of these are mapped as Symfony services, it only needs a bigger customization.

But the main topic is: prestashop data model is built to be consistent. If you remove one piece of data, we cannot guarantee the integrity of the data model. CQRS enforces this integrity by requiring every field and requiring it is valid. By wishing to remove something from a page, you either need to hardcode it (= keep model integrity) or hack your way into PrestaShop data model to modify it deep enough to make sure it can live without this value.

[mickaelandrieu]

CQRS enforces this integrity by requiring every field and requiring it is valid.

Exactly my point, I'm aware of what is CQRS I used to be software architect :trollface:

Actually, this means that PrestaShop is less configurable than it was in earlier versions: I can't say if it's for the good and/or intended when the decision about introducing CQRS was taken.

As a previous "lead Symfony Migration" developer, I've also done the same mistake in some configuration forms. Luckely, it's easier to patch as most of them are directly related to the Configuration object which accepts almost anything (at your own risks !). So it's not CQRS' fault only, but imho the way we think.

To be clear: what is the more important for you: extensibility or integrity ? Because you can't really have both easily.

[matks]

To be clear: what is the more important for you: extensibility or integrity ? Because you can't really have both easily.

Integrity. I think for a while PrestaShop has been growing with the "always more extensibility !" mindset (which is why ObjectModels can be customized for example, the crazy "dynamic schema" feature) but this has caused a gigantic number of buggy situations and now what people want is more stability. PrestaShop is not a framework. You cannot stretch it 100% and expect it to still work.

One example : the carriers module war. Some "pick it at a local store" carriers display a map to allow customer to choose where he wishes his shipment to be sent, so if you enable two of them, the checkout page is badly displayed because the two maps are not compatible.

We now aim for an isolated, strong and robust (basically "you cannot break it") core for PrestaShop while providing, on the outside, a huge extensibility layer. This is explained in "PrestaShop In 2019 And Beyond, Part 3: The Future Architecture". Well-defined but restricted extension points.

If a developer really wants to modify something deep into the Core he always can 😄someone even suggested to me a hack to override core classes: remapping Composer autoload map to replace core classes by your own classes.

[matks]

I found the perfect sentence in the Blog Post:
"Incidentally, this also means that Core behavior can be _extended_, not _modified_, by other services–or at least not in a way that it puts system coherence at risk"

Removing a field is not extending, it's modifying 😄.

[mickaelandrieu]

This means that the second issue is not one: this is the expected behavior :+1:

Thanks for the answer !