Skip to content

Conversation

@atishgoswami
Copy link
Contributor

@atishgoswami atishgoswami commented May 4, 2019

Description

Recently Viewed contains wrong product url with category path

This PR will:

  • Generate the proper product url

Fixed Issues (if relevant)

#13227

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 on Travis CI are green)

@m2-assistant
Copy link

m2-assistant bot commented May 4, 2019

Hi @atishgoswami. 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-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented May 4, 2019

CLA assistant check
All committers have signed the CLA.

@atishgoswami
Copy link
Contributor Author

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

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

@magento-engcom-team
Copy link
Contributor

Hi @atishgoswami, here is your new Magento instance.
Admin access: https://pr-22650.instances.magento-community.engineering/admin
Login: admin Password: 123123q

@rogyar
Copy link
Contributor

rogyar commented May 5, 2019

That's a very good point. If the corresponding configuration is not enabled, the system should not take into account a category ID

@rogyar rogyar self-assigned this May 5, 2019
@atishgoswami
Copy link
Contributor Author

Seems like were couple unrelated test that seems to be broken.
Is there a way to re-trigger the travis build?

@atishgoswami atishgoswami requested a review from orlangur May 22, 2019 15:52
@rogyar
Copy link
Contributor

rogyar commented May 27, 2019

Hi @atishgoswami. Sorry for the delayed answer. I believe the failing integration tests are related to the current PR. The system expects URL to have a category in the path but gets direct URL to the project. We need to adjust the tests correspondingly.

Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

Nice! Your PR already includes my notes from https://github.com/magento/magento2/pull/22322/files#r275178147 :)

Please remove redundant parentheses, adopt existing integration tests and add a new one corresponding to described case.

After changes are applied and all builds are green, please squash them into a single commit so that we have perfectly clean history 😉

@ghost ghost assigned orlangur May 28, 2019
@atishgoswami
Copy link
Contributor Author

Sure will do @rogyar and @orlangur

@magento-engcom-team
Copy link
Contributor

Hi @sidolov, thank you for the review.
ENGCOM-5300 has been created to process this Pull Request
✳️ @sidolov, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@sidolov sidolov added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jun 17, 2019
@engcom-Delta engcom-Delta self-assigned this Jun 18, 2019
@engcom-Delta
Copy link
Contributor

✔️ QA Passed

@m2-assistant
Copy link

m2-assistant bot commented Jun 26, 2019

Hi @atishgoswami, 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 magento-engcom-team added this to the Release: 2.3.3 milestone Jun 26, 2019
magento-engcom-team pushed a commit that referenced this pull request Jun 26, 2019
@atishgoswami atishgoswami deleted the issue-13227 branch June 26, 2019 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants