-
Notifications
You must be signed in to change notification settings - Fork 22
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
Configurable variants saving to cache OLD #5
Conversation
-Module and changes to existing infrastructure to support product variants -Added configurable product variants -Added tests for product variants
Minor syntax fixes
-Changed entity type to string
-Test fixes
-Minor fixes
-Moved test from variants to configurable variants and other minor adjustments
@magento run all tests |
* @param string[] $ids | ||
* @return \Magento\CatalogExportApi\Api\Data\ProductVariant[] | ||
*/ | ||
public function get(array $ids): array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where this API is used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\Magento\CatalogMessageBroker\Model\FetchProductVariants::execute
The getByProductIds is currently not really used, however I think we will likely need it in future so I added it.
public function resolve(string ...$params): string | ||
{ | ||
array_unshift($params, ConfigurableOptionValueUid::OPTION_TYPE); | ||
return implode('/', $params); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
propose to change a little bit this approach to make is more safe, because in this case we do not know nothing about order of incoming parameters
// \Magento\ConfigurableProductDataExporter\Model\Provider\Product\ProductVariants::getVariants
$id = $idResolver->resolve(\...(['parentId' => $row['parentId'], 'childId' => $row['childId']));
....
in this case we can control data:
$params = [
'parentId' => $row['parentId'],
'childId' => $row['childId']
];
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
use Magento\DataExporter\Exception\UnableRetrieveData; | ||
use Magento\Framework\App\ResourceConnection; | ||
use Psr\Log\LoggerInterface; | ||
|
||
/** | ||
* Configurable product variant data provider | ||
* TODO: Deprecated. Remove this class and its query class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add id of the issue here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
-Comment fix -Made product variant id provider params more defined
…ta-provider MC-29580: Complex product data provider
SF API test for categories and product dynamic attributes
-Removed indexation reliance on product index -Changed mview to product_relations
-Fixed indexing issues
…ommerce-data-export-3 [Imported] Category Breadcrumb sort order
MDEE-46: Prepare queries to get stock item status data from Magento
-Module and changes to existing infrastructure to support product variants
-Added configurable product variants
-Added tests for product variants
Description (*)
Related Pull Requests
Fixed Issues (if relevant)
Questions or comments
Code Review Checklist (*)