fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #15137

DanielRuf · 2018-05-10T14:50:57Z

Description

This reverts the addition of the enforced area in d3aef7c which caused #14968 and introduced this regression

Fixed Issues (if relevant)

Can't change the applied theme in 2.2.4 #14968: Can't change the applied theme in 2.2.4
"Template file 'header.html' is not found" error while trying to save Design Configuration. #13530: "Template file 'header.html' is not found" error while trying to save Design Configuration.

Manual testing scenarios

comment / disable line 117 in app/code/Magento/Theme/Model/Design/Config/Validator.php
change theme in store view

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

…\Unit\Model\Design\Config\Validator

hostep · 2018-05-10T19:05:35Z

For completeness sake, this PR also fixes #13530 (which was already fixed by d3aef7c, but that was an incomplete fix)

DanielRuf · 2018-05-10T19:12:02Z

For completeness sake, this PR also fixes #13530 (which was already fixed by d3aef7c, but that was an incomplete fix)

Thanks for the reference, updated the list in the PR comment to link to it.

orlangur · 2018-05-12T10:05:16Z

app/code/Magento/Email/Model/AbstractTemplate.php

@@ -534,10 +534,9 @@ protected function cancelDesignConfig()
     */
    public function setForcedArea($templateId)
    {
-        if ($this->area) {
-            throw new \LogicException(__('Area is already set'));
+        if (!isset($this->area)) {


Can $this->area be empty and not null? Looks like simple if (!$this->area) is sufficient here.

Not that I knew, it is either set or null according to the class where it is set in the constructor.

Ok, then simplify condition please. Commit amend is preferable.

empty and isset check are different, what should I change here? It can just be null or set and isset checks for it already.

@DanielRuf I meant that there is no reason to change if ($this->area) {

isset is more strict and still correct so I think we can keep it like this.

orlangur · 2018-05-12T10:05:37Z

app/code/Magento/Email/Model/AbstractTemplate.php

@@ -534,10 +534,9 @@ protected function cancelDesignConfig()
     */
    public function setForcedArea($templateId)
    {
-        if ($this->area) {
-            throw new \LogicException(__('Area is already set'));


Was there any logic in this exception before? Any idea why it was introduced?

This was still the original code from 3 years ago which was untouched. The annotations and exception class are not correct imo but should be solved in another PR.

Check the blame view of the file in the original repo and you see that the code was never really touched ;-)

@DanielRuf Is there any patch available for temporary. I need to fix this in my Magento v.2.2.4.

@hitesh-wagento simply add .patch to the pull/number URL and you get https://patch-diff.githubusercontent.com/raw/magento/magento2/pull/15137.patch

…5137

magento-engcom-team · 2018-05-16T20:27:03Z

Hi @DanielRuf. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.2.6 release.

pfortin-expertime · 2018-06-07T03:14:00Z

Hi guys,

a small module to fix it without touching the core:

https://github.com/pfortin-expertime/MageFix-Misc

Enjoy ;)

DanielRuf · 2018-06-22T10:50:35Z

Hi guys,

a small module to fix it without touching the core:

pfortin-expertime/MageFix-Misc

Enjoy ;)

A composer patch would be better ;-)

0xpwn · 2018-06-22T12:11:02Z

@DanielRuf Can you please explain in short how to apply this patch using composer-patches?

I have the following in my composer.json:

{
  "config": {
    "preferred-install": "source"
  },
  "extra": {
    "patches": {
      "magento/product-community-edition": {
        "Fix force switch theme": "https://patch-diff.githubusercontent.com/raw/magento/magento2/pull/15137.patch"
      }
    }
  }
}

But I get the following errors:

composer update magento/product-community-edition -vvv

  - Applying patches for magento/product-community-edition
    https://patch-diff.githubusercontent.com/raw/magento/magento2/pull/15137.patch (Fix force switch theme)
Downloading https://patch-diff.githubusercontent.com/raw/magento/magento2/pull/15137.patch
patch '-p1' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
Executing command (CWD): patch '-p1' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
patch: **** Can't change to directory  : No such file or directory

patch '-p0' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
Executing command (CWD): patch '-p0' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
patch: **** Can't change to directory  : No such file or directory

patch '-p2' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
Executing command (CWD): patch '-p2' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
patch: **** Can't change to directory  : No such file or directory

patch '-p4' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
Executing command (CWD): patch '-p4' --no-backup-if-mismatch -d '' < '/var/folders/n1/0cl9jxm90lg1r5qkrqnwf3580000gn/T/5b2ce62c2819b.patch'
patch: **** Can't change to directory  : No such file or directory

   Could not apply patch! Skipping. The error was: Cannot apply patch https://patch-diff.githubusercontent.com/raw/magento/magento2/pull/15137.patch



  [ErrorException]
  file_put_contents(/PATCHES.txt): failed to open stream: Permission denied

DanielRuf · 2018-06-22T12:12:48Z

file_put_contents(/PATCHES.txt): failed to open stream: Permission denied

Looks like some chown / rights issues.

hostep · 2018-06-23T17:51:16Z

@1x0: a patch won't just work from Github, since the file paths in the patch won't match the composer installation (app/code/Magento/Theme/ vs vendor/magento/module-theme/).

You can find information about how to create, edit and apply patches using this recently introduced Knowledge Base article from Magento: https://support.magento.com/hc/en-us/articles/360005484154
Hope this helps :)

DanielRuf · 2018-06-23T17:53:55Z

(app/code/Magento/Theme/ vs vendor/magento/module-theme/).

Ah right forgot that the Git repo and composer based installs are different but we can still use app/code to overwrite the original files.

Changing the file paths in the patch file should be sufficient.

DanielRuf · 2018-06-23T17:54:51Z

Edit the file and remove vendor// from all paths so that they are relative to the vendor// directory.

Simply forgot this one =)

