Skip to content

Fix use_default not removing store-specific value of url_key and url_path for Categories #27291

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

Conversation

lbajsarowicz
Copy link
Contributor

@lbajsarowicz lbajsarowicz commented Mar 14, 2020

Description (*)

This PR is my 2nd approach to the issues with Categories and support of Scopes (Different value per store_id). Firstly I focus on the issues related to url_key and url_path.

All the reported bugs appear when performing actions from API, some of them also appear when you perform actions manually in Admin Panel.

Work done

  1. Added Integration test that covers issues
  2. Moved Observer functionality to beforeSave plugin, extracting UrlPathUpdate to separate Service class
  3. Deep refactoring of CategoryRepository

Related Pull Requests

Fixed Issues (if relevant)

  1. magento/magento2# TBD

Manual testing scenarios (*)

✔️ Bug 1: Copy value instead of removing (API)

  1. Create Category A
  2. Update url_key for Store X to category-z
  3. Reset url_key to use_default for Store X
    (At this point the result is correct: url_key returns category-a)
  4. Update url_key for Global Scope to category-n

Expected: url_key returned for Category A in Store X should be category-n
Actual: url_key for Category A in Store X is category-a

✔️ Bug 2: Override global's scope url_path

Note: When you update url_key, Magento should take care of all child categories in all affected scopes to update url_path automatically according to your configuration.

  1. Create Category A
  2. Create Subcategory B under Category A
  3. Update url_key for Store X to category-z

Expected: url_path returned for Subcategory B in Global scope should be category-a/subcategory-b, but for Store X this should be category-z/subcategory-b
Actual: url_path returned for Subcategory B in Global scope is category-z/subcategory-b

The value is saved in Global scope in Database.

❌ Bug 3:

  1. Create Category A
  2. Create Subcategory B under Category A
  3. Update url_key for Store X to category-z
  4. Reset url_key to use_default for Store X

Expected: url_path returned for Subcategory B in Global scope should be category-a/subcategory-b
Actual: url_path returned for Subcategory B in Global scope is category-z/subcategory-b

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)

1. Added Integration test that covers change
2. Moved Observer functionality to beforeSave plugin
@m2-assistant
Copy link

m2-assistant bot commented Mar 14, 2020

Hi @lbajsarowicz. 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.

@lbajsarowicz lbajsarowicz changed the title WIP: Fix bug with use_default not removing store-specific value WIP: Fix use_default not removing store-specific value of url_key and url_path for Categories Mar 15, 2020
@lbajsarowicz
Copy link
Contributor Author

According to Integration Tests there are still 2 steps to be done:

  1. Fix 1st bug from the PR description
  2. Find out why triggering 2nd call to the same endpoint returns null

@lbajsarowicz
Copy link
Contributor Author

I'm reducing the scope of the PR once again.

@lbajsarowicz lbajsarowicz force-pushed the bugfix/category-use-default branch from 023b473 to 3d3e19c Compare March 21, 2020 20:51
@lbajsarowicz lbajsarowicz changed the title WIP: Fix use_default not removing store-specific value of url_key and url_path for Categories Fix use_default not removing store-specific value of url_key and url_path for Categories Mar 21, 2020
@lbajsarowicz
Copy link
Contributor Author

I'm really sorry to disappoint you, as due to limited time, I was unable to accomplish that bugfix before 2.4 feature freeze. I'm going to fix this before 2.5

@m2-assistant
Copy link

m2-assistant bot commented Apr 18, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants