-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
[4.0] [com_workflow] Fix default value for not nullable datetime columns and make checkout_time nullable #26428
[4.0] [com_workflow] Fix default value for not nullable datetime columns and make checkout_time nullable #26428
Conversation
Ready for review and test. |
@alikon Could you test? |
@alikon @Quy @SharkyKZ @twister65 Could you test this? I've just updated the testing instructions for testing the update path, too. Beside the datetime stuff this PR fixes an error in the update sql for workflows for postgresql, which makes update on postgresql fail. Since this PR does not require PR #26295 , but the update test of PR #26295 fails due to the error fixed here, please test this PR here first, if possible also with PostgreSQL, so we get this here merged soon and @wilsonge can update then his PR #26295 to latest 4.0-dev . |
I have tested this item ✅ successfully on ff5a696 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26428. |
I have tested this item ✅ successfully on 6c4fdc7 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26428. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26428. |
Thanks! |
Pull Request for Issue # .
Summary of Changes
This PR fixes all datetime columns of the
com_workflow
component's database tables so there will not be anyInvalid value '0000-00-00 00:00:00' for datetime
error anymore on MySQL 5.7 or later when strict mode is enabled.In opposite to PR #26372 , this PR here does not depend on PR #26295 , because this one here does not have anything to change in table
#__ucm_content
.Following is done for following tables:
#__workflows
: The 2 not nullable columnscreated
andmodified
will be handled like the same columns of the#__banners
table in PR [4.0] [com_banners] Finish nullable and fix default values for not nullable datetime columns #26372 , i.e. they will get no default value. The default value is only used when inserting new records without specifying values for that particular column. Not having a default will enforce to insert new records with values for these columns being provided and throw an SQL error if some of these values is not specified, i.e. such errors will not be hidden anymore.#__workflows
,#__workflow_stages
and#__workflow_transitions
: Thechecked_out_time
columns added to these table with PR [4.0] Prepare Database for checked_out in workflow tables #24059 are made nullable.Because
com_wokflow
and its table have been added in 4.0, there is no update for any old records in any sql update script, and because we are not in Beta phase yet and so don't have to support updates from 4.0-Alpha-x to 4.0-Alpha-y or between nightly builds, we don't need such update statements.Therefore it is possible just to change the sql update scripts
4.0.0-2018-06-03.sql
which add the new table, what this PR does.PHP code is changed so that the tables
#__workflows
,#__workflow_stages
and#__workflow_transitions
support null values.Finally, this PR fixes a bug in the update sql script
4.0.0-2018-05-15.sql
for postgresql, where columnschecked_out_time
andchecked_out
missing in the columns list for the insert statements for tables#__workflow_stages
and#__workflow_transitions
, but the values where in the statement's values list, which would result in an SQL error. The MySQL update sql script was ok regarding this issue.Testing Instructions
New Installation
configuration.php
and delete all Joomla database tables in PhpMyAdmin or PhpPgAdmin (depending on your database type).datetime
/timestamp without timezone
columns.Update from 3.9.12 or staging
datetime
/timestamp without timezone
columns.Expected result
New Installation
Workflows work as well as without this PR. In a MySQL database there are no columns
created
ormodified
orchecked_out_time
having value '0000-00-00 00:00:00' in any of the tables#__workflows
,#__workflow_stages
and#__workflow_transitions
, and there is no invalid default value anymore in MySQL >= 5.7 with strict mode on.Update from 3.9.12 or staging
Update works, no errors in SQL or PHP about workflows. Rest see above for new installation.
Actual result
New Installation
Same as expected result, but the default value of these database column is invalid in MySQL >= 5.7 with strict mode on.
Update from 3.9.12 or staging
On MySQL same as expected, on PostgreSQL the update fails due to an SQL error when inserting the standard workflow (fixed here for new installs but not for the update: 81d0526), and later other errors related to workflows appear.
Documentation Changes Required
Maybe core developer docs and extension developer docs should be updated to encourage them not to use '0000-00-00 00:00:00' on MySQL anymore but use real NULL and not abuse '1970-01-01 00:00:00' on PostgreSQL as a speudo null date anymore and use real NULL values also there.