Skip to content

[4.4] Add information about reasons to Joomla Update when an update was found which doesn't fulfil PHP or DB requirements#42489

Merged
MacJoom merged 8 commits intojoomla:4.4-devfrom
richard67:4.4-dev-joomlaupdate-not-matching-updates
Dec 20, 2023
Merged

[4.4] Add information about reasons to Joomla Update when an update was found which doesn't fulfil PHP or DB requirements#42489
MacJoom merged 8 commits intojoomla:4.4-devfrom
richard67:4.4-dev-joomlaupdate-not-matching-updates

Conversation

@richard67
Copy link
Member

@richard67 richard67 commented Dec 10, 2023

Pull Request for Issue #42228 .

Summary of Changes

This pull request (PR) extends the "Update" object of the updater library to provide additional information about failed checks for PHP and database server requirements and extends the Joomla Update Component's "noupdate" view to show details when there was no suitable update found but one which failed one of these checks.

The change is made in the most b/c way so it uses a new property of the "Update" object and not any existing one, and in case if a suitable update was found there is no change in behaviour.

The language string "COM_JOOMLAUPDATE_NODOWNLOAD_EMPTYSTATE_REASON" for such cases, which was added with PR #42103 , is not modified regarding its meaning or its sprintf parameters. This PR only removes the last part "Please contact your web host to update your server." from it, which will be appended after the new details information again with a new string, and it changes the link to the technical requirements to the page the currently linked site refers to. The latter I was reluctant to do because the currently linked site that linking to another site is just a temporary thing, but the discussion in the referred issue and on social media has shown that people would prefer the link to go directly to the site where currently the requirements can be found. So worst case is that with an outdated translation the removed part of the sting will still be there and the link will be the old one.

Testing Instructions

You can find a full installation package and an update package of 4.4.2-dev + this PR here: https://artifacts.joomla.org/drone/joomla/joomla-cms/4.4-dev/42489/downloads/72341/ .

If you have a Joomla 4 site which does not meet the PHP or database requirements to update to Joomla 5 you can use that for testing.

If not, you can fake the database or the PHP version or both as described below.

Then make sure that an update is found where the requirements are not met and go to the Joomla Update component.

In addition, check that the Joomla Update Components still works as usual when there is an update found where all requirements are fulfilled.

How to fake a too low PHP version

Edit file "libraries/src/Updater/Update.php" and search for this line in the "_endElement" method:

