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.55k stars 9.32k forks source link

Can't change the applied theme in 2.2.4 #14968

Closed Krapulat closed 6 years ago

Krapulat commented 6 years ago

Preconditions

  1. Magento 2.2.4
  2. PHP 7.1

Steps to reproduce

  1. Go to Content > Design > Configuration
  2. Edit the Store View and try to change the "Applied theme" to another
  3. Save Configuration

Expected result

  1. Change the theme

Actual result

  1. Appears the error: "Something went wrong while saving this configuration: Area is already set"
ne0nlight commented 6 years ago

Can verify, also experiencing this exact issue.

hostep commented 6 years ago

Indeed, can confirm as well on a clean 2.2.4 installation. It only happens on storeview level, on website or global level, it works as expected.

I can not reproduce this on a clean 2.2.3 installation, so it looks like this is a new bug in 2.2.4, yay!

sferreira-nicopu commented 6 years ago

@sferreira-nicopu just adding myself to the conversation since we experienced the same issue yesterday after updating our Magento Commerce Cloud to this version

SamB-GB commented 6 years ago

I am seeing this issue as well but trying to update logo and save.

gtlt commented 6 years ago

+1 Class Magento\Email\Model\AbstractTemplate : $this->area is already setted to adminhtml when $this->emailConfig->getTemplateArea($templateId) return frontend. note: $templateId is equal to "design_email_header_template"

    public function setForcedArea($templateId)
    {
        if ($this->area) {
            throw new \LogicException(__('Area is already set'));
        }
        $this->area = $this->emailConfig->getTemplateArea($templateId);
        return $this;
    }

shouldn't we have this code instead ??

   public function setForcedArea($templateId)
    {
        if (!isset($this->area)) {
           $this->area = $this->emailConfig->getTemplateArea($templateId);
        }
        return $this;
    }

here a stacktrace :

[2018-05-04 10:44:23] main.CRITICAL: Exception message: Area is already set
Trace: #0 /var/www/mysite/www.mysite/vendor/magento/module-theme/Model/Design/Config/Validator.php(117): Magento\Email\Model\AbstractTemplate->setForcedArea('design_email_he...')
#1 /var/www/mysite/www.mysite/vendor/magento/module-theme/Model/Design/Config/Validator.php(68): Magento\Theme\Model\Design\Config\Validator->getTemplateText('design_email_he...', Object(Magento\Theme\Model\Data\Design\Config))
#2 /var/www/mysite/www.mysite/vendor/magento/module-theme/Model/DesignConfigRepository.php(91): Magento\Theme\Model\Design\Config\Validator->validate(Object(Magento\Theme\Model\Data\Design\Config))
#3 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Theme\Model\DesignConfigRepository->save(Object(Magento\Theme\Model\Data\Design\Config))
#4 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Theme\Model\DesignConfigRepository\Interceptor->___callParent('save', Array)
#5 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Theme\Model\DesignConfigRepository\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Theme\Model\Data\Design\Config))
#6 /var/www/mysite/www.mysite/generated/code/Magento/Theme/Model/DesignConfigRepository/Interceptor.php(39): Magento\Theme\Model\DesignConfigRepository\Interceptor->___callPlugins('save', Array, Array)
#7 /var/www/mysite/www.mysite/vendor/magento/module-theme/Controller/Adminhtml/Design/Config/Save.php(75): Magento\Theme\Model\DesignConfigRepository\Interceptor->save(Object(Magento\Theme\Model\Data\Design\Config))
#8 /var/www/mysite/www.mysite/generated/code/Magento/Theme/Controller/Adminhtml/Design/Config/Save/Interceptor.php(24): Magento\Theme\Controller\Adminhtml\Design\Config\Save->execute()
#9 /var/www/mysite/www.mysite/vendor/magento/framework/App/Action/Action.php(107): Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor->execute()
#10 /var/www/mysite/www.mysite/vendor/magento/module-backend/App/AbstractAction.php(229): Magento\Framework\App\Action\Action->dispatch(Object(Magento\Framework\App\Request\Http))
#11 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Backend\App\AbstractAction->dispatch(Object(Magento\Framework\App\Request\Http))
#12 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor->___callParent('dispatch', Array)
#13 /var/www/mysite/www.mysite/vendor/magento/module-backend/App/Action/Plugin/Authentication.php(143): Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Framework\App\Request\Http))
#14 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(135): Magento\Backend\App\Action\Plugin\Authentication->aroundDispatch(Object(Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor), Object(Closure), Object(Magento\Framework\App\Request\Http))
#15 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Framework\App\Request\Http))
#16 /var/www/mysite/www.mysite/generated/code/Magento/Theme/Controller/Adminhtml/Design/Config/Save/Interceptor.php(39): Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor->___callPlugins('dispatch', Array, NULL)
#17 /var/www/mysite/www.mysite/vendor/magento/framework/App/FrontController.php(55): Magento\Theme\Controller\Adminhtml\Design\Config\Save\Interceptor->dispatch(Object(Magento\Framework\App\Request\Http))
#18 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(58): Magento\Framework\App\FrontController->dispatch(Object(Magento\Framework\App\Request\Http))
#19 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(138): Magento\Framework\App\FrontController\Interceptor->___callParent('dispatch', Array)
#20 /var/www/mysite/www.mysite/vendor/mymodule/m2-module-core/Plugin/App/FrontController.php(122): Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Framework\App\Request\Http))
#21 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(135): Module\Core\Plugin\App\FrontController->aroundDispatch(Object(Magento\Framework\App\FrontController\Interceptor), Object(Closure), Object(Magento\Framework\App\Request\Http))
#22 /var/www/mysite/www.mysite/vendor/magento/framework/Interception/Interceptor.php(153): Magento\Framework\App\FrontController\Interceptor->Magento\Framework\Interception\{closure}(Object(Magento\Framework\App\Request\Http))
#23 /var/www/mysite/www.mysite/generated/code/Magento/Framework/App/FrontController/Interceptor.php(26): Magento\Framework\App\FrontController\Interceptor->___callPlugins('dispatch', Array, Array)
#24 /var/www/mysite/www.mysite/vendor/magento/framework/App/Http.php(135): Magento\Framework\App\FrontController\Interceptor->dispatch(Object(Magento\Framework\App\Request\Http))
#25 /var/www/mysite/www.mysite/generated/code/Magento/Framework/App/Http/Interceptor.php(24): Magento\Framework\App\Http->launch()
#26 /var/www/mysite/www.mysite/vendor/magento/framework/App/Bootstrap.php(256): Magento\Framework\App\Http\Interceptor->launch()
#27 /var/www/mysite/www.mysite/pub/index.php(37): Magento\Framework\App\Bootstrap->run(Object(Magento\Framework\App\Http\Interceptor))
#28 {main} [] []
13CHILLIES commented 6 years ago

