#14061 Fix Configurable Product Type resource model 'getChildrenIds' method return format

[vasilii-b]

Description (*)

This PR fixes the return type of Magento\ConfigurableProduct\Model\ResourceModel\Product\Type\Configurable:getChildrenIds method.

Related Pull Requests

#15167 - Closed and inactive

Fixed Issues (if relevant)

1. Configurable Product Type resource model 'getChildrenIds' method incorrect return format. #14061: Configurable Product Type resource model 'getChildrenIds' method incorrect return format

Manual testing scenarios (*)

1. 1nject class \Magento\ConfigurableProduct\Model\ResourceModel\Product\Type
2. Apply the array of multiple configurable ids to method getChildrenIds
3. Expecting array to have such structure:

[
"parent id" => ["child id"]
]

Questions or comments

N/A

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @vasilii-b. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[dmytro-ch]

@magento run all tests

[eduard13]

@magento run Integration Tests

[dmytro-ch]

UPD
The EE and B2B Integration test fails due to the already existing select query which is not compatible with Content Staging.

[vasilii-b]

Hi @dmytro-ch,
Thanks for jumping in with the suggestions. The PR has been adjusted accordingly and now the Integration Tests has passed ✅
Could you please proceed with the review?
Thanks!

[dmytro-ch]

@vasilii-b thank you for the updates!

I've tested the current solution with Content Staging Updates and it works like a charm now. 👍
I also confirm this is the appropriate solution since in case of Commerce version the catalog_product_super_link.parent_id field is linked to row_id instead of entity_id.

Example of resulting query:

SELECT `l`.`product_id`, `p`.`entity_id` AS `parent_id` FROM `catalog_product_super_link` AS `l`
 INNER JOIN `catalog_product_entity` AS `p` ON p.row_id = l.parent_id AND (p.created_in <= '1585149600' AND p.updated_in > '1585149600')
 INNER JOIN `catalog_product_entity` AS `e` ON e.entity_id = l.product_id AND e.required_options = 0 AND (e.created_in <= '1585149600' AND e.updated_in > '1585149600') WHERE (p.entity_id IN ('273'));

@vasilii-b, could you please just squash all commits into a single commit in order to cleanup history?

Thank you!

[vasilii-b]

Hi @dmytro-ch,
Sure thing! I'll take care of that.

[vasilii-b]

@dmytro-ch the changes have been squashed ✅
Thank you!

[magento-engcom-team]

Hi @dmytro-ch, thank you for the review.
ENGCOM-7205 has been created to process this Pull Request

[vasilii-b]

Hi @engcom-Alfa, @dmytro-ch,
Can you please update on the current state here?
Thank you!

[engcom-Alfa]

✔️ QA Passed

Have configurable product where a parent with id=5 and 3 childs (2,3,4 ids)

Before:

After:

The first item of the array is kept for backward-compatibility.

[VladimirZaets]

Hi @vasilii-b, @dmytro-ch. Thanks for collaboration. I not sure that structure like
[ "parent id" => ["child id"]]
is correct in this case. The method name is getChildrenIds and it should return just an array of ids, like ['child id', 'child id', 'child id'], without nested structure and mentions parent id.

[vasilii-b]

Hi @VladimirZaets,
Thanks for jumping in!

Let me please bring some light here.
Please have a look over the method we're adjusting getChildrenIds:

magento2/app/code/Magento/ConfigurableProduct/Model/ResourceModel/Product/Type/Configurable.php

Lines 150 to 188 in e5ea17f

	/**
	* Retrieve Required children ids
	* Return grouped array, ex array(
	* group => array(ids)
	* )
	*
	* @param int|array $parentId
	* @param bool $required
	* @return array
	* @SuppressWarnings(PHPMD.UnusedFormalParameter)
	*/
	public function getChildrenIds($parentId, $required = true)
	{
	$select = $this->getConnection()->select()->from(
	['l' => $this->getMainTable()],
	['product_id', 'parent_id']
	)->join(
	['p' => $this->getTable('catalog_product_entity')],
	'p.' . $this->optionProvider->getProductEntityLinkField() . ' = l.parent_id',
	[]
	)->join(
	['e' => $this->getTable('catalog_product_entity')],
	'e.entity_id = l.product_id AND e.required_options = 0',
	[]
	)->where(
	'p.entity_id IN (?)',
	$parentId
	);
	$childrenIds = [
	0 => array_column(
	$this->getConnection()->fetchAll($select),
	'product_id',
	'product_id'
	)
	];
	return $childrenIds;
	}

It accepts as the first parameter $parentId, which can be a) int or b) array.

When you call the method getChildrenIds([4, 10, 99]) with an array of parent ids, you get a result like (e.g.)

[
   0 => [ 40, 5, 20, 23, 98 ]
];
// in other words
[
  0 => ['child id', 'child id', 'child id', 'child id']
]

Tell me please, how do you know which child id belongs to what parent id you have passed to the original method?

With the fix from this PR the result will be

[
   0 => [ 40, 5, 20, 23, 98 ], // for backward compatibility
   4 => [40, 5],
   10 => [20, 23]
   99 => [98]
];
// in other words
[
    0 => ['child id', 'child id', 'child id', 'child id'], // backward compatibility
    'parent id' => ['child id', 'child id', 'child id', 'child id']
]

Please let me know should there be any questions. Thank you!.

[VladimirZaets]

@vasilii-b yes, sorry I missed that we can pass an array of parent ids there, but I still have concerns due to this method. Return type array of arrays, a method that can receive as parameter int or array, required param that is unused, all of this looks weird. Maybe we should deprecate this method and create a new one. I think this PR is a good candidate for group code review

[dmytro-ch]

Hello @vasilii-b, unfortunately, I have to close this PR due to inactivity.
Please reopen and update if you wish to continue.
Thank you!

[m2-assistant]

Hi @vasilii-b, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.