[4.0 - PHP8.1] Remove call to deprecated strftime - with unit test#36469
[4.0 - PHP8.1] Remove call to deprecated strftime - with unit test#36469PhilETaylor wants to merge 10 commits intojoomla:4.1-devfrom PhilETaylor:strftime
Conversation
|
We had a proposal to do an automatic conversion of the strftime to Date format in the past. It didn't work well. |
This comment was marked as abuse.
This comment was marked as abuse.
Famous last words... FWIW if @Bakual is ok I'm more than happy with the changes |
This comment was marked as abuse.
This comment was marked as abuse.
|
@PhilETaylor only when php dates support all non gregorian calendars |
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
As I wrote in the other issue 2022 will be the year that all browsers will roll |
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
|
Current PR is okay as temporary solution.
@brianteeman it can, try this: $formatter = new \IntlDateFormatter('fa_IR', \IntlDateFormatter::LONG, \IntlDateFormatter::NONE);
echo $formatter->format(time());It just not easy to integrate to the api we have. It should be something new (kind of calendar2 field). For proper date format we need to think about PHP Intl date formater for server side and "temporal js" for client side. And with it we should drop the translations for patterns in the inputs, it a horror. |
YES, I totally agree here. A (probably radical) idea: PHP should only (by default) deal ONLY with the SQL format (also the JS part should also return SQL formatted datetime string). That would simplify immensely the code. A quick explanation: the PHP should NOT be used to directly handle input from the UI (that's the JS's part), thus handling only one default format is totally fine. |
This comment was marked as abuse.
This comment was marked as abuse.
|
It seems works, on quick test, |
Summary of Changes
@Bakual @dgrammatiko
strftimeis deprecated, Joomla Core only uses it (AFAIK) for the frontend calendars for the Smart Search Filters (not enabled by default either).This PR provides a test case that can be run before the code change, and you can see it passes.
This PR provides a test case that can be run after the code change (The removal of
strftimecalls), and you can see it passes.This PR is a light touch approach, simply converting
strftimeformat toDateTimeInterfaceformat using a simple mapping table thanks to @relipse on StackOverflow. Its not perfect, as the comments point out, but its the best that can be expected - especially as its only used in Core Joomla a few places, and might only effect 3PD more than Joomla Core. The formats that are not mapped, and thus dont work, are not the kind of formats you would use in a calendar field anyway (Like, why would you select a Date and then only show the month name?! you wouldn't)...Testing Instructions
You can test in any PHP version.
In Joomla 4 admin -> Smart Search -> Options -> Enable Date Filters
Create Menu item to the Smart Search
Visit frontend menu item for Smart Search, Click Advanced Search
and/or:
Any 3PD that uses the calendar field is invited to test this at a PHP code level implementing the calendar field with
Actual result BEFORE applying this Pull Request
Calendar works fine, and after selecting a date, the formatted date is shown in the text box (for Joomla Core this is %Y-%m-%d like 1978-03-08 by default)
Expected result AFTER applying this Pull Request
Calendar works fine, and after selecting a date, the formatted date is shown in the text box (for Joomla Core this is %Y-%m-%d like 1978-03-08 by default)
Note
Im aware there is a CalendarField.php call with
strftimein it, that will be done next.