Skip to content

Commit f899234

Browse files
x86demonvitaliyberdylo
x86demon
authored and
vitaliyberdylo
committed
BB-7908: Product import fails (#8298)
1 parent 43eda8a commit f899234

File tree

5 files changed

+243
-2
lines changed

5 files changed

+243
-2
lines changed

Diff for: ‎src/Oro/Bundle/ProductBundle/ImportExport/Processor/ProductImportProcessor.php

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
namespace Oro\Bundle\ProductBundle\ImportExport\Processor;
4+
5+
use Oro\Bundle\BatchBundle\Item\Support\ClosableInterface;
6+
use Oro\Bundle\ImportExportBundle\Processor\ImportProcessor;
7+
8+
class ProductImportProcessor extends ImportProcessor implements ClosableInterface
9+
{
10+
/**
11+
* {@inheritdoc}
12+
*/
13+
public function close()
14+
{
15+
if ($this->strategy instanceof ClosableInterface) {
16+
$this->strategy->close();
17+
}
18+
}
19+
}

Diff for: ‎src/Oro/Bundle/ProductBundle/ImportExport/Strategy/ProductStrategy.php

+39-2
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@
55
use Doctrine\Common\Collections\ArrayCollection;
66
use Doctrine\Common\Collections\Collection;
77
use Doctrine\Common\Util\ClassUtils;
8+
use Oro\Bundle\BatchBundle\Item\Support\ClosableInterface;
89
use Oro\Bundle\LocaleBundle\ImportExport\Strategy\LocalizedFallbackValueAwareStrategy;
910
use Oro\Bundle\OrganizationBundle\Entity\BusinessUnit;
1011
use Oro\Bundle\ProductBundle\Entity\Product;
1112
use Oro\Bundle\ProductBundle\ImportExport\Event\ProductStrategyEvent;
1213
use Oro\Bundle\SecurityBundle\SecurityFacade;
1314
use Oro\Bundle\UserBundle\Entity\User;
1415

15-
class ProductStrategy extends LocalizedFallbackValueAwareStrategy
16+
class ProductStrategy extends LocalizedFallbackValueAwareStrategy implements ClosableInterface
1617
{
1718
/**
1819
* @var SecurityFacade
@@ -34,6 +35,19 @@ class ProductStrategy extends LocalizedFallbackValueAwareStrategy
3435
*/
3536
protected $productClass;
3637

38+
/**
39+
* @var array|Product[]
40+
*/
41+
protected $processedProducts = [];
42+
43+
/**
44+
* {@inheritdoc}
45+
*/
46+
public function close()
47+
{
48+
$this->processedProducts = [];
49+
}
50+
3751
/**
3852
* @param SecurityFacade $securityFacade
3953
*/
@@ -89,7 +103,11 @@ protected function afterProcessEntity($entity)
89103
$event = new ProductStrategyEvent($entity, $this->context->getValue('itemData'));
90104
$this->eventDispatcher->dispatch(ProductStrategyEvent::PROCESS_AFTER, $event);
91105

92-
return parent::afterProcessEntity($entity);
106+
/** @var Product $entity */
107+
$entity = parent::afterProcessEntity($entity);
108+
$this->processedProducts[$entity->getSku()] = $entity;
109+
110+
return $entity;
93111
}
94112

95113
/**
@@ -215,6 +233,25 @@ protected function updateRelations($entity, array $itemData = null)
215233
}
216234
}
217235

236+
/**
237+
* {@inheritdoc}
238+
*/
239+
protected function processEntity(
240+
$entity,
241+
$isFullData = false,
242+
$isPersistNew = false,
243+
$itemData = null,
244+
array $searchContext = [],
245+
$entityIsRelation = false
246+
) {
247+
if ($entity instanceof Product && array_key_exists($entity->getSku(), $this->processedProducts)) {
248+
return $this->processedProducts[$entity->getSku()];
249+
}
250+
251+
return parent::processEntity($entity, $isFullData, $isPersistNew, $itemData, $searchContext, $entityIsRelation);
252+
}
253+
254+
218255
/**
219256
* Get additional search parameter name to find only related entities
220257
*

Diff for: ‎src/Oro/Bundle/ProductBundle/Resources/config/importexport.yml

+1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ services:
4343
oro_product.importexport.processor.import.product:
4444
public: false
4545
parent: oro_importexport.processor.import_abstract
46+
class: Oro\Bundle\ProductBundle\ImportExport\Processor\ProductImportProcessor
4647
calls:
4748
- [setDataConverter, ['@oro_product.importexport.data_converter.product']]
4849
- [setStrategy, ['@oro_product.importexport.strategy.product']]

Diff for: ‎src/Oro/Bundle/ProductBundle/Tests/Functional/ImportExport/Strategy/ProductStrategyTest.php

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
<?php
2+
3+
namespace Oro\Bundle\ProductBundle\Tests\Functional\ImportExport\Strategy;
4+
5+
use Oro\Bundle\EntityConfigBundle\Attribute\Entity\AttributeFamily;
6+
use Oro\Bundle\EntityExtendBundle\Entity\AbstractEnumValue;
7+
use Oro\Bundle\EntityExtendBundle\Tools\ExtendHelper;
8+
use Oro\Bundle\ImportExportBundle\Context\Context;
9+
use Oro\Bundle\LocaleBundle\Entity\LocalizedFallbackValue;
10+
use Oro\Bundle\ProductBundle\Entity\Product;
11+
use Oro\Bundle\ProductBundle\Entity\ProductUnit;
12+
use Oro\Bundle\ProductBundle\Entity\ProductUnitPrecision;
13+
use Oro\Bundle\ProductBundle\Entity\ProductVariantLink;
14+
use Oro\Bundle\ProductBundle\ImportExport\Strategy\ProductStrategy;
15+
use Oro\Bundle\ProductBundle\Tests\Functional\DataFixtures\LoadProductData;
16+
use Oro\Bundle\ProductBundle\Tests\Functional\DataFixtures\LoadProductUnits;
17+
use Oro\Bundle\TestFrameworkBundle\Test\WebTestCase;
18+
use Oro\Component\Testing\Unit\EntityTrait;
19+
20+
/**
21+
* @dbIsolation
22+
*/
23+
class ProductStrategyTest extends WebTestCase
24+
{
25+
use EntityTrait;
26+
27+
/**
28+
* @var ProductStrategy
29+
* */
30+
protected $strategy;
31+
32+
protected function setUp()
33+
{
34+
$this->initClient();
35+
$this->loadFixtures([LoadProductData::class]);
36+
$container = $this->getContainer();
37+
38+
$container->get('oro_importexport.field.database_helper')->onClear();
39+
$this->strategy = new ProductStrategy(
40+
$container->get('event_dispatcher'),
41+
$container->get('oro_importexport.strategy.import.helper'),
42+
$container->get('oro_entity.helper.field_helper'),
43+
$container->get('oro_importexport.field.database_helper'),
44+
$container->get('oro_entity.entity_class_name_provider'),
45+
$container->get('translator'),
46+
$container->get('oro_importexport.strategy.new_entities_helper'),
47+
$container->get('oro_entity.doctrine_helper')
48+
);
49+
$this->strategy->setEntityName(Product::class);
50+
$this->strategy->setVariantLinkClass(ProductVariantLink::class);
51+
$this->strategy->setLocalizedFallbackValueClass(LocalizedFallbackValue::class);
52+
$this->strategy->setSecurityFacade($container->get('oro_security.security_facade'));
53+
}
54+
55+
/**
56+
* Impossible to import product when it's variant is in the same batch.
57+
* Caused because variant is not in DB it could not be loaded by SKU and variant link contains invalid relation.
58+
*
59+
* @link https://magecore.atlassian.net/browse/BB-7908
60+
*/
61+
public function testProcessWithVariantLinks()
62+
{
63+
$context = new Context([]);
64+
$context->setValue('itemData', []);
65+
$this->strategy->setImportExportContext($context);
66+
67+
$inventoryStatusClassName = ExtendHelper::buildEnumValueClassName('prod_inventory_status');
68+
/** @var AbstractEnumValue $inventoryStatus */
69+
$inventoryStatus = $this->getContainer()
70+
->get('doctrine')
71+
->getRepository($inventoryStatusClassName)
72+
->find('in_stock');
73+
74+
/** @var ProductUnit $unit */
75+
$unit = $this->getReference(LoadProductUnits::BOX);
76+
/** @var AttributeFamily $attributeFamily */
77+
$attributeFamily = $this->getEntity(AttributeFamily::class, ['code' => 'default_family']);
78+
$newProductSku = 'PR-V1';
79+
80+
// Prepare new product that is imported in same batch and will be used later as variant link
81+
$newProduct = $this->createProduct($newProductSku, $attributeFamily, $unit, $inventoryStatus);
82+
/** @var Product $processedNewProduct */
83+
$processedNewProduct = $this->strategy->process($newProduct);
84+
$this->assertEquals([], $context->getErrors());
85+
$this->assertInstanceOf(Product::class, $processedNewProduct);
86+
$this->assertSame($newProductSku, $processedNewProduct->getSku());
87+
88+
// Get existing product that should be found by SKU as variant link relation
89+
/** @var Product $existingProduct */
90+
$existingProduct = $this->getReference(LoadProductData::PRODUCT_1);
91+
92+
$linkToNewProduct = new ProductVariantLink();
93+
$linkToNewProduct->setProduct((new Product())->setSku($newProductSku));
94+
$linkToExistingProduct = new ProductVariantLink();
95+
$linkToExistingProduct->setProduct((new Product())->setSku($existingProduct->getSku()));
96+
97+
// Add prepared variant links to newly imported product
98+
$productWithVariants = $this->createProduct('PR-VV', $attributeFamily, $unit, $inventoryStatus);
99+
$productWithVariants->addVariantLink($linkToNewProduct);
100+
$productWithVariants->addVariantLink($linkToExistingProduct);
101+
102+
// Check that all variant links present and were attached correctly
103+
/** @var Product $processedProductWithVariants */
104+
$processedProductWithVariants = $this->strategy->process($productWithVariants);
105+
$this->assertEquals([], $context->getErrors());
106+
$this->assertInstanceOf(Product::class, $processedProductWithVariants);
107+
$this->assertSame($productWithVariants->getSku(), $processedProductWithVariants->getSku());
108+
$this->assertCount(2, $processedProductWithVariants->getVariantLinks());
109+
$usedVariantLinksProductSkus = array_map(
110+
function (ProductVariantLink $variantLink) {
111+
return $variantLink->getProduct()->getSku();
112+
},
113+
$processedProductWithVariants->getVariantLinks()->toArray()
114+
);
115+
$this->assertContains($newProductSku, $usedVariantLinksProductSkus);
116+
$this->assertContains($existingProduct->getSku(), $usedVariantLinksProductSkus);
117+
}
118+
119+
/**
120+
* @param string $sku
121+
* @param AttributeFamily $attributeFamily
122+
* @param ProductUnit $unit
123+
* @param AbstractEnumValue $inventoryStatus
124+
* @return Product
125+
*/
126+
protected function createProduct(
127+
$sku,
128+
AttributeFamily $attributeFamily,
129+
ProductUnit $unit,
130+
AbstractEnumValue $inventoryStatus
131+
) {
132+
$newProduct = new Product();
133+
$newProduct->setSku($sku);
134+
$newProduct->setAttributeFamily($attributeFamily);
135+
$newProduct->setInventoryStatus($inventoryStatus);
136+
$newProduct->setPrimaryUnitPrecision(
137+
(new ProductUnitPrecision())->setUnit($unit)
138+
);
139+
140+
return $newProduct;
141+
}
142+
}

Diff for: ‎src/Oro/Bundle/ProductBundle/Tests/Unit/ImportExport/Processor/ProductImportProcessorTest.php

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php
2+
3+
namespace Oro\Bundle\ProductBundle\Tests\Unit\ImportExport\Processor;
4+
5+
use Oro\Bundle\ImportExportBundle\Strategy\StrategyInterface;
6+
use Oro\Bundle\ProductBundle\ImportExport\Processor\ProductImportProcessor;
7+
use Oro\Bundle\ProductBundle\ImportExport\Strategy\ProductStrategy;
8+
9+
class ProductImportProcessorTest extends \PHPUnit_Framework_TestCase
10+
{
11+
/**
12+
* @var ProductImportProcessor
13+
*/
14+
private $processor;
15+
16+
protected function setUp()
17+
{
18+
$this->processor = new ProductImportProcessor();
19+
}
20+
21+
public function testCloseWithClosableStrategy()
22+
{
23+
/** @var ProductStrategy|\PHPUnit_Framework_MockObject_MockObject $strategy */
24+
$strategy = $this->getMockBuilder(ProductStrategy::class)
25+
->disableOriginalConstructor()
26+
->getMock();
27+
$strategy->expects($this->once())
28+
->method('close');
29+
$this->processor->setStrategy($strategy);
30+
$this->processor->close();
31+
}
32+
33+
public function testCloseWithNonClosableStrategy()
34+
{
35+
/** @var StrategyInterface|\PHPUnit_Framework_MockObject_MockObject $strategy */
36+
$strategy = $this->createMock(StrategyInterface::class);
37+
$strategy->expects($this->never())
38+
->method($this->anything());
39+
$this->processor->setStrategy($strategy);
40+
$this->processor->close();
41+
}
42+
}

0 commit comments

Comments
 (0)