getting the same fault.

magentochile commented 6 years ago

Hi gtlt, You are absolutely right in the world !!! When we change in Magento\Email\Model\AbstractTemplate.php this: public function setForcedArea($templateId) { if ($this->area) { throw new \LogicException(__('Area is already set')); } $this->area = $this->emailConfig->getTemplateArea($templateId); return $this; }

For this: public function setForcedArea($templateId) { if (!isset($this->area)) { $this->area = $this->emailConfig->getTemplateArea($templateId); } return $this; } work!!! And they fill correctly core_config_data and design_config_grid_flat:

core_config_data: https://www.magentochile.cl/blog/wikis-images/bug/core_config_data.jpg

design_config_grid_flat: https://www.magentochile.cl/blog/wikis-images/bug/design_config_grid_flat.jpg

DanielRuf commented 6 years ago

Would be good to have some regression testing for such things to potentially prevent such issues. How can we help here with tests?

gtlt commented 6 years ago

@magentochile the class Magento\Email\Model\AbstractTemplate didn't changed since last August, so I'm not sure changing the method setForcedArea is the right solution, I didn't investigated to check if it has side effects (like on email generation, etc).

itsmeit268 commented 6 years ago

Hi, I also met the same problem

tomasmarcik commented 6 years ago

Same issue. Applied the code from above but I don't expect this is final solution. When it will be fixed?

magentochile commented 6 years ago

Dear sirs, I think you have to look at:

$this->area = isset($data['area']) ? $data['area'] : null;

And trace it back as far as it goes ... now I'm setting up a Magento 2 store and I'm not letting go ... if I manage to finish the store, I'll follow up. Maybe see what he throws with a: echo $data['area']; var_dump($data['area']);

and look what it throws ... what's in $data['area'] that says it Area is already set!

DanielRuf commented 6 years ago

Hm, if the class did not change did the interface or consumers change its usage?

kunsal commented 6 years ago

Trying to change theme in fresh Magento 2.2.4 threw error "Something went wrong while saving this configuration: Area is already set". Any fix yet?

bhaveybansal commented 6 years ago

I'm too facing this issue on fresh install of magento 2.2.4

pocketprogrammer commented 6 years ago

I experience the same issue on fresh install of 2.2.4

Not sure if this helps, but I CAN save the theme at the website level. The error only occurs for me at the store view level. Changing the theme and saving at the website level updates the store view's theme.

DanielRuf commented 6 years ago

Right, global and website level work but not the store view level.

chrisjefferies commented 6 years ago

I am also having this issue. 2.2.4.

I was able to get around it by changing the theme_id value in core_config_data table manually. No known side effects yet.

mariamghalleb commented 6 years ago

Ive also start having this problem since I upgraded from 2.2.3 to 2.2.4 ...

DanielRuf commented 6 years ago

