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

Prices displayed without taxes when Set default country from browser language is enabled #34636

Closed
2 tasks done
ChillCode opened this issue Nov 21, 2023 · 15 comments
Closed
2 tasks done
Labels
8.1.x Branch Bug Type: Bug Customer groups Label: Which BO under menu is concerned FO Category: Front Office Invalid Resolution: issue closed because invalid Taxes and Prices Component: Which BO section is concerned

Comments

@ChillCode
Copy link

Prerequisites

Describe the bug and add attachments

When a user visits a site that have a group to display prices without taxes and have no cookies already set, PS will display prices without taxes even to visitors, but if the same page is reloaded (we have cookies) prices are shown as configured in the group.

Expected behavior

Show prices as configured.

Steps to reproduce

  1. Install PS 1.7.8.10 in French
  2. Add EN-US as a second language.
  3. Add a new customer Group called B2B and set to show prices without taxes.
  4. Go to International->Localization->Configuration and enable Set default country from browser language.
  5. Open a Browser configured with EN-US as the default language.
  6. Clean cookies or open a private session. [IMPORTANT]
  7. Visit a product and note that prices are shown without taxes to a visitor.

image

  1. Reload the page and prices are shown with taxes to the same visitor now PS is using cookies.

image

  1. Do the same with a Browser with FR and both times prices are shown with taxes.

PrestaShop version(s) where the bug happened

1.7.8.10

PHP version(s) where the bug happened

No response

If your bug is related to a module, specify its name and its version

No response

Your company or customer's name goes here (if applicable).

No response

@ChillCode ChillCode added Bug Type: Bug New New issue not yet processed by QA labels Nov 21, 2023
@florine2623
Copy link
Contributor

Hello @ChillCode ,

I've tested with 8.1.x, I can't reproduce your issue :

Screen.Recording.2023-11-21.at.14.50.49.mov

Am I missing something ?

@florine2623 florine2623 added Waiting for author Status: action required, waiting for author feedback FO Category: Front Office NMI Status: issue needs more information Customer groups Label: Which BO under menu is concerned Taxes and Prices Component: Which BO section is concerned 8.1.x Branch and removed New New issue not yet processed by QA labels Nov 21, 2023
@ChillCode
Copy link
Author

Hi @florine2623

Some time I'm bad at communication.

Just created a demo on prestashop.com, explore BO, added a B2B group without taxes and the rest is on the following video:

Hummingbird.printed.t-shirt.Mozilla.Firefox.2023-11-21.21-25-39.mp4

Thanks for testing.

@prestashop-issue-bot prestashop-issue-bot bot removed Waiting for author Status: action required, waiting for author feedback labels Nov 21, 2023
@ChillCode
Copy link
Author

ChillCode commented Nov 24, 2023

After some digging, the issue resides on FrontController and affects all versions.

if (!$has_currency && Validate::isLoadedObject($country) && $this->context->country->id !== $country->id) {
$this->context->country = $country;
$this->context->cookie->id_currency = (int) Currency::getCurrencyInstance($country->id_currency ? (int) $country->id_currency : Currency::getDefaultCurrencyId())->id;
$this->context->cookie->iso_code_country = strtoupper($country->iso_code);
}

!$has_currency is blocking to set the context country from the browser when we already have a cookie, since it's always set, if we remove that check the prices always display without taxes on a browser with US and with taxes on a browser with FR.

Not sure which is the expected behavior but don't think should be this, since customers coming from search engines are getting a different price when reload.

Just disabled Set default country from browser language as a temp solution.

@ChillCode
Copy link
Author

Now I thought to search for term has_currency and found this:

#27903

@florine2623
Copy link
Contributor

ping @PrestaShop/qa-functional can someone test this ? On my end, I can't reproduce it.

@florine2623 florine2623 added the Waiting for QA Status: action required, waiting for test feedback label Dec 7, 2023
@ChillCode
Copy link
Author

As we can see I did not login at any time, just removed cookies to show how a customer sees a product price when first visits a PrestaShop site with Set default country from browser enabled , and this issue is the culprit of many other issues related to addresses, taxes etc...

It's worth a watch.

@AureRita
Copy link
Contributor

AureRita commented Jan 4, 2024

Hi @ChillCode

Thank you for your report, Despite my effort, I can't reproduce your issue as you can see :

recording.38.webm
recording.39.webm

It seems that your issue is not a PrestaShop core bug but most likely a server configuration or customization problem.

Maybe your issue is more related to the FR taxes and not for the taxes of the other country.

Because this one 'll appear only with the cookie, as you said it. And we can't reproduce it. We will close this issue.

