[4.0] [com_users] Fix default value for not nullable datetime columns and make some nullable

[richard67]

Pull Request for Issue #24535 (part).

Summary of Changes

This Pull Request (PR) fixes all datetime columns of the com_users database tables #__users and #__user_notes 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 user notes are handled like other tables in my other PR's: Columns publish_up, publish_down and checked_out_time will be nullable and will have default value NULL, and the same is done with column review_time. Columns created_time and modified_time will not allow null values like before, but the default value will be removed. This 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.

The users are a bit special: There are not the usual created and modified columns. But in column field_mappings of table content_types you can find for the record for com_users (type_alias = 'com_users.user') you can see that there column registerDate or table #__users is mapped to the core_created_time column of the #__ucm_content table, and lastvisitDate is mapped to core_modified_time. Other datetime columns are not mapped for com_user.

For this reason, the lastvisitDate will be handled like modified or modified_time columns in my other PR's for the datetime issue: It will not allow null values and not have a default value, and if a user has never visited the site (i.e. new user), the lastvisitDate will be equal to the registerDate column. This might not look very logical, but the decision was made to handle modified columns of other tables in that way for consistenty because it was already done like that at several places, and because the users table is also relevant for UCM and we have that columns mapping mentioned above, I think we have to do it that way. Suggestions for a better way to handle it without changing handling of all modified time columns of other tables would be welcome.

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

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. Play around with user notes.
6. After any action in steps 4 and 5, 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.

5. Copy the content of file administrator/components/com_admin/sql/updates/mysql/4.0.0-2019-09-28.sql or administrator/components/com_admin/sql/updates/postgresql/4.0.0-2019-09-28.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.
6. Replace #__ by your database prefix in the SQL statements pasted before in the SQL input window.
7. Put the cursor to the beginning of the 1st SQL statement in the SQL input window and now execute all SQL commands.
8. Play around with users, create some, modify some, use all possible options for activation, check the list display and its filters and sorting.
9. Play around with user notes.
10. After any action in steps 7 and 8, check in your database the relevant datetime/timestamp without timezone columns.

Result: See section "Expected result" below.

Expected result

New Installation

Tags 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 tables #__users and #__user_notes, 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.

There is one exception: When checking in a user note using com_checkin, the checked_out_time in table #__user_notes is set to '0000-00-00 00:00:00' on MySQL and '1970-01-01 00:00:00' on PosgreSQL. This will be changed with a separate, future PR for com_checkin. Checking in an item with the lock icon in list display works, i.e. there checked_out_time is set to NULL.

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.

[roland-d]

I have tested this item ✅ successfully on e583fce

I have tested this on 10.4.6-MariaDB and everything works as expected. The values are set to NULL if empty and the UTC timezone is used when saving dates.

This is done using an existing and a new installation.

---

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

[richard67]

@roland-d And what is your opinion on setting last visit date equal to registration date for new users who never have visted the site?

[roland-d]

@richard67 To me this is fine because the user did actually visit the site, just didn't login. Performing the registration is a kind of act of logging in if you ask me.

[richard67]

@roland-d Thanks for testing and feedback.

[Hackwar]

Please don't set the date of the lastvisit to the date of registration. Having that field as nullable, is a regular way to find failed registrations or inactive accounts. It is pretty important to see if an account has never been actually used.

[HLeithner]

I think the same as Hannes, a registration is not a login. Please make it null able.

[richard67]

@Hackwar @HLeithner I am open for it, but then we have to make core_modified_time in the ucm_content table, too. That would not be a big problem, but as I don't want to make design decisions on my own I have discussed that with @wilsonge . Maybe he can have a look on it again. I will implement it like he advices me.

[richard67]

So currently we have 2 vote for "ok as it is", and 2 votes for "make last visit time nullable". I am open for both, just let me know any final decision, if necessary by PLT.

[richard67]

@Hackwar @HLeithner But just by the way, the google doc and worksheet are available in Glip since weeks for discussion ... and I often advertised it ... don't you think it is a bit late to come now with this discusson? Anyway, I'll let others decide and then do what is decided.

[HLeithner]

I didn't changed my opinion I had on jday. Modifed should be null and this is a similar thing.

[richard67]

Guys in PLT or whoever else, pleace decide who will decide, or decide who will decide who will decide, or whatever, and then tell me what I shall do and I will do it.

[richard67]

@Hackwar @HLeithner Maybe an idea how to solve it: We make a new column for modified time in the users table, call it "modifiedDate" to fit to the other excentric names in that table, use this column like elsewhere, e.g. save when user data has been modified. We don't have such column yet so it would be useful. We map this to the #__ucm_content.core_modified_time and let the lastVisitDate column be nullable and not map it to ucm content. What do you think about this? It would solve your issue with the users table here.

[richard67]

P.S.: Or we drop UCM soon ;-)

[HLeithner]

The problem with the last loggin time set to the same time as registration is that you can't see if the user ever logged in. For example if you automatically login on registration both times are the same time. (not 100% sure if this thoughts reflects the code)

[richard67]

@HLeithner It does not help much to explain me the problem again, except you think I am so stupid that I did not understand it at the first time. I would make more sense to read why I have done it this way, mapping of columns to ucm_content, and maybe comment my proposal above or make an own proposal on how to solve it. Btw, the easiest solution of course would be just not to map the LastVisitDate column to ucm_content anymore.

[HLeithner]

I explained it because it'different then the modified solution of com_content for example. There it seams to be ok because if modified === created then it has never been modified.

Here it is different, if you set it to the same time it's doesn't mean that the user has never logged in because it's possible that he logged in on registration. I only explained the difference, maybe not for you, maybe only for my self.

Do we need it in the ucm table? what do other extensions if they want to map a null date to modified time column of ucm? does the ucm code automatically use our nullDate value?

[richard67]

Other components do not map null dates to the core_modified_time of ucm_content, that's why core_modified_time is not nullable. We could make it nullable, let com_users map the lastVisitDate column like it does now but let it write null values, and the other components still will not write null values to it. But I need a conrimation of someone who knows ucm_content stuff bettee than I do that we can do it that way.

[richard67]

@Hackwar @HLeithner Will change the last visit time of users to be nullable, but needs a bit investigation before. Stay tuned.

[richard67]

Closing in favour of PR #26609 for user notes and another PR to be created soon for the users table.

[wilsonge]

Keep this open. Just merge 4.0 into this branch :)

[richard67]

No, merge conflicts would be crazy. I prefer to make a new one, am already working on it, and we can link there to the discussion here.

[richard67]

PR for the users table is #26611 .