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.

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

Theme templates overrides don't work in environment emulation cross-area #10402

Closed
alessandroniciforo opened this issue Aug 2, 2017 · 4 comments

Comments

@alessandroniciforo
Copy link

commented Aug 2, 2017

The theme templates overrides are not resolved correctly if we're in an environment emulation cross-area (e.g.: we're in adminhtml emulating frontend).

My particular case has been this: I have a custom template overriding the payment method info template, I login in the Magento admin, find an order, click "Send email". The email won't have the overridden template.

Techincal details

To render the payment method info block Magento emulates the order's store in the frontend area (https://github.com/magento/magento2/blob/develop/app/code/Magento/Payment/Helper/Data.php#L208) and then calls toHtml() on the block. The template file resolution will pass through Magento\Framework\View\Asset\Repository::updateDesignParams(), will execute line #145 setting $theme to the theme ID, and will execute line #149 that tries to load the theme by full path rather than by ID, thus returning no theme.

The code of this method normally works because if we're not in an environment emulation cross-area, the execution will not fall into that elseif and will not execute line #145 going likely into line #154.

I'm going to submit a pull request with what I believe is a possible solution.

Preconditions

  1. Magento CE 2.1.7

Steps to reproduce

  1. Create a new custom frontend template that inherits from Magento/Luma
  2. Inside the template directory (app/design/frontend/<vendor>/<themeName>) create the folder Magento_OfflinePayments/templates/info/ and create checkmo.phtml into it. The content of this file can be, for example, "Payment method override test"
  3. Go on Magento admin, set the store design to use this template and flush cache
  4. Go on frontend, login, complete an order with payment method "Check / Money order"
  5. Go in your account area, open this order and verify that it shows "Payment method override test" as payment method
  6. Go on Magento admin, find this order and click "Send email" on it

Expected result

  1. The email received has "Payment method override test" as well as payment method

Actual result

  1. The email doesn't have "Payment method override test" but it has the standard payment method label provided by the module template
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 2, 2017
magento#10402 fix theme templates overrides in environment emulation …
…cross-area

Row 141 and 145 are obviously returning a theme id, so we'd better load the theme by id. I'm not sure if execution will fall into row 143 what to expect.
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 2, 2017
magento#10402 fix theme templates overrides in environment emulation …
…cross-area

Row 141 and 145 are obviously returning a theme id, so we'd better load the theme by id. I'm not sure if execution will fall into row 143 what to expect.
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 2, 2017
magento#10402 fix theme templates overrides in environment emulation…
… cross-area

 Load theme by full path only in case we received a 'theme' in $params
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 3, 2017
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 3, 2017
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 3, 2017
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 3, 2017
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 4, 2017
magento#10402 change implementation to detect automatically whether t…
…he theme param returned by getConfigurationDesignTheme is an id or a path
alessandroniciforo added a commit to alessandroniciforo/magento2 that referenced this issue Aug 4, 2017
@magento-engcom-team

This comment has been minimized.

Copy link
Contributor

commented Sep 25, 2017

@alessandroniciforo, thank you for your report.
We've created internal ticket(s) MAGETWO-78298 to track progress on the issue.

@magento-engcom-team

This comment has been minimized.

Copy link
Contributor

commented Aug 20, 2018

Hi @alessandroniciforo

This ticket has been marked as "Triage Wanted" due to low user involvement over time. Over the next 2 weeks we are looking for additional community feedback to decide if it should be archived or not. More information on this is available on the GitHub wiki.

Thank you for collaboration.

@magento-engcom-team

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

@alessandroniciforo, thank you for your report.

Unfortunately, we are archiving this ticket now as it did not get much attention from both Magento Community and Core developers for an extended period. This is done in an effort to create a quality, community-driven backlog which will allow us to allocate the required attention more easily.

You may learn more about this initiative following this link.

Please feel free to comment or reopen the ticket if you think it should be reviewed once more. Thank you for collaboration.

@diamondavocado

This comment has been minimized.

Copy link

commented Jun 4, 2019

This issue should be re-opened as it is quite a fundamental flaw. It makes it impossible to properly override templates for transactional emails which is pretty serious.

It looks like @alessandroniciforo has already created a solution for this, including unit tests, so couldn't this fairly easily be solved? @okorshenko

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.