if (!isset($this->currentUpdate->php_minimum) || version_compare(PHP_VERSION, $this->currentUpdate->php_minimum->_data, '>=')) {

In the unmodified file on the 4.4-dev branch it is line 348, in the modified file from this PR it is line 360.

Change the PHP_VERSION to e.g. '7.4.0'.

In the modified file from this PR you should also do the same change a few lines later in line 367 so you get the faked PHP version also reported in the new details information.

How to fake a too low database version

Depending on which database driver you use, "MySQLi", "MySQL (PDO)" or "PostgreSQL", modify the "getVersion" method of the particular driver to return a version string with a too small version, e.g. '5.7.0-0ubuntu0.22.04.1' for a MySQL, '10.2.0-0ubuntu0.22.04.1' for a MariaDB or '11.0-0ubuntu0.22.04.1' for PostgreSQL database.

  • For "MySQLi" file "libraries/vendor/joomla/database/src/Mysqli/MysqliDriver.php"
  • For "MySQL (PDO)" file "libraries/vendor/joomla/database/src/Mysql/MysqlDriver.php"
  • For "PostgreSQL" file "libraries/vendor/joomla/database/src/Pdo/PdoDriver.php"

Actual result BEFORE applying this Pull Request

See issue #42228 : There are no details on which of the requirements is not met, and the link to the technical requirements goes to a site which refers to another site.

Expected result AFTER applying this Pull Request

When the PHP version requirement is not met, an additional sentence shows that, telling the current and the required version.

When the database version requirement is not met, an additional sentence shows that, telling the database type and the current and required versions.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

Co-authored-by: Quy <quy@nomonkeybiz.com>
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.4-dev labels Dec 10, 2023
Co-authored-by: Quy <quy@nomonkeybiz.com>
@richard67 richard67 added the bug label Dec 10, 2023
richard67 and others added 2 commits December 10, 2023 17:51
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
@richard67
Copy link
Member Author

richard67 commented Dec 12, 2023

Please wait with testing. It seems that it doesn't work when there are also updates for extensions available. The "noupdate" view is not shown in that case for some reason. Could be an issue not related to this PR, but I will check and see if I can fix it.

No, my mistake when testing. Modified the wrong DB driver file for faking db version.

@richard67
Copy link
Member Author

@nasirkhan @josephsimony as you were reporting the issue which should be fixed with this pull request (PR) here, or commented in it: Could you test this PR? You know, every non-trivial PR needs 2 successful human tests before it is accepted, so if you want that fix in the upcoming 4.4.2 and 5.0.2 releases, you can help with testing.

You can download a full installation package or an update package of 4.4.2-dev + this PR here: https://artifacts.joomla.org/drone/joomla/joomla-cms/4.4-dev/42489/downloads/72341/ .

Just make a new installation of that, or update an existing 4.x to that. Do that on a testing environment, preferably on the one with which you had the issue, e.g. in a subdomain or a sub-folder.

If that environment would fulfil the PHP and database version requirements for updating to 5.0.1, you can find instructions on how to fake the current versions in the description of this pull request.

Then test that if some requirement is not fulfilled so that the "No update" view is shown like described in the issue, a sentence in the text on that screen is telling that, together with the actual and the required version, and if all requirements would not be fulfilled the "normal" update view is shown with the available update and the pre-update checker and so on.

If all that works as described, it is a successful test for this PR here.

@TLWebdesign
Copy link
Contributor

Awesome work! Very insightfull to see what you changed to make this work. Was just a little bit out of my league still. 😅

@TLWebdesign
Copy link
Contributor

Tried two different existing J4 sites and updated them to the package listed in this PR. But both sites won't show a J5 update when i set the update channel to Joomla Next.

@alikon
Copy link
Contributor

alikon commented Dec 18, 2023

@TLWebdesign i've used custom url with https://update.joomla.org/core/nightlies/next_major_list.xml

@alikon
Copy link
Contributor

alikon commented Dec 18, 2023

unable to mark successfull test on the tracker

image

tested succesfully

@TLWebdesign
Copy link
Contributor

Can't mark a successful test but i did test succesfully on local install that does not meet requirements and on a live website i tested it still works when it does meet requirements.

@alikon
Copy link
Contributor

alikon commented Dec 18, 2023

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 18, 2023
@brianteeman
Copy link
Contributor

Is it not possible to make this without the html <br> in the language string?

@richard67
Copy link
Member Author

Is it not possible to make this without the html <br> in the language string?

Yes it is, but I am busy with paid job. Maybe you can make a follow-up PR for that after this one got merged?

@richard67
Copy link
Member Author

richard67 commented Dec 18, 2023

@brianteeman Would it be ok to remove the <br>s from the new language stings as well as from the existing one which you had introduced, and add them with PHP in the loop here? https://github.com/joomla/joomla-cms/pull/42489/files#diff-b2fc94302ab7ee49e36e1d6e47320938ed9a9fc6fe81a956061cb9fad0c2564aR22-R40

@dautrich
Copy link

It's not possible to mark my successful test in the Issue Tracker.
grafik
I tested successfully.

@MacJoom MacJoom self-assigned this Dec 20, 2023
@MacJoom MacJoom added this to the Joomla! 4.4.2 milestone Dec 20, 2023
@MacJoom MacJoom merged commit 1b4fea4 into joomla:4.4-dev Dec 20, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 20, 2023
@MacJoom
Copy link
Contributor

MacJoom commented Dec 20, 2023

Thanks a lot - good improvement for the update process!

@richard67
Copy link
Member Author

@MacJoom Thanks for merging. I'd like to know your opinion on something which Brian had mentioned. The new language strings as well as the existing ones contain <br>, which he thinks it should be avoided, and I think I agree. I had prepared a change like this: 4.4-dev...richard67:joomla-cms:4.4-dev-joomlaupdate-not-matching-updates-2 . What do you think? Shall I make a follow-up PR with that change?

@richard67 richard67 deleted the 4.4-dev-joomlaupdate-not-matching-updates branch December 20, 2023 09:57
@brianteeman
Copy link
Contributor

@richard67 not tested it but that proposed change looks perfect.

@MacJoom
Copy link
Contributor

MacJoom commented Dec 20, 2023

We have a lot of language strings including <br> but the others do not have the <br> at the end. I agree that we should not have <br> at the end.

@richard67
Copy link
Member Author

Ok, follow-up PR see #42550 . I think it can be tested by review.

@richard67
Copy link
Member Author

@brianteeman Could you at least test or just review my PR #42550 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

Comments