search

Smile-SA / magento2-module-store-locator

Magento 2 store locator module
50 stars 57 forks source link

Explicitly set YYYY-MM-dd format for Zend_Date constructor #147

Open fnogatz opened 1 year ago

fnogatz commented 1 year ago

In the Adminhtml's Renderer.php, the call of $this->date->date()->format('Y-m-d') correctly returns the string 2023-02-06 for today, February 6th of 2023. However, in installations with a different locale (like de_DE), this is wrongly interpreted as June 2nd of 2023 by the Zend_Date() constructor. For every date that might possibly be parsed both ways, this results in the problems described in #91 and #124: the set opening hours are incorrectly displayed at the very right of the range bar. If you look closely, you will notice that this problem was reported to be "magically gone" only after the 12th day of a month has passed, since this takes away the ambiguity in the date parsing process.

This PR explicitly states the date format of the given string, so it is parsed correctly independently on the set locale. It is more straightforward than my original proposal from three years ago in https://github.com/Smile-SA/magento2-module-store-locator/issues/91#issue-578937698.

pravaliterabricoman commented 1 year ago

Hello ! I discovered this bug yesterday, and was about to send an mail about it :wink: What a coincidence !

From my part, i analyzed that it came from the locale sent in the HTTP headers Accept-Language, by the browser.

If no format is given to Zend_Date, it will try to guess it, and it does it through the priority given in Accept-Language: everything was fine when fr_FR was firt, but if it was not in, or after en_US, i had the same bugs !

It's a very dangerous bug, because if you edit the retailer for another thing than the opening hours, and you don't notice they are not ok, you could possibly corrupt the data on save ! And if you have a shipping method using those hours... :cry:

Thx a lot for this fix !

pravaliterabricoman commented 1 year ago

@fnogatz Hello ! Any news on when it would be merged and deployed ?

Thx :smile:

fnogatz commented 1 year ago

No idea when the Smile folks will look into this, sorry :innocent:

In the meantime, you could apply this patch via composer patches.

fnogatz commented 1 year ago

Maybe @PrigentMatthieu or @Coosos can help out getting this merged?

Coosos commented 1 year ago

I sent a message to my colleague :)

fnogatz commented 1 year ago

Thank you, @Coosos. Did you get any response?