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

Transport variable can not be altered in email_invoice_set_template_vars_before Event #10210

Closed
diybook opened this issue Jul 11, 2017 · 20 comments
Assignees
Labels
bug report Component: Payment Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release

Comments

@diybook
Copy link

diybook commented Jul 11, 2017

We waht to change the payment_html for banktransfer invoices. Unfortunately the instruction is also sent in invioce email. And there the customer already has paid the bill.

Preconditions

  1. Magento 2.1.7
  2. PHP 7.0.2
  3. MySql 5.7

Steps to reproduce

  1. Declare the event
<event name="email_invoice_set_template_vars_before">
        <observer name="mod_mail_variables_of_Invoice" instance="Vendor\Module\Observer\ChangePaymentHtml" />
    </event>
  1. Alter variable payment_html in Vendor\Module\Observer\ChangePaymentHtml like
<?php

namespace Venoor\Module\Observer;

use Magento\Framework\Event\ObserverInterface;
use Magento\Framework\Event\Observer;
use Magento\Framework\App\Request\DataPersistorInterface;
use Magento\Framework\App\ObjectManager;

use Vendor\Module\Helper\Data as PaymentHelper;
use Magento\Sales\Model\Order;

class ChangePaymentHtml implements ObserverInterface
{

    protected $paymentHelper;
    public $logger;

    public function __construct(
        PaymentHelper $paymentHelper,
        \Psr\Log\LoggerInterface $logger
    ) {
        $this->paymentHelper = $paymentHelper;
        $this->logger = $logger;
    }

    public function execute(\Magento\Framework\Event\Observer $observer)
    {
        $transport = $observer->getEvent()->getTransport();
        $paymentInfo = $this->getPayment($transport['order'], $transport['store']->getStoreId());
        
        if ($paymentInfo->getTemplate() == 'info/instructions.phtml') {
            $paymentInfo->setTemplate('Vendor_Module::payment/info/invoicepaymentinfo.phtml');
        }

        $transport['payment_html'] = $paymentInfo->toHtml();
        $observer->setData('transport', $transport);
    }

    protected function getPayment(Order $order, $storeid)
    {
        return $this->paymentHelper->getRawInfoBlock(
            $order->getPayment(),
            $storeid
        );
    }
}

Expected result

  1. Email should be modified and new or changed variables should be output in email.

Actual result

  1. Email is unchanged, and $transport variable seems not to be changed.
@magento-engcom-team magento-engcom-team added 2.1.x bug report Component: Payment Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Sep 20, 2017
@magento-engcom-team
Copy link
Contributor

@diybook, thank you for your report.
The issue is already fixed in develop branch
But we will consider to backport the fix to patch releases

@okorshenko okorshenko added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Sep 21, 2017
@reviskar
Copy link

Hi.
Meanwhile could anyone pinpoint us to a patch for this?

@magento-engcom-team magento-engcom-team added 2.2.x Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release labels Oct 12, 2017
@gwharton
Copy link
Contributor

@reviskar Here is a link to the commit in the development branch that fixes this issue.

68e7d8b

@reviskar
Copy link

Hi, @gwharton!

Thanks very much, will check it out!

@gwharton
Copy link
Contributor

gwharton commented Nov 7, 2017

@magento-engcom-team Is there a process to follow to request the backporting of the above commit into 2.1 and 2.2. Apologies if it is covered somewhere, but I did search to no avail.

@okorshenko
Copy link
Contributor

The issue has been fixed and delivered to 2.2-develop branch

@okorshenko okorshenko added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label Nov 27, 2017
@dan-ding
Copy link

Unfortunately it didn't make it in time for 2.2.2.

or 2.2.3

@gwharton
Copy link
Contributor

gwharton commented May 3, 2018

Although this fix has made it into 2.2.4 There is an error in the commit that was made to 2.2-develop.

The fix is missing for file app/code/Magento/Sales/Model/Order/Email/Sender/OrderSender.php

Infact, on further investigation this is still broken in 2.1-develop, 2.2-develop and 2.3-develop for the above reason.

@gwharton gwharton reopened this May 3, 2018
@gwharton gwharton removed Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line labels May 3, 2018
@gwharton
Copy link
Contributor

gwharton commented May 3, 2018

@RomaKis or @okorshenko can you look at this again and check the status of app/code/Magento/Sales/Model/Order/Email/Sender/OrderSender.php in 2.1, 2.2 and 2.3 Somethings gone wrong with the commit and the changes have been only partially applied for the above file.

