Delete existing URL rewrite IDs in After Replace

[amenk]

It is possible that the entries in the catalog_url_rewrite_product_category table already exist. So as we are in a replace interceptor, we have to take care of this and not just insert additional entries.

Description (*)

We saw errors like

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '51541850' for key 'PRIMARY', query was:
INSERT  INTO `catalog_url_rewrite_product_category` 
(`url_rewrite_id`,`category_id`,`product_id`) VALUES (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?), (?, ?, ?)

after saving categories. This pull request fixes the issue.

I do not have a single clue if this reproducible in a stock Magento installation.

Manual testing scenarios (*)

• not sure if reproducible

Questions or comments

Fixed issues

• Fixes [Issue] Ensure that previously existing records in catalog_url_rewrite_produc… #34210

Please tell me if that makes sense

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)
• README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @amenk. Thank you for your contribution
Here are 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

❗ Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s)
  For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE,
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here

ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[amenk]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[amenk]

@magento give me 2.4.3 instance

[magento-deployment-service]

Hi @amenk. Thank you for your request. I'm working on Magento instance for you.

[magento-deployment-service]

Hi @amenk, here is your Magento Instance: https://9345e2348ec7151e91a385b1e42b2423-2-4-3.instances.magento-community.engineering
Admin access: https://9345e2348ec7151e91a385b1e42b2423-2-4-3.instances.magento-community.engineering/admin_62e4
Login: 580044d6
Password: a574c79528d0

[amenk]

related: #33770

[amenk]

The functional tests seem to fail because of other reasons, I cannot imagine those errors are related to my PR. @ihor-sviziev can you review this as you were involved in the releated issue?

[ihor-sviziev]

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

Hi @amenk,
Could you provide the steps to reproduce this issue?

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[amenk]

@ihor-sviziev

Thank you for your feedback.

I was not able to reproduce this on a clean Magento installation, it seems a complex case.
But as you see in #33770, many are facing this issue.

From a logical point of view it makes sense to me - when hooking in the afterReplace - to actually replace items and not just insert them. That's why I inserted the delete before the insert.

[ihor-sviziev]

@amenk, I wonder how we can check that this change will fix the issue if we can't reproduce it.

[amenk]

@ihor-sviziev Yes, very good point.
Anyways, on my local customer's instance I can reproduce it (and verified the solution) - but I was not able to create a minimal example. Would it make sense to share the database dump of the product catalog privately?

[ihor-sviziev]

@amenk, sorry, I don't have enough time to test it now. Maybe you could share with someone who will be checking this PR?

[ihor-sviziev]

@magento run Functional Tests EE, Functional Tests CE

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[engcom-Delta]

✔️ QA Passed

Preconditions:

1. We should have fresh installation of Magento 2.4 Develop.

Manual testing scenario:

1. Create che website with de_ch, en_ch, es_ch, fr_ch, zh_ch store views
2. Create exp website with de_ex, en_ex, es_ex, fr_ex, zh_ex store views
3. Create the Products category inside default catagory
4. Go on the admin panel under System > Import
5. Choose Products entity type with Add/Update behaviour and let default settings for the rest
6. Upload provided csv file and launch the import
7. Validate file import successful

Before: ✖️ We had been getting exception while importing.

After: ✔️ flies getting imported successfully.

[ihor-sviziev]

@magento run all tests

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-9380 has been created to process this Pull Request

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[magento-automated-testing]

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

[charlescurrier]

Any movement on this? This issue is affecting every developer on our team, and the PR appears to have been sitting stagnant for over a month.

[burgh8wp]

@amenk the patch created doesn't seem to address the lack of auto increment on this catalog_url_rewrite_product_category table?

So for example in our setup for this client, it should be auto incrementing from around 118669 as the next new ID to go in, starting off from previous of 118668, but the Magento system is trying to add various that start 117xxx when a product is saved in a new category, which suggests it has wrong stamp on where to start the ID? As far as I can see, the patch doesn't address any of this?

[amenk]

@burgh8wp nothing was changed with the auto increments, yes. Probably a different issue?

[burgh8wp]

@amenk unsure, but just keen to understand why auto increment is not on url_rewrite_id for this table? Would that not be a simple solution that fully resolves and prevents duplicates occurring where it has a primary key there?