Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#26611] - [4.0] [com_users] Fix default value for datetime columns and make some nullable

? ? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
16 Oct 2019

Pull Request for Issue #24535 (part).

Replacement for #26469 (part).

Summary of Changes

This Pull Request (PR) fixes all datetime columns of database table #__users so there will not be any Invalid value '0000-00-00 00:00:00' for datetime error anymore on MySQL 5.7 or later when strict mode is enabled.

The registerDate is handled like columns created or created_time at other places with other recently merged PR's for datetime columns.

The lastvisitDate column was subject of a discussion in the first PR I made for com_users for that purpose, #26469 , and also oin Glip, and finally it turned out that we can remove the reference for the lastvisitDate column to #__ucm_content, what this PR here does, too, and so can make this column nullable, which is more logical than like it was in PR #26469 .

The lastResetTime (time of last password reset) column of the users table will also allow null values.

The user notes part which was part of PR #26469 has been separated into a separate PR, #26609 , which meanwhile has been merged, so user notes are not handled in this PR here.

Testing Instructions

Testers please report back the database kind (MySQL or PostgreSQL) on which you have tested.

If you have both MySQL and PostgreSQL, please test on both if possible.

Test 1: New installation

  1. Apply the patch on a clean 4.0-dev branch using git or merging manually, or apply it on an installation of current 4.0-dev using patchtester.
  2. If used patchtester on an existing installation in step 1, delete file configuration.php and delete all Joomla database tables in PhpMyAdmin or PhpPgAdmin (depending on your database type).
  3. Do a new installation, login to backend, confirm the statistics dialog, go to global config and set error reporting to maximum or development in server settings.
  4. Play around with users, create some, modify some, use all possible options for activation, check the list display and its filters and sorting.
  5. After any action in step 4, check in your database the relevant datetime/timestamp without timezone columns.

Result: See section "Expected result" below.

Test 2: Update sql script

  1. Install a clean clean 4.0-dev, login to backend, confirm the statistics dialog, go to global config and set error reporting to maximum or development in server settings.
  2. Apply the changes from this PR e.g. manually or with patch tester.
  3. Open PhpMyAdmin or PhpPgAdmin (depending on your database type), select your database and then go to the SQL command input.
  4. On MySQL copy the first line of file installation/sql/mysql/joomla.sql into the SQL command window but don't execute the commands yet:
SET SQL_MODE = "NO_AUTO_VALUE_ON_ZERO";

This switches off strict mode to the SQL will run on MySQL 5.7 or later.

  1. Copy the content of file administrator/components/com_admin/sql/updates/mysql/4.0.0-2019-10-17.sql or administrator/components/com_admin/sql/updates/postgresql/4.0.0-2019-10-17.sql (depending on your database type) into the SQL command window, in case of MySQL below the previously pasted commands, but don't execute the commands yet.
  2. Replace #__ by your database prefix in the SQL statements pasted before in the SQL input window.
  3. Put the cursor to the beginning of the 1st SQL statement in the SQL input window and now execute all SQL commands.
  4. Play around with users, create some, modify some, use all possible options for activation, check the list display and its filters and sorting.
  5. After any action in step 7, check in your database the relevant datetime/timestamp without timezone columns.

Result: See section "Expected result" below.

Expected result

New Installation

Users work as well as without this PR. In a MySQL database there are no columns of data type datetime having value '0000-00-00 00:00:00' in table #__users, and there is no invalid default value anymore in MySQL >= 5.7 with strict mode on. On PostgreSQL there are no columns of data type timestamp without time zone having value '0000-00-00 00:00:00' in this table.

Update sql script

The statements are processed without error. The expected result is the same as for a new installation.

Actual result

New Installation

On MySQL same as expected, but the default value '0000-00-00 00:00:00' of database columns of data type datetime is invalid in MySQL >= 5.7 with strict mode on, and there might be values '0000-00-00 00:00:00'. On postgreSQL same as expected, but there might be values '1970-01-01 00:00:00' in this table.

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.

avatar richard67 richard67 - open - 16 Oct 2019
avatar richard67 richard67 - change - 16 Oct 2019
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 16 Oct 2019
Category ⇒ SQL Administration com_admin Postgresql Installation
avatar richard67 richard67 - change - 16 Oct 2019
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 16 Oct 2019
Category SQL Administration com_admin Postgresql Installation ⇒ Administration SQL com_admin Postgresql com_users Front End Installation Libraries
avatar richard67 richard67 - change - 16 Oct 2019
Title
…
[4.0] [WiP] [com_users] Fix default value for datetime columns and make some nullable
[4.0] [com_users] Fix default value for datetime columns and make some nullable
avatar richard67 richard67 - edited - 16 Oct 2019
avatar richard67 richard67 - change - 16 Oct 2019
The description was changed
avatar richard67 richard67 - edited - 16 Oct 2019
avatar richard67
richard67 - comment - 16 Oct 2019

@roland-d Because you had tested my PR #26469 for the same thing, could you test this here, too? Thanks in advance.