Squashed commit of the following: commit db9fa53cce1c8042caf24c855cfc18d1d948a6b8 Author: Daniel Ruf <[email protected]> Date: Thu May 10 19:18:37 2018 +0200 fix: change unit test back to test for the setForcedArea method commit af6d5c137ff57b13fbd8d6c9e097f7455f67cf7e Author: Daniel Ruf <[email protected]> Date: Thu May 10 19:10:11 2018 +0200 fix: just set the area if it is not set commit 2fd7e5d84abcdea5306f2bd81dd44b86aaa970d7 Author: Daniel Ruf <[email protected]> Date: Thu May 10 16:55:35 2018 +0200 fix: do not expect setForcedArea in unit tests for Magento\Theme\Test\Unit\Model\Design\Config\Validator commit a1bf00c10d5fc01e5e592f38472b820141a47133 Author: Daniel Ruf <[email protected]> Date: Thu May 10 16:46:04 2018 +0200 fix: do not set forced area in template, fixes magento#14968

ihor-sviziev · 2018-07-10T10:56:54Z

FYI I created squashed commit that can used for composer patches:
https://github.com/ihor-sviziev/magento2/commit/e4ec8adef586c615cda168175ad8950c701b0874.patch

hostep · 2018-07-10T11:08:11Z

@ihor-sviziev : @sidolov also already did that on 16 May, you can view his commit if you scroll up a bit, see: 7019a0a

And it's available on here as well: https://magento.com/tech-resources/download (MAGETWO-93036), don't know if this got communicated or something, because I only found out about that by accident.

But thanks anyways! :)

ihor-sviziev · 2018-07-11T04:56:14Z

Hi @hostep,
We tried to apply this patch from commit 7019a0a with composer patches and got an error. Reason is simple commit that you showed - when you trying to use it as a patch - it contains changes both form magneto email and magneto theme modules + reverting changes in magento theme also. While mine contains only changes in magento theme because it contains squashed chanages into 1 commit

DanielRuf · 2018-07-11T05:04:14Z

There should be just one file changed. Do you use Magento 2.2.5?

DanielRuf · 2018-07-11T05:05:23Z

Only app/code/Magento/Email/Model/AbstractTemplate.php should be affected.

ihor-sviziev · 2018-07-11T05:09:02Z

@DanielRuf sure, just take a look at https://github.com/magento/magento2/commit/7019a0a1392095185505ff3ca7b97dd3e9cb4ef2.patch - it contains all changes form all commits - adds changes to magneto theme, removes changed files in magento theme and adds changes to magneto email.
That's why it's better to squash commits into single one when you have final result

DanielRuf · 2018-07-11T05:18:03Z

Try https://github.com/magento/magento2/pull/15137.diff if the other one is problematic.

DanielRuf · 2018-07-11T05:22:15Z

Also the other should be problematic, the result should be the same (in general).

fix: do not set forced area in template, fixes #14968

0ae1230

DanielRuf added the Release Line: 2.2 label May 10, 2018

DanielRuf mentioned this pull request May 10, 2018

Can't change the applied theme in 2.2.4 #14968

Closed

magento-engcom-team assigned VladimirZaets May 10, 2018

magento-engcom-team added this to the May 2018 milestone May 10, 2018

DanielRuf added 2 commits May 10, 2018 16:56

fix: do not expect setForcedArea in unit tests for Magento\Theme\Test…

3df51d4

…\Unit\Model\Design\Config\Validator

fix: just set the area if it is not set

0dec988