I'll give it a try and try some bisecting tomorrow since this seems to affect many users and we do not yet have a solution or the cause of this.

actonsys commented 6 years ago

Experiencing the same issue here Magento 2.2.4

magentochile commented 6 years ago

Hey friends,

I stopped at my work, and I took another look at the error. And I did the essay to add echo $this->area, but in the "throw new" and it gave me the following result:

public function setForcedArea($templateId) { if ($this->area) {
// throw new \LogicException(('Area is already set'));
throw new \LogicException(
($this->area));
} $this->area = $this->emailConfig->getTemplateArea($templateId); return $this; }

And it gave me the result of: Something went wrong while saving this configuration: adminhtml

The area "adminhtml" is that I think you should not assign the area to what corresponds to the frontend template. Here is documentation of that: https://devdocs.magento.com/guides/v2.0/architecture/archi_perspectives/components/modules/mod_and_areas.html

So what could be done a small fix, although it's not the right thing, could be like that (other than the "adminhtml" area):

public function setForcedArea($templateId) { // if ($this->area) { if ($this->area !== 'adminhtml') {
throw new \LogicException(__('Area is already set'));
} $this->area = $this->emailConfig->getTemplateArea($templateId); return $this; } Look at the image below: https://www.magentochile.cl/blog/wikis-images/bug/save-adminhtml-area.jpg

Regards,

Boris Durán R.

DanielRuf commented 6 years ago

This is the cause.

git bisect bad
d3aef7c3eef658baa00d3233d0352b5fed0f1cbe is the first bad commit
commit d3aef7c3eef658baa00d3233d0352b5fed0f1cbe
Author: Andrii Meysar <andrii.meysar@transoftgroup.com>
Date:   Wed Mar 21 18:16:42 2018 +0200

    MAGETWO-89261: Template file 'header.html' is not found.

:040000 040000 7437b710bf3a7e2a357f9bfd71a6d488aa225812 9f8cf78d8757c7a8d1aa17ad0ec0b2ef6b433cf2 M      app
DanielRuf commented 6 years ago

Please all thoroughly test https://github.com/magento/magento2/pull/15137

DanielRuf commented 6 years ago

https://github.com/magento/magento2/commit/3f6464da2c7cd4d7db8bf07cfbf51cc17ee1993a

Where can we get details about MAGETWO-89261: Template file 'header.html' is not found.? Which issue did it solve and should it be solved in another way?

hostep commented 6 years ago

@DanielRuf: it might be related to https://github.com/magento/magento2/issues/13530 Can you verify after your fix that this bug isn't triggered again (was only on global or website scope). Thanks!

DanielRuf commented 6 years ago

Can you verify after your fix that this bug isn't triggered again (was only on global or website scope). Thanks!

I tested it in a fresh setup where I cloned magento/magento2 and did the bisecting directly and can reproduct it with the line and it is not happening without the line.

DanielRuf commented 6 years ago

@DanielRuf: it might be related to #13530

Not reproducible with my change with a fresh 2.2-develop.

DanielRuf commented 6 years ago

But I'll do some more tests now.

DanielRuf commented 6 years ago

Hm, you are right Will provide another changeset for my PR.

fahadmunir158 commented 6 years ago

@Krapulat I faced the same issue.

Open Magento\Email\Model\AbstractTemplate.php file and replace the function setForcedArea with this code

public function setForcedArea($templateId) { if (!isset($this->area)) { $this->area = $this->emailConfig->getTemplateArea($templateId); } return $this; }

This worked for me :100:

DanielRuf commented 6 years ago

@DanielRuf: it might be related to #13530 Can you verify after your fix that this bug isn't triggered again (was only on global or website scope). Thanks!

It is not directly related as the error with header.htl comes from the mail config as it seems. The forced area call is not the right solution here.

@fahadmunir158 thanks, will test it for my PR. But so far it feels like fixing one thing and another breaks it might be another issue.

In general I would do another bisecting to find the culprit of your issue @hostep, anything else is more like guessing and choosing any solution which might be the cause. When did it work and when was this introduced?

DanielRuf commented 6 years ago

Also the setForcedArea docblock does not seem correct to me.

     * @throws \Magento\Framework\Exception\MailException
...
throw new \LogicException(__('Area is already set'));

@fahadmunir158 your solution might be the correct one as the current code is 3 years old there and can be probably outdated.

https://github.com/magento/magento2/blame/2.2-develop/app/code/Magento/Email/Model/AbstractTemplate.php#L538

DanielRuf commented 6 years ago

@fahadmunir158 I pushed your proposed change to my PR. Please let me know if I should add you as Co-Author =)

DanielRuf commented 6 years ago

@hostep I do not yet understand the connection between your issue number and the one which is referenced in the commit that caused this regression.

hostep commented 6 years ago

When did it work and when was this introduced?

