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

Add label, date and datetime picker to system.xml #2739

Closed
wants to merge 15 commits into from
Closed

Add label, date and datetime picker to system.xml #2739

wants to merge 15 commits into from

Conversation

luigifab
Copy link
Contributor

@luigifab luigifab commented Nov 17, 2022

Latest version is here with IPv6, but incomplete.

Description

Some new fields for system.xml, example:

image

XML of the screen shot:

<head>
	<label>Crash test</label>
	<frontend_model>adminhtml/system_config_form_field_heading</frontend_model>
	<sort_order>1001</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</head>
<label_1>
	<label>label</label>
	<value>Pouet</value>
	<frontend_model>adminhtml/system_config_form_field_label</frontend_model>
	<sort_order>1002</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</label_1>
<labelbold_2>
	<label>label bold</label>
	<value>Pouet Pouet</value>
	<bold>1</bold>
	<frontend_type>label</frontend_type><!-- optionnal and useless -->
	<frontend_model>adminhtml/system_config_form_field_label</frontend_model>
	<sort_order>1003</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</labelbold_2>
<datetime_1>
	<label>datetime (original)</label>
	<frontend_model>adminhtml/system_config_form_field_datetime</frontend_model>
	<sort_order>1004</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</datetime_1>
<datetime_2>
	<label>datetime with format</label>
	<frontend_type>datetime</frontend_type><!-- optionnal and useless -->
	<frontend_model>adminhtml/system_config_form_field_datetime</frontend_model>
	<format>FORMAT_TYPE_SHORT</format>
	<sort_order>1005</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</datetime_2>
<date_3>
	<label>date</label>
	<frontend_model>adminhtml/system_config_form_field_date</frontend_model>
	<sort_order>1006</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</date_3>
<date_4>
	<label>date with format</label>
	<frontend_type>date</frontend_type><!-- optionnal and useless -->
	<frontend_model>adminhtml/system_config_form_field_date</frontend_model>
	<format>FORMAT_TYPE_SHORT</format>
	<sort_order>1007</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</date_4>
<datetime_5>
	<label>editable datetime</label>
	<frontend_model>adminhtml/system_config_form_field_datetime</frontend_model>
	<backend_model>adminhtml/system_config_backend_datetime</backend_model>
	<editable>1</editable>
	<sort_order>1008</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</datetime_5>
<datetime_6>
	<label>editable datetime with format</label>
	<frontend_type>datetime</frontend_type><!-- optionnal and useless -->
	<frontend_model>adminhtml/system_config_form_field_datetime</frontend_model>
	<backend_model>adminhtml/system_config_backend_datetime</backend_model>
	<format>FORMAT_TYPE_SHORT</format>
	<editable>1</editable>
	<sort_order>1009</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</datetime_6>
<date_7>
	<label>editable date</label>
	<frontend_model>adminhtml/system_config_form_field_date</frontend_model>
	<backend_model>adminhtml/system_config_backend_date</backend_model>
	<editable>1</editable>
	<sort_order>1010</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</date_7>
<date_8>
	<label>editable date with format</label>
	<frontend_type>date</frontend_type><!-- optionnal and useless -->
	<frontend_model>adminhtml/system_config_form_field_date</frontend_model>
	<backend_model>adminhtml/system_config_backend_date</backend_model>
	<format>FORMAT_TYPE_SHORT</format>
	<editable>1</editable>
	<sort_order>1011</sort_order>
	<show_in_default>1</show_in_default>
	<show_in_website>1</show_in_website>
	<show_in_store>1</show_in_store>
</date_8>

There are some bugs with the JS calendar, sometimes it doesn't understand the format, so it's better to use the short format. But this is also another problem.

