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

Expose product variants via export API #160

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions app/code/Magento/CatalogExport/Model/Data/Variant.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\CatalogExport\Model\Data;

use Magento\CatalogExportApi\Api\Data\VariantInterface;
use Magento\Framework\Model\AbstractModel;

/**
* Variant entity.
*/
class Variant extends AbstractModel implements VariantInterface
{
private const ID = 'id';
private const PARENT_ID = 'parent_id';
private const PRODUCT_ID = 'product_id';
private const DEFAULT_QTY = 'default_qty';
private const QTY_MUTABILITY = 'qty_mutability';
private const OPTION_VALUE_IDS = 'option_value_ids';
private const PRICE = 'price';

/**
* @inheritDoc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our static tests will possible failed for inheritDoc (with camel case)

*/
public function getId()
{
return $this->getData(self::ID);
}

/**
* @inheritDoc
*/
public function getParentId(): string
{
return $this->getData(self::PARENT_ID);
}

/**
* @inheritDoc
*/
public function getProductId(): string
{
return $this->getData(self::PRODUCT_ID);
}

/**
* @inheritDoc
*/
public function getDefaultQty(): float
{
return $this->getData(self::DEFAULT_QTY);
}

/**
* @inheritDoc
*/
public function getQtyMutability(): bool
{
return $this->getData(self::QTY_MUTABILITY);
}

/**
* @inheritDoc
*/
public function getOptionValueIds(): array
Copy link
Member

@mslabko mslabko Jul 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DTO and schema does not match: https://github.com/magento/saas-export/pull/67/files#diff-f3c59568cea39e5d8221a349960c1da1R118. (it will lead to error when you will try to use Export API)

I propose to create correct et_shema fist, and then generate classes from schema, so we will have one source of true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema doesn't exists yet. You are referencing on the PR that isn't under review yet.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shema should be created first as a source of true as was agreed with @akaplya
Any manual work is redundant since we have a tool to generate classes

please check comment in #160 (review)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual work has already done. I don't see any needs to undo this work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may see after connecting 2 parts and use them together to propagate data from one system to another.

if something you can automate, just do it, don't spend your time on routine work

{
return $this->getData(self::OPTION_VALUE_IDS);
}

/**
* @inheritDoc
*/
public function getPrice(): float
{
return $this->getData(self::PRICE);
}
}
43 changes: 43 additions & 0 deletions app/code/Magento/CatalogExport/Model/Data/VariantFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\CatalogExport\Model\Data;

