Skip to content

Conversation

kassner
Copy link
Contributor

@kassner kassner commented Dec 12, 2016

FIX: Magento\Bundle\Model\Product\Type::_prepareProduct cannot be called twice
with different product selections, throws fatal error:

PHP Fatal error: Uncaught Error: Call to a member function getPosition() on
null in /var/www/html/vendor/magento/module-bundle/Model/Product/Type.php:937

@vrann vrann self-assigned this Mar 25, 2017
@vrann vrann added this to the March 2017 milestone Mar 25, 2017
@vrann
Copy link
Contributor

vrann commented Mar 25, 2017

@kassner unfortunately, we don't process PRs to 2.1 branch at the moment. Please re-create it against develop branch.

@vrann vrann closed this Mar 25, 2017
@kassner kassner changed the base branch from 2.1 to develop March 25, 2017 19:31
@kassner
Copy link
Contributor Author

kassner commented Mar 25, 2017

@vrann can you reopen? GitHub allows to rebase the PR from the interface.

@vrann vrann reopened this Mar 26, 2017
@vrann
Copy link
Contributor

vrann commented Mar 26, 2017

@kassner can you please merge with the latest develop?

@kassner kassner force-pushed the fix-bundle-optionscollection-append-selections branch from 5b24902 to fbfd7ce Compare March 26, 2017 20:47
@kassner
Copy link
Contributor Author

kassner commented Mar 26, 2017

@vrann done.

@vrann
Copy link
Contributor

vrann commented Apr 12, 2017

@kassner trying to reproduce the issue, can you please provide steps?

@kassner
Copy link
Contributor Author

kassner commented Apr 18, 2017

@vrann Using Magento's sample data, you can go simulate with this controller:

<?php

namespace My\Test\Controller;

class Index extends \Magento\Framework\App\Action\Action
{

    public function __construct(
        \Magento\Framework\App\Action\Context $context,
        \Magento\Catalog\Model\ProductRepository $productRepository,
        \Magento\Quote\Model\QuoteFactory $quoteFactory
    ) {
        parent::__construct($context);
        $this->productRepository = $productRepository;
        $this->quoteFactory = $quoteFactory;
    }

    protected function addAndCalculate($product, $request)
    {
        $quote = $this->quoteFactory->create();
        $quote->addProduct($product, $request);
        $quote->getShippingAddress(); // Needed to go around another bug
        $quote->setTotalsCollectedFlag(false);
        $quote->collectTotals();

        return $quote->getGrandTotal();
    }

    public function execute()
    {
        $p1 = $this->productRepository->get('24-WG080');

        var_dump($this->addAndCalculate($p1, new \Magento\Framework\DataObject([
            'product' => $p1->getId(),
            'qty' => 1,
            'bundle_option' => [
                1 => 1,
                2 => 4,
                3 => 5,
                4 => 8,
            ],
            'bundle_option_qty' => [
                1 => 1,
                2 => 1,
                3 => 1,
                4 => 1,
            ],
        ])));

        $p2 = $this->productRepository->get('24-WG080');

        var_dump($this->addAndCalculate($p2, new \Magento\Framework\DataObject([
            'product' => $p2->getId(),
            'qty' => 1,
            'bundle_option' => [
                1 => 3,
                2 => 4,
                3 => 7,
                4 => 8,
            ],
            'bundle_option_qty' => [
                1 => 1,
                2 => 1,
                3 => 1,
                4 => 1,
            ],
        ])));
    }

}

You might need to adjust the bundle_option IDs to match your database (I have a clean one).

@okorshenko okorshenko modified the milestones: April 2017, May 2017 May 9, 2017
@okorshenko okorshenko modified the milestones: May 2017, June 2017 Jun 1, 2017
@okorshenko okorshenko modified the milestones: June 2017, July 2017 Jul 2, 2017
@okorshenko okorshenko assigned okorshenko and unassigned vrann Jul 12, 2017
@okorshenko
Copy link
Contributor

Hi @kassner
Thank you for your contribution. After several internal discussions, we decided that adding a new flag to the Selection collection is not the best approach. Also, it is clear that having the flag in the Option collection is a legacy code that must be refactored but in a different way (without moving this state flag to another object). This logic must be refactored in a way when option collection and selections won't depend on each other.
Back to your sample code:

$p1 = $this->productRepository->get('24-WG080');
$p2 = $this->productRepository->get('24-WG080');

it is clear that
$p1 === $p2
Since method qoute->addProduct modifis the object state, we can not perform this action in one request.
Here you have few options:

  1. refactor the code in the suggested way (long way)
  2. get product from repository using argument $forceReload = true (quick solution, but may affect performance if you iterate a lot of products)
  3. unset option collection before adding to the cart (dirty way)
    $product->unsetData('_cache_instance_options_collection');

Closing this Pull Request for now. Please, let us know if you would like to work on this refactoring so that we can discuss implementation details and design.

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

Successfully merging this pull request may close these issues.

3 participants