[#31450] - [3.10] [WiP] Add postinstall message for invalid created date check
- Closed
- 2 Jan 2021
- Medium
- Build: staging
- Draft PR
- # 31450
- Diff
- richard67:staging-add-postinstall-msg-for-null-created-dates
User tests: Successful: Unsuccessful:
Pull Request for Issue #30546 (part).
Related to Pull Request #31449 .
Summary of Changes
WiP - work in progress
With pull request (PR) #31449 , the update SQL scripts of Joomla 4 will be extended by setting a valid default date value (January 1, 1980 00:00:00 UTC) for the created date of core content which has an old style pseudo null date as created date, in order to fix issue #30546 .
This PR here adds a new postinstall message to Joomla 3 to inform the admin about this and so enable him or her to correct bad created dates to a different value than the default mentioned above.
The message is only shown if such bad created date values have been found in database.
If the admin wants to set custom created dates (e.g. the date when the site was created would make sense), it should be done latest before updating to Joomla 4.
But it makes sense to show the postinstallation message already in 3.9.x and not only in 3.10 so the admins can start early with fixing their created dates.
To be done Add a link to a docs page (also to be done) with the necessary SQL statements to find out which records of which kind of content have the bad old null date value for created dates.
Testing Instructions
This pull request should be tested with different database types (MySQL or MariaDB and PostgreSQL, and theoretically also MS SQL Server).
Testers please report back which kind of database you have used for testing. If you have more than one type, test with all you have. Thanks in advance.
- Have a Joomla 3.9.x installation, either current staging branch or latest 3.9.x nightly or 3.9.22, which has some content of each of the following types:
- Articles
- Banners
- Categories
- Contacts
- News Feeds
- Tags
- User Notes
If you don't have such content, either create some for each type or make a new installation and chose to install "Test English (GB) Sample Data" at the final step and then create a user note in backend.
- Use a database client like e.g. phpMyAdmin for a MySQL or MariaDB database or phpPgAdmin for a PostgreSQL database to set an old pseudo null date as created date for some content.
(to be completed)
Actual result BEFORE applying this Pull Request
There is no postinstall message about invalid created dates in database.
Expected result AFTER applying this Pull Request
If some core content contains an old style null date as created date, the following postinstall message is shown:
The message lists all types of content which could be affected, i.e. not only those where the bad created date was found.
To be done: I have to add a link to a doc which provides the necessary SQL statements to find the affected records for each content type.
If no content was found with a bad created date, the postinstall message is not shown.
Documentation Changes Required
It needs some doc which provides the necessary SQL statements to find the affected records for each content type.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_admin SQL Postgresql MS SQL Language & Strings Installation |
| Title |
|
||||||
| Labels |
Added:
?
?
|
||
I think the message needs to be clear that only the tables of core components will be updated.
I have to rework the message anyway and will make that clear.
My plan is to write something on Joomla Docs and link to that in the message.
Presumably users will need to check the tables of extensions and update those as well manually?
Since we don't touch datetimes in non-core tables with the update to J4, there won't happen the SQL error mentioned in the issue with those tables. But it could make sense to mention in the doc to be created that one should also check 3rd party extensions.
is it just the created date or all dates? This message only talks about created date.
The other dates are already handled by the update SQL scripts for J4, but when we did the null date stuff we assumed that we always have a valid created date, so we did not touch this. This was a mistake, as we could see by the issue.
do you think there should be a pre-update message for that?
Yes, that's the 3rd item in my comment to the issue #30546 (comment) .
This PR here and #31449 for J4 and the one to be created for 3.10 for the pre-update check are the result of a discussion between @zero-24 and me.
Because (pseudo) null created dates can make problems not only when updating to J4 but also before, e.g. when sorting by that in lists, we thought it could be good to inform about that already in 3.9.x and not only in 3.10.
If relevant maintainers disagree on that, this PR can be rebased to 3.10-dev.
As you have multiple queries to check for the presence of invalid dates in different places why dont you report the results of that instead of the generic message - it would be much more helpful.
As you have multiple queries to check for the presence of invalid dates in different places why dont you report the results of that instead of the generic message - it would be much more helpful.
@brianteeman If you tell me how to do that I'd be happy to implement it. The only thing I could find for check routines of postinstall messages was to return true or false. I have no idea how I can include some result from a database query into the postinstall message. Or did you mean to implement different postinstall messages for the particular types of content?
@zero-24 As I remember you are very familiar with postinstall messages. Maybe you have an idea?
The more I think of this the more I think this should be forcibly displayed after update and not left to the optional messages. Especially as one service provider thought it was a good idea to automatically hide them
The more I think of this the more I think this should be forcibly displayed after update and not left to the optional messages. Especially as one service provider thought it was a good idea to automatically hide them
Sounds reasonable. I just don't know how to do that. Do we have some example?
@zero-24 As I remember you are very familiar with postinstall messages. Maybe you have an idea?
I'm not aware of anything other than true and false in this case. The only workaround i could think of with the current system would be to have different messages for each issue.
In 3.10 we could have the pre-update check for that in addition ... or I make a new tab for that in the database checker... what do you think about that?
I think you have some good ideas
- pre-update check
- post-installation message that points to
- database checker tab
I guess it depends on
- how big an issue this
- how serious an issue this
Thinking more about it, I think I should rebase or redo this PR here for 3.10-dev. The pre-update check we won't have in 3.9.x, and if we can show the details about where invalid created dates were found in the pre-update check, we won't need an extra tab in the database checker. In the post-install message I could link then to the pre-update check (or a new tab in the database checker if it goes this way) instead of linking to an article in docs.
@zero-24 @HLeithner What do you think? Leave this PR here for staging, or rebase/redo for 3.10-dev?
Fine for me.
The PR in staging was the idea to have them upfront when that is to complicated or not possible with postinstall messages. Its fine for me to move that check to the pre upgrade checker.
@richard67 yes please move it to 3.10, 3.9 is more less eol now.
I think it's also better in the pre upgrade checker
Thanks for feedback. Will do that on weekend.
| Title |
|
||||||
| Category | Administration com_admin SQL Postgresql MS SQL Language & Strings Installation | ⇒ | Administration com_admin SQL Postgresql MS SQL com_config Language & Strings External Library Composer Change Installation Layout Libraries |
Rebase to 3.10-dev did not work. I will create a new PR instead.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-01-02 15:15:00 |
| Closed_By | ⇒ | richard67 | |
| Labels |
Added:
?
?
|
||
I think the message needs to be clear that only the tables of core components will be updated. Presumably users will need to check the tables of extensions and update those as well manually?
Sorry not been following this issue - is it just the created date or all dates? This message only talks about created date.
Also as the update only happens when you do the update to J4 do you think there should be a pre-update message for that?