Skip to content

Fix category position migrator visibility join for CE/Mage-OS#3837

Closed
mrrobot-magedev wants to merge 7 commits intoSmile-SA:2.11.xfrom
mrrobot-magedev:fix/category-position-migrator-visibility-join
Closed

Fix category position migrator visibility join for CE/Mage-OS#3837
mrrobot-magedev wants to merge 7 commits intoSmile-SA:2.11.xfrom
mrrobot-magedev:fix/category-position-migrator-visibility-join

Conversation

@mrrobot-magedev
Copy link
Copy Markdown
Contributor

Description

This PR fixes the visibility join in CategoryPositionMigrator.

The current query joins catalog_product_entity_int using:

cpei.row_id = ccp.product_id

However, in Magento Open Source / Mage-OS CE, product EAV backend tables use entity_id, not row_id.

This causes the category position migration command to fail on CE/Mage-OS installations because the column catalog_product_entity_int.row_id does not exist.


Fix

Replaced:

cpei.row_id = ccp.product_id

with:

cpei.entity_id = ccp.product_id

Steps to reproduce

  1. Install ElasticSuite 2.11.x on Magento Open Source / Mage-OS CE.
  2. Run the category position migration CLI command.
  3. Observe SQL error due to missing row_id column in catalog_product_entity_int.

Expected result

Migration runs successfully and product visibility is correctly joined.


Type of change

  • Bug fix

Notes

This change ensures compatibility with Magento Open Source / Mage-OS CE environments.

@romainruaud
Copy link
Copy Markdown
Collaborator

Hi, thanks for this PR but we should rather write something that would support both cases, as we do in our indexers, eg, here : https://github.com/Smile-SA/elasticsuite/blob/2.11.x/src/module-elasticsuite-catalog/Model/ResourceModel/Product/Indexer/Fulltext/Datasource/AttributeData.php#L211

@mrrobot-magedev
Copy link
Copy Markdown
Contributor Author

Thanks for the review.

You're right — hardcoding entity_id only fixes Magento Open Source / Mage-OS CE and breaks compatibility with Adobe Commerce where row_id is used.

I've updated the implementation to follow the same approach as in the indexers by resolving the product link field dynamically using MetadataPool.

This ensures compatibility for both:

  • Magento Open Source / Mage-OS CE (entity_id)
  • Adobe Commerce (row_id)

Please let me know if anything else should be improved.

@romainruaud
Copy link
Copy Markdown
Collaborator

Looks good to me, you can add a PHPMD ignore for the coupling so that the Code Quality check will be OK, but other than that, we'll have someone in the team to test it on both Magento Open Source and Adobe Commerce, and we should be ok.

Thanks for submitting the issue but more than that, for contributing a fix, we always appreciate !

@vahonc
Copy link
Copy Markdown
Collaborator

vahonc commented Apr 27, 2026

@romainruaud, I've tested this fix on both OS and EE versions of Magento.

m247p8ce:

Details
$ php bin/magento elasticsuite:category-position:migrate --category 6
Transfer negative positions to positive and contiguous ones (y/n) y
Transfer zero positions? (y/n) y

WARNING: This may migrate a large number of unused positions, significantly slowing down category pages in the BO and potentially impacting boost application.
Enabling this option is generally not recommended.

Are you absolutely sure you want to continue? (y/n) y
Transfer positive positions? (y/n) y
What to do with products sharing the same position?
[0] reorder
[1] delete

0
Processing category ID: 6
Preview mode: showing up to 100 rows.
+-------------+------------+--------------+--------------+
| Category ID | Product ID | Old Position | New Position |
+-------------+------------+--------------+--------------+
| 6 | 36 | -2 | 1 |
| 6 | 37 | -1 | 2 |
| 6 | 38 | 0 | 3 |
| 6 | 39 | 1 | 4 |
| 6 | 40 | 2 | 5 |
| 6 | 41 | 3 | 6 |
| 6 | 42 | 4 | 7 |
| 6 | 43 | 5 | 8 |
| 6 | 44 | 6 | 9 |
+-------------+------------+--------------+--------------+
Migration finished. Total categories: 1 | Migrated: 9

m248p3ee:

Details
$ php bin/magento elasticsuite:category-position:migrate --category 6
Transfer negative positions to positive and contiguous ones (y/n) n
Transfer zero positions? (y/n) y

WARNING: This may migrate a large number of unused positions, significantly slowing down category pages in the BO and potentially impacting boost application.
Enabling this option is generally not recommended.

Are you absolutely sure you want to continue? (y/n) y
Transfer positive positions? (y/n) y
What to do with products sharing the same position?
[0] reorder
[1] delete

0
Processing category ID: 6
Preview mode: showing up to 100 rows.
+-------------+------------+--------------+--------------+
| Category ID | Product ID | Old Position | New Position |
+-------------+------------+--------------+--------------+
| 6 | 36 | 0 | 1 |
| 6 | 37 | 0 | 2 |
| 6 | 38 | 0 | 3 |
| 6 | 39 | 0 | 4 |
| 6 | 40 | 0 | 5 |
| 6 | 41 | 0 | 6 |
| 6 | 42 | 0 | 7 |
| 6 | 43 | 0 | 8 |
| 6 | 44 | 0 | 9 |
+-------------+------------+--------------+--------------+
Migration finished. Total categories: 1 | Migrated: 9

Therefore, it looks good to me too.

@mrrobot-magedev, thanks for spotting that!

BR,
Vadym

@rbayet
Copy link
Copy Markdown
Collaborator

rbayet commented Apr 27, 2026

Hello @mrrobot-magedev,

Would you be so kind to squash all your commits into the first one using a git rebase ?

Regards

@rbayet
Copy link
Copy Markdown
Collaborator

rbayet commented Apr 28, 2026

Hello @mrrobot-magedev,

Would you be so kind to squash all your commits into the first one using a git rebase ?

Regards

OK, nevermind, I saw that the PR was based on top of branch 2.11.x so I created a version on top of branch 2.10.x (#3838) and squashed the commits there.

#3838 will replace this one.

Regards,

@rbayet rbayet closed this Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants