search

mollie / PrestaShop

iDEAL, Creditcard, Bancontact, SOFORT, Bank transfer, PayPal & paysafecard for Prestashop
http://www.mollie.com
BSD 2-Clause "Simplified" License
69 stars 44 forks source link

Configuration keys set for ALL shops in multishop context when editing settings for one shop #772

Closed Ashley-Butcher closed 1 year ago

Ashley-Butcher commented 1 year ago

Background

When setting up Mollie in a multishop Prestashop environment, setting the API key applies the API key to all shops, not just the one in the current context.

This behaviour is not expected, and not wanted in our scenario as we have shops where payments must be taken by a different legal entity. For example, we have a European company based in Belgium, and a UK company based in England, both using the same products and design but different shops. The English and Belgium companies must remain separate, and even Mollie has different rules for how payments are handled in a UK company versus an EU company.

Additionally, we have multiple shops for different countries within the EU. These are linked to the same Mollie account, but for reporting reasons we separate transactions on each shop using different API keys.

As the API keys cannot be set per shop, this must be done in the database directly.

Expected behavior

When editing the API keys in a multishop environment on a shop level context, the API keys should only change for that shop. When editing on a store group or "all shops" context, it would make sense to apply the API keys to the relevant shops.

Actual behavior

When editing the API keys in a multishop environment on a shop level context, the API keys are changed for all shops. In the configuration database table, this is done not on the "all shops" level (id_shop_group = null, id_shop = null) but rather it is applied to every shop individually (id_shop_group = null, id_shop = x).

This behaviour contradicts Prestashop's UI, and results in the API keys being unexpectedly changed in other shop contexts.

Steps to reproduce the behavior

  1. Use a multishop enabled Prestashop environment with multiple shops defined.
  2. Switch to a particular shop context
  3. Set the API keys
  4. Switch to another shop context
  5. Set the API keys
  6. Observe in the database that all shops have been updated with the API keys from step 5.

Environment

justelis22 commented 1 year ago

Hi there @Ashley-Butcher,

Could you clarify if the shops are in a different shop group or if all the shops are in the same group but with different id_shop? As I am not able to recreate the same issue if I save the API keys for one shop, the same settings are not applied for the other shop in the same group.

Waiting for your clarification!

Best Regards, Invertus Support team.

Ashley-Butcher commented 1 year ago

Hey @justelis22, sorry for the delay in getting back to you.

In this scenario the installation has 12 shops, grouped into 2 groups. The change applied to all 12 shops, as confirmed by checking the modified date on the API key configuration records in the configuration table. The change of the API key was in a specific shop's context, but was then carried to all other shops.

It's not a high priority thing, but the behaviour was unexpected. We just updated the keys via the database directly.

ausrinekr commented 1 year ago

Hi there @Ashley-Butcher,

Thank you for additional details.

I am transferring this issue to our development team and I will come back to you with more information as soon as we analyze this issue.

Best Regards, Invertus Support team.

Ashley-Butcher commented 1 year ago

Just an update from my side, I've watched this happen again today. User was updating settings in one shop (in that shop's context) to change payment settings, and the MOLLIE_API_KEY and MOLLIE_API_KEY_TEST values from that store were then propagated to all other shops (not only shops in that group), necessitating an update directly on the database to restore the keys for the other stores.

Frustratingly it was not only the API keys. By looking at date_upd in ps_configuration, it's clear this impacted other keys too, including MOLLIE_API, MOLLIE_API_KEY, MOLLIE_API_KEY_TEST, MOLLIE_VOUCHER_CATEGORY, MOLLIE_APPLE_PAY_DIRECT_PRODUCT, MOLLIE_APPLE_PAY_DIRECT_CART, and MOLLIE_AS_MAIN for all shops, even those outside of that shop group.

This is not really expected behaviour, as previously described.

I've instructed users not to make setting changes in the Mollie module until this is fixed, as it seems to have unexpected and unwanted consequences on the settings for other shops.

Ashley-Butcher commented 1 year ago

After having a casual look since this is bothering me, I narrowed this down to ConfigurationAdapter::updateValue() (in src/Adapter/ConfigurationAdapter.php) - some kind of simplistic façade class for the Prestashop configuration class.

If $idShop is null, this code decides instead to loop through each shop and apply the value, which is contrary to how Prestashop works (saving this as null is already for all shops, with a value being an override for that particular shop).

I suppose this behaviour would be okay, except for the fact that not a single call to updateValue() specifies the shop ID, or the shop group ID, or anything really. Across SettingsSaveService::, CarrierService:: or the Installer:: class, nothing ever uses anything other than the $key and the $value parameters when calling updateValue() so of course this will always just clobber all values in all other shops.

At no point is the shop context considered - all values in all shops get clobbered.

This is HORRIBLE behaviour and I can't understand why this was even implemented this way.

sebastian-mollie commented 1 year ago

@Ashley-Butcher thanks for the report on this. We currently are finishing up the release which will include the fix for this issue. Our plan is to release a beta version next week and then have the official release the following week.

It would be good to hold on any API key change if possible until we release next version. Feel free to reach out in case you find something else.

justelis22 commented 1 year ago

Hi there @Ashley-Butcher, Thanks for waiting!

The issue should be fixed in the new release. You can try it out here - https://github.com/mollie/PrestaShop/releases/tag/v6.0.4-beta

Your feedback is greatly appreciated.

Best Regards, Invertus Support team.

Ashley-Butcher commented 1 year ago

Hey @justelis22 - sorry for the delay in getting back to you. Thanks for this, I've tested and it is now working as expected! Thanks for the quick turnaround.

justelis22 commented 1 year ago

Hi @Ashley-Butcher, Glad to hear that it is working!

I am closing the issue.

Best Regards, Invertus Support team.