Skip to content
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

#26499 Always transliterate product url key #26506

Conversation

DanieliMi
Copy link
Contributor

@DanieliMi DanieliMi commented Jan 23, 2020

Description (*)

\Magento\CatalogUrlRewrite\Model\ProductUrlPathGenerator::prepareProductUrlKey will always call \Magento\Catalog\Model\Product::formatUrlKey in order to transliterate it and avoid spaces, non latin characters, etc. in url_key.

Fixed Issues (if relevant)

  1. Fixes Product url key is not transliterated anymore if already set #26499 : Product url key is not transliterated anymore if already set

Manual testing scenarios (*)

  1. Edit a product and add "test url key" as url key
  2. Save the product
  3. Now the url key will be "test-url-key" as it behaved prior to 2.3.3 and 2.2.9

This can be reproduced via rest API too by updating a product url key.

Questions or comments

It might conflict with the issue that has been fixed in 6f4f511. If the tests fail we'll have to think of a different solution.

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
Copy link

m2-assistant bot commented Jan 23, 2020

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

@rogyar rogyar self-assigned this Jan 24, 2020
@rogyar
Copy link
Contributor

rogyar commented Jan 24, 2020

@magento run all tests

@rogyar rogyar added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jan 24, 2020
Copy link
Contributor

@rogyar rogyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @DanieliMi.

Thank you for your collaboration. I would ask you to take a look at the failing functional tests. According to the screenshot, it looks like the system tries to open a testable product using the wrong URL after changes introduced in the current PR.

@DanieliMi
Copy link
Contributor Author

Exactly it tries to visit the product page based on the product name with spaces in it. This PR aims to replace the spaces with '_'. I'll have to update the according tests.

@engcom-Echo
Copy link
Contributor

@magento run all tests

@DanieliMi
Copy link
Contributor Author

@rogyar I've had a look at the functional tests and fixed them. Currently static tests and database tests are failing.

The static tests fail due to a missing class description, I am not sure what it best could be since I only changed a line in the class do you have any ideas?

I am not sure how the database compare tests work and how to fix them could you assist me with them?

@engcom-Echo engcom-Echo self-assigned this Mar 4, 2020
@engcom-Echo
Copy link
Contributor

I will try fix it.

@engcom-Echo
Copy link
Contributor

@magento run all tests

@engcom-Echo engcom-Echo force-pushed the always_transliterate_product_url_key branch from bdc8030 to 27dd58a Compare March 6, 2020 14:04
@engcom-Echo engcom-Echo requested a review from rogyar March 6, 2020 15:14
@magento-engcom-team
Copy link
Contributor

Hi @rogyar, thank you for the review.
ENGCOM-7064 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@DanieliMi thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@m2-assistant
Copy link

m2-assistant bot commented Mar 15, 2020

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

Product url key is not transliterated anymore if already set
5 participants