User tests: Successful: Unsuccessful:
default database creation sql file has a timestamp in table menu instead of the datetime, but has the datetime default value.
on fresh installation check if table _menu contains a field with the type "datetime" called checked_out_time
Category | ⇒ | Installation SQL |
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
What about existing sites?
ALTER TABLE #__menu
CHANGE checked_out_time
checked_out_time
datetime NOT NULL DEFAULT '0000-00-00 00:00:00';
should be added in a new file in 'administrator/components/com_admin/sql/updates/mysql'
dunno the version number i should use, so i dont push it in this pull request.
@wmchris Better use
ALTER TABLE #__menu
MODIFY checked_out_time datetime
NOT NULL DEFAULT '0000-00-00 00:00:00';
The CHANGE statement will work, too, but because you do not want to rename the column there is no need to it, and so with 1 column name to be specified in the statement there is 1 chance less for a typo.
And it would fit to the existing update scripts, where we always used MODIFY if sufficient and CHANGE only if necessary.
I have tested this item
applied changes in joomla.sql before installing the site.
The structure in mysql was correct; fieldtype is datetime.
Checking out a menu item showed correct date/time.
@jeckodevelopment added to my todo list
it lacks the update
the file name should be something like 3.7.0-2016-11-21.sql
it lacks the update
the file name should be something like 3.7.0-2016-11-21.sql
Well, ok, iirc, @wmchris you should change the DB also in a "dedicated" SQL file specific for this release, in order to apply the patch also to updated websites and not just on fresh installations.
To do it, you should create the following files:
administrator/components/com_admin/sql/updates/mysql/3.7.0-2016-11-21.sql
administrator/components/com_admin/sql/updates/postgresql/3.7.0-2016-11-21.sql
administrator/components/com_admin/sql/updates/sqlazure/3.7.0-2016-11-21.sql
syntax is: X.X.X-YYYY-MM-DD (where obviously X.X.X is the target version of this PR and YYYY-MM-DD represents the date of this PR).
Consider, as you may understand from the above mentioned files, that you should replicate changes also to other supported databases not just MySQL.
Status | Pending | ⇒ | Information Required |
If this PR get no Response, it will be closed at 22th June 2017.
@franz-wohlkoenig It seems that @wmchris has deleted the branch for this PR. But the PR makes sense: All columns "checked_out_time" in Joomla core tables on mysql are of type DATETIME, except the one where this PR wants to fix it. For postgresql and sqlazure (aka mssql) the types of these columns are consistent among all tables. So this PR still makes sense.
@wmchris and @franz-wohlkoenig : The only 2 things which are not done yet in this PR here is the missing update scipt in 'administrator/components/com_admin/sql/updates/mysql', e.g. 3.7.3-2017-05-28.sql, and the use of "ALTER TABLE ... MODIFY" in that SQL script instead of "ALTER TABLE ... CHANGE", so it fits to the existing scripts, see my comment above.
@wmchris If you want and let me know which branch in your repository this PR comes from, I can make a pull request against your branch with my recommended changes. Then you can accept them and this PR would be OK. It could be tested by review from my point of view. If you don' have time for continuing this PR, let me know and I will take it over as described below.
@franz-wohlkoenig If no reaction from @wmchris , shall I takeover this PR, i.e. make a new one with these changes and you close this one in favour of the new one? I would then at least give some thanks to @wmchris in my PR's description.
I have tested this item
Tested by code review.
All columns "checked_out_time" in Joomla core tables on mysql are of type DATETIME, except the one which is fixed by this PR.
For postgresql and sqlazure (aka mssql) the types of these columns are consistent among all tables, so for these database types nothing has to be done.
@franz-wohlkoenig If this PR here will be merged I will make an own, new one for adding the missing schema update script for mysql.
I meanwhile have decided to redo this PR with a new one, see #16469 .
So please check the new one, and this one can be closed.
Of course almost all credits should go tho @wmchris , but there was no reaction about the missing schema update script, and the branch for this PR here is shown as unknown, so I decided for a redo.
Status | Information Required | ⇒ | Ready to Commit |
RTC after two successful tests.
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-06-03 12:54:31 |
Closed_By | ⇒ | franz-wohlkoenig |
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/11527
closed in favor of PR #16469
this patch has been created at @icampus