Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GenerateThemeMailTemplatesCommand during upgrade #434

Merged
merged 6 commits into from
Feb 16, 2022
Merged

Use GenerateThemeMailTemplatesCommand during upgrade #434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
abe45b1
Use GenerateThemeMailTemplatesCommand during upgrade instead of legac…
jolelievre Nov 2, 2021
6ea0622
Create new CoreUpgrade80 to handle backward compatibility regardless …
jolelievre Nov 3, 2021
ddf316d
Remove comment
atomiix Dec 16, 2021
be6d365
Get command bus from moduleAdapter
atomiix Jan 7, 2022
b99c34d
Fix ps version detection & mail generation output folders
atomiix Jan 7, 2022
86678ea
Fix tests
atomiix Feb 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/php.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
- name: PHP syntax checker 5.6
uses: prestashop/github-action-php-lint/5.6@master
with:
folder-to-exclude: "! -path \"./.github/*\""
folder-to-exclude: "! -path \"./.github/*\" ! -path \"./classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php\""

- name: PHP syntax checker 7.2
uses: prestashop/github-action-php-lint/7.2@master
Expand Down
9 changes: 7 additions & 2 deletions classes/TaskRunner/Upgrade/UpgradeDb.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
use PrestaShop\Module\AutoUpgrade\UpgradeException;
use PrestaShop\Module\AutoUpgrade\UpgradeTools\CoreUpgrader\CoreUpgrader16;
use PrestaShop\Module\AutoUpgrade\UpgradeTools\CoreUpgrader\CoreUpgrader17;
use PrestaShop\Module\AutoUpgrade\UpgradeTools\CoreUpgrader\CoreUpgrader80;
use PrestaShop\Module\AutoUpgrade\UpgradeTools\SettingsFileWriter;

class UpgradeDb extends AbstractTask
Expand Down Expand Up @@ -59,11 +60,15 @@ public function run()