DanielRuf changed the title ~~fix: do not set forced area in template, fixes #14968~~ fix: do not forcefully set area in template if it is already set, fixes #14968 May 10, 2018

fix: change unit test back to test for the setForcedArea method

d92b679

DanielRuf mentioned this pull request May 10, 2018

"Template file 'header.html' is not found" error while trying to save Design Configuration. #13530

Closed

DanielRuf changed the title ~~fix: do not forcefully set area in template if it is already set, fixes #14968~~ fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 May 11, 2018

orlangur requested changes May 12, 2018

View reviewed changes

orlangur self-assigned this May 12, 2018

orlangur added the Progress: needs update label May 12, 2018

magento-engcom-team added Progress: accept and removed Progress: needs update labels May 16, 2018

magento-engcom-team merged commit d92b679 into magento:2.2-develop May 16, 2018

magento-engcom-team pushed a commit that referenced this pull request May 16, 2018

ENGCOM-1516: fix: do not set forced area in template, fixes #14968 #1…

7019a0a

…5137

DanielRuf deleted the fix/theme-revert-forced-area-14968 branch May 16, 2018 21:08

hostep mentioned this pull request May 19, 2018

theme can not be changed in magento 2.2.4 #15359

Closed

DanielRuf mentioned this pull request Jun 1, 2018

When I try to change frontend theme : something went wrong while saving this configuration: Area is already set #15651

Closed

This was referenced Jul 9, 2018

fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #16660

Closed

fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #16661

Closed

MarsArt pushed a commit to ConvertGroupsAS/magento2-patches that referenced this pull request Aug 9, 2019

[SPAR-187] created patch by pull magento/magento2#15137

9536715

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #15137

fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #15137

DanielRuf commented May 10, 2018 •

edited

Loading

hostep commented May 10, 2018 •

edited

Loading

DanielRuf commented May 10, 2018

orlangur May 12, 2018

DanielRuf May 12, 2018

orlangur May 12, 2018

DanielRuf May 12, 2018

orlangur Jun 4, 2018

DanielRuf Jun 4, 2018

orlangur May 12, 2018

DanielRuf May 12, 2018

DanielRuf May 12, 2018

hitesh-wagento Jun 22, 2018

DanielRuf Jun 22, 2018

magento-engcom-team commented May 16, 2018 •

edited by ihor-sviziev

Loading

pfortin-expertime commented Jun 7, 2018 •

edited

Loading

DanielRuf commented Jun 22, 2018

0xpwn commented Jun 22, 2018

DanielRuf commented Jun 22, 2018

hostep commented Jun 23, 2018 •

edited

Loading

DanielRuf commented Jun 23, 2018

DanielRuf commented Jun 23, 2018

ihor-sviziev commented Jul 10, 2018

hostep commented Jul 10, 2018 •

edited

Loading

ihor-sviziev commented Jul 11, 2018

DanielRuf commented Jul 11, 2018

DanielRuf commented Jul 11, 2018 •

edited

Loading

ihor-sviziev commented Jul 11, 2018 •

edited

Loading

DanielRuf commented Jul 11, 2018

DanielRuf commented Jul 11, 2018

fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #15137

fix: do not forcefully set area in template if it is already set, fixes #14968, fixes #13530 #15137

Conversation

DanielRuf commented May 10, 2018 • edited Loading

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

hostep commented May 10, 2018 • edited Loading

DanielRuf commented May 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magento-engcom-team commented May 16, 2018 • edited by ihor-sviziev Loading

pfortin-expertime commented Jun 7, 2018 • edited Loading

DanielRuf commented Jun 22, 2018

0xpwn commented Jun 22, 2018

DanielRuf commented Jun 22, 2018

hostep commented Jun 23, 2018 • edited Loading

DanielRuf commented Jun 23, 2018

DanielRuf commented Jun 23, 2018

ihor-sviziev commented Jul 10, 2018

hostep commented Jul 10, 2018 • edited Loading

ihor-sviziev commented Jul 11, 2018

DanielRuf commented Jul 11, 2018

DanielRuf commented Jul 11, 2018 • edited Loading

ihor-sviziev commented Jul 11, 2018 • edited Loading

DanielRuf commented Jul 11, 2018

DanielRuf commented Jul 11, 2018

DanielRuf commented May 10, 2018 •

edited

Loading

hostep commented May 10, 2018 •

edited

Loading

magento-engcom-team commented May 16, 2018 •

edited by ihor-sviziev

Loading

pfortin-expertime commented Jun 7, 2018 •

edited

Loading

hostep commented Jun 23, 2018 •

edited

Loading

hostep commented Jul 10, 2018 •

edited

Loading

DanielRuf commented Jul 11, 2018 •

edited

Loading

ihor-sviziev commented Jul 11, 2018 •

edited

Loading