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.

Sign up for GitHub

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

Jump to bottom

Use product url (including combination) rather than canonical one in the miniatures #15964

Merged
merged 1 commit into from
Oct 16, 2019
Merged

Use product url (including combination) rather than canonical one in the miniatures #15964

merged 1 commit into from
Oct 16, 2019

Conversation

jolelievre
Copy link
Contributor

@jolelievre jolelievre commented Oct 15, 2019 •
edited by eternoendless
Loading

Questions Answers
Branch? 1.7.6.x
Description? A modification was previously integrated in 1.7.6 which used the product canonical url in the miniatures templates This was used to boost SEO and highlight the canonical url but it caused UX issues The url in list could not target a precise combination So we revert back to the original behaviour and use the product url Besides this doesn't affect the SEO since any way all combination urls have the same canonical
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #14765
How to test? Go to a product list and perform a search by criteria, look at the url of a product with combination The url of the miniature must include the combination id (something like /fr/hommes/1-1-hummingbird-printed-t-shirt.html#/1-size-s/8-color-white)

This change is Reviewable

@jolelievre jolelievre added this to the 1.7.6.2 milestone Oct 15, 2019
@jolelievre jolelievre requested a review from a team as a code owner October 15, 2019 14:44
@prestonBot prestonBot added 1.7.6.x Branch Bug Type: Bug labels Oct 15, 2019
matthieu-rolland
matthieu-rolland previously approved these changes Oct 15, 2019
View reviewed changes
Copy link
Contributor

@matthieu-rolland matthieu-rolland left a comment

Choose a reason for hiding this comment

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

lgtm, thanx @jolelievre !

@jolelievre jolelievre added the Waiting for QA Status: action required, waiting for test feedback label Oct 15, 2019
prestamodule
prestamodule previously approved these changes Oct 15, 2019
View reviewed changes
@Robin-Fischer-PS Robin-Fischer-PS self-assigned this Oct 16, 2019
@Robin-Fischer-PS Robin-Fischer-PS added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Oct 16, 2019
@jolelievre jolelievre closed this Oct 16, 2019
@jolelievre jolelievre reopened this Oct 16, 2019
@matks
Copy link
Contributor

matks commented Oct 16, 2019

Thank you @jolelievre you are the MVP !

@matks matks merged commit 1298644 into PrestaShop:1.7.6.x Oct 16, 2019
@matks matks mentioned this pull request Oct 16, 2019
Closed
@matks matks mentioned this pull request Nov 28, 2019
Merged
@Amazzing
Copy link
Contributor

@jolelievre, this is a useful commit.
But same commit was already merged in develop branch: #15013
Wasn't it supposed to be automatically merged with latest version PS 1.7.6.2 ?

@khouloudbelguith khouloudbelguith mentioned this pull request Jan 2, 2020
Closed
@jolelievre jolelievre deleted the product-url branch February 2, 2021 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matks matks matks approved these changes

@prestamodule prestamodule prestamodule left review comments

@matthieu-rolland matthieu-rolland matthieu-rolland left review comments

Assignees

@Robin-Fischer-PS Robin-Fischer-PS

Labels
1.7.6.x Branch Bug Type: Bug QA ✔️ Status: check done, code approved
Projects
None yet
Milestone
1.7.6.2
Development

Successfully merging this pull request may close these issues.
7 participants
@jolelievre @matks @Amazzing @prestamodule @matthieu-rolland @prestonBot @Robin-Fischer-PS