Wrong Table Layout (timestamp instead of datetime with wrong default value)

[krim404]

Summary of Changes

default database creation sql file has a timestamp in table menu instead of the datetime, but has the datetime default value.

Testing Instructions

on fresh installation check if table _menu contains a field with the type "datetime" called checked_out_time

[krim404]

this patch has been created at @icampus

[brianteeman]

What about existing sites?

[krim404]

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.

[richard67]

@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.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/11527.}

[Luchen6]

I have tested this item ✅ successfully on 94368ba

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.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11527.}

[jeckodevelopment]

@alikon can you please test/review this?

[alikon]

@jeckodevelopment added to my todo list

[alikon]

it lacks the update
the file name should be something like 3.7.0-2016-11-21.sql

[jeckodevelopment]

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.

[ghost]

@wmchris is this PR now for testing?

[ghost]

If this PR get no Response, it will be closed at 22th June 2017.

[richard67]

@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.

[richard67]

I have tested this item ✅ successfully on 94368ba

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.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11527.}

[richard67]

@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.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11527.}

[richard67]

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.

[ghost]

RTC after two successful tests.

[joomla-cms-bot]

Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/11527

[ghost]

closed in favor of PR #16469

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/11527.}