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

Modify Link::getAdminLink in MultipleShop mode #9450

Merged
merged 1 commit into from Aug 22, 2018

Conversation

Projects
None yet
8 participants
@jolelievre
Contributor

jolelievre commented Aug 14, 2018

Questions Answers
Branch? develop
Description? modify Link::getAdminLink to keep consistency with request uri in MultipleShop mode, the base url was the one from the selected shop whereas we want to keep the same base url in all the back office links no matter what shop is selected
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-3607
How to test? Create multiple shops, check that the urls in the left column keep the one in your address bar, even when you select a specific shop, and even if you have a subfolder installation instead of a domain one Perform the same check on the domain/address of a another shop

This change is Reviewable

@jolelievre jolelievre added this to the 1.7.5.0 milestone Aug 14, 2018

Show outdated Hide outdated classes/Link.php
foreach ($result as $row) {
// A shop matching current URL was found
if (preg_match('#^'.preg_quote($row['uri'], '#').'#i', $request_uri)) {

This comment has been minimized.

@PierreRambaud

PierreRambaud Aug 14, 2018

Contributor

What kind of uri do you get in $row?
You only check if the request uri starts with $row['uri'], can be false if you have a request uri: test.com/product/beer/awesome and $row['uri']: test.com/project/beer

@PierreRambaud

PierreRambaud Aug 14, 2018

Contributor

What kind of uri do you get in $row?
You only check if the request uri starts with $row['uri'], can be false if you have a request uri: test.com/product/beer/awesome and $row['uri']: test.com/project/beer

This comment has been minimized.

@jolelievre

jolelievre Aug 16, 2018

Contributor

$row['uri'] contains the shop domain concatenated to the virtual part, so it will contain test.com/my_virtual_folder wich will be the base of all the urls of this shop context, thus you will have urls like test.com/my_virtual_folder/product/beer/awesome and test.com/my_virtual_folder/project/beer which will all match this uri
I the url does not match it means you are in another shop context

@jolelievre

jolelievre Aug 16, 2018

Contributor

$row['uri'] contains the shop domain concatenated to the virtual part, so it will contain test.com/my_virtual_folder wich will be the base of all the urls of this shop context, thus you will have urls like test.com/my_virtual_folder/product/beer/awesome and test.com/my_virtual_folder/project/beer which will all match this uri
I the url does not match it means you are in another shop context

Show outdated Hide outdated classes/Link.php
Show outdated Hide outdated classes/Link.php
@tomlev

tomlev approved these changes Aug 20, 2018

@marionf marionf self-assigned this Aug 21, 2018

@marionf

This comment has been minimized.

Show comment
Hide comment
@marionf

marionf Aug 21, 2018

Contributor

@jolelievre

I have created 2 groups
If I try to select a shop in the second group, the shop isn't selected

capture d ecran_184

Contributor

marionf commented Aug 21, 2018

@jolelievre

I have created 2 groups
If I try to select a shop in the second group, the shop isn't selected

capture d ecran_184

@marionf marionf removed their assignment Aug 21, 2018

@jolelievre

This comment has been minimized.

Show comment
Hide comment
@jolelievre

jolelievre Aug 21, 2018

Contributor

OK I'm gonna check this bug

Contributor

jolelievre commented Aug 21, 2018

OK I'm gonna check this bug

@marionf marionf self-assigned this Aug 22, 2018

@marionf marionf added QA ✔️ and removed waiting for QA labels Aug 22, 2018

@marionf marionf removed their assignment Aug 22, 2018

@mickaelandrieu mickaelandrieu merged commit e8bc902 into PrestaShop:develop Aug 22, 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
@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 22, 2018

Contributor

Thanks!

Contributor

mickaelandrieu commented Aug 22, 2018

Thanks!

@sroubekog

This comment has been minimized.

Show comment
Hide comment
@sroubekog

sroubekog Aug 23, 2018

Hello,
i just applied diffs to my 1.7.4.2 multistore Link.php.

I have primary shopA and others shops B, C, D. Now if i switch to shopB and go to modules the URL is OK still shopA. But when I try to change configuration of some module it will change URL to shopB. So I am afraid, problem is still there.

sroubekog commented Aug 23, 2018

Hello,
i just applied diffs to my 1.7.4.2 multistore Link.php.

I have primary shopA and others shops B, C, D. Now if i switch to shopB and go to modules the URL is OK still shopA. But when I try to change configuration of some module it will change URL to shopB. So I am afraid, problem is still there.

@mickaelandrieu

This comment has been minimized.

Show comment
Hide comment
@mickaelandrieu

mickaelandrieu Aug 23, 2018

Contributor

But when I try to change configuration of some module it will change URL to shopB. So I am afraid, problem is still there.

it's a slightly different issue: would you mind to create a new issue for that specific use case?

Contributor

mickaelandrieu commented Aug 23, 2018

But when I try to change configuration of some module it will change URL to shopB. So I am afraid, problem is still there.

it's a slightly different issue: would you mind to create a new issue for that specific use case?

@sroubekog

This comment has been minimized.

Show comment
Hide comment
@sroubekog

sroubekog Aug 23, 2018

it's a slightly different issue: would you mind to create a new issue for that specific use case?

Oki, its done: #10043

sroubekog commented Aug 23, 2018

it's a slightly different issue: would you mind to create a new issue for that specific use case?

Oki, its done: #10043

@jolelievre

This comment has been minimized.

Show comment
Hide comment
@jolelievre

jolelievre Aug 23, 2018

Contributor

Thanks @sroubekog I'll look into it as soon as I can ;)

Contributor

jolelievre commented Aug 23, 2018

Thanks @sroubekog I'll look into it as soon as I can ;)

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

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