in updates XML always resulted in an error#38121
in updates XML always resulted in an error#38121HLeithner merged 3 commits intojoomla:4.2-devfrom nikosdion:fix/38117-dbsupport-updates
Conversation
In most cases this might be right, but in some languages like e.g. Chinese there might be transcripts of product names in their writing system, so I think it's good to have them translatable. |
|
@richard67 They are language strings already, as I said. If a language needs to transliterate the product names, they can. |
@nikosdion Yes, I understood that. I only wanted to say why I think this PR is really useful. Am preparing for tests now. Will create some own updatesite for that extension and modify it to use that so I can play around a bit with the supported_databases values. |
|
@richard67 Oh! Sorry, I misunderstood what you wrote before. All clear now :) |
|
Unfortunately the extension linked in the description doesn't have 4.2 yet in the targetplatform tag of the update site https://raw.githubusercontent.com/lucid-fox/social-magick/main/update/socialmagick.xml , so on a 4.2-dev the test will not work as described with that extension. So I have to create an own update site anyway. |
or modify the version number of your install. One file to edit. Very quick |
@brianteeman For that purpose only yes, but I also want to play with the supported_databases values of the update site. |
|
@richard67 You can download the update XML and host it locally, amending the supported platforms. Then edit your DB to point its update site to your local file and Bob's your uncle. |
@nikosdion Yes, I'm doing it that way, change the URL of the update site in the db and also in the manifest XML of the plugin. What I found out up to now is that your PR works as described, but I did not get the version check work up to now here .I had a When I removed the "mysql" completely with Am investigating what the reason could be. Will let you know soon. |
|
@nikosdion The problem is that the array keys of If we can rely on the keys always being uppercase we could just change that to P.S. What we should not to is to change |
|
Hmm, it is an XML element attribute ... if we can rely on it always being made uppercase by the XML parser, we can just use |
|
@nikosdion In addition to my 2 previous review comments, I found that it needs to change the error message also here fromText::_($db->name), to Text::_('JLIB_DB_SERVER_TYPE_' . $dbTypeUcase), if you follow my suggestions, or to Text::_('JLIB_DB_SERVER_TYPE_' . $dbType), otherwise.
|
|
For testing I have created a few update sites:
Just change the |
|
Another thing I found is that there is very similar functionality here https://github.com/joomla/joomla-cms/blob/4.2-dev/libraries/src/Updater/Update.php#L357-L388 . I'm not sure, but I think that's used for the core update. It would make sense to use the new language strings also there when this PR here has been merged. I could make a follow up or for that in this case. Finally, as the language strings are only used for reporting the current database type on which Joomla is running, and not the supported one from the update site, and Joomla 4 runs only on MySQL or PostgreSQL, we currently do not really need the language strings for the other database types (Oracle, MS SQL, SQLite). But as the drivers support to connect with them, I think it is good to have them for future use at other places. But that's just my opinion, others might disagree. |
|
@nikosdion If you want I can make a PR for you in your repo to apply my above suggestions. |
|
Yup, please do a PR. As for the lang strings, best to have all keys than ending up with an untranslated string 5 years down the line if, say, we decide that a site can have multiple DB connections and people write integration plugins which talk to the secondary database which might be of a different technology than what is supported for the primary database. |
@nikosdion Done, see https://github.com/nikosdion/joomla-cms/pull/10 . Regarding the language strings I fully agree with you. I just mentioned it in case others may disagree. |
…upport-updates-mod1 [CMS PR 38121] Use uppercase for array key and language string key
|
Thank you! I merged your changes. |
|
I have tested this item ✅ successfully on a4be380 I have created 3 update sites for 3 tests:
Then I have installed the plugin linked in the description. Then I have updated the I've done this for all 3 sites without and with the PR applied. Results will come with screenshot in my next comment. Other testers please follow my inscrutions above. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38121. |
|
I have tested this item ✅ successfully on a4be380 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38121. |
|
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38121. |
|
thanks |
joomla/joomla-cms#36591 + joomla/joomla-cms#37362 + joomla/joomla-cms#37404 - (позже был Revert PR) joomla/joomla-cms#37453 + joomla/joomla-cms#37583 + joomla/joomla-cms#37459 + joomla/joomla-cms#36751 + joomla/joomla-cms#36752 + joomla/joomla-cms#37912 + joomla/joomla-cms#37838 + joomla/joomla-cms#38002 + joomla/joomla-cms#38036 - (только для en-GB, у нас давно исправлено) joomla/joomla-cms#38009 + joomla/joomla-cms#38064 + joomla/joomla-cms#37911 + joomla/joomla-cms#38065 + joomla/joomla-cms#38075 + joomla/joomla-cms#38071 + joomla/joomla-cms#38080 + joomla/joomla-cms#38082 + joomla/joomla-cms#38092 + joomla/joomla-cms#38113 + joomla/joomla-cms#38121 + joomla/joomla-cms#37910 + joomla/joomla-cms#38165 + joomla/joomla-cms#37747 +




Pull Request for Issue #38117 .
Summary of Changes
Updates
\Joomla\CMS\Updater\Adapter\ExtensionAdapterto support the<supported_databases>tag in XML update files correctly.Moreover, fixes the message when there is no supported database to display the DB server technology type, not the DB connector type (therefore mysqli and pdomysql / mysql will all be referred to as “MySQL”, whereas the same connectors on a MariaDB server will be referred to as “MariaDB”).
Testing Instructions
Actual result BEFORE applying this Pull Request
A bunch of misleading warnings:
Expected result AFTER applying this Pull Request
Update found, no warnings.
Documentation Changes Required
None
Translation impact
Added six new translation strings: JLIB_DB_SERVER_TYPE_MARIADB, JLIB_DB_SERVER_TYPE_MYSQL, JLIB_DB_SERVER_TYPE_POSTGRESQL, JLIB_DB_SERVER_TYPE_ORACLE, JLIB_DB_SERVER_TYPE_SQLITE, JLIB_DB_SERVER_TYPE_MSSQL
However, these strings are product names and do not need to be translated; they just need to be copied verbatim into language files by the translation teams.