avatar richard67
richard67 - comment - 19 Oct 2019

Adapted to latest changes for prepared statements and so solved merge conflicts.

13c4d5d 19 Oct 2019 avatar richard67 CS
avatar richard67
richard67 - comment - 19 Oct 2019

When solving merge conflicts it seems I've made some mistakes which resulted in PHP syntax errors. those are fixed now, and suggestions by @Quy have been implemented, too.

avatar uglyeoin
uglyeoin - comment - 19 Oct 2019

Forgive me if I haven't tested this correctly. Here's what I did.

I've been using my J4 all day testing patches (and reverting them). Hopefully that is ok but you are very specific about a brand new instance.

Before the patch, as expected 00 datetimes which fail strict MySql.

After the patch when I edit a user and then save I get:

Save failed with the following error: Column 'lastvisitDate' cannot be null

New users seem to work fine. The last visit date is still saved as

0000-00-00 00:00:00

If you let me know if I am testing correctly I am happy to retest this.

avatar richard67
richard67 - comment - 19 Oct 2019

@uglyeoin No, new users should not be saved with '0000-00-00 00:00:00'. You have to test exactly as described in the testing instructions, following every step.

avatar richard67
richard67 - comment - 19 Oct 2019

@uglyeoin P.S. It seems you've done something wrong when testing so the database schema changes were not applied. The last visit date should be NULL of a new user, and database should allow NULL values for this database column lastvisitDate.

avatar richard67
richard67 - comment - 19 Oct 2019

@roland-d PR is ready for test now.

avatar uglyeoin
uglyeoin - comment - 19 Oct 2019

Ah I see why it's 8mportsnt. No problems I will restest in a little while.

avatar uglyeoin uglyeoin - test_item - 19 Oct 2019 - Tested successfully
avatar uglyeoin
uglyeoin - comment - 19 Oct 2019

I have tested this item ✅ successfully on 1a1f77d

Thanks for your patience. Great instructions. Really good pull request, strict mode is really annoying. Works well I couldn't find any issues with block/unblock/notes/batch/changing user group/sorting. Thanks for this.

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

avatar alikon alikon - test_item - 26 Oct 2019 - Tested successfully
avatar alikon
alikon - comment - 26 Oct 2019

I have tested this item ✅ successfully on 1a1f77d

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

avatar alikon alikon - change - 26 Oct 2019
Status Pending ⇒ Ready to Commit
avatar alikon
alikon - comment - 26 Oct 2019

RTC

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

avatar wilsonge wilsonge - close - 2 Nov 2019
avatar wilsonge wilsonge - merge - 2 Nov 2019
avatar wilsonge wilsonge - change - 2 Nov 2019
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2019-11-02 10:06:15
Closed_By ⇒ wilsonge
Labels Added: ?
avatar wilsonge
wilsonge - comment - 2 Nov 2019

Thanks!

avatar richard67
richard67 - comment - 2 Nov 2019

Thanks too.

avatar Quy
Quy - comment - 2 Nov 2019
  • Go to Users > Manage
  • Click Filter Options > Select Last Visit Date > more than a year ago
500 No data supplied for parameters in prepared statement No data supplied for parameters in prepared statement 

Call stack
--
# | Function | Location
1 | () | JROOT\administrator\components\com_users\View\Users\HtmlView.php:107
2 | Joomla\Component\Users\Administrator\View\Users\HtmlView->display() | JROOT\libraries\src\MVC\Controller\BaseController.php:690
3 | Joomla\CMS\MVC\Controller\BaseController->display() | JROOT\administrator\components\com_users\Controller\DisplayController.php:120
4 | Joomla\Component\Users\Administrator\Controller\DisplayController->display() | JROOT\libraries\src\MVC\Controller\BaseController.php:728
5 | Joomla\CMS\MVC\Controller\BaseController->execute() | JROOT\libraries\src\Dispatcher\ComponentDispatcher.php:146
6 | Joomla\CMS\Dispatcher\ComponentDispatcher->dispatch() | JROOT\libraries\src\Component\ComponentHelper.php:383
7 | Joomla\CMS\Component\ComponentHelper::renderComponent() | JROOT\libraries\src\Application\AdministratorApplication.php:117
8 | Joomla\CMS\Application\AdministratorApplication->dispatch() | JROOT\libraries\src\Application\AdministratorApplication.php:160
9 | Joomla\CMS\Application\AdministratorApplication->doExecute() | JROOT\libraries\src\Application\CMSApplication.php:242
10 | Joomla\CMS\Application\CMSApplication->execute() | JROOT\administrator\includes\app.php:63
11 | require_once() | JROOT\administrator\index.php:36
avatar richard67
richard67 - comment - 2 Nov 2019

Will check after sleep.

avatar richard67
richard67 - comment - 3 Nov 2019

@Quy Thanks for the good find. I have a fix, see PR #26964 . I would be happy if you could test.

Add a Comment

Login with GitHub to post a comment