Skip to content

[4.1] Use the date function in calendar field#37329

Closed
laoneo wants to merge 3 commits intojoomla:4.2-devfrom
Digital-Peak:j4/field/calendar-81
Closed

[4.1] Use the date function in calendar field#37329
laoneo wants to merge 3 commits intojoomla:4.2-devfrom
Digital-Peak:j4/field/calendar-81

Conversation

@laoneo
Copy link
Member

@laoneo laoneo commented Mar 20, 2022

Pull Request for Issue #34952 and takeover from #36395.

Summary of Changes

The calendar field uses the strftime to format a date. This function got deprecated in PHP 8.1 and will be removed in 9. This pr uses the widely accepted date function.
There are probably edge cases for format conversion, which are not covered by this pr, but can be added later.

Full credit goes to @PhilETaylor, I was just taking it over and made some small modifications.

Testing Instructions

  • Create a new article in the back end
  • Click on the publishing tab

Actual result BEFORE applying this Pull Request

The following message is displayed:
Deprecated: Function strftime() is deprecated in /libraries/src/Form/Field/CalendarField.php on line 277

Expected result AFTER applying this Pull Request

No error message.

@laoneo laoneo added the PHP 8.x PHP 8.x deprecated issues label Mar 20, 2022
@laoneo laoneo requested a review from rdeutz as a code owner March 20, 2022 13:47
@laoneo laoneo mentioned this pull request Mar 20, 2022
@tecpromotion
Copy link
Contributor

I have tested this item ✅ successfully on fca7309


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37329.

@brianteeman
Copy link
Contributor

Surely this has to go in 4.2

@BPBlueprint
Copy link

I have tested this item ✅ successfully on fca7309


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37329.

@laoneo laoneo added PHP 8 RTC This Pull Request is Ready To Commit and removed Unit/System Tests PHP 8.x PHP 8.x deprecated issues labels Mar 22, 2022
@laoneo
Copy link
Member Author

laoneo commented Mar 22, 2022

rtc


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37329.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 22, 2022
@laoneo laoneo added Unit/System Tests PHP 8.x PHP 8.x deprecated issues PHP 8 RTC This Pull Request is Ready To Commit and removed PHP 8 Unit/System Tests PHP 8.x PHP 8.x deprecated issues labels Mar 22, 2022
@laoneo
Copy link
Member Author

laoneo commented Mar 22, 2022

rtc


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37329.

@laoneo laoneo added Unit/System Tests PHP 8.x PHP 8.x deprecated issues and removed PHP 8 labels Mar 22, 2022
@MacJoom
Copy link
Contributor

MacJoom commented Mar 22, 2022

I have tested this item ✅ successfully on fca7309


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37329.

@bembelimen
Copy link
Contributor

Surely this has to go in 4.2

Yes, or 5.0

@bembelimen bembelimen changed the base branch from 4.1-dev to 4.2-dev March 24, 2022 21:37
@laoneo
Copy link
Member Author

laoneo commented Mar 25, 2022

If this has to go into 5, then Joomla 4 will never be able to run on PHP 9. If it has to go into 4.2, then I suggest an alternative parameter where extension developers can define a format in normal PHP date format. Something like date_format. So the tag would look like:

<field
	name="begin"
	type="calendar"
	label="COM_BANNERS_BEGIN_LABEL"
	hint="COM_BANNERS_BEGIN_HINT"
	format="%Y-%m-%d"
	date_format="Y-m-d"
	filter="user_utc"
/>

Extension developers can then still use the normal format parameter to keep backwards compatibility with Joomla 3.

What do you guys think?

@brianteeman
Copy link
Contributor

If you go down the route of the extra date_format then you should also update all date fields in joomla so that they are using the new way and nothing in core is using the old and presumably deprecated method.

@nibra See I think I understand now your comments in the RFC

@joomdonation
Copy link
Contributor

Actually, we can use a hidden parameter filterFormat

. It is not documented, but used on all of our calendar form fields when translateformat="true" , see https://github.com/joomla/joomla-cms/blob/4.1-dev/libraries/src/Form/Field/CalendarField.php#L207

So Calendar Form fields from core will still work as how it is. For third party extensions, we can just add filterFormat attribute with the right value and it is working OK, too. If the attribute is not available, we can use the code to convert format like how we are doing in this PR (maybe just for PHP >= 8.1.0)

@laoneo
Copy link
Member Author

laoneo commented Mar 25, 2022

@joomdonation can you make an alternative pr with the filterFormat? But please make it compatible with all PHP versions.

@joomdonation
Copy link
Contributor

@laoneo If it is OK to make PR without writing unit test, I will give it a try (I don't know how to write unit tests at the moment). Also, I will have to borrow the code of strftimeFormatToDateFormat method in this PR.

@laoneo
Copy link
Member Author

laoneo commented Mar 25, 2022

Sure

@laoneo
Copy link
Member Author

laoneo commented Mar 25, 2022

CLosing in favor of #37373.

@laoneo laoneo closed this Mar 25, 2022
@laoneo laoneo deleted the j4/field/calendar-81 branch March 25, 2022 16:19
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PHP 8.x PHP 8.x deprecated issues Unit/System Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants