dotmailer / dotmailer-magento2-extension

The official Dotdigital for Magento2 extension
https://dotdigital.com/integrations/magento
MIT License
49 stars 63 forks source link

[BUG] Incorrect configuration #505

Closed aleksandrosadchiy closed 6 years ago

aleksandrosadchiy commented 6 years ago

ISSUE Invalid configs cause an issue when extension installed but not configured

[2018-03-09 16:03:17] report.CRITICAL: Invalid type given. String expected {"exception":"[object] (Zend_Mail_Protocol_Exception(code: 0): Invalid type given. String expected at /var/www/html/vendor/magento/zendframework1/library/Zend/Mail/Protocol/Abstract.php:147)"} []

Zend_Mail_Protocol_Abstract

public function __construct($host = '127.0.0.1', $port = null)
    {
        $this->_validHost = new Zend_Validate();
        $this->_validHost->addValidator(new Zend_Validate_Hostname(Zend_Validate_Hostname::ALLOW_ALL));
//host will be NULL because extension override parameters
        if (!$this->_validHost->isValid($host)) {

            /**
             * @see Zend_Mail_Protocol_Exception
             */
            #require_once 'Zend/Mail/Protocol/Exception.php';
            throw new Zend_Mail_Protocol_Exception(join(', ', $this->_validHost->getMessages()));
        }

        $this->_host = $host;
        $this->_port = $port;
    }

STEPS

  1. Look at https://github.com/dotmailer/dotmailer-magento2-extension/blob/master/Helper/Transactional.php
 const XML_PATH_DDG_TRANSACTIONAL_ENABLED = 'transactional_emails/ddg_transactional/enabled';
    const XML_PATH_DDG_TRANSACTIONAL_HOST = 'transactional_emails/ddg_transactional/host';
    const XML_PATH_DDG_TRANSACTIONAL_USERNAME = 'transactional_emails/ddg_transactional/username';
    const XML_PATH_DDG_TRANSACTIONAL_PASSWORD = 'transactional_emails/ddg_transactional/password';
    const XML_PATH_DDG_TRANSACTIONAL_PORT = 'transactional_emails/ddg_transactional/port';
    const XML_PATH_DDG_TRANSACTIONAL_DEBUG = 'transactional_emails/ddg_transactional/debug';
  1. Look at https://github.com/dotmailer/dotmailer-magento2-extension/blob/master/etc/config.xml

you will find "connector_transactional_emails" instead of "transactional_emails"

This will cause an issue like:

[2018-03-09 16:03:17] report.CRITICAL: Invalid type given. String expected {"exception":"[object] (Zend_Mail_Protocol_Exception(code: 0): Invalid type given. String expected at /var/www/html/vendor/magento/zendframework1/library/Zend/Mail/Protocol/Abstract.php:147)"} []

EXPECTED RESULT

Configs are correct and extension disabled after installation

ACTUAL RESULT

Configs are incorrect and extension affect email functionality dotmailer-magento2-extension/Helper/Transactional.php

/**
     * Is transactional email enabled.
     *
     * @return bool
     */
    public function isEnabled()
    {
        $store = $this->storeManager->getStore();
//always set and have NULL value
        return $this->scopeConfig->isSetFlag(self::XML_PATH_DDG_TRANSACTIONAL_ENABLED, 'store', $store);
    }

Always return TRUE and check passed.

maybe one of possible ways is change config in config.xml from connector_transactional_emails to transactional_emails

 <connector_transactional_emails>
            <ddg_transactional>
                <enabled>0</enabled>
            </ddg_transactional>
        </connector_transactional_emails>

to

 <transactional_emails>
            <ddg_transactional>
                <enabled>0</enabled>
            </ddg_transactional>
        </ransactional_emails>
aleksandrosadchiy commented 6 years ago

@cdiacon Steps to reproduce

User able to save config without host etc. After extension is enabled plugins will always override email functionality.

cdiacon commented 6 years ago

@aleksandrosadchiy

Awesome, thanks for the details and for great replication steps, I'll get this replicated and fixed,

Spasibo.