public function getCoreUpgrader()
{
if (version_compare($this->container->getState()->getInstallVersion(), '1.7.0.0', '<=')) {
if (version_compare($this->container->getState()->getInstallVersion(), '1.7.0.0', '<')) {
return new CoreUpgrader16($this->container, $this->logger);
}

return new CoreUpgrader17($this->container, $this->logger);
if (version_compare($this->container->getState()->getInstallVersion(), '8.0.0', '<')) {
return new CoreUpgrader17($this->container, $this->logger);
}

return new CoreUpgrader80($this->container, $this->logger);
}

public function init()
Expand Down
103 changes: 103 additions & 0 deletions classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
<?php
/**
* Copyright since 2007 PrestaShop SA and Contributors
* PrestaShop is an International Registered Trademark & Property of PrestaShop SA
*
* NOTICE OF LICENSE
*
* This source file is subject to the Open Software License (OSL 3.0)
* that is bundled with this package in the file LICENSE.md.
* It is also available through the world-wide-web at this URL:
* https://opensource.org/licenses/OSL-3.0
* If you did not receive a copy of the license and are unable to
* obtain it through the world-wide-web, please send an email
* to license@prestashop.com so we can send you a copy immediately.
*
* DISCLAIMER
*
* Do not edit or add to this file if you wish to upgrade PrestaShop to newer
* versions in the future. If you wish to customize PrestaShop for your
* needs please refer to https://devdocs.prestashop.com/ for more information.
*
* @author PrestaShop SA and Contributors <contact@prestashop.com>
* @copyright Since 2007 PrestaShop SA and Contributors
* @license https://opensource.org/licenses/OSL-3.0 Open Software License (OSL 3.0)
*/

declare(strict_types=1);

namespace PrestaShop\Module\AutoUpgrade\UpgradeTools\CoreUpgrader;

use PrestaShop\Module\AutoUpgrade\UpgradeException;
use PrestaShop\PrestaShop\Core\CommandBus\CommandBusInterface;
use PrestaShop\PrestaShop\Core\Domain\MailTemplate\Command\GenerateThemeMailTemplatesCommand;
use PrestaShop\PrestaShop\Core\Exception\CoreException;

class CoreUpgrader80 extends CoreUpgrader
{
protected function initConstants()
{
parent::initConstants();

// Container may be needed to run upgrade scripts
$this->container->getSymfonyAdapter()->initAppKernel();
}

protected function upgradeDb($oldversion)
{
parent::upgradeDb($oldversion);

$commandResult = $this->container->getSymfonyAdapter()->runSchemaUpgradeCommand();
if (0 !== $commandResult['exitCode']) {
throw (new UpgradeException($this->container->getTranslator()->trans('Error upgrading Doctrine schema', [], 'Modules.Autoupgrade.Admin')))->setQuickInfos(explode("\n", $commandResult['output']));
}
}

protected function upgradeLanguage($lang)
{
$isoCode = $lang['iso_code'];

if (!\Validate::isLangIsoCode($isoCode)) {
return;
}
$errorsLanguage = [];

if (!\Language::downloadLanguagePack($isoCode, _PS_VERSION_, $errorsLanguage)) {
throw new UpgradeException($this->container->getTranslator()->trans('Download of the language pack %lang% failed. %details%', ['%lang%' => $isoCode, '%details%' => implode('; ', $errorsLanguage)], 'Modules.Autoupgrade.Admin'));
}

$lang_pack = \Language::getLangDetails($isoCode);
\Language::installSfLanguagePack($lang_pack['locale'], $errorsLanguage);

if (!$this->container->getUpgradeConfiguration()->shouldKeepMails()) {
$mailTheme = \Configuration::get('PS_MAIL_THEME', null, null, null, 'modern');

$frontTheme = _THEME_NAME_;
$frontThemeMailsFolder = _PS_ALL_THEMES_DIR_ . $frontTheme . '/mails';
$frontThemeModulesFolder = _PS_ALL_THEMES_DIR_ . $frontTheme . '/modules';

$generateCommand = new GenerateThemeMailTemplatesCommand(
$mailTheme,
$lang_pack['locale'],
true,
is_dir($frontThemeMailsFolder) ? $frontThemeMailsFolder : '',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these folders supposed to always exist? Is it really needed to check their existence?

I just checked the handler, which actually handles the default folder internally when the command parameters are empty, so maybe specifying these parameters is actually not useful here? We could ignore this and let that handler handle?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These folders aren't supposed to always exist, if you check the themes/classic/ folder, you'll see no mails folder.
Also, I see that's what we do in src/PrestaShopBundle/Controller/Admin/Improve/Design/MailThemeController.php

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see any modif related to the bug you mentioned regarding LegacyLoader, is it fixed in the core?

@jolelievre yep, it's fixed in the core ;)

is_dir($frontThemeModulesFolder) ? $frontThemeModulesFolder : ''
);
/** @var CommandBusInterface $commandBus */
$commandBus = $this->container->getModuleAdapter()->getCommandBus();

try {
$commandBus->handle($generateCommand);
} catch (CoreException $e) {
throw new UpgradeException($this->container->getTranslator()->trans('Cannot generate email templates: %s.', [$e->getMessage()], 'Modules.Autoupgrade.Admin'));
}
}

if (!empty($errorsLanguage)) {
throw new UpgradeException($this->container->getTranslator()->trans('Error while updating translations for lang %lang%. %details%', ['%lang%' => $isoCode, '%details%' => implode('; ', $errorsLanguage)], 'Modules.Autoupgrade.Admin'));
}
\Language::loadLanguages();

// TODO: Update AdminTranslationsController::addNewTabs to install tabs translated
}
}
1 change: 1 addition & 0 deletions tests/phpstan/phpstan-1.6.1.18.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ parameters:
- ./../../classes/pclzip.lib.php
- ./../../functions.php
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader17.php
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
- ./../../classes/UpgradeTools/SymfonyAdapter.php
ignoreErrors:
- '#Access to an undefined property Autoupgrade::\$bootstrap.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.2.5.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Autoupgrade::\$bootstrap.#'
- '#Access to an undefined property Module::\$installed.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.3.4.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Autoupgrade::\$bootstrap.#'
- '#Access to an undefined property Module::\$installed.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.4.4.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Autoupgrade::\$bootstrap.#'
- '#Access to an undefined property Module::\$installed.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.5.1.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Autoupgrade::\$bootstrap.#'
- '#Access to an undefined property Module::\$installed.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.6.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Autoupgrade::\$bootstrap.#'
- '#Access to an undefined property Module::\$installed.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.7.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Module::\$installed.#'
- '#Call to method fetchLocale\(\) on an unknown class PrestaShop\\PrestaShop\\Core\\Cldr\\Update.#'
Expand Down
2 changes: 2 additions & 0 deletions tests/phpstan/phpstan-1.7.8.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ includes:
- %currentWorkingDirectory%/tests/phpstan/phpstan.neon

parameters:
excludes_analyse:
- ./../../classes/UpgradeTools/CoreUpgrader/CoreUpgrader80.php
ignoreErrors:
- '#Access to an undefined property Module::\$installed.#'
- '#Call to method fetchLocale\(\) on an unknown class PrestaShop\\PrestaShop\\Core\\Cldr\\Update.#'
Expand Down