Fix bug with all shops context in WebService #28537
Conversation
classes/shop/Shop.php
Outdated
| @@ -396,7 +396,7 @@ public static function initialize() | |||
| Configuration::getMultiShopValues('PS_MEDIA_SERVER_3') | |||
| ); | |||
|
|
|||
| if ((!$id_shop && defined('_PS_ADMIN_DIR_')) || Tools::isPHPCLI() || in_array($http_host, $all_media)) { | |||
| if ((!$id_shop && defined('_PS_ADMIN_DIR_')) || ('all' === $id_shop && defined('_PS_API_IN_USE_') && _PS_API_IN_USE_) || Tools::isPHPCLI() || in_array($http_host, $all_media)) { | |||
There was a problem hiding this comment.
I suggest to use vars to improve the code readability
Something like :
$isAllShop = 'all' === $id_shop;
$isApiInUse = defined('_PS_API_IN_USE_') && _PS_API_IN_USE_;There was a problem hiding this comment.
I updated the commit with your suggestions.
make sure value only defined once Update Shop.php suggestions from pululuk Fix for issue 12412
Progi1984
changed the title
There was a problem hiding this comment.
Hello @Prestaworks and thank you for the PR! Apologies for the late review; these last weeks we have been focused on PrestaShop 8 freeze so we're very late in processing contributions.
I improved a bit the PR description 😉
For me the code is valid. But I have one question: because of the bug, today nobody can use the webservice in multistore, right? Then if we merge your PR we remove the bug. But do we know if webservices work with multistore ? 😄 I wonder if we have opened a door but behind the door the stuff is broken
|
it's possible to use in multistore, but when creating a size like "Large" for instance, you can't create it for all shops, only for one specific shop at a time. and that will create multiple entries with different IDs. |
There was a problem hiding this comment.
Thank you for your answer @Prestaworks and my apologies for late answer. I add 'waiting for QA' label so that QA tests this contribution.
matks
added
Waiting for QA
|
Woups I thought this was the PSWebservice repo (1 approval is enough). Ping @PrestaShop/committers this PR needs a 2nd approval to move forward :) |
matks
added
the
Waiting for QA
There was a problem hiding this comment.
Hello @Prestaworks ,
Tested the following:
- Single store & multistore (with 2 groups)
- Enabling of webservice
- Creation of webservice for a specific store
- Creation of webservice for all store
- Creation of specific product for a specific store
- Switched from single to multistore a few times
- Tested only the GET method.
My shop architecture:
- group default
- shop1
- shop2
- second group
- shop3
Created webservices for all of the shops, no more redirection to myshop.com/api/products?url=products when accessing myshop.com/api/products&id_shop=all
Each webservice created for each shop has its products.
Screen.Recording.2022-10-19.at.10.27.58.mov
Well noted for what you said in the comment :
you can't create it for all shops, only for one specific shop at a time
For instance, it is QA ✅
florine2623
added
QA ✔️
|
QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge. |
|
Thanks @Prestaworks & @florine2623 |
_PS_API_IN_USE_to allow PrestaShop to detect calls to webservice in multistore and avoid redirecting them.