From 15a05531c42a9c7de785177ad2a03e752d818d29 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Mon, 28 Oct 2024 18:53:32 +0000 Subject: [PATCH 01/13] Plug the update options page to the app --- README.md | 2 +- _dev/src/ts/pages/UpdatePageUpdateOptions.ts | 59 +++++++++++++++++++ classes/Analytics.php | 2 +- classes/Parameters/ConfigurationValidator.php | 2 +- classes/Parameters/UpgradeConfiguration.php | 12 ++-- classes/Router/Router.php | 8 +++ classes/Router/Routes.php | 2 + classes/Twig/Form/UpgradeOptionsForm.php | 6 +- .../CoreUpgrader/CoreUpgrader17.php | 2 +- .../CoreUpgrader/CoreUpgrader80.php | 2 +- .../admin/AdminSelfUpgradeController.php | 6 +- .../AbstractPageWithStepController.php | 9 ++- .../UpdatePageUpdateOptionsController.php | 41 +++++++++++-- .../UpdatePageVersionChoiceController.php | 2 +- tests/unit/AnalyticsTest.php | 2 +- .../templates/steps/update-options.html.twig | 16 ++++- 16 files changed, 146 insertions(+), 27 deletions(-) diff --git a/README.md b/README.md index d9bee25f9..0e1f4fc2a 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ Here is an example of the different fields that can be found in it: "archive_num": "8.0.0", // Release number, specific to the archive channel "PS_AUTOUP_CUSTOM_MOD_DESACT": 1, // Disable non-native modules "PS_AUTOUP_CHANGE_DEFAULT_THEME": 0, // Keep the current theme - "PS_AUTOUP_KEEP_MAILS": 0, // Retain customized email templates + "PS_AUTOUP_REGEN_EMAIL": 1, // Retain customized email templates "PS_AUTOUP_BACKUP": 0, // Do not create a store backup "PS_AUTOUP_KEEP_IMAGES": 1, // Retain images "PS_DISABLE_OVERRIDES": 1 // Disable all overrides diff --git a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts index 7b769aa70..b9c6ea8de 100644 --- a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts +++ b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts @@ -1,13 +1,72 @@ import UpdatePage from './UpdatePage'; +import api from '../api/RequestHandler'; export default class UpdatePageUpdateOptions extends UpdatePage { protected stepCode = 'update-options'; + protected form: HTMLFormElement; + protected formFields: Element[] = []; constructor() { super(); + this.form = this.assertAndGetForm(); } public mount() { this.initStepper(); + this.initFormFields(this.form); + this.form.addEventListener('submit', this.onFormSubmit); } + + public beforeDestroy() { + this.form.removeEventListener('submit', this.onFormSubmit); + this.formFields.forEach((element) => { + element.removeEventListener('change', this.onInputChange); + }); + } + + private assertAndGetForm(): HTMLFormElement { + const form = document.forms.namedItem('update-options-page-form'); + if (!form) { + throw new Error('Form not found'); + } + + ['routeToSave', 'routeToSubmit'].forEach((data) => { + if (!form.dataset[data]) { + throw new Error(`Missing data ${data} from form dataset.`); + } + }); + + return form; + } + + private initFormFields(form: HTMLFormElement): void { + Array.from(form.elements).forEach((element) => { + if (element.nodeName !== "INPUT") { + return; + } + + this.formFields.push(element); + + element.addEventListener('change', this.onInputChange); + }); + } + + private onInputChange = (ev: Event) => { + const optionInput = (ev.target as Element); + + optionInput.setAttribute('disabled', 'true'); + + const data = new FormData(); + data.append('name', optionInput.name); + data.append('value', optionInput.checked); + api.post(this.form.dataset.routeToSave!, data); + + optionInput.removeAttribute('disabled'); + } + + private onFormSubmit = (event: Event) => { + event.preventDefault(); + + api.post(this.form.dataset.routeToSubmit!, new FormData(this.form)); + }; } diff --git a/classes/Analytics.php b/classes/Analytics.php index 8b3d6cc42..7e751f033 100644 --- a/classes/Analytics.php +++ b/classes/Analytics.php @@ -124,7 +124,7 @@ public function getProperties($type): array 'upgrade_channel' => $this->upgradeConfiguration->getChannel(), 'disable_non_native_modules' => $this->upgradeConfiguration->shouldDeactivateCustomModules(), 'switch_to_default_theme' => $this->upgradeConfiguration->shouldSwitchToDefaultTheme(), - 'keep_customized_email_templates' => $this->upgradeConfiguration->shouldKeepMails(), + 'keep_customized_email_templates' => !$this->upgradeConfiguration->shouldRegenerateMailTemplates(), ]; $upgradeProperties = $this->properties[self::WITH_UPDATE_PROPERTIES] ?? []; $additionalProperties = array_merge($upgradeProperties, $additionalProperties); diff --git a/classes/Parameters/ConfigurationValidator.php b/classes/Parameters/ConfigurationValidator.php index a7460509b..d7dc12b67 100644 --- a/classes/Parameters/ConfigurationValidator.php +++ b/classes/Parameters/ConfigurationValidator.php @@ -64,7 +64,7 @@ public function validate(array $array = []): array $error = $this->validateArchiveXml($value, $isLocal); break; case UpgradeConfiguration::PS_AUTOUP_CUSTOM_MOD_DESACT: - case UpgradeConfiguration::PS_AUTOUP_KEEP_MAILS: + case UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL: case UpgradeConfiguration::PS_AUTOUP_KEEP_IMAGES: case UpgradeConfiguration::PS_DISABLE_OVERRIDES: $error = $this->validateBool($value, $key); diff --git a/classes/Parameters/UpgradeConfiguration.php b/classes/Parameters/UpgradeConfiguration.php index 847446742..099a97b27 100644 --- a/classes/Parameters/UpgradeConfiguration.php +++ b/classes/Parameters/UpgradeConfiguration.php @@ -41,7 +41,7 @@ class UpgradeConfiguration extends ArrayCollection { const PS_AUTOUP_CUSTOM_MOD_DESACT = 'PS_AUTOUP_CUSTOM_MOD_DESACT'; const PS_AUTOUP_CHANGE_DEFAULT_THEME = 'PS_AUTOUP_CHANGE_DEFAULT_THEME'; - const PS_AUTOUP_KEEP_MAILS = 'PS_AUTOUP_KEEP_MAILS'; + const PS_AUTOUP_REGEN_EMAIL = 'PS_AUTOUP_REGEN_EMAIL'; const PS_AUTOUP_BACKUP = 'PS_AUTOUP_BACKUP'; const PS_AUTOUP_KEEP_IMAGES = 'PS_AUTOUP_KEEP_IMAGES'; const PS_DISABLE_OVERRIDES = 'PS_DISABLE_OVERRIDES'; @@ -56,7 +56,7 @@ class UpgradeConfiguration extends ArrayCollection const UPGRADE_CONST_KEYS = [ self::PS_AUTOUP_CUSTOM_MOD_DESACT, self::PS_AUTOUP_CHANGE_DEFAULT_THEME, - self::PS_AUTOUP_KEEP_MAILS, + self::PS_AUTOUP_REGEN_EMAIL, self::PS_AUTOUP_BACKUP, self::PS_AUTOUP_KEEP_IMAGES, self::PS_DISABLE_OVERRIDES, @@ -69,7 +69,7 @@ class UpgradeConfiguration extends ArrayCollection const PS_CONST_DEFAULT_VALUE = [ self::PS_AUTOUP_CUSTOM_MOD_DESACT => true, self::PS_AUTOUP_CHANGE_DEFAULT_THEME => false, - self::PS_AUTOUP_KEEP_MAILS => false, + self::PS_AUTOUP_REGEN_EMAIL => true, self::PS_AUTOUP_BACKUP => true, self::PS_AUTOUP_KEEP_IMAGES => true, ]; @@ -201,11 +201,11 @@ public function shouldDeactivateCustomModules(): bool } /** - * @return bool true if we should keep the merchant emails untouched + * @return bool true if we should regenerate the merchant emails */ - public function shouldKeepMails(): bool + public function shouldRegenerateMailTemplates(): bool { - return $this->computeBooleanConfiguration(self::PS_AUTOUP_KEEP_MAILS); + return $this->computeBooleanConfiguration(self::PS_AUTOUP_REGEN_EMAIL); } /** diff --git a/classes/Router/Router.php b/classes/Router/Router.php index e1107095a..51e6b456d 100644 --- a/classes/Router/Router.php +++ b/classes/Router/Router.php @@ -82,6 +82,14 @@ public function __construct(UpgradeContainer $upgradeContainer) 'controller' => UpdatePageUpdateOptionsController::class, 'method' => 'step', ], + Routes::UPDATE_STEP_UPDATE_OPTIONS_SAVE_OPTION => [ + 'controller' => UpdatePageUpdateOptionsController::class, + 'method' => 'saveOption', + ], + Routes::UPDATE_STEP_UPDATE_OPTIONS_SUBMIT_FORM => [ + 'controller' => UpdatePageUpdateOptionsController::class, + 'method' => 'submit', + ], Routes::UPDATE_PAGE_BACKUP => [ 'controller' => UpdatePageBackupController::class, 'method' => 'index', diff --git a/classes/Router/Routes.php b/classes/Router/Routes.php index 8e9e047c3..60740aa45 100644 --- a/classes/Router/Routes.php +++ b/classes/Router/Routes.php @@ -18,6 +18,8 @@ class Routes /* step: update options */ const UPDATE_PAGE_UPDATE_OPTIONS = 'update-page-update-options'; const UPDATE_STEP_UPDATE_OPTIONS = 'update-step-update-options'; + const UPDATE_STEP_UPDATE_OPTIONS_SAVE_OPTION = 'update-step-update-options-save-option'; + const UPDATE_STEP_UPDATE_OPTIONS_SUBMIT_FORM = 'update-step-update-options-submit-form'; /* step: backup */ const UPDATE_PAGE_BACKUP = 'update-page-backup'; diff --git a/classes/Twig/Form/UpgradeOptionsForm.php b/classes/Twig/Form/UpgradeOptionsForm.php index c8b805ea0..66f40767c 100644 --- a/classes/Twig/Form/UpgradeOptionsForm.php +++ b/classes/Twig/Form/UpgradeOptionsForm.php @@ -79,13 +79,13 @@ public function __construct(Translator $translator, FormRenderer $formRenderer) 'desc' => $translator->trans('This will change your theme: your shop will then use the default theme of the version of PrestaShop you are upgrading to.'), ], - UpgradeConfiguration::PS_AUTOUP_KEEP_MAILS => [ - 'title' => $translator->trans('Keep the customized email templates'), + UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL => [ + 'title' => $translator->trans('Regenerate the customized email templates'), 'cast' => 'intval', 'validation' => 'isBool', 'type' => 'bool', 'desc' => $translator->trans('This will not upgrade the default PrestaShop e-mails.') . '
' - . $translator->trans('If you customized the default PrestaShop e-mail templates, enabling this option will keep your modifications.'), + . $translator->trans('If you customized the default PrestaShop e-mail templates, switching off this option will keep your modifications.'), ], ]; } diff --git a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader17.php b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader17.php index 3a1d98e76..a3893039b 100644 --- a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader17.php +++ b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader17.php @@ -62,7 +62,7 @@ protected function upgradeLanguage($lang): void $lang_pack = \Language::getLangDetails($isoCode); \Language::installSfLanguagePack($lang_pack['locale'], $errorsLanguage); - if (!$this->container->getUpgradeConfiguration()->shouldKeepMails()) { + if ($this->container->getUpgradeConfiguration()->shouldRegenerateMailTemplates()) { \Language::installEmailsLanguagePack($lang_pack, $errorsLanguage); } diff --git a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php index 7d477cc98..6bee53506 100644 --- a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php +++ b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php @@ -87,7 +87,7 @@ protected function upgradeLanguage($lang): void $lang_pack = \Language::getLangDetails($isoCode); \Language::installSfLanguagePack($lang_pack['locale'], $errorsLanguage); - if (!$this->container->getUpgradeConfiguration()->shouldKeepMails()) { + if ($this->container->getUpgradeConfiguration()->shouldRegenerateMailTemplates()) { $this->logger->debug($this->container->getTranslator()->trans('Generating mail templates for %lang%', ['%lang%' => $isoCode])); $mailTheme = \Configuration::get('PS_MAIL_THEME', null, null, null, 'modern'); diff --git a/controllers/admin/AdminSelfUpgradeController.php b/controllers/admin/AdminSelfUpgradeController.php index ad3aa54e9..0e3d89f88 100644 --- a/controllers/admin/AdminSelfUpgradeController.php +++ b/controllers/admin/AdminSelfUpgradeController.php @@ -238,13 +238,13 @@ private function _setFields() 'type' => 'bool', 'desc' => $this->trans('This will change your theme: your shop will then use the default theme of the version of PrestaShop you are upgrading to.'), ], - UpgradeConfiguration::PS_AUTOUP_KEEP_MAILS => [ - 'title' => $this->trans('Keep the customized email templates'), + UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL => [ + 'title' => $this->trans('Regenerate the customized email templates'), 'cast' => 'intval', 'validation' => 'isBool', 'type' => 'bool', 'desc' => $this->trans('This will not upgrade the default PrestaShop e-mails.') . '
' - . $this->trans('If you customized the default PrestaShop e-mail templates, enabling this option will keep your modifications.'), + . $this->trans('If you customized the default PrestaShop e-mail templates, switching off this option will keep your modifications.'), ], ]; } diff --git a/controllers/admin/self-managed/AbstractPageWithStepController.php b/controllers/admin/self-managed/AbstractPageWithStepController.php index 92c74141c..d9ea1ea6c 100644 --- a/controllers/admin/self-managed/AbstractPageWithStepController.php +++ b/controllers/admin/self-managed/AbstractPageWithStepController.php @@ -40,11 +40,18 @@ public function step() return new Response('Unexpected call to a step route outside an ajax call.', 404); } + // It may be tempting to move this line inside the parameters of the method + // `getTwig()->render()`. Please refrain to do so as this makes Twig + // called BEFORE the call to the function sent as parameters. Initiating it too early + // can be misleading when rendering the templates as more autoloaders can be loaded + // in the meantime (i.e the core). + $params = $this->getParams(); + return AjaxResponseBuilder::hydrationResponse( PageSelectors::STEP_PARENT_ID, $this->getTwig()->render( '@ModuleAutoUpgrade/steps/' . $this->getStepTemplate() . '.html.twig', - $this->getParams() + $params ), $this->displayRouteInUrl() ); diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index 6a292c55a..8ba696827 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -27,8 +27,12 @@ namespace PrestaShop\Module\AutoUpgrade\Controller; +use PrestaShop\Module\AutoUpgrade\AjaxResponseBuilder; +use PrestaShop\Module\AutoUpgrade\Parameters\UpgradeConfiguration; +use PrestaShop\Module\AutoUpgrade\Parameters\UpgradeFileNames; use PrestaShop\Module\AutoUpgrade\Router\Routes; use PrestaShop\Module\AutoUpgrade\Twig\UpdateSteps; +use Symfony\Component\HttpFoundation\JsonResponse; class UpdatePageUpdateOptionsController extends AbstractPageWithStepController { @@ -49,6 +53,31 @@ protected function displayRouteInUrl(): ?string return Routes::UPDATE_PAGE_UPDATE_OPTIONS; } + public function saveOption(): JsonResponse + { + $name = $this->request->request->get('name'); + if ($name === 'PS_DISABLE_OVERRIDES') { + UpgradeConfiguration::updatePSDisableOverrides((bool) $this->request->request->get('value')); + } + + $upgradeConfiguration = $this->upgradeContainer->getUpgradeConfiguration(); + $upgradeConfigurationStorage = $this->upgradeContainer->getUpgradeConfigurationStorage(); + + // TODO: Call the validator + + $upgradeConfiguration->merge([ + $name => $this->request->request->get('value'), + ]); + + $success = $upgradeConfigurationStorage->save($upgradeConfiguration, UpgradeFileNames::CONFIG_FILENAME); + return new JsonResponse(['success' => $success]); + } + + public function submit(): JsonResponse + { + return AjaxResponseBuilder::nextRouteResponse(Routes::UPDATE_PAGE_BACKUP); + } + /** * @return array * @@ -56,15 +85,19 @@ protected function displayRouteInUrl(): ?string */ protected function getParams(): array { + $this->upgradeContainer->initPrestaShopCore(); + $upgradeConfiguration = $this->upgradeContainer->getUpgradeConfiguration(); $updateSteps = new UpdateSteps($this->upgradeContainer->getTranslator()); return array_merge( $updateSteps->getStepParams(self::CURRENT_STEP), [ - 'default_deactive_non_native_modules' => true, - 'default_regenerate_email_templates' => true, - 'switch_the_theme' => '1', - 'disable_all_overrides' => false, + 'form_route_to_save' => Routes::UPDATE_STEP_UPDATE_OPTIONS_SAVE_OPTION, + 'form_route_to_submit' => Routes::UPDATE_STEP_UPDATE_OPTIONS_SUBMIT_FORM, + + 'default_deactive_non_native_modules' => $upgradeConfiguration->shouldDeactivateCustomModules(), + 'default_regenerate_email_templates' => $upgradeConfiguration->shouldRegenerateMailTemplates(), + 'disable_all_overrides' => !$upgradeConfiguration->isOverrideAllowed(), ] ); } diff --git a/controllers/admin/self-managed/UpdatePageVersionChoiceController.php b/controllers/admin/self-managed/UpdatePageVersionChoiceController.php index d4fec2eb2..cafdebe7c 100644 --- a/controllers/admin/self-managed/UpdatePageVersionChoiceController.php +++ b/controllers/admin/self-managed/UpdatePageVersionChoiceController.php @@ -41,7 +41,7 @@ use PrestaShop\Module\AutoUpgrade\VersionUtils; use Symfony\Component\HttpFoundation\JsonResponse; -class UpdatePageVersionChoiceController extends AbstractPageController +class UpdatePageVersionChoiceController extends AbstractPageWithStepController { const CURRENT_STEP = UpdateSteps::STEP_VERSION_CHOICE; const FORM_NAME = 'version_choice'; diff --git a/tests/unit/AnalyticsTest.php b/tests/unit/AnalyticsTest.php index 9c3ab0a26..5e53ea3d3 100644 --- a/tests/unit/AnalyticsTest.php +++ b/tests/unit/AnalyticsTest.php @@ -43,7 +43,7 @@ public function testProperties() $upgradeConfiguration = (new UpgradeConfiguration([ UpgradeConfiguration::PS_AUTOUP_CUSTOM_MOD_DESACT => 0, UpgradeConfiguration::PS_AUTOUP_CHANGE_DEFAULT_THEME => 1, - UpgradeConfiguration::PS_AUTOUP_KEEP_MAILS => 0, + UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL => 1, UpgradeConfiguration::PS_AUTOUP_BACKUP => 1, UpgradeConfiguration::PS_AUTOUP_KEEP_IMAGES => 0, UpgradeConfiguration::CHANNEL => 'major', diff --git a/views/templates/steps/update-options.html.twig b/views/templates/steps/update-options.html.twig index da934f788..c1b17665a 100644 --- a/views/templates/steps/update-options.html.twig +++ b/views/templates/steps/update-options.html.twig @@ -7,7 +7,14 @@ {% endblock %} {% block content %} -
+
{% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { id: "PS_AUTOUP_CUSTOM_MOD_DESACT", name: "PS_AUTOUP_CUSTOM_MOD_DESACT", @@ -34,11 +41,14 @@ "you to avoid conflicts during and after updates.", value: disable_all_overrides } %} -
+ {% endblock %} {% block buttons_inner %} - From 2dc6f9d9d44bfa44eeee6f8cd4389a1a8f68e35c Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Mon, 28 Oct 2024 19:11:07 +0000 Subject: [PATCH 02/13] Plug switch of overrides --- classes/Parameters/UpgradeConfiguration.php | 2 +- .../self-managed/UpdatePageUpdateOptionsController.php | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/classes/Parameters/UpgradeConfiguration.php b/classes/Parameters/UpgradeConfiguration.php index 099a97b27..8758336fa 100644 --- a/classes/Parameters/UpgradeConfiguration.php +++ b/classes/Parameters/UpgradeConfiguration.php @@ -232,7 +232,7 @@ private function computeBooleanConfiguration(string $const): bool public static function isOverrideAllowed(): bool { - return (bool) Configuration::get(self::PS_DISABLE_OVERRIDES); + return !Configuration::get(self::PS_DISABLE_OVERRIDES); } public static function updateDisabledOverride(bool $value, ?int $shopId = null): void diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index 8ba696827..df1ce5130 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -57,7 +57,9 @@ public function saveOption(): JsonResponse { $name = $this->request->request->get('name'); if ($name === 'PS_DISABLE_OVERRIDES') { - UpgradeConfiguration::updatePSDisableOverrides((bool) $this->request->request->get('value')); + $this->upgradeContainer->initPrestaShopCore(); + UpgradeConfiguration::updatePSDisableOverrides($this->request->request->getBoolean('value')); + return new JsonResponse(['success' => true]); } $upgradeConfiguration = $this->upgradeContainer->getUpgradeConfiguration(); @@ -66,7 +68,7 @@ public function saveOption(): JsonResponse // TODO: Call the validator $upgradeConfiguration->merge([ - $name => $this->request->request->get('value'), + $name => $this->request->request->getBoolean('value'), ]); $success = $upgradeConfigurationStorage->save($upgradeConfiguration, UpgradeFileNames::CONFIG_FILENAME); From 20e39139c3ac7c137c03e26f885e30a2a9b9a679 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Tue, 29 Oct 2024 11:38:53 +0000 Subject: [PATCH 03/13] Plug the validator --- _dev/src/ts/pages/UpdatePageUpdateOptions.ts | 8 +-- .../UpdatePageUpdateOptionsController.php | 62 ++++++++++++++----- .../templates/steps/update-options.html.twig | 21 ++++--- 3 files changed, 63 insertions(+), 28 deletions(-) diff --git a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts index b9c6ea8de..05acb503d 100644 --- a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts +++ b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts @@ -41,7 +41,7 @@ export default class UpdatePageUpdateOptions extends UpdatePage { private initFormFields(form: HTMLFormElement): void { Array.from(form.elements).forEach((element) => { - if (element.nodeName !== "INPUT") { + if (element.nodeName !== 'INPUT') { return; } @@ -52,17 +52,17 @@ export default class UpdatePageUpdateOptions extends UpdatePage { } private onInputChange = (ev: Event) => { - const optionInput = (ev.target as Element); + const optionInput = ev.target as HTMLInputElement; optionInput.setAttribute('disabled', 'true'); const data = new FormData(); data.append('name', optionInput.name); - data.append('value', optionInput.checked); + data.append('value', JSON.stringify(optionInput.checked)); api.post(this.form.dataset.routeToSave!, data); optionInput.removeAttribute('disabled'); - } + }; private onFormSubmit = (event: Event) => { event.preventDefault(); diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index df1ce5130..28cadde90 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -31,6 +31,7 @@ use PrestaShop\Module\AutoUpgrade\Parameters\UpgradeConfiguration; use PrestaShop\Module\AutoUpgrade\Parameters\UpgradeFileNames; use PrestaShop\Module\AutoUpgrade\Router\Routes; +use PrestaShop\Module\AutoUpgrade\Twig\PageSelectors; use PrestaShop\Module\AutoUpgrade\Twig\UpdateSteps; use Symfony\Component\HttpFoundation\JsonResponse; @@ -55,24 +56,32 @@ protected function displayRouteInUrl(): ?string public function saveOption(): JsonResponse { - $name = $this->request->request->get('name'); - if ($name === 'PS_DISABLE_OVERRIDES') { - $this->upgradeContainer->initPrestaShopCore(); - UpgradeConfiguration::updatePSDisableOverrides($this->request->request->getBoolean('value')); - return new JsonResponse(['success' => true]); - } - $upgradeConfiguration = $this->upgradeContainer->getUpgradeConfiguration(); $upgradeConfigurationStorage = $this->upgradeContainer->getUpgradeConfigurationStorage(); - // TODO: Call the validator + $name = $this->request->request->get('name'); - $upgradeConfiguration->merge([ + $config = [ $name => $this->request->request->getBoolean('value'), - ]); + ]; + + // TODO: Remove after rebase + $upgradeConfiguration->validate($config); + $error = null; + // TODO: Uncomment after rebase + // $error = $this->upgradeContainer->getConfigurationValidator()->validate($config); + + if (empty($error)) { + if ($name === 'PS_DISABLE_OVERRIDES') { + $this->upgradeContainer->initPrestaShopCore(); + UpgradeConfiguration::updatePSDisableOverrides($this->request->request->getBoolean('value')); + } else { + $upgradeConfiguration->merge($config); + $upgradeConfigurationStorage->save($upgradeConfiguration, UpgradeFileNames::CONFIG_FILENAME); + } + } - $success = $upgradeConfigurationStorage->save($upgradeConfiguration, UpgradeFileNames::CONFIG_FILENAME); - return new JsonResponse(['success' => $success]); + return $this->getRefreshOfForm(array_merge($this->getParams(), ['error' => $error])); } public function submit(): JsonResponse @@ -97,10 +106,33 @@ protected function getParams(): array 'form_route_to_save' => Routes::UPDATE_STEP_UPDATE_OPTIONS_SAVE_OPTION, 'form_route_to_submit' => Routes::UPDATE_STEP_UPDATE_OPTIONS_SUBMIT_FORM, - 'default_deactive_non_native_modules' => $upgradeConfiguration->shouldDeactivateCustomModules(), - 'default_regenerate_email_templates' => $upgradeConfiguration->shouldRegenerateMailTemplates(), - 'disable_all_overrides' => !$upgradeConfiguration->isOverrideAllowed(), + 'form_fields' => [ + 'deactive_non_native_modules' => [ + 'field' => 'PS_AUTOUP_CUSTOM_MOD_DESACT', + 'value' => $upgradeConfiguration->shouldDeactivateCustomModules(), + ], + 'regenerate_email_templates' => [ + 'field' => 'PS_AUTOUP_REGEN_EMAIL', + 'value' => $upgradeConfiguration->shouldRegenerateMailTemplates(), + ], + 'disable_all_overrides' => [ + 'field' => 'PS_DISABLE_OVERRIDES', + 'value' => !$upgradeConfiguration->isOverrideAllowed(), + ], + ], ] ); } + + private function getRefreshOfForm(array $params): JsonResponse + { + return AjaxResponseBuilder::hydrationResponse( + PageSelectors::STEP_PARENT_ID, + $this->getTwig()->render( + '@ModuleAutoUpgrade/steps/' . $this->getStepTemplate() . '.html.twig', + $params + ), + $this->displayRouteInUrl() + ); + } } diff --git a/views/templates/steps/update-options.html.twig b/views/templates/steps/update-options.html.twig index c1b17665a..b95d59e6c 100644 --- a/views/templates/steps/update-options.html.twig +++ b/views/templates/steps/update-options.html.twig @@ -16,30 +16,33 @@ name="update-options-page-form" > {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { - id: "PS_AUTOUP_CUSTOM_MOD_DESACT", - name: "PS_AUTOUP_CUSTOM_MOD_DESACT", + id: form_fields.deactive_non_native_modules.field, + name: form_fields.deactive_non_native_modules.field, title: "Deactivate non-native modules", description: "All the modules installed after creating your store are considered non-native modules. They might be " ~ "incompatible with the new version of PrestaShop. We recommend deactivating them during the update.", - value: default_deactive_non_native_modules + value: form_fields.deactive_non_native_modules.value, + error_message: error[form_fields.deactive_non_native_modules.field], } %} {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { - id: "PS_AUTOUP_REGEN_EMAIL", - name: "PS_AUTOUP_REGEN_EMAIL", + id: form_fields.regenerate_email_templates.field, + name: form_fields.regenerate_email_templates.field, title: "Regenerate email templates", description: "If you've customized email templates, your changes will be lost if you activate this option.", - value: default_regenerate_email_templates + value: form_fields.regenerate_email_templates.value, + error_message: error[form_fields.regenerate_email_templates.field], } %} {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { - id: "PS_AUTOUP_DISABLE_OVERRIDE", - name: "PS_AUTOUP_DISABLE_OVERRIDE", + id: form_fields.disable_all_overrides.field, + name: form_fields.disable_all_overrides.field, title: "Disable all overrides", description: "Overriding is a way to replace business behaviors (class files and controller files) to target only " ~ "one method or as many as you need. This option disables all classes & controllers overrides, allowing " ~ "you to avoid conflicts during and after updates.", - value: disable_all_overrides + value: form_fields.disable_all_overrides.value, + error_message: error[form_fields.disable_all_overrides.field], } %} {% endblock %} From fd0145c196a65ba7deeeb8166a1b4fd8d4783e91 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Wed, 30 Oct 2024 11:56:58 +0000 Subject: [PATCH 04/13] Update storybook story --- .../stories/pages/UpdateOptions.stories.js | 26 ++++++++++++++++--- .../templates/steps/update-options.html.twig | 6 ++--- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/storybook/stories/pages/UpdateOptions.stories.js b/storybook/stories/pages/UpdateOptions.stories.js index fa6867f4f..033bb462b 100644 --- a/storybook/stories/pages/UpdateOptions.stories.js +++ b/storybook/stories/pages/UpdateOptions.stories.js @@ -39,12 +39,30 @@ export const UpdateOptions = { code: "update-options", title: "Update options", }, - default_deactive_non_native_modules: true, - default_regenerate_email_templates: true, - switch_the_theme: 1, - disable_all_overrides: false, + form_fields: { + deactive_non_native_modules: { + field: 'PS_AUTOUP_CUSTOM_MOD_DESACT', + value: true, + }, + regenerate_email_templates: { + field: 'PS_AUTOUP_REGEN_EMAIL', + value: true, + }, + disable_all_overrides: { + field: 'PS_DISABLE_OVERRIDES', + value: false, + }, + }, step_parent_id: "ua_container", stepper_parent_id: "stepper_content", + + form_route_to_save: "update-step-update-options-save-option", + form_route_to_submit: "update-step-update-options-submit-form", + + error: { + 'PS_AUTOUP_REGEN_EMAIL': 'Example of an error that occured when switching the value!', + }, + // Stepper ...Stepper.args, }, diff --git a/views/templates/steps/update-options.html.twig b/views/templates/steps/update-options.html.twig index b95d59e6c..c1dd33e45 100644 --- a/views/templates/steps/update-options.html.twig +++ b/views/templates/steps/update-options.html.twig @@ -22,7 +22,7 @@ description: "All the modules installed after creating your store are considered non-native modules. They might be " ~ "incompatible with the new version of PrestaShop. We recommend deactivating them during the update.", value: form_fields.deactive_non_native_modules.value, - error_message: error[form_fields.deactive_non_native_modules.field], + error_message: error[form_fields.deactive_non_native_modules.field] is defined ? error[form_fields.deactive_non_native_modules.field] : null, } %} {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { @@ -31,7 +31,7 @@ title: "Regenerate email templates", description: "If you've customized email templates, your changes will be lost if you activate this option.", value: form_fields.regenerate_email_templates.value, - error_message: error[form_fields.regenerate_email_templates.field], + error_message: error[form_fields.regenerate_email_templates.field] is defined ? error[form_fields.regenerate_email_templates.field] : null, } %} {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { @@ -42,7 +42,7 @@ "one method or as many as you need. This option disables all classes & controllers overrides, allowing " ~ "you to avoid conflicts during and after updates.", value: form_fields.disable_all_overrides.value, - error_message: error[form_fields.disable_all_overrides.field], + error_message: error[form_fields.disable_all_overrides.field] is defined ? error[form_fields.disable_all_overrides.field] : null, } %} {% endblock %} From 24ab9d8f743a3f59b443f6a1ae1cb3a92f4b108b Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Mon, 4 Nov 2024 17:46:33 +0000 Subject: [PATCH 05/13] Handle comments waiting for rebase --- _dev/src/ts/pages/UpdatePageUpdateOptions.ts | 35 ++++--------------- .../UpdatePageUpdateOptionsController.php | 8 ++--- .../templates/steps/update-options.html.twig | 12 +++---- 3 files changed, 15 insertions(+), 40 deletions(-) diff --git a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts index 05acb503d..95d42547f 100644 --- a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts +++ b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts @@ -3,28 +3,19 @@ import api from '../api/RequestHandler'; export default class UpdatePageUpdateOptions extends UpdatePage { protected stepCode = 'update-options'; - protected form: HTMLFormElement; - protected formFields: Element[] = []; - - constructor() { - super(); - this.form = this.assertAndGetForm(); - } public mount() { this.initStepper(); - this.initFormFields(this.form); - this.form.addEventListener('submit', this.onFormSubmit); + this.form.addEventListener('submit', this.onSubmit); + this.form.addEventListener('change', this.onChange); } public beforeDestroy() { - this.form.removeEventListener('submit', this.onFormSubmit); - this.formFields.forEach((element) => { - element.removeEventListener('change', this.onInputChange); - }); + this.form.removeEventListener('submit', this.onSubmit); + this.form.removeEventListener('change', this.onChange); } - private assertAndGetForm(): HTMLFormElement { + private get form(): HTMLFormElement { const form = document.forms.namedItem('update-options-page-form'); if (!form) { throw new Error('Form not found'); @@ -39,19 +30,7 @@ export default class UpdatePageUpdateOptions extends UpdatePage { return form; } - private initFormFields(form: HTMLFormElement): void { - Array.from(form.elements).forEach((element) => { - if (element.nodeName !== 'INPUT') { - return; - } - - this.formFields.push(element); - - element.addEventListener('change', this.onInputChange); - }); - } - - private onInputChange = (ev: Event) => { + private readonly onChange = (ev: Event) => { const optionInput = ev.target as HTMLInputElement; optionInput.setAttribute('disabled', 'true'); @@ -64,7 +43,7 @@ export default class UpdatePageUpdateOptions extends UpdatePage { optionInput.removeAttribute('disabled'); }; - private onFormSubmit = (event: Event) => { + private readonly onSubmit = (event: Event) => { event.preventDefault(); api.post(this.form.dataset.routeToSubmit!, new FormData(this.form)); diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index 28cadde90..021708fb6 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -62,14 +62,10 @@ public function saveOption(): JsonResponse $name = $this->request->request->get('name'); $config = [ - $name => $this->request->request->getBoolean('value'), + $name => $this->request->request->get('value'), ]; - // TODO: Remove after rebase - $upgradeConfiguration->validate($config); - $error = null; - // TODO: Uncomment after rebase - // $error = $this->upgradeContainer->getConfigurationValidator()->validate($config); + $error = $this->upgradeContainer->getConfigurationValidator()->validate($config); if (empty($error)) { if ($name === 'PS_DISABLE_OVERRIDES') { diff --git a/views/templates/steps/update-options.html.twig b/views/templates/steps/update-options.html.twig index c1dd33e45..b19eb4d83 100644 --- a/views/templates/steps/update-options.html.twig +++ b/views/templates/steps/update-options.html.twig @@ -18,9 +18,9 @@ {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { id: form_fields.deactive_non_native_modules.field, name: form_fields.deactive_non_native_modules.field, - title: "Deactivate non-native modules", + title: "Deactivate non-native modules"|trans({}), description: "All the modules installed after creating your store are considered non-native modules. They might be " ~ - "incompatible with the new version of PrestaShop. We recommend deactivating them during the update.", + "incompatible with the new version of PrestaShop. We recommend deactivating them during the update."|trans({}), value: form_fields.deactive_non_native_modules.value, error_message: error[form_fields.deactive_non_native_modules.field] is defined ? error[form_fields.deactive_non_native_modules.field] : null, } %} @@ -28,8 +28,8 @@ {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { id: form_fields.regenerate_email_templates.field, name: form_fields.regenerate_email_templates.field, - title: "Regenerate email templates", - description: "If you've customized email templates, your changes will be lost if you activate this option.", + title: "Regenerate email templates"|trans({}), + description: "If you've customized email templates, your changes will be lost if you activate this option."|trans({}), value: form_fields.regenerate_email_templates.value, error_message: error[form_fields.regenerate_email_templates.field] is defined ? error[form_fields.regenerate_email_templates.field] : null, } %} @@ -37,10 +37,10 @@ {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { id: form_fields.disable_all_overrides.field, name: form_fields.disable_all_overrides.field, - title: "Disable all overrides", + title: "Disable all overrides"|trans({}), description: "Overriding is a way to replace business behaviors (class files and controller files) to target only " ~ "one method or as many as you need. This option disables all classes & controllers overrides, allowing " ~ - "you to avoid conflicts during and after updates.", + "you to avoid conflicts during and after updates."|trans({}), value: form_fields.disable_all_overrides.value, error_message: error[form_fields.disable_all_overrides.field] is defined ? error[form_fields.disable_all_overrides.field] : null, } %} From 93cab0073c3e961c9d6cdd1de2eec6db023013a0 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Fri, 8 Nov 2024 11:06:31 +0000 Subject: [PATCH 06/13] Set window properties immediately --- _dev/jest.setup.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/_dev/jest.setup.ts b/_dev/jest.setup.ts index b75fe11d1..57a0f94d7 100644 --- a/_dev/jest.setup.ts +++ b/_dev/jest.setup.ts @@ -1,8 +1,9 @@ -beforeAll(() => { - window.AutoUpgradeVariables = { - token: 'test-token', - admin_url: 'http://localhost', - admin_dir: '/admin_directory', - stepper_parent_id: 'stepper_content' - }; -}); +// We don't wait for the call to beforeAll to define window properties. +window.AutoUpgradeVariables = { + token: 'test-token', + admin_url: 'http://localhost', + admin_dir: '/admin_directory', + stepper_parent_id: 'stepper_content' +}; + +beforeAll(() => {}); From 4888f0fd93ad86d3a160175daa1f05eeeeb138e8 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Fri, 8 Nov 2024 11:12:35 +0000 Subject: [PATCH 07/13] Add await on API calls --- _dev/src/ts/pages/UpdatePageUpdateOptions.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts index 95d42547f..921f006de 100644 --- a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts +++ b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts @@ -30,7 +30,7 @@ export default class UpdatePageUpdateOptions extends UpdatePage { return form; } - private readonly onChange = (ev: Event) => { + private readonly onChange = async (ev: Event) => { const optionInput = ev.target as HTMLInputElement; optionInput.setAttribute('disabled', 'true'); @@ -38,14 +38,14 @@ export default class UpdatePageUpdateOptions extends UpdatePage { const data = new FormData(); data.append('name', optionInput.name); data.append('value', JSON.stringify(optionInput.checked)); - api.post(this.form.dataset.routeToSave!, data); + await api.post(this.form.dataset.routeToSave!, data); optionInput.removeAttribute('disabled'); }; - private readonly onSubmit = (event: Event) => { + private readonly onSubmit = async (event: Event) => { event.preventDefault(); - api.post(this.form.dataset.routeToSubmit!, new FormData(this.form)); + await api.post(this.form.dataset.routeToSubmit!, new FormData(this.form)); }; } From 0e8ae81b22877797c52ce9e7ee028f4c2415698c Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Mon, 11 Nov 2024 17:34:27 +0000 Subject: [PATCH 08/13] Add formater for validator errors --- classes/Twig/ValidatorToFormFormater.php | 25 +++++++++++++++++++ .../UpdatePageUpdateOptionsController.php | 10 +++++--- .../UpdatePageVersionChoiceController.php | 14 ++--------- .../templates/steps/update-options.html.twig | 6 ++--- 4 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 classes/Twig/ValidatorToFormFormater.php diff --git a/classes/Twig/ValidatorToFormFormater.php b/classes/Twig/ValidatorToFormFormater.php new file mode 100644 index 000000000..aa73db0c5 --- /dev/null +++ b/classes/Twig/ValidatorToFormFormater.php @@ -0,0 +1,25 @@ + $errors + * + * @return array<'global'|string, string> + */ + public static function format($errors): array + { + return array_column( + array_map(function ($error) { + return [ + 'key' => $error['target'] ?? 'global', + 'value' => $error['message'], + ]; + }, $errors), + 'value', + 'key' + ); + } +} diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index 021708fb6..29243195e 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -33,6 +33,7 @@ use PrestaShop\Module\AutoUpgrade\Router\Routes; use PrestaShop\Module\AutoUpgrade\Twig\PageSelectors; use PrestaShop\Module\AutoUpgrade\Twig\UpdateSteps; +use PrestaShop\Module\AutoUpgrade\Twig\ValidatorToFormFormater; use Symfony\Component\HttpFoundation\JsonResponse; class UpdatePageUpdateOptionsController extends AbstractPageWithStepController @@ -65,9 +66,9 @@ public function saveOption(): JsonResponse $name => $this->request->request->get('value'), ]; - $error = $this->upgradeContainer->getConfigurationValidator()->validate($config); + $errors = $this->upgradeContainer->getConfigurationValidator()->validate($config); - if (empty($error)) { + if (empty($errors)) { if ($name === 'PS_DISABLE_OVERRIDES') { $this->upgradeContainer->initPrestaShopCore(); UpgradeConfiguration::updatePSDisableOverrides($this->request->request->getBoolean('value')); @@ -77,7 +78,10 @@ public function saveOption(): JsonResponse } } - return $this->getRefreshOfForm(array_merge($this->getParams(), ['error' => $error])); + return $this->getRefreshOfForm(array_merge( + $this->getParams(), + ['errors' => ValidatorToFormFormater::format($errors)] + )); } public function submit(): JsonResponse diff --git a/controllers/admin/self-managed/UpdatePageVersionChoiceController.php b/controllers/admin/self-managed/UpdatePageVersionChoiceController.php index cafdebe7c..cb7914543 100644 --- a/controllers/admin/self-managed/UpdatePageVersionChoiceController.php +++ b/controllers/admin/self-managed/UpdatePageVersionChoiceController.php @@ -36,6 +36,7 @@ use PrestaShop\Module\AutoUpgrade\Services\PhpVersionResolverService; use PrestaShop\Module\AutoUpgrade\Twig\PageSelectors; use PrestaShop\Module\AutoUpgrade\Twig\UpdateSteps; +use PrestaShop\Module\AutoUpgrade\Twig\ValidatorToFormFormater; use PrestaShop\Module\AutoUpgrade\UpgradeContainer; use PrestaShop\Module\AutoUpgrade\UpgradeSelfCheck; use PrestaShop\Module\AutoUpgrade\VersionUtils; @@ -222,24 +223,13 @@ public function save(): JsonResponse if ($channel !== null) { $params[$channel . '_requirements'] = $this->getRequirements(); } - } else { - $errors = array_column( - array_map(function ($error) { - return [ - 'key' => $error['target'] ?? 'global', - 'value' => $error['message'], - ]; - }, $errors), - 'value', - 'key' - ); } $params = array_merge( $params, [ 'current_values' => $requestConfig, - 'errors' => $errors, + 'errors' => ValidatorToFormFormater::format($errors), ] ); diff --git a/views/templates/steps/update-options.html.twig b/views/templates/steps/update-options.html.twig index b19eb4d83..43230825b 100644 --- a/views/templates/steps/update-options.html.twig +++ b/views/templates/steps/update-options.html.twig @@ -22,7 +22,7 @@ description: "All the modules installed after creating your store are considered non-native modules. They might be " ~ "incompatible with the new version of PrestaShop. We recommend deactivating them during the update."|trans({}), value: form_fields.deactive_non_native_modules.value, - error_message: error[form_fields.deactive_non_native_modules.field] is defined ? error[form_fields.deactive_non_native_modules.field] : null, + error_message: errors[form_fields.deactive_non_native_modules.field] is defined ? errors[form_fields.deactive_non_native_modules.field] : null, } %} {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { @@ -31,7 +31,7 @@ title: "Regenerate email templates"|trans({}), description: "If you've customized email templates, your changes will be lost if you activate this option."|trans({}), value: form_fields.regenerate_email_templates.value, - error_message: error[form_fields.regenerate_email_templates.field] is defined ? error[form_fields.regenerate_email_templates.field] : null, + error_message: errors[form_fields.regenerate_email_templates.field] is defined ? errors[form_fields.regenerate_email_templates.field] : null, } %} {% include "@ModuleAutoUpgrade/components/render-switch.html.twig" with { @@ -42,7 +42,7 @@ "one method or as many as you need. This option disables all classes & controllers overrides, allowing " ~ "you to avoid conflicts during and after updates."|trans({}), value: form_fields.disable_all_overrides.value, - error_message: error[form_fields.disable_all_overrides.field] is defined ? error[form_fields.disable_all_overrides.field] : null, + error_message: errors[form_fields.disable_all_overrides.field] is defined ? errors[form_fields.disable_all_overrides.field] : null, } %} {% endblock %} From 208f186ef9f0989ed92a2305cb1957c376850b19 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Tue, 12 Nov 2024 11:41:11 +0000 Subject: [PATCH 09/13] Use UpgradeConfiguration constants --- .../self-managed/UpdatePageUpdateOptionsController.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index 29243195e..45a487194 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -69,7 +69,7 @@ public function saveOption(): JsonResponse $errors = $this->upgradeContainer->getConfigurationValidator()->validate($config); if (empty($errors)) { - if ($name === 'PS_DISABLE_OVERRIDES') { + if ($name === UpgradeConfiguration::PS_DISABLE_OVERRIDES) { $this->upgradeContainer->initPrestaShopCore(); UpgradeConfiguration::updatePSDisableOverrides($this->request->request->getBoolean('value')); } else { @@ -108,15 +108,15 @@ protected function getParams(): array 'form_fields' => [ 'deactive_non_native_modules' => [ - 'field' => 'PS_AUTOUP_CUSTOM_MOD_DESACT', + 'field' => UpgradeConfiguration::PS_AUTOUP_CUSTOM_MOD_DESACT, 'value' => $upgradeConfiguration->shouldDeactivateCustomModules(), ], 'regenerate_email_templates' => [ - 'field' => 'PS_AUTOUP_REGEN_EMAIL', + 'field' => UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL, 'value' => $upgradeConfiguration->shouldRegenerateMailTemplates(), ], 'disable_all_overrides' => [ - 'field' => 'PS_DISABLE_OVERRIDES', + 'field' => UpgradeConfiguration::PS_DISABLE_OVERRIDES, 'value' => !$upgradeConfiguration->isOverrideAllowed(), ], ], From b8538176994a040a44e70579824a807203160cae Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Tue, 12 Nov 2024 15:50:01 +0000 Subject: [PATCH 10/13] Rename segment property --- classes/Analytics.php | 2 +- tests/unit/AnalyticsTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/classes/Analytics.php b/classes/Analytics.php index 7e751f033..d1dc000e3 100644 --- a/classes/Analytics.php +++ b/classes/Analytics.php @@ -124,7 +124,7 @@ public function getProperties($type): array 'upgrade_channel' => $this->upgradeConfiguration->getChannel(), 'disable_non_native_modules' => $this->upgradeConfiguration->shouldDeactivateCustomModules(), 'switch_to_default_theme' => $this->upgradeConfiguration->shouldSwitchToDefaultTheme(), - 'keep_customized_email_templates' => !$this->upgradeConfiguration->shouldRegenerateMailTemplates(), + 'regenerate_customized_email_templates' => $this->upgradeConfiguration->shouldRegenerateMailTemplates(), ]; $upgradeProperties = $this->properties[self::WITH_UPDATE_PROPERTIES] ?? []; $additionalProperties = array_merge($upgradeProperties, $additionalProperties); diff --git a/tests/unit/AnalyticsTest.php b/tests/unit/AnalyticsTest.php index 5e53ea3d3..d17ff0bc4 100644 --- a/tests/unit/AnalyticsTest.php +++ b/tests/unit/AnalyticsTest.php @@ -97,7 +97,7 @@ public function testProperties() 'upgrade_channel' => 'major', 'disable_non_native_modules' => false, 'switch_to_default_theme' => true, - 'keep_customized_email_templates' => false, + 'regenerate_customized_email_templates' => true, 'regenerate_rtl_stylesheet' => false, ], ], From 1065239953f8f236b2592300462c0112478e62c2 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Wed, 13 Nov 2024 10:02:10 +0000 Subject: [PATCH 11/13] Move form retrieval logic from TS to PHP --- _dev/src/ts/pages/UpdatePageUpdateOptions.ts | 6 +----- classes/Parameters/ConfigurationValidator.php | 7 +++++-- classes/Parameters/UpgradeConfiguration.php | 4 +++- .../UpdatePageUpdateOptionsController.php | 15 +++++++-------- .../Parameters/ConfigurationValidatorTest.php | 2 +- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts index 921f006de..8b6fbefb3 100644 --- a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts +++ b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts @@ -33,13 +33,9 @@ export default class UpdatePageUpdateOptions extends UpdatePage { private readonly onChange = async (ev: Event) => { const optionInput = ev.target as HTMLInputElement; + const data = new FormData(this.form); optionInput.setAttribute('disabled', 'true'); - - const data = new FormData(); - data.append('name', optionInput.name); - data.append('value', JSON.stringify(optionInput.checked)); await api.post(this.form.dataset.routeToSave!, data); - optionInput.removeAttribute('disabled'); }; diff --git a/classes/Parameters/ConfigurationValidator.php b/classes/Parameters/ConfigurationValidator.php index d7dc12b67..377aab185 100644 --- a/classes/Parameters/ConfigurationValidator.php +++ b/classes/Parameters/ConfigurationValidator.php @@ -106,9 +106,12 @@ private function validateArchiveXml(string $xml, bool $isLocal): ?string return null; } - private function validateBool(string $boolValue, string $key): ?string + /** + * @param string|bool $boolValue + */ + private function validateBool($boolValue, string $key): ?string { - if ($boolValue === '' || filter_var($boolValue, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === null) { + if (!is_bool($boolValue) && ($boolValue === '' || filter_var($boolValue, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE) === null)) { return $this->translator->trans('Value must be a boolean for %s', [$key]); } diff --git a/classes/Parameters/UpgradeConfiguration.php b/classes/Parameters/UpgradeConfiguration.php index 8758336fa..0a61b1d6c 100644 --- a/classes/Parameters/UpgradeConfiguration.php +++ b/classes/Parameters/UpgradeConfiguration.php @@ -262,7 +262,9 @@ public static function updatePSDisableOverrides(bool $value): void public function merge(array $array = []): void { foreach ($array as $key => $value) { - $this->set($key, $value); + if (in_array($key, self::UPGRADE_CONST_KEYS)) { + $this->set($key, $value); + } } } } diff --git a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php index 45a487194..5e1d991cb 100644 --- a/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php +++ b/controllers/admin/self-managed/UpdatePageUpdateOptionsController.php @@ -60,22 +60,21 @@ public function saveOption(): JsonResponse $upgradeConfiguration = $this->upgradeContainer->getUpgradeConfiguration(); $upgradeConfigurationStorage = $this->upgradeContainer->getUpgradeConfigurationStorage(); - $name = $this->request->request->get('name'); - $config = [ - $name => $this->request->request->get('value'), + UpgradeConfiguration::PS_AUTOUP_CUSTOM_MOD_DESACT => $this->request->request->getBoolean(UpgradeConfiguration::PS_AUTOUP_CUSTOM_MOD_DESACT, false), + UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL => $this->request->request->getBoolean(UpgradeConfiguration::PS_AUTOUP_REGEN_EMAIL, false), + UpgradeConfiguration::PS_DISABLE_OVERRIDES => $this->request->request->getBoolean(UpgradeConfiguration::PS_DISABLE_OVERRIDES, false), ]; $errors = $this->upgradeContainer->getConfigurationValidator()->validate($config); if (empty($errors)) { - if ($name === UpgradeConfiguration::PS_DISABLE_OVERRIDES) { + if (isset($config[UpgradeConfiguration::PS_DISABLE_OVERRIDES])) { $this->upgradeContainer->initPrestaShopCore(); - UpgradeConfiguration::updatePSDisableOverrides($this->request->request->getBoolean('value')); - } else { - $upgradeConfiguration->merge($config); - $upgradeConfigurationStorage->save($upgradeConfiguration, UpgradeFileNames::CONFIG_FILENAME); + UpgradeConfiguration::updatePSDisableOverrides($config[UpgradeConfiguration::PS_DISABLE_OVERRIDES]); } + $upgradeConfiguration->merge($config); + $upgradeConfigurationStorage->save($upgradeConfiguration, UpgradeFileNames::CONFIG_FILENAME); } return $this->getRefreshOfForm(array_merge( diff --git a/tests/unit/Parameters/ConfigurationValidatorTest.php b/tests/unit/Parameters/ConfigurationValidatorTest.php index c60304cf8..9ac901ab3 100644 --- a/tests/unit/Parameters/ConfigurationValidatorTest.php +++ b/tests/unit/Parameters/ConfigurationValidatorTest.php @@ -117,7 +117,7 @@ public function testValidateXmlFail() public function testValidateBoolSuccess() { - $validValues = ['1', '0', 'true', 'false', 'on', 'off']; + $validValues = ['1', '0', 'true', 'false', 'on', 'off', true, false]; foreach ($validValues as $value) { $result = $this->validator->validate([UpgradeConfiguration::PS_AUTOUP_CUSTOM_MOD_DESACT => $value]); From 0ea40cf09fab01b7c2707da34f247266d1054f40 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Wed, 13 Nov 2024 12:12:36 +0000 Subject: [PATCH 12/13] Fix beforeDestroy and caller of isOverrideAllowed --- _dev/src/ts/pages/UpdatePageUpdateOptions.ts | 8 ++++++-- classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts index 8b6fbefb3..da646580f 100644 --- a/_dev/src/ts/pages/UpdatePageUpdateOptions.ts +++ b/_dev/src/ts/pages/UpdatePageUpdateOptions.ts @@ -11,8 +11,12 @@ export default class UpdatePageUpdateOptions extends UpdatePage { } public beforeDestroy() { - this.form.removeEventListener('submit', this.onSubmit); - this.form.removeEventListener('change', this.onChange); + try { + this.form.removeEventListener('submit', this.onSubmit); + this.form.removeEventListener('change', this.onChange); + } catch { + // Do Nothing, page is likely removed from the DOM already + } } private get form(): HTMLFormElement { diff --git a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php index 333df0b91..92721a3ee 100644 --- a/classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php +++ b/classes/UpgradeTools/CoreUpgrader/CoreUpgrader.php @@ -136,10 +136,10 @@ public function finalizeCoreUpdate(): void $this->cleanXmlFiles(); if (UpgradeConfiguration::isOverrideAllowed()) { + $this->logger->info($this->container->getTranslator()->trans('Keeping overrides in place')); + } else { $this->logger->info($this->container->getTranslator()->trans('Disabling overrides')); $this->disableOverrides(); - } else { - $this->logger->info($this->container->getTranslator()->trans('Keeping overrides in place')); } $this->updateTheme(); From b8b13514e6f0ad0b83ab704ba9ae0d9003261f27 Mon Sep 17 00:00:00 2001 From: Thomas Nabord Date: Wed, 13 Nov 2024 17:47:39 +0000 Subject: [PATCH 13/13] Revert filtering of data pushed to Configuration --- classes/Parameters/UpgradeConfiguration.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/classes/Parameters/UpgradeConfiguration.php b/classes/Parameters/UpgradeConfiguration.php index 0a61b1d6c..8758336fa 100644 --- a/classes/Parameters/UpgradeConfiguration.php +++ b/classes/Parameters/UpgradeConfiguration.php @@ -262,9 +262,7 @@ public static function updatePSDisableOverrides(bool $value): void public function merge(array $array = []): void { foreach ($array as $key => $value) { - if (in_array($key, self::UPGRADE_CONST_KEYS)) { - $this->set($key, $value); - } + $this->set($key, $value); } } }