search

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.46k stars 9.28k forks source link

Fetching invoice items with \Magento\Sales\Model\Order\Invoice getItems function sometimes returns a collection #13180

Open koenner01 opened 6 years ago

koenner01 commented 6 years ago

When looking at \Magento\Sales\Model\Order\Invoice the getItems function states that the return value should be an array of \Magento\Sales\Api\Data\InvoiceItemInterface. 

/**
 * Returns invoice items
 *
 * @return \Magento\Sales\Api\Data\InvoiceItemInterface[]
 */
public function getItems()
{
    if ($this->getData(InvoiceInterface::ITEMS) === null && $this->getId()) {
        $collection = $this->_invoiceItemCollectionFactory->create()->setInvoiceFilter($this->getId());
        foreach ($collection as $item) {
            $item->setInvoice($this);
        }
        $this->setData(InvoiceInterface::ITEMS, $collection->getItems());
    }
    return $this->getData(InvoiceInterface::ITEMS);
}

However, when calling this function it will some times return a Magento\Sales\Model\ResourceModel\Order\Invoice\Item\Collection.

This is caused by the fact that when items are added to the invoice the addItem function is calling getItemsCollection function which will set the items data equal to a collection if the data hasn't been set yet.

public function addItem(\Magento\Sales\Model\Order\Invoice\Item $item)
{
    $item->setInvoice($this)->setParentId($this->getId())->setStoreId($this->getStoreId());

    if (!$item->getId()) {
        $this->getItemsCollection()->addItem($item);
    }
    return $this;
}
public function getItemsCollection()
{
    if (!$this->hasData(InvoiceInterface::ITEMS)) {
        $this->setItems($this->_invoiceItemCollectionFactory->create()->setInvoiceFilter($this->getId()));

        if ($this->getId()) {
            foreach ($this->getItems() as $item) {
                $item->setInvoice($this);
            }
        }
    }
    return $this->getItems();
}

Preconditions

  1. PHP7.2
  2. Magento 2.4-developer

Steps to reproduce

  1. Create a new adminhtml observer for the 'sales_order_invoice_save_after' event which fetches the invoice from the observer and outputs the invoice getItems() (preferably add a die() of some sorts to stop code from continuing execution): 
    $invoice = $observer->getEvent()->getInvoice();
$items = $invoice->getItems();
  2. Create a new order through the backend and then create a new invoice.

Expected result

  1. The invoice getItems function should return an array of \Magento\Sales\Api\Data\InvoiceItemInterface

Actual result

  1. getItems is returning a Magento\Sales\Model\ResourceModel\Order\Invoice\Item\Collection

I know this seems like a trivial thing but when trying to process the invoice with incorrect return values for getItems with a \Magento\Framework\Webapi\ServiceOutputProcessor, it will fail.

The getItems and getItemsCollection should not be sharing the same data value because their return types are not the same.

koenner01 commented 6 years ago

The same faulty logic is applied to the getComments function in \Magento\Sales\Model\Order\Invoice

magento-engcom-team commented 6 years ago

@koenner01, thank you for your report. We've acknowledged the issue and added to our backlog.

m2-assistant[bot] commented 4 years ago

Hi @engcom-Echo. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

magento-engcom-team commented 4 years ago

:white_check_mark: Confirmed by @engcom-Echo Thank you for verifying the issue. Based on the provided information internal tickets MC-30489 were created

Issue Available: @engcom-Echo, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

m2-assistant[bot] commented 4 years ago

Hi @manish-sirvi. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

hostep commented 4 years ago

@manish-sirvi: please don't close issues without any explanation, thanks!

m2-assistant[bot] commented 4 years ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

magento-engcom-team commented 4 years ago

:white_check_mark: Confirmed by @engcom-Hotel Thank you for verifying the issue. Based on the provided information internal tickets MC-30489 were created

Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

m2-assistant[bot] commented 4 years ago

Hi @AleksLi. Thank you for working on this issue. Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction: