? Success

User tests: Successful: Unsuccessful:

avatar wmchris
wmchris
9 Aug 2016

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

avatar joomla-cms-bot joomla-cms-bot - change - 9 Aug 2016
Category Installation SQL
avatar wmchris wmchris - open - 9 Aug 2016
avatar wmchris wmchris - change - 9 Aug 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 9 Aug 2016
Labels Added: ?
avatar wmchris
wmchris - comment - 9 Aug 2016

this patch has been created at @icampus

avatar brianteeman
brianteeman - comment - 9 Aug 2016

What about existing sites?

avatar wmchris wmchris - reference | a8d612e - 9 Aug 16
avatar wmchris
wmchris - comment - 9 Aug 2016

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.

avatar wmchris wmchris - change - 9 Aug 2016
The description was changed
avatar wmchris wmchris - edited - 9 Aug 2016
avatar richard67
richard67 - comment - 10 Aug 2016

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

avatar Luchen6 Luchen6 - test_item - 4 Nov 2016 - Tested successfully
avatar Luchen6
Luchen6 - comment - 4 Nov 2016

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.

avatar jeckodevelopment
jeckodevelopment - comment - 20 Nov 2016

@alikon can you please test/review this?

avatar alikon
alikon - comment - 21 Nov 2016

@jeckodevelopment added to my todo list

avatar alikon
alikon - comment - 21 Nov 2016

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

avatar jeckodevelopment
jeckodevelopment - comment - 23 Nov 2016

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.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 10 Jan 2017

@wmchris is this PR now for testing?

avatar franz-wohlkoenig franz-wohlkoenig - change - 28 May 2017
The description was changed
Status Pending Information Required
avatar joomla-cms-bot joomla-cms-bot - edited - 28 May 2017
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 28 May 2017

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

avatar richard67
richard67 - comment - 28 May 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.

avatar richard67
richard67 - comment - 3 Jun 2017

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.

avatar richard67 richard67 - test_item - 3 Jun 2017 - Tested successfully
avatar richard67
richard67 - comment - 3 Jun 2017

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

avatar richard67
richard67 - comment - 3 Jun 2017

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.

avatar franz-wohlkoenig franz-wohlkoenig - change - 3 Jun 2017
Status Information Required Ready to Commit
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 3 Jun 2017

RTC after two successful tests.

avatar joomla-cms-bot joomla-cms-bot - close - 3 Jun 2017
avatar franz-wohlkoenig franz-wohlkoenig - change - 3 Jun 2017
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2017-06-03 12:54:31
Closed_By franz-wohlkoenig
avatar joomla-cms-bot
joomla-cms-bot - comment - 3 Jun 2017
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 3 Jun 2017

closed in favor of PR #16469


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

Add a Comment

Login with GitHub to post a comment