[#21901] - [4.0] Replace the zero datetime value '0000-00-00 00:00:00' by the real NULL value
- Fixed in Code Base
- 19 Mar 2019
- Medium
- Build: 4.0-dev
- # 21901
- Diff
- csthomas:com_module_null_datetime
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
This PR changes only datetimes in the module table.
No more zero dates like "0000-00-00 00:00:00" in the module table.
Method checkIn() is only added (almost copy/paste) to set column check_out_time to NULL.
At method store() the default value has been changed to $updateNulls = true to be able to save NULL values.
TODO
missing changes for the install files (Mysql, PostgreSQL)missing changes in the update files for Postgresql version
Changes for other tables should be made in the next PRs
Testing Instructions
First way:
- On existed 4.0-dev installation upload changes and then click on the database "Fix" button.
TheUPDATE ...query won't be applied by theFIXbutton. You can do it manually (in phpMyAdmin), but it is not required, the system will work anyway!- on Postgresql database Fix button does not work because PR #19707 has not yet been merged in 4.0.
Second way:
- Upload changes, then remove file
configuration.phpand install joomla 4.0 again (also you can test sample data).
In both cases:
- After changes will be applied you can create and save modules with real NULL dates.
Test publish_up,publish_downandcheck_out_timecolumns. Everything work as before.
Expected result
You can create, edit and save modules.
Modules are loaded normally on backend and frontend.
How to revert changes in your test db
SET sql_mode = '';
ALTER TABLE `#__modules` MODIFY `publish_up` DATETIME NOT NULL DEFAULT '0000-00-00 0000:00:00';
ALTER TABLE `#__modules` MODIFY `publish_down` DATETIME NOT NULL DEFAULT '0000-00-00 0000:00:00';
ALTER TABLE `#__modules` MODIFY `checked_out_time` DATETIME NOT NULL DEFAULT '0000-00-00 0000:00:00';| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | SQL Administration com_admin Libraries |
| Labels |
Added:
?
|
||
we should do this now in 4.0 or never more
| Category | SQL Administration com_admin Libraries | ⇒ | SQL Administration com_admin Postgresql Installation Libraries |
| Title |
|
||||||
Ready for core review and test:)
By adding patch from curl https://patch-diff.githubusercontent.com/raw/joomla/joomla-cms/pull/19707.diff | git apply --reject you can test the update file for postgresql
In the "Files changed" tab it is probably not visible. I copy&paste checkIn() method from Table.php and pasted it into Module.php. Then I changed only the comment and two lines to change the sql zero date with NULL and empty string with null.
I will change the order of methods in the file, and the diff view should be more readable.
The diff between Table.php and Module.php for checkIn() method.
I did not want to add changes in Table.php because it breaks other tables.
Would be good if we can do it without copy paste. Can you not expand the query with an "or is null" in Table.php?
Can you not expand the query with an "or is null" in Table.php?
Read carefully, this is an update query (SET ....):
- ->set($this->_db->quoteName($checkedOutTimeField) . ' = ' . $this->_db->quote($this->_db->getNullDate()));
+ ->set($this->_db->quoteName($checkedOutTimeField) . ' = NULL');
I changed the code and now should be more readable, without overloading checkIn() method.
@alikon @tonypartridge @ggppdk @Bakual I am asking you for a test, at least for a part, I know people are very busy.
If this go then I can work on the rest of tables.
I have tested this item
@csthomas thanks for making such PRs to improve / fix Database API and Database handling
I have tested with MySQL
frontend and backend modules,
also looked to see if there are more places to be updated with usage of isNullDatetime() by looking at files that have #__modules as
but did not find any more places
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21901.
on my to do list
Can we get here some tests? Would be nice to get it in.
I have tested this item
sorry for the delay
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21901.
| Labels |
Added:
?
|
||
I rebased PR. There were conflicts in sql files. Now there is need one more test.
Would be good to get some more tests here and the conflicts fixed, so we can get it merged.
I did what I could. Your turn.
Let's go for this. I think we still have other compatibility issues. But this seems like a positive starting point.
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-03-19 20:03:22 |
| Closed_By | ⇒ | wilsonge |
@franz-wohlkoenig needs more. this rolls it out for modules. this will need rolling out everywhere that has a publish up/down column (com_content etc etc)
Members may I help with further PRs ?
@wilsonge @franz-wohlkoenig
Fine by me
Fine by me
Where in are the further changes to be made?
Tested that and works.
Fixes the error messages related with empty published_up, published_down and checked_out_time in the module saving as well.
I like the change. I was always wondering why we bloat the rows with such 0 dates instead of null.