magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.45k stars 9.29k forks source link

Wrong asset urls in transactional email #15559

Open chipsmaster opened 6 years ago

chipsmaster commented 6 years ago

Theme assets are managed by Magento\Framework\View\Asset\Repository and there are problems with it when using "app emulation" (used by email template processing) :

I put the two issues in same ticket to keep them in mind when reworking this asset management if any

I used the "Reset password" feature to demonstrate the issues but it's the same with order confirmation email.

Preconditions

  1. Magento 2.2.4
  2. Apache 2.4 / PHP 7.0.27

Steps to reproduce

  1. Setup a Magento with 2 websites with different domains as described here https://devdocs.magento.com/guides/v2.2/config-guide/multi-site/ms_apache.html + with a custom admin url to a different domain (Advanced > Admin > Admin Base URL > Custom Admin URL) ; so 3 domains in total
  2. Create "Customer 1" in the frontend of Website 1
  3. Create "Customer 2" in the frontend of Website 2
  4. Edit "Customer 2" in the backend and click "reset password"
  5. Create a CLI Command and/or a cronjob that calls the password reset process the same way the backend controller does (see cronjob class sample below) and make it run
namespace Xxx\MailBug\Cron;

class Test
{
    protected $storeManager;
    protected $customerRepository;
    protected $accountManagement;

    public function __construct(
        \Magento\Store\Model\StoreManagerInterface $storeManager,
        \Magento\Customer\Api\CustomerRepositoryInterface $customerRepository,
        \Magento\Customer\Api\AccountManagementInterface $accountManagement)
    {
        $this->storeManager = $storeManager;
        $this->customerRepository = $customerRepository;
        $this->accountManagement = $accountManagement;
    }

    public function execute()
    {
        echo "cron test email\n";

        /** @see \Magento\Customer\Controller\Adminhtml\Index\ResetPassword::execute() */
        $customer = $this->customerRepository->getById(/* Customer 2 id */ 2);
        $this->accountManagement->initiatePasswordReset(
            $customer->getEmail(),
            \Magento\Customer\Model\AccountManagement::EMAIL_REMINDER,
            $customer->getWebsiteId()
        );
        $customer = $this->customerRepository->getById(/* Customer 1 id */ 1);
        $this->accountManagement->initiatePasswordReset(
            $customer->getEmail(),
            \Magento\Customer\Model\AccountManagement::EMAIL_REMINDER,
            $customer->getWebsiteId()
        );
        echo "done\n";
    }
}

Expected result

  1. Received mails should have asset urls matching the desired store (for Customer password reset mails : the store or website in which the user registered), including logo url and inlined css assets in head section

Actual result

  1. Mail sent using the backend Reset password feature has admin static urls
  2. First mail sent using CLI/cron is ok (except "pub" problem), second mail sent from the same process has static urls from the wrong frontend store
chipsmaster commented 6 years ago

I'll detail here the fix i may use in my project (this is not necessarily the fix to be used by Magento)

The safest choice i think is to use a dedicated Asset Repository instance (instead of a singleton) during one Email Template processing (there is also its "baseUrl" argument to instantiate and change its class to Magento\Framework\Url instead of the backend one). Changing that could be achieved using di.xml configuration only. But i realized there are also a chain of objects during inline style generation that depend on Asset Repository and it could be too complicated to change everything

So my solution is to replace Asset Repository class by a custom () that detects when there is a app emulation and delegates public calls to either the default code or an Asset Repository that would have been instantiated when emulation started.

Chips.zip

ghost commented 6 years ago

@chipsmaster, thank you for your report. We've acknowledged the issue and added to our backlog.

heldchen commented 5 years ago

same problem on 2.3.1: sending password recovery emails from the backend results in wrong asset urls.

@chipsmaster thanks for the workaround example code. unfortunately I found that it requires even more tweaking than this, as the locale isn't properly set. my workaround was to enforce the new emulated store's locale by adding it to $params in a updateDesignParams override.

ihor-sviziev commented 3 years ago

Hi @chipsmaster, Could you confirm that this issue still reproducing on 2.4-develop branch?

heldchen commented 3 years ago

@ihor-sviziev this is still an issue in 2.4.2-p1 - static assets in order confirmation mails sent from the magento admin gui have admin static assets included. to reproduce it, ensure the magento backend uses a different domain, f.e. frontend: www.shop.com, backend: admin.shop.com