Date and datetime are stored in UTC time in core_config_data. If you check with Adminer, use SET @@session.time_zone='+00:00'; (https://dba.stackexchange.com/q/20217/242650).

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Core Relates to Mage_Core Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* labels Nov 17, 2022
fballiano
fballiano previously approved these changes Nov 27, 2022
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love some new feature

@fballiano
Copy link
Contributor

but there's a conflict now

@sreichel
Copy link
Contributor

@luigifab
Copy link
Contributor Author

luigifab commented Dec 1, 2022

I updated some things, not sure if the good ones.
I rebase...

@sreichel
Copy link
Contributor

sreichel commented Dec 1, 2022

I updated some things, not sure if the good ones.
I rebase...

Please do not force push when your previous commit is used (like in my PR). Now there are conflicts and i cant see what you recently changed. :(

@github-actions github-actions bot removed the Component: Core Relates to Mage_Core label Dec 1, 2022
@luigifab
Copy link
Contributor Author

luigifab commented Dec 2, 2022

Oh sorry, ok I didn't check correctly what you did, I only supposed that the problem is the rebase.

@sreichel
Copy link
Contributor

sreichel commented Dec 4, 2022

I use force push to, but only when i missied something in my last commit.

At the end its more clear to use merge instead of rebase.

However .... a hope it was okay to push to your branch. If there is smomething wrong, pls let me know.

@sreichel sreichel requested review from fballiano and kiatng December 4, 2022 00:40
@fballiano
Copy link
Contributor

Should we write something in the README? In the "what's different" section?

fballiano
fballiano previously approved these changes Dec 14, 2022
@fballiano
Copy link
Contributor

Should we write something in the README? In the "what's different" section?

done

@fballiano fballiano requested a review from sreichel December 14, 2022 15:08
@sreichel
Copy link
Contributor

sreichel commented Dec 14, 2022

$class = (!empty($field->frontend_type) && ($field->frontend_type != 'text')) ? 'Varien_Data_Form_Element_' . ucfirst(strtolower($field->frontend_type)) : '';

Causes Fatal error: Uncaught TypeError: strtolower(): Argument #1 ($string) must be of type string, Mage_Core_Model_Config_Element given in /var/www/html/app/code/core/Mage/Adminhtml/Block/System/Config/Form/Field/Label.php:41

edit:

Error comes from declare(strict_types=1);

W/o it works b/c Mage_Core_Model_Config_Element has a __toString method. Still testing ...

@sreichel sreichel marked this pull request as draft December 14, 2022 23:52
@sreichel
Copy link
Contributor

When saving date (w/o format) ...

Fatal error:  Uncaught TypeError: number_format(): Argument #1 ($num) must be of type float, string given in /var/www/html/lib/Zend/Locale/Math.php:161
Stack trace:
#0 /var/www/html/lib/Zend/Locale/Math.php(161): number_format('DEC 14, 2022 07...', 0, '.', '')
#1 /var/www/html/lib/Zend/Locale/Format.php(305): Zend_Locale_Math::floatalize('DEC 14, 2022 07...')
#2 /var/www/html/lib/Zend/Locale/Format.php(662): Zend_Locale_Format::toNumber('Dec 14, 2022 07...', Array)
#3 /var/www/html/lib/Zend/Filter/NormalizedToLocalized.php(106): Zend_Locale_Format::toFloat('Dec 14, 2022 07...', Array)
#4 /var/www/html/app/code/core/Mage/Adminhtml/Model/System/Config/Backend/Date.php(89): Zend_Filter_NormalizedToLocalized->filter('Dec 14, 2022 07...')
#5 /var/www/html/app/code/core/Mage/Adminhtml/Model/System/Config/Backend/Date.php(59): Mage_Adminhtml_Model_System_Config_Backend_Date->filterDateTime('Dec 14, 2022 07...')

@sreichel
Copy link
Contributor

I've change some parts. Everything works, but save datetime is still not correct ...

Took fields from @luigifab post and added to cms config section ...

There is a 12h offset for datetimes ...

| path | value |
| :--- | :--- |
| cms/wysiwyg/datetime\_5 | 2022-12-18 03:04:01 |
| cms/wysiwyg/datetime\_6 | 2022-12-17 15:04:00 |

@sreichel
Copy link
Contributor

sreichel commented Dec 15, 2022

I reverted my changes and tested @luigifab initial commit. It does not work. I cant save dates (same error as in my tests yesterday) and also displayed time is wrong. Now it shows Dec 15, 2022 2:56:53 PM (6h diff to correct time)

@luigifab
Copy link
Contributor Author

luigifab commented Jan 1, 2023

I tested again with the original commit, with PHP 8.0, with OpenMage locale fr_FR, with browser locale fr_FR, with OpenMage timezone utc+1, with branch 1.9.4.x.

For me, when I save my config page with:

  • 15 janv. 2023 13:32:00, I have 2023-01-15 12:32:00 in database
  • 16/01/2023 13:32, I have 2023-01-16 12:32:00 in database
  • 17 janv. 2023, I have 2023-01-16 23:00:00 in database
  • 18/01/2023, I have 2023-01-17 23:00:00 in database

I can save my page multiple times, there are no problems.

With: <?php echo $this->formatDate(Mage::getStoreConfig('xyz/xyz/datetime_5'), Mage_Core_Model_Locale::FORMAT_TYPE_LONG, true) ?>, I have the good value: 15 janvier 2023 13:32:00 CET.

But when I change OpenMage locale to en_US, you are right there are some bugs.


With your changes, I have a problem: date(time)s are not save in UTC.
For example, when I save my config page with:

  • 15 janv. 2023 13:32:00, I have 2023-01-15 13:32:00 in database, backend display 15 janv. 2023 14:32:00
  • 16/01/2023 13:32, I have 2023-01-16 13:32:00 in database, backend display 16/01/2023 14:32
  • 17 janv. 2023, I have 2023-01-17 00:00:00 in database, backend display 17 janv. 2023
  • 18/01/2023, I have 2023-01-18 00:00:00 in database, backend display 18/01/2023

With: <?php echo $this->formatDate(Mage::getStoreConfig('xyz/xyz/datetime_5'), Mage_Core_Model_Locale::FORMAT_TYPE_LONG, true) ?>, I have: 15 janvier 2023 14:32:00 CET.

@luigifab
Copy link
Contributor Author

luigifab commented May 31, 2023

Closed because this PR can be split in multiple PR, and there are issues... and I will be out there until 2025.
Anyway, I continue us it as is.

@luigifab luigifab closed this May 31, 2023
@luigifab luigifab deleted the new-config-fields branch May 31, 2023 08:26
@kyrena kyrena mentioned this pull request Oct 4, 2023
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants