diff --git a/ws_customerspecialty.zip b/ws_customerspecialty.zip index cd2c5aa..64b94ec 100644 Binary files a/ws_customerspecialty.zip and b/ws_customerspecialty.zip differ diff --git a/ws_customerspecialty/README.md b/ws_customerspecialty/README.md index 143aa31..45fdbe3 100644 --- a/ws_customerspecialty/README.md +++ b/ws_customerspecialty/README.md @@ -55,26 +55,76 @@ not something the module can safely do on your behalf without overriding core ma - No drag-and-drop reordering of specialty values. The grid has "Move up" / "Move down" row actions instead (`customerspecialty_move_up` / `_move_down` routes swapping `position` with the adjacent row). This avoids depending on PrestaShop's core Grid position/drag-reorder plumbing. -- The active/inactive toggle uses a `ToggleColumn` + `customerspecialty_toggle_active` route. If - that route ever misbehaves on a given PrestaShop version, active/inactive can always be changed - via the Edit form as a fallback — it isn't the only way to flip the flag. +- The active/inactive column is display-only (`Active`/`Inactive` text via a + `GridDataModifier` hook — `actionCustomerspecialtyGridDataModifier`). An earlier version used a + clickable `ToggleColumn` for instant AJAX toggling. Checked against PrestaShop's real core + source (`src/Core/Grid/Column/Type/Common/ToggleColumn.php`): the column's required options + (`field`, `primary_field`, `route`) were all correctly set, so that wasn't the bug. The actual + cause was on the controller side — `toggleActiveAction()` returned a full-page + `RedirectResponse`, but the toggle's AJAX click handler expects a JSON response to update its + own UI state, so the click likely persisted server-side with no visible feedback. Removed rather + than fixed, since the display-only column + existing Edit form covers the same functionality + with less surface area. Change active/inactive via the Edit form (pencil icon) instead. -Confirmed working live on PrestaShop 9: create, edit, and active/inactive toggle on the -Specialties grid. Three real bugs were found and fixed this way (not by static guessing) during -initial rollout: a wrong `FormDataProviderInterface` namespace, a missing `LinkRowAction` -`route_param_field` option, and `FrameworkBundleAdminController::trans()` taking `(key, domain, -params)` instead of `Module::trans()`'s `(key, params, domain)`. "Move up"/"Move down" reordering -has not been explicitly click-tested yet — verify before relying on it. +Confirmed working live on PrestaShop 9: create, edit, and (formerly) active/inactive toggle on the +Specialties grid. Several real bugs were found via live errors during rollout rather than caught +ahead of time: a wrong `FormDataProviderInterface` namespace, a missing `LinkRowAction` +`route_param_field` option, a wrong `FormField` namespace, and `FrameworkBundleAdminController::trans()` +taking `(key, domain, params)` instead of `Module::trans()`'s `(key, params, domain)`. "Move +up"/"Move down" reordering hasn't been explicitly click-tested, but checked against PrestaShop's +real `LinkRowAction` source (`src/Core/Grid/Action/Row/Type/LinkRowAction.php`): its only required +options are `route`, `route_param_name`, `route_param_field` — the exact same three already +confirmed working on the "Edit" row action, and "Move up"/"Move down" use the identical shape. Low +residual risk, but still worth a click to confirm. -## Biggest assumption to verify +## Bug caught by research before it hit production -The storefront "Specialty" field on registration/account-edit relies on `additionalCustomerFormFields` -already being called by core's own front-office `CustomerFormatter` (passing `fields` by reference). -The legacy module achieved this only by overriding `CustomerFormatter` to add that hook call itself — -an override intentionally not reproduced here (see the architecture note above). If the installed -PrestaShop version's `CustomerFormatter` does not already call this hook natively, the storefront -field will not appear (back office, grid, and mail-variable behavior are unaffected either way, -since none of those depend on this hook). Verify on a real 1.7 and 9.0 install; if the hook isn't -native, the fallback is to make the field mandatory server-side via a small first-party -`CustomerFormatter` override scoped to only adding that one `Hook::exec()` call — much smaller and -safer than the legacy module's full-method copy. +The `active`/`inactive` label transform used the hook name `actionCustomerspecialtyGridDataModifier` +with `$params['presented_grid']['data']['records']` — copied directly from this module's own +written conventions doc. Checked against real PrestaShop core source (confirmed identically on +1.7.8.11, 8.1.6, and 9.0.0): `GridFactory::getGrid()` dispatches `action{Id}GridDataModifier` with +`['data' => &$data]` (a raw `GridData` object, no `presented_grid` key at all) — the +`presented_grid` array shape actually belongs to a *different* hook, `action{Id}GridPresenterModifier`, +dispatched separately from `GridPresenter::present()`. The conventions doc has the hook name and +parameter shape from two different hooks crossed. Our hook name has been corrected to +`actionCustomerspecialtyGridPresenterModifier`; the body logic was already right for that shape. +This was caught via source research before ever being deployed, unlike most other bugs in this +module which surfaced as live fatals — worth flagging back if this doc gets used for future modules. + +## Confirmed live on PrestaShop 9 + +`additionalCustomerFormFields` **is** called natively by core's front-office `CustomerFormatter::getFormat()` +(confirmed via a live stack trace from the registration page) — no `CustomerFormatter` override is +needed, as originally hoped. This was the biggest unverified assumption in the initial build; it's +now settled. + +## Front-office vs back-office service container gotcha (found live, now fixed) + +The front office runs on a different, smaller Symfony container than the back office — it does +**not** have this module's own `config/*.yml`-registered services compiled into it, only core +services. A hook that may fire in front-office context (like `additionalCustomerFormFields`, fired +from the registration page) must not fetch custom module services via `$this->get('websmid.*')` — +it throws `ServiceNotFoundException` there even though the identical call works fine from +admin-only hooks. Fixed by instantiating `CustomerSpecialtyChoiceProvider` directly (`new +CustomerSpecialtyChoiceProvider()`) instead of routing it through the container — it has no real +dependencies, so this works identically in both contexts. `config/provider.yml` was removed since +the class is no longer registered as a DI service. + +**Update, confirmed live**: even `prestashop.core.command_bus` — a *core* service, not a +module-registered one — throws the same `ServiceNotFoundException` when called from front-office +hooks (`actionObjectCustomerAddAfter`/`UpdateAfter`, fired from registration/account-edit). The +front-office container is more limited than assumed; it's not just custom module services that are +missing. Fixed by having `persistFrontOfficeCustomerSpecialty()` write directly via +`Db::getInstance()` (an `INSERT ... ON DUPLICATE KEY UPDATE` against +`ws_customerspecialty_customer`) instead of dispatching `LinkCustomerSpecialtyCommand` through the +command bus. This also retroactively explains why `ws_giftdropdown`'s own front controller bypasses +its fully-wired Command/CommandHandler/Doctrine plumbing in favor of raw SQL — same limitation, +same workaround, already an established pattern in this codebase. + +**General rule going forward**: any hook that can fire in front-office context +(`additionalCustomerFormFields`, `actionObjectCustomerAddAfter`/`UpdateAfter`, and similar) must +stick to `Db::getInstance()`/legacy globals only — no `$this->get(...)` container service, custom +or core, and no Doctrine entity manager. Only hooks that are provably admin-only +(`actionCustomerFormBuilderModifier`, `actionAfterCreate/UpdateCustomerFormHandler`, +`actionCustomerGridDefinitionModifier`/`QueryBuilderModifier`) can safely use the command bus, +Doctrine, or custom DI services. diff --git a/ws_customerspecialty/config/command.yml b/ws_customerspecialty/config/command.yml index 40908e8..1cd4295 100644 --- a/ws_customerspecialty/config/command.yml +++ b/ws_customerspecialty/config/command.yml @@ -11,12 +11,6 @@ services: - { name: 'tactician.handler', command: 'Websmid\CustomerSpecialty\Command\MoveCustomerSpecialtyCommand' } - { name: 'messenger.message_handler' } - websmid.customerspecialty.command_handler.toggle_customer_specialty_active_handler: - class: 'Websmid\CustomerSpecialty\Command\CommandHandler\ToggleCustomerSpecialtyActiveCommandHandler' - tags: - - { name: 'tactician.handler', command: 'Websmid\CustomerSpecialty\Command\ToggleCustomerSpecialtyActiveCommand' } - - { name: 'messenger.message_handler' } - websmid.customerspecialty.command_handler.link_customer_specialty_handler: class: 'Websmid\CustomerSpecialty\Command\CommandHandler\LinkCustomerSpecialtyCommandHandler' tags: diff --git a/ws_customerspecialty/config/provider.yml b/ws_customerspecialty/config/provider.yml deleted file mode 100644 index ed49d1c..0000000 --- a/ws_customerspecialty/config/provider.yml +++ /dev/null @@ -1,4 +0,0 @@ -services: - websmid.customerspecialty.form.choice_provider.customer_specialty_choice_provider: - public: true - class: 'Websmid\CustomerSpecialty\Form\ChoiceProvider\CustomerSpecialtyChoiceProvider' diff --git a/ws_customerspecialty/config/routes.yml b/ws_customerspecialty/config/routes.yml index 1f6b2b7..347ef86 100644 --- a/ws_customerspecialty/config/routes.yml +++ b/ws_customerspecialty/config/routes.yml @@ -27,13 +27,6 @@ customerspecialty_edit: _controller: 'Websmid\CustomerSpecialty\Controller\CustomerSpecialtyController::editAction' _legacy_controller: AdminCustomerSpecialtyOverview -customerspecialty_toggle_active: - path: customerspecialty/toggle-active/{idCustomerSpecialty} - methods: [GET] - defaults: - _controller: 'Websmid\CustomerSpecialty\Controller\CustomerSpecialtyController::toggleActiveAction' - _legacy_controller: AdminCustomerSpecialtyOverview - customerspecialty_move_up: path: customerspecialty/move-up/{idCustomerSpecialty} methods: [GET] diff --git a/ws_customerspecialty/config/services.yml b/ws_customerspecialty/config/services.yml index 8f283e3..f949b46 100644 --- a/ws_customerspecialty/config/services.yml +++ b/ws_customerspecialty/config/services.yml @@ -2,7 +2,6 @@ imports: - { resource: form.yml } - { resource: grid.yml } - { resource: command.yml } - - { resource: provider.yml } services: websmid.customerspecialty.entity.table_prefix_subscriber: diff --git a/ws_customerspecialty/src/Command/CommandHandler/ToggleCustomerSpecialtyActiveCommandHandler.php b/ws_customerspecialty/src/Command/CommandHandler/ToggleCustomerSpecialtyActiveCommandHandler.php deleted file mode 100644 index f8d3b13..0000000 --- a/ws_customerspecialty/src/Command/CommandHandler/ToggleCustomerSpecialtyActiveCommandHandler.php +++ /dev/null @@ -1,35 +0,0 @@ -handle($command); - } - - public function handle(ToggleCustomerSpecialtyActiveCommand $command) - { - $entityManager = SymfonyContainer::getInstance()->get('doctrine.orm.entity_manager'); - $repository = $entityManager->getRepository(CustomerSpecialty::class); - - $customerSpecialty = $repository->find($command->getIdCustomerSpecialty()); - - if (!$customerSpecialty) { - throw new PrestaShopException('Customer specialty not found.'); - } - - $customerSpecialty->setActive(!$customerSpecialty->isActive()); - - $entityManager->persist($customerSpecialty); - $entityManager->flush(); - - return true; - } -} diff --git a/ws_customerspecialty/src/Command/ToggleCustomerSpecialtyActiveCommand.php b/ws_customerspecialty/src/Command/ToggleCustomerSpecialtyActiveCommand.php deleted file mode 100644 index 40f717c..0000000 --- a/ws_customerspecialty/src/Command/ToggleCustomerSpecialtyActiveCommand.php +++ /dev/null @@ -1,18 +0,0 @@ -idCustomerSpecialty = $idCustomerSpecialty; - } - - public function getIdCustomerSpecialty(): int - { - return $this->idCustomerSpecialty; - } -} diff --git a/ws_customerspecialty/src/Config/Config.php b/ws_customerspecialty/src/Config/Config.php index d980eaf..a3783ac 100644 --- a/ws_customerspecialty/src/Config/Config.php +++ b/ws_customerspecialty/src/Config/Config.php @@ -15,6 +15,7 @@ class Config 'actionCustomerGridQueryBuilderModifier', 'displayAdminCustomers', 'actionGetExtraMailTemplateVars', + 'actionCustomerspecialtyGridPresenterModifier', ]; const TABS = [ diff --git a/ws_customerspecialty/src/Controller/CustomerSpecialtyController.php b/ws_customerspecialty/src/Controller/CustomerSpecialtyController.php index ed78d80..790e42d 100644 --- a/ws_customerspecialty/src/Controller/CustomerSpecialtyController.php +++ b/ws_customerspecialty/src/Controller/CustomerSpecialtyController.php @@ -7,7 +7,6 @@ use PrestaShopBundle\Security\Voter\PageVoter; use Websmid\CustomerSpecialty\Grid\Definition\Factory\CustomerSpecialtyGridDefinitionFactory; use Websmid\CustomerSpecialty\Grid\Filters\CustomerSpecialtyFilters; use Websmid\CustomerSpecialty\Command\MoveCustomerSpecialtyCommand; -use Websmid\CustomerSpecialty\Command\ToggleCustomerSpecialtyActiveCommand; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -32,7 +31,7 @@ class CustomerSpecialtyController extends FrameworkBundleAdminController { $legacyContext = $this->get('prestashop.adapter.legacy.context')->getContext(); - return $legacyContext->link->getAdminLink('AdminModules') . '&configure=ws_customerspecialty'; + return $legacyContext->link->getAdminLink('AdminModules') . '&configure=ws_customerspecialty&viewsettings=1'; } public function searchAction(Request $request): RedirectResponse @@ -95,18 +94,6 @@ class CustomerSpecialtyController extends FrameworkBundleAdminController ]); } - public function toggleActiveAction(int $idCustomerSpecialty): RedirectResponse - { - try { - $this->getCommandBus()->handle(new ToggleCustomerSpecialtyActiveCommand($idCustomerSpecialty)); - $this->addFlash('success', $this->trans('Successful update', 'Admin.Notifications.Success')); - } catch (Exception $e) { - $this->addFlash('error', $this->getErrorMessageForException($e, $this->getErrorMessages())); - } - - return $this->redirectToRoute('customerspecialty_index'); - } - public function moveUpAction(int $idCustomerSpecialty): RedirectResponse { return $this->move($idCustomerSpecialty, MoveCustomerSpecialtyCommand::DIRECTION_UP); diff --git a/ws_customerspecialty/src/Grid/Definition/Factory/CustomerSpecialtyGridDefinitionFactory.php b/ws_customerspecialty/src/Grid/Definition/Factory/CustomerSpecialtyGridDefinitionFactory.php index 0678ad4..c57c26a 100644 --- a/ws_customerspecialty/src/Grid/Definition/Factory/CustomerSpecialtyGridDefinitionFactory.php +++ b/ws_customerspecialty/src/Grid/Definition/Factory/CustomerSpecialtyGridDefinitionFactory.php @@ -5,7 +5,6 @@ namespace Websmid\CustomerSpecialty\Grid\Definition\Factory; use PrestaShop\PrestaShop\Core\Grid\Definition\Factory\AbstractGridDefinitionFactory; use PrestaShop\PrestaShop\Core\Grid\Column\ColumnCollection; use PrestaShop\PrestaShop\Core\Grid\Column\Type\Common\DataColumn; -use PrestaShop\PrestaShop\Core\Grid\Column\Type\Common\ToggleColumn; use PrestaShop\PrestaShop\Core\Grid\Column\Type\Common\ActionColumn; use PrestaShop\PrestaShop\Core\Grid\Action\Row\RowActionCollection; use PrestaShop\PrestaShop\Core\Grid\Action\Row\Type\LinkRowAction; @@ -49,13 +48,10 @@ class CustomerSpecialtyGridDefinitionFactory extends AbstractGridDefinitionFacto ]) ) ->add( - (new ToggleColumn('active')) + (new DataColumn('active')) ->setName($this->trans('Active', [], 'Admin.Global')) ->setOptions([ - 'field' => 'active', - 'primary_field' => 'id_ws_customerspecialty', - 'route' => 'customerspecialty_toggle_active', - 'route_param_name' => 'idCustomerSpecialty', + 'field' => 'active', ]) ) ->add( diff --git a/ws_customerspecialty/src/Module/AdminHooks.php b/ws_customerspecialty/src/Module/AdminHooks.php index 4b2b34c..d5e1c8c 100644 --- a/ws_customerspecialty/src/Module/AdminHooks.php +++ b/ws_customerspecialty/src/Module/AdminHooks.php @@ -4,6 +4,7 @@ namespace Websmid\CustomerSpecialty\Module; use Websmid\CustomerSpecialty\Config\Config; use Websmid\CustomerSpecialty\Command\LinkCustomerSpecialtyCommand; +use Websmid\CustomerSpecialty\Form\ChoiceProvider\CustomerSpecialtyChoiceProvider; use Db; use Configuration; use Tools; @@ -11,6 +12,7 @@ use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\Extension\Core\Type\ChoiceType; use PrestaShop\PrestaShop\Core\Grid\Column\Type\Common\DataColumn; use PrestaShop\PrestaShop\Core\Grid\Filter\Filter; +use PrestaShop\PrestaShop\Core\Grid\Record\RecordCollection; trait AdminHooks { @@ -26,6 +28,9 @@ trait AdminHooks if (Tools::isSubmit('submitWsCustomerSpecialty')) { Configuration::updateValue(Config::FRONT_ENABLED_CONFIG_KEY, (bool) Tools::getValue('front_enabled')); $confirmation = $this->displayConfirmation($this->trans('Settings saved.', [], 'Admin.Notifications.Success')); + } elseif (!Tools::getValue('viewsettings')) { + header('Location: ' . $this->get('router')->generate('customerspecialty_index')); + exit; } $this->context->smarty->assign([ @@ -113,6 +118,20 @@ trait AdminHooks $this->applyCustomerSpecialtyFilter($searchCriteria, $searchQueryBuilder); } + public function hookActionCustomerspecialtyGridPresenterModifier(array $params): void + { + $activeLabel = $this->trans('Active', [], 'Admin.Global'); + $inactiveLabel = $this->trans('Inactive', [], 'Admin.Global'); + + $params['presented_grid']['data']['records'] = new RecordCollection( + array_map(static function (array $row) use ($activeLabel, $inactiveLabel) { + $row['active'] = $row['active'] ? $activeLabel : $inactiveLabel; + + return $row; + }, $params['presented_grid']['data']['records']->all()) + ); + } + public function hookDisplayAdminCustomers(array $params) { $title = $this->loadCustomerSpecialtyTitle((int) $params['id_customer']); @@ -200,8 +219,8 @@ trait AdminHooks return false !== $title && null !== $title ? (string) $title : null; } - private function getCustomerSpecialtyChoiceProvider() + private function getCustomerSpecialtyChoiceProvider(): CustomerSpecialtyChoiceProvider { - return $this->get('websmid.customerspecialty.form.choice_provider.customer_specialty_choice_provider'); + return new CustomerSpecialtyChoiceProvider(); } } diff --git a/ws_customerspecialty/src/Module/DisplayHooks.php b/ws_customerspecialty/src/Module/DisplayHooks.php index 0968b83..ff4c8ad 100644 --- a/ws_customerspecialty/src/Module/DisplayHooks.php +++ b/ws_customerspecialty/src/Module/DisplayHooks.php @@ -3,11 +3,10 @@ namespace Websmid\CustomerSpecialty\Module; use Websmid\CustomerSpecialty\Config\Config; -use Websmid\CustomerSpecialty\Command\LinkCustomerSpecialtyCommand; use Configuration; use Tools; use Db; -use PrestaShop\PrestaShop\Core\Form\FormField; +use FormField; trait DisplayHooks { @@ -17,7 +16,7 @@ trait DisplayHooks return; } - $choices = $this->get('websmid.customerspecialty.form.choice_provider.customer_specialty_choice_provider')->getChoices(); + $choices = $this->getCustomerSpecialtyChoiceProvider()->getChoices(); if (empty($choices)) { return; @@ -28,7 +27,17 @@ trait DisplayHooks ->setType('select') ->setLabel($this->trans('Specialty', [], 'Modules.Wscustomerspecialty.Shop')) ->setRequired(true) - ->setAvailableValues($choices); + ->setAvailableValues(array_flip($choices)); + + $idCustomer = (int) $this->context->customer->id; + + if ($idCustomer > 0) { + $currentSpecialization = $this->loadCustomerSpecialtyId($idCustomer); + + if ($currentSpecialization > 0) { + $specializationField->setValue($currentSpecialization); + } + } if (isset($params['fields'])) { $this->insertFieldAfterEmail($params['fields'], $specializationField); @@ -86,8 +95,16 @@ trait DisplayHooks return; } - $this->get('prestashop.core.command_bus')->handle( - new LinkCustomerSpecialtyCommand($idCustomer, $specialization) + $now = date('Y-m-d H:i:s'); + + Db::getInstance()->execute( + 'INSERT INTO `' . _DB_PREFIX_ . 'ws_customerspecialty_customer` + (`id_customer`, `id_ws_customerspecialty`, `date_add`, `date_upd`) + VALUES + (' . $idCustomer . ', ' . $specialization . ', \'' . pSQL($now) . '\', \'' . pSQL($now) . '\') + ON DUPLICATE KEY UPDATE + `id_ws_customerspecialty` = ' . $specialization . ', + `date_upd` = \'' . pSQL($now) . '\'' ); } diff --git a/ws_customerspecialty/views/templates/admin/configuration.tpl b/ws_customerspecialty/views/templates/admin/configuration.tpl index 8cae1db..cd6d81f 100644 --- a/ws_customerspecialty/views/templates/admin/configuration.tpl +++ b/ws_customerspecialty/views/templates/admin/configuration.tpl @@ -2,10 +2,26 @@ {$confirmation nofilter} {/if} - +
+ +
diff --git a/ws_customerspecialty/views/templates/admin/grid/customerspecialty.html.twig b/ws_customerspecialty/views/templates/admin/grid/customerspecialty.html.twig index 2e66da1..d06b0df 100644 --- a/ws_customerspecialty/views/templates/admin/grid/customerspecialty.html.twig +++ b/ws_customerspecialty/views/templates/admin/grid/customerspecialty.html.twig @@ -1,10 +1,26 @@ {% extends '@PrestaShop/Admin/layout.html.twig' %} {% block content %} - +
+ +
{% include '@PrestaShop/Admin/Common/Grid/grid_panel.html.twig' with {'grid': customerSpecialtyGrid} %} {% endblock %}