/**
* Variant factory.
*/
class VariantFactory
{
/**
* Object Manager
*
* @var \Magento\Framework\ObjectManagerInterface
*/
private $objectManager;

/**
* @param \Magento\Framework\ObjectManagerInterface $objectManager
*/
public function __construct(\Magento\Framework\ObjectManagerInterface $objectManager)
{
$this->objectManager = $objectManager;
}

/**
* Create instance of Variant.
*
* @param array $data
* @return \Magento\CatalogExportApi\Api\Data\VariantInterface
*/
public function create(array $data)
{
return $this->objectManager->create(
\Magento\CatalogExportApi\Api\Data\VariantInterface::class,
$data
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add empty line at the end of the file

124 changes: 124 additions & 0 deletions app/code/Magento/CatalogExport/Model/VariantRepository.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\CatalogExport\Model;

use Magento\CatalogExportApi\Api\VariantRepositoryInterface;

/**
* Default implementation of product variants repository.
*/
class VariantRepository implements VariantRepositoryInterface
{
/**
* Constant value for setting max items in response.
*/
private const MAX_ITEMS_IN_RESPONSE = 250;

/**
* Config path of max numbers of variants allowed in response.
*/
private const CATALOG_EXPORT_MAX_VARIANTS_IN_RESPONSE = 'catalog_export/max_variants_in_response';

/**
* @var \Magento\CatalogDataExporter\Model\Feed\Variant
*/
private $variantFeed;

/**
* @var LoggerInterface
*/
private $logger;

/**
* @var \Magento\Framework\App\DeploymentConfig
*/
private $deploymentConfig;

/**
* @var \Magento\CatalogExport\Model\Data\VariantFactory
*/
private $variantFactory;

/**
* @param \Magento\CatalogDataExporter\Model\Feed\Variant $variantFeed
* @param \Magento\CatalogExport\Model\Data\VariantFactory $variantFactory
* @param \Magento\Framework\App\DeploymentConfig $deploymentConfig
* @param LoggerInterface $logger
*/
public function __construct(
\Magento\CatalogDataExporter\Model\Feed\Variant $variantFeed,
\Magento\CatalogExport\Model\Data\VariantFactory $variantFactory,
\Magento\Framework\App\DeploymentConfig $deploymentConfig,
LoggerInterface $logger
) {
$this->variantFeed = $variantFeed;
$this->variantFactory = $variantFactory;
$this->deploymentConfig = $deploymentConfig;
$this->logger = $logger;
}

/**
* @inheritDoc
*/
public function get(array $ids): array
{
$this->validateIds($ids);

$variants = [];
foreach ($this->fetchData($ids) as $feedItem) {
$feedItem['id'] = $feedItem['productId'];
$variants[] = $this->variantFactory->create($feedItem);
}

return $variants;
}

/**
* Get max items in response.
*
* @return int
*/
private function getMaxItemsInResponse()
{
$maxItemsInResponse = (int) $this->deploymentConfig->get(self::CATALOG_EXPORT_MAX_VARIANTS_IN_RESPONSE);
return $maxItemsInResponse ?: self::MAX_ITEMS_IN_RESPONSE;
}

/**
* Validate ids input array.
*
* @param string[] $ids
* @throws \InvalidArgumentException
*/
private function validateIds(array $ids): void
{
if (count($ids) > $this->getMaxItemsInResponse()) {
throw new \InvalidArgumentException(
'Max items in the response can\'t exceed '
. $this->getMaxItemsInResponse()
. '.'
);
}
}

/**
* Retrieve list of variants' data from Variant feed by ids.
*
* @param string[] $ids
* @return array
*/
private function fetchData(array $ids): array
{
$feedData = $this->variantFeed->getFeedByIds($ids);
if (empty($feedData['feed'])) {
$this->logger->error(\sprintf('Cannot find Variants in feed with ids "%s"', $ids));
}

return $feedData['feed'];
}
}
3 changes: 3 additions & 0 deletions app/code/Magento/CatalogExport/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
-->
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
<preference for="Magento\CatalogExportApi\Api\Data\ProductInterface" type="Magento\CatalogExport\Model\Data\Product"/>
<preference for="Magento\CatalogExportApi\Api\Data\VariantInterface" type="Magento\CatalogExport\Model\Data\Variant"/>
<preference for="Magento\CatalogExportApi\Api\Data\AttributeInterface" type="Magento\CatalogExport\Model\Data\Attribute"/>
<preference for="Magento\CatalogExportApi\Api\ProductRepositoryInterface" type="Magento\CatalogExport\Model\ProductRepository"/>
<preference for="Magento\CatalogExportApi\Api\VariantRepositoryInterface" type="Magento\CatalogExport\Model\VariantRepository"/>
<preference for="Magento\CatalogDataExporter\Model\Indexer\IndexerCallbackInterface" type="Magento\CatalogExport\Model\Indexer\IndexerCallback" />
<preference for="Magento\CatalogExportApi\Api\Data\VariantInterfaceFactory" type="Magento\CatalogExport\Model\Data\VariantFactory" />

<type name="Magento\CatalogExport\Model\ProductRepository">
<arguments>
Expand Down
6 changes: 6 additions & 0 deletions app/code/Magento/CatalogExport/etc/webapi.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,10 @@
<resource ref="Magento_Catalog::products" />
</resources>
</route>
<route url="/V1/catalog-export/variants" method="GET">
<service class="Magento\CatalogExportApi\Api\VariantRepositoryInterface" method="get"/>
<resources>
<resource ref="Magento_Catalog::products" />
</resources>
</route>
</routes>
49 changes: 49 additions & 0 deletions app/code/Magento/CatalogExportApi/Api/Data/VariantInterface.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\CatalogExportApi\Api\Data;

/**
* Product variant data-object interface.
*/
interface VariantInterface
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We agreed to do not use interfaces for DTO.
Also, you can generate DTO with this tool: https://github.com/magento/saas-export/pull/69/files#diff-cd518b16965245507c614234e1e945e1R1. (PR is still in progress due to related need to be fixed fist: https://github.com/magento/catalog-storefront/pull/159/files, but we can use it already)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will delete interface if we agreed to it. But don't see any need to use those tool in my case.

{
/**
* @return string
*/
public function getId();

/**
* @return string
*/
public function getParentId(): string;

/**
* @return string
*/
public function getProductId(): string;

/**
* @return float
*/
public function getDefaultQty(): float;

/**
* @return bool
*/
public function getQtyMutability(): bool;

/**
* @return int[]
*/
public function getOptionValueIds(): array;

/**
* @return float
*/
public function getPrice(): float;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\CatalogExportApi\Api;

use Magento\CatalogExportApi\Api\Data\VariantInterface;

/**
* Product variants repository interface.
*/
interface VariantRepositoryInterface
{
/**
* Get variants by ids.
*
* @param string[] $ids
* @return VariantInterface[]
* @throws \InvalidArgumentException
*/
public function get(array $ids): array;
}