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

Unable to create a new Mailchimp Store in admin #1208

Closed
pascal-novatize opened this issue Jun 8, 2021 · 10 comments · Fixed by #1212
Closed

Unable to create a new Mailchimp Store in admin #1208

pascal-novatize opened this issue Jun 8, 2021 · 10 comments · Fixed by #1212

Comments

@pascal-novatize
Copy link

Preconditions

  1. Magento 2.3.5-p2
  2. Module mc-magento2 102.3.42 (Should also be reproducible in other releases)
  3. The base configuration of the mc-magento2 module has been done in Magento admin. This means enabling the module and putting your API key.

Steps to reproduce

  1. Login to Magento admin.
  2. Navigate to "Mailchimp Stores" page.
  3. In my case, I had no previous store created (I'm not sure if this is important).
  4. Click "Create new Mailchimp Store" button on the page.
  5. Enter your store information. Can be anything you want for this test.
  6. Click "Save" button.

Actual and Expected result

Expected result:
The Mailchimp Store is created. You are then redirected to the previous page where you can see your new store.
Actual result:
Magento crashes, showing a 500 error page.

Additional information

Looking in my php-fpm logs, I found this error:

PHP Fatal error: Uncaught Error: Call to undefined method Ebizmarts\MailChimp\Helper\Data::getUrl() in /var/www/###/vendor/magento/module-backend/App/AbstractAction.php:364

I believe this issue was reported before, but never fixed (see issue #930).

Cause of the problem

If we look in the constructor of the class Ebizmarts\MailChimp\Controller\Adminhtml\Stores, we can see this line:

$this->_helper                  = $helper;

The problem is that, at that point, this protected property is also declared and already initialized in our parent class Magento\Backend\App\AbstractAction:

 $this->_helper = $context->getHelper();

So, we can quickly see why the following method is failing:

// Magento\Backend\App\AbstractAction
public function getUrl($route = '', $params = [])
{
   return $this->_helper->getUrl($route, $params);
}

The AbstractAction class is expecting the _helper property to be an instance of \Magento\Backend\Helper\Data, but instead it has been replaced by your helper.

How to fix

In short, the _helper property defined in the class Ebizmarts\MailChimp\Controller\Adminhtml\Stores should be renamed something else.

@gonzaloebiz
Copy link
Collaborator

Hi @pascal-novatize

thanks for pointing that.
I've tested the use case and works ok for me. (in fact we have more than 650k installations and works ok)
No matter this, is interesting so i'm asking if the protected $_helper in the vendor/magento/module-backend/App/AbstractAction.php class and the protected $_helper in the https://github.com/mailchimp/mc-magento2/blob/develop-2.3/Controller/Adminhtml/Stores.php#L39 are the same variable as you said, and the conclusion is not, there is not the same variable.
You can check it with this small example

<?php
class MyClass
{
    public $public = 'Public';
    protected $protected = 'Protected';
    private $private = 'Private';
    function printHello()
    {
        echo "MyClass public [".$this->public."] protected [".$this->protected."] private [".$this->private."]\n";
    }
}

$obj = new MyClass();
$obj->printHello();

class MyClass2 extends MyClass
{
    public $public = 'Public2';
    protected $protected = 'Protected2';
    function printHello()
    {
        echo "MyClass2 public [".$this->public."] protected [".$this->protected."]\n";
    }
}
$obj2 = new MyClass2();
$obj2->printHello();
$obj->printHello();
?>

@pascal-novatize
Copy link
Author

Hi @gonzaloebiz

Thank you for taking the time to check my issue.

In the example given, both variables $obj and $obj2 represents completely different and independent objects. It's normal that their properties have different values.
So I don't think your example reflects the situation I was describing.

The class \Ebizmarts\MailChimp\Controller\Adminhtml\Stores inherits the abstract class \Magento\Backend\App\AbstractAction. Since both class declares the same protected property _helper, the last initialization of the property is applied. This is part of how Object Inheritance works with PHP.

Here is your example, but modified to show this:

<?php
abstract class MyClass
{
    public $public = 'Public';
    protected $protected;
    private $private = 'Private';
    
    function __construct()
    {
    	$this->protected = 'Protected';
    }
    
    function printHello()
    {
        $className = static::class;
        echo "{$className} public [".$this->public."] protected [".$this->protected."] private [".$this->private."]\n";
    }
}

class MyClass2 extends MyClass
{
    public $public = 'Public2';
    protected $protected;
    
    function __construct()
    {
        parent::__construct();
    	$this->protected = 'Protected2';
    }
}

$obj2 = new MyClass2();
$obj2->printHello();
?>

@gonzaloebiz
Copy link
Collaborator

Weird, in my tests all work OK, even without any predefined store (and i already say, we have more than 650k installations, which is so weird) I'm not saying that your point is wrong, but I'm trying to understand if we have an issue and if this is the case why is working in 650k installations, I know that the easy way is to rename the variable but I'm trying to understand the problem.

What version of PHP are you using?

@gonzaloebiz gonzaloebiz reopened this Jun 8, 2021
@pascal-novatize
Copy link
Author

I'm running on PHP 7.2.29, with nginx 1.16.1.

I agree with you that this is really weird. I'm able to reproduce my issue on my production server, but not on my local environment. One of the difference between those environnement is the Magento mode used. Maybe this issue can only reproduced using 'production' mode with Magento.

I'll have to try that later today. I'll report back when I have more information.

@gonzaloebiz
Copy link
Collaborator

Hi @pascal-novatize

i just try with production mode and works ok.
Also try with php 7.2.34 and works ok.
do you have some extra extension in your production server?

@gonzaloebiz
Copy link
Collaborator

Hi @pascal-novatize

can you try installing the dev-Issue1208-2.3 version? run:

composer require mailchimp/mc-magento2:dev-Issue1208-2.3
Let me know if this version works for you

Best

@pascal-novatize
Copy link
Author

Hi @gonzaloebiz

Sorry, I didn't had time yesterday to answer your previous question. Yes, we have many other extensions installed on our Magento. I don't think they would interfere with the admin Controllers of the Mailchimp module.

Thank you for the fix, I will install it on my production server later today and report back.

@pascal-novatize
Copy link
Author

Hi @gonzaloebiz

I was able to test your patched version, earlier than expected, on my production server. I can confirm that I was able to create a new Mailchimp store with your fix.

I'm still not sure why others don't have the same issue.

Thank you for the fix.

@gonzaloebiz gonzaloebiz added this to the 10x.x.44 milestone Jun 10, 2021
gonzaloebiz added a commit that referenced this issue Jun 10, 2021
gonzaloebiz added a commit that referenced this issue Jun 10, 2021
gonzaloebiz added a commit that referenced this issue Jun 10, 2021
gonzaloebiz added a commit that referenced this issue Jun 10, 2021
gonzaloebiz added a commit that referenced this issue Jun 10, 2021
gonzaloebiz added a commit that referenced this issue Jun 10, 2021
@gonzaloebiz
Copy link
Collaborator

Thanks @pascal-novatize for your contribution

@nickalmeida
Copy link

I am having the same issue on Magento 2.4.1

Fatal error: Uncaught Error: Class 'Ebizmarts\MailChimp\Controller\Adminhtml\Stores' not found in /home/xxxxx/xxxxx/public_html/app/code/Ebizmarts/Mailchimp/Controller/Adminhtml/Stores/Index.php:17
Stack trace:
#0 /home/xxxxx/xxxxx/public_html/setup/src/Magento/Setup/Module/Di/Code/Reader/ClassesScanner.php(136): require_once()
#1 /home/xxxxx/xxxxx/public_html/setup/src/Magento/Setup/Module/Di/Code/Reader/ClassesScanner.php(117): Magento\Setup\Module\Di\Code\Reader\ClassesScanner->includeClass()
#2 /home/xxxxx/xxxxx/public_html/setup/src/Magento/Setup/Module/Di/Code/Reader/ClassesScanner.php(87): Magento\Setup\Module\Di\Code\Reader\ClassesScanner->extract()
#3 /home/xxxxx/xxxxx/public_html/setup/src/Magento/Setup/Module/Di/App/Task/Operation/RepositoryGenerator.php(61): Magento\Setup\Module\Di\Code\Reader\ClassesScanner->getList()
#4 /home/xxxxx/xxxxx/public_html/setup/src/Magento in /home/xxxxx/xxxxx/public_html/app/code/Ebizmarts/Mailchimp/Controller/Adminhtml/Stores/Index.php on line 17

any help please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants