From 21ca61318899b7c1e76761f1f68cb57ca618f90f Mon Sep 17 00:00:00 2001 From: Dmitry Khrysev Date: Tue, 26 Jul 2016 12:34:42 +0300 Subject: [PATCH 1/3] BB-3950: Product units are not updated correctly after update --- .../FrontendProductPricesProvider.php | 16 ++--- .../Schema/v1_3/OroB2BPricingBundle.php | 1 - .../Schema/v1_3/PostMigrationUpdates.php | 2 + .../FrontendProductPricesProviderTest.php | 66 +++++++++---------- .../Entity/ProductUnitPrecision.php | 4 +- .../Schema/OroB2BProductBundleInstaller.php | 2 +- .../Schema/v1_4/OroB2BProductBundle.php | 23 ++++++- .../Schema/v1_4/PostMigrationUpdates.php | 38 +++++++++++ .../v1_4/RemoveImageRelationOnProduct.php | 1 - 9 files changed, 102 insertions(+), 51 deletions(-) create mode 100644 src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/PostMigrationUpdates.php diff --git a/src/OroB2B/Bundle/PricingBundle/Layout/DataProvider/FrontendProductPricesProvider.php b/src/OroB2B/Bundle/PricingBundle/Layout/DataProvider/FrontendProductPricesProvider.php index e833c16db86..972281a5a82 100644 --- a/src/OroB2B/Bundle/PricingBundle/Layout/DataProvider/FrontendProductPricesProvider.php +++ b/src/OroB2B/Bundle/PricingBundle/Layout/DataProvider/FrontendProductPricesProvider.php @@ -7,6 +7,7 @@ use Oro\Bundle\EntityBundle\ORM\DoctrineHelper; +use OroB2B\Bundle\PricingBundle\Entity\CombinedProductPrice; use OroB2B\Bundle\PricingBundle\Entity\ProductPrice; use OroB2B\Bundle\PricingBundle\Entity\Repository\ProductPriceRepository; use OroB2B\Bundle\PricingBundle\Model\PriceListRequestHandler; @@ -78,20 +79,19 @@ public function getData(ContextInterface $context) ] ); if (count($prices)) { - $unitPrecisions = current($prices)->getProduct()->getUnitPrecisions(); + $unitPrecisions = $product->getUnitPrecisions(); $unitsToSell = []; foreach ($unitPrecisions as $unitPrecision) { - if ($unitPrecision->isSell()) { - $unitsToSell[] = $unitPrecision->getUnit(); - } + $unitsToSell[$unitPrecision->getUnit()->getCode()] = $unitPrecision->isSell(); } - foreach ($prices as $key => $combinedProductPrice) { - if (!in_array($combinedProductPrice->getUnit(), $unitsToSell)) { - unset($prices[$key]); + $prices = array_filter( + $prices, + function (CombinedProductPrice $price) use ($unitsToSell) { + return !empty($unitsToSell[$price->getProductUnitCode()]); } - } + ); } $this->data[$productId] = $prices; diff --git a/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/OroB2BPricingBundle.php b/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/OroB2BPricingBundle.php index c322266ee6b..c9eecd7b239 100644 --- a/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/OroB2BPricingBundle.php +++ b/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/OroB2BPricingBundle.php @@ -284,7 +284,6 @@ protected function addOrob2BCmbPriceListToAccForeignKeys(Schema $schema) ['id'], ['onUpdate' => null, 'onDelete' => 'CASCADE'] ); - } /** diff --git a/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/PostMigrationUpdates.php b/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/PostMigrationUpdates.php index 7d841f7ca3c..914d30ab1a9 100644 --- a/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/PostMigrationUpdates.php +++ b/src/OroB2B/Bundle/PricingBundle/Migrations/Schema/v1_3/PostMigrationUpdates.php @@ -30,6 +30,7 @@ public function up(Schema $schema, QueryBag $queries) /** * @param Schema $schema + * @param QueryBag $queries * @throws \Doctrine\DBAL\Schema\SchemaException */ protected function updatePriceListTable(Schema $schema, QueryBag $queries) @@ -46,6 +47,7 @@ protected function updatePriceListTable(Schema $schema, QueryBag $queries) /** * @param Schema $schema + * @param QueryBag $queries * @throws \Doctrine\DBAL\Schema\SchemaException */ protected function updatePriceListCombinedTable(Schema $schema, QueryBag $queries) diff --git a/src/OroB2B/Bundle/PricingBundle/Tests/Unit/Layout/DataProvider/FrontendProductPricesProviderTest.php b/src/OroB2B/Bundle/PricingBundle/Tests/Unit/Layout/DataProvider/FrontendProductPricesProviderTest.php index 068744b25cb..4cddc33ba19 100644 --- a/src/OroB2B/Bundle/PricingBundle/Tests/Unit/Layout/DataProvider/FrontendProductPricesProviderTest.php +++ b/src/OroB2B/Bundle/PricingBundle/Tests/Unit/Layout/DataProvider/FrontendProductPricesProviderTest.php @@ -7,9 +7,13 @@ use Oro\Bundle\EntityBundle\ORM\DoctrineHelper; +use OroB2B\Bundle\PricingBundle\Entity\CombinedProductPrice; use OroB2B\Bundle\PricingBundle\Layout\DataProvider\FrontendProductPricesProvider; use OroB2B\Bundle\PricingBundle\Model\PriceListRequestHandler; use OroB2B\Bundle\PricingBundle\Manager\UserCurrencyManager; +use OroB2B\Bundle\ProductBundle\Entity\Product; +use OroB2B\Bundle\ProductBundle\Entity\ProductUnit; +use OroB2B\Bundle\ProductBundle\Entity\ProductUnitPrecision; class FrontendProductPricesProviderTest extends \PHPUnit_Framework_TestCase { @@ -68,13 +72,13 @@ public function testGetDataWithEmptyContext() public function testGetData() { - $prices = [1 => 'test', 2 => 'test2']; $priceListId = 23; $priceList = $this->getEntity('OroB2B\Bundle\PricingBundle\Entity\PriceList', ['id' => $priceListId]); $productId = 24; - $product = $this->getMockBuilder('OroB2B\Bundle\ProductBundle\Entity\Product') - ->disableOriginalConstructor() - ->getMock(); + /** @var Product|\PHPUnit_Framework_MockObject_MockObject $product */ + $product = $this->getMockBuilder('OroB2B\Bundle\ProductBundle\Entity\Product') + ->disableOriginalConstructor() + ->getMock(); $product->expects($this->once()) ->method('getId') @@ -84,13 +88,13 @@ public function testGetData() $unitPrecisions[] = $this->createUnitPrecision('set', false); $product->expects($this->once()) - ->method('getUnitPrecisions') - ->willReturn($unitPrecisions); + ->method('getUnitPrecisions') + ->willReturn($unitPrecisions); $productPrice1 = $this->createProductPrice('each', $product); $productPrice2 = $this->createProductPrice('set', $product); $prices = [$productPrice1, $productPrice2]; - + $priceSorting = ['unit' => 'ASC', 'currency' => 'DESC', 'quantity' => 'ASC']; $context = new LayoutContext(); $context->data()->set('product', null, $product); @@ -117,29 +121,20 @@ public function testGetData() ->willReturn('EUR'); $actual = $this->provider->getData($context); - $this->assertEquals(1, count($actual)); + $this->assertCount(1, $actual); $this->assertEquals('each', current($actual)->getUnit()); } /** * @param string $unitCode - * @param boolen $sell - * @return productUnitPresion + * @param boolean $sell + * @return ProductUnitPrecision */ private function createUnitPrecision($unitCode, $sell) { - $p = $this->getMockBuilder('OroB2B\Bundle\ProductBundle\Entity\ProductUnionPrecision') - ->setMethods(array('isSell', 'getUnit')) - ->disableOriginalConstructor() - ->getMock(); - - $p->expects($this->once()) - ->method('isSell') - ->willReturn($sell); - - $p->expects($this->any()) - ->method('getUnit') - ->willReturn($unitCode); + $p = new ProductUnitPrecision(); + $p->setSell($sell); + $p->setUnit($this->getUnit($unitCode)); return $p; } @@ -151,21 +146,22 @@ private function createUnitPrecision($unitCode, $sell) */ private function createProductPrice($unit, $product) { - $p = $this->getMockBuilder( - 'OroB2B\Bundle\PricingBundle\Entity\CombinedProductPrice', - array('getUnit', 'getProduct') - ) - ->disableOriginalConstructor() - ->getMock(); + $p = new CombinedProductPrice(); + $p->setProduct($product); + $p->setUnit($this->getUnit($unit)); - $p->expects($this->any()) - ->method('getProduct') - ->willReturn($product); + return $p; + } - $p->expects($this->any()) - ->method('getUnit') - ->willReturn($unit); + /** + * @param string $unitCode + * @return ProductUnit + */ + private function getUnit($unitCode) + { + $unit = new ProductUnit(); + $unit->setCode($unitCode); - return $p; + return $unit; } } diff --git a/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php b/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php index b174acd33a7..d7d1510baeb 100644 --- a/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php +++ b/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php @@ -96,7 +96,7 @@ class ProductUnitPrecision implements ProductUnitHolderInterface /** * @var bool * - * @ORM\Column(name="sell",type="boolean",nullable=true) + * @ORM\Column(name="sell",type="boolean",nullable=false) * @ConfigField( * defaultValues={ * "importexport"={ @@ -105,7 +105,7 @@ class ProductUnitPrecision implements ProductUnitHolderInterface * } * ) */ - protected $sell; + protected $sell = true; public function __clone() { diff --git a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/OroB2BProductBundleInstaller.php b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/OroB2BProductBundleInstaller.php index 5c91d8ca1e7..822d9285a75 100644 --- a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/OroB2BProductBundleInstaller.php +++ b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/OroB2BProductBundleInstaller.php @@ -157,7 +157,7 @@ protected function createOroB2BProductUnitPrecisionTable(Schema $schema) $table->addColumn('unit_code', 'string', ['notnull' => false, 'length' => 255]); $table->addColumn('unit_precision', 'integer', []); $table->addColumn('conversion_rate', 'float', ['notnull' => false]); - $table->addColumn('sell', 'boolean', ['notnull' => false]); + $table->addColumn('sell', 'boolean', ['notnull' => true]); $table->setPrimaryKey(['id']); $table->addUniqueIndex(['product_id', 'unit_code'], 'product_unit_precision__product_id__unit_code__uidx'); } diff --git a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/OroB2BProductBundle.php b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/OroB2BProductBundle.php index 4462d99ac37..00f2ef866c6 100644 --- a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/OroB2BProductBundle.php +++ b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/OroB2BProductBundle.php @@ -5,12 +5,14 @@ use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Doctrine\DBAL\Types\Type; use Doctrine\DBAL\Schema\Schema; use Oro\Bundle\AttachmentBundle\Migration\Extension\AttachmentExtension; use Oro\Bundle\AttachmentBundle\Migration\Extension\AttachmentExtensionAwareInterface; use Oro\Bundle\MigrationBundle\Migration\Extension\RenameExtension; use Oro\Bundle\MigrationBundle\Migration\Extension\RenameExtensionAwareInterface; +use Oro\Bundle\MigrationBundle\Migration\ParametrizedSqlMigrationQuery; use Oro\Bundle\MigrationBundle\Migration\Migration; use Oro\Bundle\MigrationBundle\Migration\OrderedMigrationInterface; use Oro\Bundle\MigrationBundle\Migration\QueryBag; @@ -94,7 +96,7 @@ public function up(Schema $schema, QueryBag $queries) 'oro_fallback_localization_val', ['localized_value_id'] ); - $this->updateOroB2BProductUnitPrecisionTable($schema); + $this->updateOroB2BProductUnitPrecisionTable($schema, $queries); $this->updateOroB2BProductTable($schema); $this->addOroB2BProductForeignKeys($schema); @@ -132,12 +134,27 @@ protected function createConstraint(Schema $schema, QueryBag $queries, $tableNam * Update orob2b_product_unit_precision table * * @param Schema $schema + * @param QueryBag $queries */ - protected function updateOroB2BProductUnitPrecisionTable(Schema $schema) + protected function updateOroB2BProductUnitPrecisionTable(Schema $schema, QueryBag $queries) { $table = $schema->getTable(self::PRODUCT_UNIT_PRECISION_TABLE_NAME); $table->addColumn('conversion_rate', 'float', ['notnull' => false]); $table->addColumn('sell', 'boolean', ['notnull' => false]); + + $queries->addQuery( + new ParametrizedSqlMigrationQuery( + 'UPDATE orob2b_product_unit_precision SET conversion_rate = :conversion_rate, sell = :sell', + [ + 'conversion_rate' => 1.0, + 'sell' => true, + ], + [ + 'conversion_rate' => Type::FLOAT, + 'sell' => Type::BOOLEAN + ] + ) + ); } /** @@ -289,6 +306,6 @@ protected function getImageTypesSubSelect() $selects[] = sprintf('SELECT \'%s\' as type', $imageType->getName()); } - return join(' UNION ', $selects); + return implode(' UNION ', $selects); } } diff --git a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/PostMigrationUpdates.php b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/PostMigrationUpdates.php new file mode 100644 index 00000000000..af6fe3d058b --- /dev/null +++ b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/PostMigrationUpdates.php @@ -0,0 +1,38 @@ +updateProductUnitPrecisionTable($schema); + } + + /** + * @param Schema $schema + * @throws \Doctrine\DBAL\Schema\SchemaException + */ + protected function updateProductUnitPrecisionTable(Schema $schema) + { + $table = $schema->getTable('orob2b_product_unit_precision'); + $table->getColumn('sell')->setNotnull(true); + } +} diff --git a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/RemoveImageRelationOnProduct.php b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/RemoveImageRelationOnProduct.php index c5e80ba96eb..1b5506b86fd 100644 --- a/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/RemoveImageRelationOnProduct.php +++ b/src/OroB2B/Bundle/ProductBundle/Migrations/Schema/v1_4/RemoveImageRelationOnProduct.php @@ -50,7 +50,6 @@ public function up(Schema $schema, QueryBag $queries) self::PRODUCT_IMAGE_ASSOCCIATION_NAME ) ); - } /** From 63758ffce4f96b2f01aad86215c14038bbdbf689 Mon Sep 17 00:00:00 2001 From: Dmitry Khrysev Date: Tue, 26 Jul 2016 12:44:55 +0300 Subject: [PATCH 2/3] BB-3950: Product units are not updated correctly after update - tests fix --- src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php b/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php index d7d1510baeb..392a08f8baf 100644 --- a/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php +++ b/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php @@ -105,7 +105,7 @@ class ProductUnitPrecision implements ProductUnitHolderInterface * } * ) */ - protected $sell = true; + protected $sell; public function __clone() { From da237847166ab9481951e53b13967c6a3178d238 Mon Sep 17 00:00:00 2001 From: Dmitry Khrysev Date: Tue, 26 Jul 2016 14:54:11 +0300 Subject: [PATCH 3/3] BB-3950: Product units are not updated correctly after update - tests fix --- .../Entity/ProductUnitPrecision.php | 2 +- .../DataConverter/ProductDataConverter.php | 18 ++++++++++-------- .../Normalizer/ProductNormalizer.php | 10 ++++++++-- .../ProductPrimaryUnitPrecisionTypeTest.php | 8 ++++++-- .../Form/Type/ProductUnitPrecisionTypeTest.php | 6 ++++-- 5 files changed, 29 insertions(+), 15 deletions(-) diff --git a/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php b/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php index 392a08f8baf..d7d1510baeb 100644 --- a/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php +++ b/src/OroB2B/Bundle/ProductBundle/Entity/ProductUnitPrecision.php @@ -105,7 +105,7 @@ class ProductUnitPrecision implements ProductUnitHolderInterface * } * ) */ - protected $sell; + protected $sell = true; public function __clone() { diff --git a/src/OroB2B/Bundle/ProductBundle/ImportExport/DataConverter/ProductDataConverter.php b/src/OroB2B/Bundle/ProductBundle/ImportExport/DataConverter/ProductDataConverter.php index 8a7153887c6..e38df306e8b 100644 --- a/src/OroB2B/Bundle/ProductBundle/ImportExport/DataConverter/ProductDataConverter.php +++ b/src/OroB2B/Bundle/ProductBundle/ImportExport/DataConverter/ProductDataConverter.php @@ -30,20 +30,22 @@ public function setEventDispatcher(EventDispatcherInterface $eventDispatcher) protected function getBackendHeader() { $backendHeader = parent::getBackendHeader(); - + // According to business logic Product should have Primary and Additional unit precisions. // But Product Entity has primaryUnitPrecision property and unitPrecisions property which // is collection of all unit precisions. AdditionalUnitPrecisions is calculated by excluding // PrimaryUnitPrecision from all UnitPrecisions. This fix was done in order to display // correct column headers during Products Export. foreach ($backendHeader as $k => &$v) { - $arr = explode(":", $v); - if ($arr[0] == "unitPrecisions" && $arr[1] == "0") { - unset($backendHeader[$k]); - } elseif ($arr[0] == "unitPrecisions") { - $arr[0] = "additionalUnitPrecisions"; - $arr[1] = $arr[1] - 1; - $v = implode(":", $arr); + $arr = explode(':', $v); + if ($arr[0] === 'unitPrecisions') { + if ((int)$arr[1] === 0) { + unset($backendHeader[$k]); + } else { + $arr[0] = 'additionalUnitPrecisions'; + --$arr[1]; + $v = implode(':', $arr); + } } } unset($v); diff --git a/src/OroB2B/Bundle/ProductBundle/ImportExport/Normalizer/ProductNormalizer.php b/src/OroB2B/Bundle/ProductBundle/ImportExport/Normalizer/ProductNormalizer.php index 8b1fcb16af3..c3ce1e8089c 100644 --- a/src/OroB2B/Bundle/ProductBundle/ImportExport/Normalizer/ProductNormalizer.php +++ b/src/OroB2B/Bundle/ProductBundle/ImportExport/Normalizer/ProductNormalizer.php @@ -38,15 +38,17 @@ public function setProductClass($productClass) } /** + * @param Product $object + * * {@inheritdoc} */ public function normalize($object, $format = null, array $context = []) { $data = parent::normalize($object, $format, $context); - if (array_key_exists('unitPrecisions', $data)) { + if (array_key_exists('unitPrecisions', $data) && is_array($data['unitPrecisions'])) { foreach ($data['unitPrecisions'] as $v) { - if ($v['unit']['code'] != $object->getPrimaryUnitPrecision()->getUnit()->getCode()) { + if ($v['unit']['code'] !== $object->getPrimaryUnitPrecision()->getUnit()->getCode()) { $data['additionalUnitPrecisions'][] = $v; } } @@ -69,6 +71,10 @@ public function denormalize($data, $class, $format = null, array $context = []) if (array_key_exists('additionalUnitPrecisions', $data)) { $data['unitPrecisions'] = $data['additionalUnitPrecisions']; unset($data['additionalUnitPrecisions']); + foreach ($data['unitPrecisions'] as &$unitPrecisionData) { + $unitPrecisionData['sell'] = !empty($unitPrecisionData['sell']); + } + unset($unitPrecisionData); } /** diff --git a/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductPrimaryUnitPrecisionTypeTest.php b/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductPrimaryUnitPrecisionTypeTest.php index 6fa0dfd21e7..05185d72c7b 100644 --- a/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductPrimaryUnitPrecisionTypeTest.php +++ b/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductPrimaryUnitPrecisionTypeTest.php @@ -138,7 +138,8 @@ public function submitProvider() ] ], 'submittedData' => [], - 'expectedData' => new ProductUnitPrecision() + 'expectedData' => (new ProductUnitPrecision()) + ->setSell(false) ], 'unit precision without value' => [ 'defaultData' => new ProductUnitPrecision(), @@ -146,7 +147,8 @@ public function submitProvider() 'unit' => [], ], 'submittedData' => [], - 'expectedData' => new ProductUnitPrecision() + 'expectedData' => (new ProductUnitPrecision()) + ->setSell(false) ], 'unit precision with value' => [ 'defaultData' => new ProductUnitPrecision(), @@ -156,10 +158,12 @@ public function submitProvider() 'submittedData' => [ 'unit' => 'kg', 'precision' => 5, + 'sell' => true ], 'expectedData' => (new ProductUnitPrecision()) ->setUnit((new ProductUnit())->setCode('kg')) ->setPrecision(5) + ->setSell(true) ] ]; } diff --git a/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductUnitPrecisionTypeTest.php b/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductUnitPrecisionTypeTest.php index f6041b19423..563ae731147 100644 --- a/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductUnitPrecisionTypeTest.php +++ b/src/OroB2B/Bundle/ProductBundle/Tests/Unit/Form/Type/ProductUnitPrecisionTypeTest.php @@ -122,7 +122,8 @@ public function submitProvider() ] ], 'submittedData' => [], - 'expectedData' => new ProductUnitPrecision() + 'expectedData' => (new ProductUnitPrecision()) + ->setSell(false) ], 'unit precision without value' => [ 'defaultData' => new ProductUnitPrecision(), @@ -131,7 +132,8 @@ public function submitProvider() 'unit_disabled' => [] ], 'submittedData' => [], - 'expectedData' => new ProductUnitPrecision() + 'expectedData' => (new ProductUnitPrecision()) + ->setSell(false) ], 'unit precision with value' => [ 'defaultData' => new ProductUnitPrecision(),