MC-40780: Moving category in hierarchy causes url_path to be incorrect when using different url keys in multiple storeviews #32598

viktorpetryk · 2021-03-25T14:37:22Z

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Fixes Moving category in hierarchy causes url_path to be incorrect when using different url keys in multiple storeviews #16202

Manual testing scenarios (*)

See #16202

Questions or comments

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 · 2021-03-25T14:37:24Z

Hi @victorpetryk. 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:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, 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, please 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

viktorpetryk · 2021-03-26T16:38:22Z

@magento run all tests

viktorpetryk · 2021-03-28T10:40:54Z

@magento run Integration Tests

viktorpetryk · 2021-03-28T12:38:33Z

@magento run Integration Tests

viktorpetryk · 2021-03-28T19:44:37Z

@magento run all tests

viktorpetryk · 2021-03-29T08:47:27Z

@magento run all tests

viktorpetryk · 2021-03-29T11:34:34Z

@magento run all tests

engcom-Kilo · 2021-03-29T12:05:02Z

app/code/Magento/CatalogUrlRewrite/Model/ResourceModel/Category/GetDefaultUrlKey.php

+            ->columns(['c.value'])
+            ->where(implode(' AND ', $whereConditions));
+
+        return current($connection->fetchCol($select));


Why not using fetchOne()?

Suggested change

return current($connection->fetchCol($select));

return $connection->fetchOne($select) ?: null;

engcom-Kilo · 2021-03-29T12:06:14Z

app/code/Magento/CatalogUrlRewrite/Model/ResourceModel/Category/GetDefaultUrlKey.php

+     * Retrieve 'url_key' value for default store.
+     *
+     * @param int $categoryId
+     * @return string|bool


I would be better to return string|null

Then we always need add check for a null, which is not good, i think it will be better return always string as per strict type, and in case of empty result empty string. What do you think ?

engcom-Kilo · 2021-03-29T12:07:02Z

app/code/Magento/CatalogUrlRewrite/Observer/CategoryUrlPathAutogeneratorObserver.php

@@ -90,6 +100,12 @@ public function execute(\Magento\Framework\Event\Observer $observer)
                $resultUrlKey = $category->formatUrlKey($category->getOrigData('name'));
                $this->updateUrlKey($category, $resultUrlKey);
            }
+            if ($category->hasChildren()) {
+                $defaultUrlKey = $this->getDefaultUrlKey->execute((int)$category->getId());
+                if (!empty($defaultUrlKey)) {


Could be simplified to:

Suggested change

if (!empty($defaultUrlKey)) {

if ($defaultUrlKey) {

…t when using different url keys in multiple storeviews

viktorpetryk · 2021-03-29T12:53:40Z

@magento run all tests

zakdma · 2021-03-30T11:08:28Z

@magento import code to https://github.com/magento-tsg/magento2ce

magento-engcom-team · 2021-03-30T11:10:51Z

@zakdma the branch with code successfully imported intomagento-tsg/magento2ce repository. Branch name: imported-magento-magento2-32598.

m2-assistant · 2021-03-31T15:45:47Z

Hi @victorpetryk, 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.

magento-engcom-team added the Component: CatalogUrlRewrite label Mar 25, 2021

m2-community-project bot added Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Mar 25, 2021

magento-engcom-team added the Release Line: 2.4 label Mar 25, 2021

viktorpetryk force-pushed the MC-40780 branch 2 times, most recently from 01bf963 to 6793d5f Compare March 26, 2021 16:37

viktorpetryk force-pushed the MC-40780 branch from 6793d5f to 2f39fc4 Compare March 28, 2021 10:38

m2-community-project bot added the Progress: pending review label Mar 29, 2021

engcom-Kilo reviewed Mar 29, 2021

View reviewed changes

m2-community-project bot assigned engcom-Kilo Mar 29, 2021

m2-community-project bot added Progress: review and removed Progress: pending review labels Mar 29, 2021

MC-40780: Moving category in hierarchy causes url_path to be incorrec…

b92d437

…t when using different url keys in multiple storeviews

viktorpetryk force-pushed the MC-40780 branch from dccfad0 to b92d437 Compare March 29, 2021 12:53

m2-community-project bot assigned Nazar65 Mar 29, 2021

hostep mentioned this pull request Mar 30, 2021

Moving category in hierarchy causes url_path to be incorrect when using different url keys in multiple storeviews #16202

Closed

magento-engcom-team merged commit 8fc0d36 into magento:2.4-develop Mar 31, 2021

	return current($connection->fetchCol($select));
	return $connection->fetchOne($select) ?: null;

MC-40780: Moving category in hierarchy causes url_path to be incorrect when using different url keys in multiple storeviews #32598

MC-40780: Moving category in hierarchy causes url_path to be incorrect when using different url keys in multiple storeviews #32598

Uh oh!

Conversation

viktorpetryk commented Mar 25, 2021

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Uh oh!

m2-assistant bot commented Mar 25, 2021

Uh oh!

viktorpetryk commented Mar 26, 2021

Uh oh!

viktorpetryk commented Mar 28, 2021

Uh oh!

viktorpetryk commented Mar 28, 2021

Uh oh!

viktorpetryk commented Mar 28, 2021

Uh oh!

viktorpetryk commented Mar 29, 2021

Uh oh!

viktorpetryk commented Mar 29, 2021

Uh oh!

engcom-Kilo Mar 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viktorpetryk Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

engcom-Kilo Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

viktorpetryk Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

Nazar65 Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

engcom-Kilo Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

viktorpetryk Mar 29, 2021

Choose a reason for hiding this comment

Uh oh!

viktorpetryk commented Mar 29, 2021

Uh oh!

zakdma commented Mar 30, 2021

Uh oh!

magento-engcom-team commented Mar 30, 2021

Uh oh!

m2-assistant bot commented Mar 31, 2021

Uh oh!

Uh oh!

engcom-Kilo Mar 29, 2021 •

edited

Loading