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

Change the default redirection behaviour to product category #9463

Merged
merged 2 commits into from Aug 24, 2018

Conversation

Projects
None yet
5 participants
@jolelievre
Contributor

jolelievre commented Aug 21, 2018

Questions Answers
Branch? develop
Description? Change the default redirection behaviour to product category in product admin page, update product controller as well so that you can leave the selected category blank, in which case the page is redirected to the main category This PR also contains the introduction of const in ProductInterface to manage redirection
Type? improvement
Category? FO / BO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-6249
How to test? Create a new product and set its main category Go to the SEO tab, check that the default value for redirection is 301 to category, save without setting a category and disable the product Go to the front product page and check that you are redirected to the main category page

This change is Reviewable

@PierreRambaud PierreRambaud added this to the 1.7.5.0 milestone Aug 22, 2018

@marionf marionf self-assigned this Aug 22, 2018

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Aug 22, 2018

Contributor

@jolelievre

If I choose a category, I am still redirected to the main category instead of the selected one
If I re-enable the product I am still redirected to the main category instead of the product page

https://drive.google.com/open?id=1yt3W9JweARuuVPDLt0nnbOTshOdj1Rk9

Contributor

marionf commented Aug 22, 2018

@jolelievre

If I choose a category, I am still redirected to the main category instead of the selected one
If I re-enable the product I am still redirected to the main category instead of the product page

https://drive.google.com/open?id=1yt3W9JweARuuVPDLt0nnbOTshOdj1Rk9

@marionf marionf removed their assignment Aug 22, 2018

@jolelievre

This comment has been minimized.

Show comment
Hide comment
@jolelievre

jolelievre Aug 22, 2018

Contributor

@marionf ok weird I am gonna retest it, maybe the refacto I made at the end broke something

Contributor

jolelievre commented Aug 22, 2018

@marionf ok weird I am gonna retest it, maybe the refacto I made at the end broke something

@jolelievre

This comment has been minimized.

Show comment
Hide comment
@jolelievre

jolelievre Aug 22, 2018

Contributor

hello @marionf I performed a few tests for this bug which did not occur at all times on my computer
I finally understood why, Chrome (and Firefox) caches the 301 redirections, since it is a permanent redirection it never tries tu update it and keep redirecting to the first url it received
To test this modification you can either clear you browser cache, or use the development tools panel and check the 'Disable cache' option

Contributor

jolelievre commented Aug 22, 2018

hello @marionf I performed a few tests for this bug which did not occur at all times on my computer
I finally understood why, Chrome (and Firefox) caches the 301 redirections, since it is a permanent redirection it never tries tu update it and keep redirecting to the first url it received
To test this modification you can either clear you browser cache, or use the development tools panel and check the 'Disable cache' option

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 22, 2018

Contributor

@jolelievre did you rebase from the develop branch? We need to ensure we don't introduce new coding styles issues.

Contributor

mickaelandrieu commented Aug 22, 2018

@jolelievre did you rebase from the develop branch? We need to ensure we don't introduce new coding styles issues.

@jolelievre

This comment has been minimized.

Show comment
Hide comment
@jolelievre

jolelievre Aug 22, 2018

Contributor

@mickaelandrieu yep! I also updated my other PR when I had conflicts (should I update all of them even when there no conflicts?)

Contributor

jolelievre commented Aug 22, 2018

@mickaelandrieu yep! I also updated my other PR when I had conflicts (should I update all of them even when there no conflicts?)

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 22, 2018

Contributor

should I update all of them even when there no conflicts?

Sadly, yes: but it will be less and less painful ^^

Contributor

mickaelandrieu commented Aug 22, 2018

should I update all of them even when there no conflicts?

Sadly, yes: but it will be less and less painful ^^

@PierreRambaud

This comment has been minimized.

Show comment
Hide comment
@PierreRambaud

PierreRambaud Aug 24, 2018

Contributor

@mickaelandrieu Merge or waiting for something?

Contributor

PierreRambaud commented Aug 24, 2018

@mickaelandrieu Merge or waiting for something?

@mickaelandrieu mickaelandrieu merged commit 3133f88 into PrestaShop:develop Aug 24, 2018

2 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jolelievre jolelievre deleted the jolelievre:BOOM-6249 branch Sep 6, 2018

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