@magento-engcom-team
Copy link
Contributor

Hi @diybook. Thank you for your report.
The issue has been fixed in #15038 by @gwharton in 2.1-develop branch
Related commit(s):

The fix will be available with the upcoming 2.1.15 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.1.x The issue has been fixed in 2.1 release line label May 18, 2018
@VladimirZaets
Copy link
Contributor

Hi @diybook. Thank you for your report.
The issue has been fixed in #15039 by @gwharton in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

@VladimirZaets VladimirZaets added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label May 20, 2018
@magento-engcom-team magento-engcom-team added the Fixed in 2.2.x The issue has been fixed in 2.2 release line label May 25, 2018
@magento-engcom-team
Copy link
Contributor

Hi @diybook. Thank you for your report.
The issue has been fixed in #15040 by @gwharton in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.5 release.

@mam08ixo
Copy link

mam08ixo commented Jul 6, 2018

@gwharton @magento-engcom-team

This is a backward incompatible change. Anyone who relied on transport being a \Magento\Framework\DataObject now receives a

Call to a member function getData() on array

fatal error, potentially breaking checkout.

@gwharton
Copy link
Contributor

gwharton commented Jul 6, 2018

Ouch, yes you are right. Looking into it, prior to the changes implemented the events have inconsistent parameter types.

Event Before Parameter 1 Before Parameter 2 After Parameter 1 After Parameter 2 After Parameter 3
email_creditmemo_set_template_vars_before Creditmemo/Sender/EmailSender $transport array() Creditmemo/Sender/EmailSender $transport array() $transportObject DataObject()
email_invoice_set_template_vars_before Invoice/Sender/EmailSender $transport array() Invoice/Sender/EmailSender $transport array() $transportObject DataObject()
email_shipment_set_template_vars_before Shipment/Sender/EmailSender $transport array() Shipment/Sender/EmailSender $transport array() $transportObject DataObject()
email_creditmemo_comment_set_template_vars_before Email/Sender/CreditmemoCommentSender $transport array() Email/Sender/CreditmemoCommentSender $transport array() $transportObject DataObject()
email_creditmemo_set_template_vars_before Email/Sender/CreditmemoSender $transport array() Email/Sender/CreditmemoSender $transport array() $transportObject DataObject()
email_invoice_comment_set_template_vars_before Email/Sender/InvoiceCommentSender $transport array() Email/Sender/InvoiceCommentSender $transport array() $transportObject DataObject()
email_invoice_set_template_vars_before Email/Sender/InvoiceSender $transport array() Email/Sender/InvoiceSender $transport array() $transportObject DataObject()
email_shipment_comment_set_template_vars_before Email/Sender/ShipmentCommentSender $transport array() Email/Sender/ShipmentCommentSender $transport array() $transportObject DataObject()
email_shipmen_set_template_vars_before Email/Sender/ShipmentSender $transport array() Email/Sender/ShipmentSender $transport array() $transportObject DataObject()
email_order_comment_set_template_vars_before Email/Sender/OrderCommentSender $transport array() Email/Sender/OrderCommentSender $transport array() $transportObject DataObject()
email_order_set_template_vars_before Email/Sender/OrderSender $transport DataObject() Email/Sender/OrderSender $transport array() $transportObject DataObject()

You can see that for the OrderSender the type of parameter "transport" was DataObject, whereas the type of parameter "transport" in all of the other email events is Array().

After the change, it was incorrectly changed to type Array(). I will create a PR to correct this.

@sidolov
Copy link
Contributor

sidolov commented Jul 11, 2018

Hi @diybook. Thank you for your report.
The issue has been fixed in #16601 by @gwharton in 2.1-develop branch
Related commit(s):

The fix will be available with the upcoming 2.1.15 release.

@sidolov
Copy link
Contributor

sidolov commented Jul 11, 2018

Hi @diybook. Thank you for your report.
The issue has been fixed in #16600 by @gwharton in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.0 release.

@VladimirZaets
Copy link
Contributor

Hi @diybook. Thank you for your report.
The issue has been fixed in #16599 by @gwharton in 2.2-develop branch
Related commit(s):

The fix will be available with the upcoming 2.2.6 release.

@topranavp
Copy link

@magento-engcom-team , issue still persists in Magento 2.2.4. have tried to append value to $transport['payment_html'] variable in extended class but still it is not allowing to alter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Payment Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Fixed in 2.3.x The issue has been fixed in 2.3 release line Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release
Projects
None yet
Development

No branches or pull requests