Thank you

@AureRita AureRita added Invalid Resolution: issue closed because invalid and removed Waiting for QA Status: action required, waiting for test feedback NMI Status: issue needs more information labels Jan 4, 2024
@AureRita AureRita closed this as completed Jan 4, 2024
@ChillCode
Copy link
Author

ChillCode commented Jan 4, 2024

Hi @AureRita

If you came to visit the product from an external link and you never visited the site before you will never have a cookie so you will see the prices without taxes until you refresh.

I just followed and even quoted the code where the issue resides... and the check @Progi1984 added while ago that is preventing to get the correct country, so I can tell you with all my short experience that there is an issue when Set default country from browser is enabled and if I follow all the steps to reproduce it I can, but the cookie must be deleted.

@metacreo
Copy link

metacreo commented Jan 11, 2024

@ChillCode hi
This is how it was intended, the function of determining the country by the browser header only works in the absence of context, once until the country appears. for example, you enter address1 and then cookies.

your bug is that when you exit, the group does not change? but after cookies clean all ok?
I reproduced this bug.

no B2B group is needed.

  1. Set all default groups to display tax incl, set PS default country FR, browser locale en-US.
  2. clean cookies and go directly to product page with configured price with FR taxes.
  3. You will see price without taxes.
  4. Refresh page and you will see other price with taxes.

the first time visitor always receives the price he receives based on the country in the context... who set by browser detection....
so... if detected en-US.. those taxes must be configured... or at all removed...
it all depends on the pricing policy in your country in relation to en-US 🙂

Do you think it’s worth removing the price dependence on browser locale determination?

@metacreo
Copy link

Screencast.from.11.01.2024.21.53.05.webm

@metacreo
Copy link

@ChillCode tryed you #27903 ?

@ChillCode
Copy link
Author

Hi @metacreo

I spend few hours with this and already got paid for it, so i understood how all this works, and my customer, a senior dev understood also why some people complain for viewing prices without taxes.

I know we don't need the group, found it later, but also wanted to include the same settings as the site I was working on so it was sure to reproduce.

I just wanted to show you where to look, and you found the changes between 1.7.8 and 8, I knew before my post you would find it, and for me that is ok, that site just set geolocation and that disabled the browser thingy.

Nice you found has_address_type thingy.

@ChillCode
Copy link
Author

@ChillCode hi This is how it was intended, the function of determining the country by the browser header only works in the absence of context, once until the country appears. for example, you enter address1 and then cookies.

your bug is that when you exit, the group does not change? but after cookies clean all ok? I reproduced this bug.

no B2B group is needed.

  1. Set all default groups to display tax incl, set PS default country FR, browser locale en-US.
  2. clean cookies and go directly to product page with configured price with FR taxes.
  3. You will see price without taxes.
  4. Refresh page and you will see other price with taxes.

the first time visitor always receives the price he receives based on the country in the context... who set by browser detection.... so... if detected en-US.. those taxes must be configured... or at all removed... it all depends on the pricing policy in your country in relation to en-US 🙂

Do you think it’s worth removing the price dependence on browser locale determination?

Also forgot to mention (was working with this a month ago) on your address issue that you will have different result if the cart is initialized or not, so if you don't have cookies, and go directly to your profile without initializing the cart you'll not have a different country...

As stated, it's worth a watch.

@metacreo
Copy link

metacreo commented Jan 12, 2024

@ChillCode hello, I think this issue #34636 and PR #27903 must be re-opened.
The code construction of PR #27903 is better and more adapted for browser detection function.

In #27903 only this part code confuses me : (int) Configuration::get('PS_CURRENCY_DEFAULT'))->id
Better to leave original : Currency::getDefaultCurrencyId())->id;
I don't think it's need mixing $has_address_type ($cart) with this issue.

Indeed, I agree that now it does not work logically and all because of the closure of normal PRs and the inability to repeat bugs. At the same moment, they accept the unfinished PR. All this violates the logic... This is nice work of the 🙄...

Bugs #34636 #34080 #33991 come from accepted unfinished #27187 which violated part of controllers logic. Now we are trying to finish it.

I tested #27903 This is a great addition to #27187

@metacreo
Copy link

Dears @florine2623 and @AureRita can you please re-open issue #34636 and PR #27903 ? Or we must create new?
Because mentioned #27187 not solve this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.1.x Branch Bug Type: Bug Customer groups Label: Which BO under menu is concerned FO Category: Front Office Invalid Resolution: issue closed because invalid Taxes and Prices Component: Which BO section is concerned
Projects
None yet
Development

No branches or pull requests

4 participants