I'm pretty sure the bug I reported didn't exist yet in 2.2.3, I ran into it while doing to some testing directly on the 2.2-develop branch at that time, so I believe that particular bug was introduced somewhere between the 2.2.3 tag and the commit which turned out the be the cause of this new issue: https://github.com/magento/magento2/commit/d3aef7c3eef658baa00d3233d0352b5fed0f1cbe (or maybe even narrower, the commit I referenced in the report: f18377b9e598fd5316963f9e4deb7e54547a638c), but it's hard to say with all the branches which get merged all over the place in the Magento2 codebase. Even though 2.2.3 was released after I reported the bug, I'm almost 100% sure the bug wasn't introduced yet in that version.

Hope this helps somehow :)

@hostep I do not yet understand the connection between your issue number and the one which is referenced in the commit that caused this regression.

It's related because the template file header.phtml is being used by the email templating system. But you probably already knew this.

DanielRuf commented 6 years ago

@hostep can you also do some bisecting here? If you do not know how bisecting works I can provide you with some hopefully information how this can be used to find regression and the causes of them.

DanielRuf commented 6 years ago
magento2$ git bisect start
ssh-w00b8941@dd20014:/www/htdocs/websites/magento-bisect/magento2$ git bisect good f3f8975c3f48ad8d2a3221153bcce9f82a5b84b8
ssh-w00b8941@dd20014:/www/htdocs/magento-bisect/magento2$ git bisect bad 61c3ba691512d36001baedb22d58bab5cb16e05f
Bisecting: 1197 revisions left to test after this (roughly 10 steps)
[83a7c1cbad1ff0c01714296733f9ee80395469ca] MAGETWO-85904: Investigate fluctuations of PAT Nightly build
ssh-w00b8941@dd20014:/www/htdocs/websites/magento-bisect#/magento2$

This would be the first step.

hostep commented 6 years ago

I'll see if I can find some time in a couple of minutes. I'm familiar with bisecting, but it's a bit of a pain in Magento 2 because of the database migration system. Going forwards is doable, going backwards means you have to re-install the database again (sometimes). But it's doable. I'll see what I can do.

DanielRuf commented 6 years ago

I'm familiar with bisecting, but it's a bit of a pain in Magento 2 because of the database migration system

These changes should not be related to actual database changes.

hostep commented 6 years ago

Bisecting revealed this commit https://github.com/magento/magento2/commit/5e93220a56bd741d3ec27b8a0fffd2f539e4e473 (is a fix for https://github.com/magento/magento2/issues/8437). Yet I believe this commit does the correct thing and only revealed a deeper problem.

This is only guessing here: somehow by saving the design configuration, the email templating system tries to load an email template (otherwise it wouldn't try to load the header.phtml file). Those email template files aren't available in the adminhtml area, they only exist in the frontend area (I think Update: this is not correct, email templates can exist both in the adminhtml & frontend area). I think we need to figure out why when saving the design configuration the email templating system is trying to load an email template and we should try to disable that, that is probably the right fix.

This is all in theory, I'm not very familiar with all these systems, so I might be completely wrong :)

DanielRuf commented 6 years ago

The email header and footer is a bit downwards at the design settings page of the global context.

hostep commented 6 years ago

It looks like the suggestion from @gtlt and the latest version of @DanielRuf's PR fixes both https://github.com/magento/magento2/issues/14968 & https://github.com/magento/magento2/issues/13530 and thinking about it, it is probably the best solution!

DanielRuf commented 6 years ago

Personally I am still unsure if this is / would be the "right" fix (of the root cause) =)

Waiting for feedback from all other affected users who upvoted and replied.

ramindas commented 6 years ago

Same Issue here "Something went wrong while saving this configuration: Area is already set" when try to change applied theme Magento CE 2.2.4.

hallleron commented 6 years ago

I tried to write a plugin for this issue, but I couldn't get it to work: Since the $emailConfig property is protected I cannot access that property in my plugin. Is it even possible?

DanielRuf commented 6 years ago

I tried to write a plugin for this issue

Why?

There is already an open PR which solves this. Please test it. See https://github.com/magento/magento2/pull/15137

magento-engcom-team commented 6 years ago

Hi @Krapulat. Thank you for your report. The issue has been fixed in magento/magento2#15137 by @DanielRuf in 2.2-develop branch Related commit(s):

The fix will be available with the upcoming 2.2.5 release.

infortis commented 6 years ago

I just want to confirm that this fix https://github.com/magento/magento2/commit/0dec988872e175e568c2bf4bb29548ab7a4698e5 did the job. I've tested it in clean Magento 2.2.4 and "Something went wrong while saving this configuration: Area is already set" error doesn't show any more.

devamitbera commented 6 years ago

@magento/community-engineering-team ,

Can you please clarify

15137 is the Final PR for this issue?