Skip to content

Wrong asset urls in transactional email #15559

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.

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

Open
chipsmaster opened this issue May 28, 2018 · 5 comments
Open

Wrong asset urls in transactional email #15559

chipsmaster opened this issue May 28, 2018 · 5 comments
Labels
Area: Analytics / Reporting Component: Email Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: ready for dev Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it

Comments

@chipsmaster
Copy link

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) :

  • Sending emails from backend use the backend base url for static urls (it is a real problem if the admin url is to be hidden and/or is inaccessible from most IP).
  • Besides the "pub" problem ( MAGETWO-84709 - Respect Magento document root in CLI #14334 ) which is way more critical ; the app emulation works in CLI/cronjobs except there are cache problems (results stored in object fields). Then multiple template processing for different stores in the same process generates wrong static urls (it uses the first generated template store url).

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
@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label May 28, 2018
@chipsmaster
Copy link
Author

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 ghost self-assigned this Aug 3, 2018
@ghost ghost added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Component: Email labels Aug 3, 2018
@ghost ghost removed their assignment Aug 3, 2018
@ghost
Copy link

ghost commented Aug 3, 2018

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

@heldchen
Copy link
Contributor

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.

@ghost ghost removed Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development labels Oct 20, 2020
@magento-engcom-team magento-engcom-team added Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Nov 30, 2020
@ihor-sviziev
Copy link
Contributor

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

@heldchen
Copy link
Contributor

heldchen commented Jul 9, 2021

@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

@engcom-Alfa engcom-Alfa added Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Area: Analytics / Reporting Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed and removed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Aug 25, 2021
@engcom-Alfa engcom-Alfa added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed and removed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed labels Aug 25, 2021
@engcom-Hotel engcom-Hotel added Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch and removed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch labels Aug 25, 2021
@engcom-Bravo engcom-Bravo added the Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it label Jun 25, 2024
@engcom-Hotel engcom-Hotel moved this to Ready for Development in High Priority Backlog Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Analytics / Reporting Component: Email Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Priority: P2 A defect with this priority could have functionality issues which are not to expectations. Progress: ready for dev Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release Reproduced on 2.4.x The issue has been reproduced on latest 2.4-develop branch Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. Triage: Dev.Experience Issue related to Developer Experience and needs help with Triage to Confirm or Reject it
Projects
None yet
Development

No branches or pull requests

9 participants