? Pending

User tests: Successful: Unsuccessful:

avatar zero-24
zero-24
13 Jul 2021

Pull Request for Issue #34599 cc @nikosdion @brianteeman

Summary of Changes

Correctly handle two critical extensions within an package that is joomla 4 compatible

Testing Instructions

Install latest akeeba, enable the system plugin (backup on update) and the action log plugin.
point your update server to Joomla 4 (custom URL: https://update.joomla.org/core/test/310to4_list.xml)
apply this PR
clear the browser / JS cache.

Actual result BEFORE applying this Pull Request

The pre upgrade checker shows a message about potencial not compatible plugins within that package

Expected result AFTER applying this Pull Request

The pre update checker shows no message about that plugins cause they are compatible.

Documentation Changes Required

none

I have touched JS code here

I'm by far not an expert in JS so please suggest changes to my JS when i just did it wrong. Thanks!

avatar zero-24 zero-24 - open - 13 Jul 2021
avatar zero-24 zero-24 - change - 13 Jul 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 13 Jul 2021
Category JavaScript
avatar brianteeman
brianteeman - comment - 13 Jul 2021

image

avatar brianteeman brianteeman - test_item - 13 Jul 2021 - Tested unsuccessfully
avatar brianteeman
brianteeman - comment - 13 Jul 2021

I have tested this item 🔴 unsuccessfully on 3026534


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

avatar zero-24
zero-24 - comment - 13 Jul 2021

WTF. Just to be sure did you cleared cache cause i changed JS files?

avatar zero-24
zero-24 - comment - 13 Jul 2021

image

Its working good here

avatar zero-24
zero-24 - comment - 13 Jul 2021

With both "critical" plugins enabled
image

avatar wilsonge
wilsonge - comment - 13 Jul 2021

Screenshot 2021-07-13 at 23 31 32

Don't have time to debug it tonight I'm afraid but I can reproduce brian's negative test

avatar zero-24
zero-24 - comment - 13 Jul 2021

Don't have time to debug it tonight I'm afraid but I can reproduce brian's negative test

Thats a differen issue cause there it claims not to be compatible. Can you double check the update server you are pointing too?

avatar zero-24 zero-24 - change - 13 Jul 2021
Labels Added: ?
avatar wilsonge
wilsonge - comment - 13 Jul 2021

Sorry my fault. I'd set it back to 3.10 as an update server rather than 4. For 4.0 as an update server indeed it indeed does work.

Screenshot 2021-07-13 at 23 36 09

We also need to fix the issue by the looks of it of the multiple plugins giving messages in the above screenshot where the server pointed to 3.10 (should only be one message for akeeba as a whole). but guess that can be a separate PR.

avatar wilsonge wilsonge - test_item - 13 Jul 2021 - Tested successfully
avatar wilsonge
wilsonge - comment - 13 Jul 2021

I have tested this item ✅ successfully on 7b3f6af

Works for me


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

avatar zero-24
zero-24 - comment - 13 Jul 2021

We also need to fix the issue by the looks of it of the multiple plugins giving messages in the above screenshot where the server pointed to 3.10 (should only be one message for akeeba as a whole). but guess that can be a separate PR.

This is the issue with the update server where it claims to be not compatible with 3.10 but ony 3.9 and 4.0 nothing I see that could be fixed on our side other than "guessing" whether an extension is compatible. But we also dont do that for updates so i dont think we should do it here and give even more confusion.

avatar zero-24 zero-24 - change - 13 Jul 2021
The description was changed
avatar zero-24 zero-24 - edited - 13 Jul 2021
avatar zero-24
zero-24 - comment - 13 Jul 2021

We also need to fix the issue by the looks of it of the multiple plugins giving messages in the above screenshot where the server pointed to 3.10 (should only be one message for akeeba as a whole). but guess that can be a separate PR.

Ah sorry got it wrong. I can not reproduce that problem, i have added "clear cache" to the test instructions does that work for you? Can you still reproduce the double messages?

avatar zero-24
zero-24 - comment - 13 Jul 2021

Ok found another issue in that code.. Will try again to fix it with my limited JS skills.

avatar brianteeman
brianteeman - comment - 13 Jul 2021

verified 1000% that the new js is being loaded

avatar brianteeman
brianteeman - comment - 13 Jul 2021

I have been able to work out why our test results differ and maybe it help you solve the bug.

With just akeeba backup pro installed - no errors
with admintools pro also installed - errors

clearly a bug there

avatar zero-24
zero-24 - comment - 13 Jul 2021

verified 1000% that the new js is being loaded

Hmm I have just pushed another hack in lack of better JS skills. I could not reproduce the issue with the double labels but maybe thats fixing it too?

avatar zero-24
zero-24 - comment - 13 Jul 2021

With just akeeba backup pro installed - no errors
with admintools pro also installed - errors

I have no access to the pro versions to test with. Has the "free" version the same issue?

With the latest changes and "just" the free versions it looks like this:
image

avatar brianteeman
brianteeman - comment - 14 Jul 2021

seems to work ok now

avatar zero-24
zero-24 - comment - 14 Jul 2021

seems to work ok now

Thats great to hear. Thanks!

avatar zero-24 zero-24 - change - 14 Jul 2021
Title
[3.10] Correctly handle two critical extensions within an package that is joomla 4 compatible
[3.10] Correctly handle multiblr critical extensions within an package that is joomla 4 compatible
avatar zero-24 zero-24 - edited - 14 Jul 2021
avatar zero-24 zero-24 - change - 14 Jul 2021
Title
[3.10] Correctly handle multiblr critical extensions within an package that is joomla 4 compatible
[3.10] Correctly handle multible critical extensions within an package that is joomla 4 compatible
avatar zero-24 zero-24 - edited - 14 Jul 2021
avatar zero-24 zero-24 - change - 14 Jul 2021
Title
[3.10] Correctly handle multible critical extensions within an package that is joomla 4 compatible
[3.10] Correctly handle multiple critical extensions within an package that is joomla 4 compatible
avatar zero-24 zero-24 - edited - 14 Jul 2021
avatar Fedik
Fedik - comment - 14 Jul 2021

a bit more readable will be:

var pluginInfo;
for (var i = PreUpdateChecker.nonCoreCriticalPlugins.length - 1; i >= 0; i--)
{
  pluginInfo = PreUpdateChecker.nonCoreCriticalPlugins[i];
  
  if (pluginInfo.package_id == extensionId || pluginInfo.extension_id == extensionId)
  {
    $('#plg_' + pluginInfo.extension_id).remove();
    PreUpdateChecker.nonCoreCriticalPlugins.splice(i, 1);
  }
}
avatar zero-24
zero-24 - comment - 14 Jul 2021

Applied here thanks @Fedik 6b8a785

avatar nikosdion
nikosdion - comment - 14 Jul 2021

Maybe a stupid question. Why don't you use forEach instead? Isn't this file going through Babel anyway? Also, why are we using jQuery when the promise is that Joomla core doesn't use it anymore?

I am confused by this JavaScript. It seems to be inconsistent with everything else in the core. I was shouted at for far lesser transgressions when I first tried to contribute my WebAuthn plugin and that was before Joomla made a public statement that it uses vanilla JavaScript in the core...

avatar zero-24
zero-24 - comment - 14 Jul 2021

Maybe a stupid question. Why don't you use forEach instead? Isn't this file going through Babel anyway? Also, why are we using jQuery when the promise is that Joomla core doesn't use it anymore?

This is 3.10 where the migration to native JS has not been done yet. I'm not sure what you mean by "going througth babel anyway"? Just to be clear this patch here is for 3.10 not 4 where native JS will be used once merged up.

avatar nikosdion
nikosdion - comment - 14 Jul 2021

Okay, so you are writing the same thing slightly differently twice, once for Joomla 3.10 and once for Joomla 4.0? Sorry, it just seemed a wasteful approach since you have to write and debug the same thing twice, written differently. If it floats your boat...

avatar zero-24
zero-24 - comment - 14 Jul 2021

As suggested by @nikosdion this now also updated the "more information" css class from important to indo and looks like this than:
image

avatar brianteeman
brianteeman - comment - 14 Jul 2021

Please check the js errors

avatar zero-24
zero-24 - comment - 14 Jul 2021

Restarted drone.

avatar nikosdion nikosdion - test_item - 18 Jul 2021 - Tested successfully
avatar nikosdion
nikosdion - comment - 18 Jul 2021

I have tested this item ✅ successfully on f059223

Tested by installing the upcoming versions of Akeeba Backup Core (8.0.7) and Admin Tools Core (6.1.0). I enabled the actionlog plugins on both extensions; Admin Tools also has a system plugin. I changed libraries/src/Version.php to pretend that Joomla is still 3.10.0-alpha8 to make it show me an update. I am also using private update XML files for my extensions which mark the upcoming versions as Joomla 3.10 compatible.

Before the patch: Joomla tells me the extensions are not compatible with Joomla 3.10 and More Info tells me to upgrade to... the installed version. That was under No Update Required. I also see the scary potential update problem message on both extensions, twice in Admin Tools which has two plugins.

After the patch: WOO-HOO! No more complains from Joomla. It tells me no update is required for these extensions.

I am happy with the way this works now.

Side note: I had a client last week who extracted the package ZIP file and installed each extension separately. Two weeks ago I had a client who was using Discover to install the plugins which appeared uninstalled after some failed database surgery. Right now, the Joomla Update component assumes that all extensions will either belong to a package OR have an update site. In both of these clients' cases — sadly, they are far from being the only ones doing things like that — this will fail. I can foresee a torrent of support requests. My current solution is to have my component check for a missing package extension and if so create it. I also check the extensions listed in a package's manifest and have it ‘adopt’ the ‘orphan’ extensions which appear in the package's manifest. The latter should really be part of Joomla itself, probably as a com_installer view in the same way we have the Database view (and its Fix button). Ideally, each extension should also declare if it is supposed to belong to a package and which package is that so that Joomla Update can provide better context to the user instead of misleading the user into believing the developer is doing something wrong and putting the onus on the developer to provide support for what is a self-inflicted issue as a result of Joomla allowing the user to do things it clearly DOES NOT want them to do. Should I open another issue about this or would you rather email me to request more info so you can discuss it with George?


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

avatar brianteeman
brianteeman - comment - 18 Jul 2021

response to the side note. the same thing happens with languages that are discovered and not installed

avatar zero-24
zero-24 - comment - 19 Jul 2021

@nikosdion please open a new issue for that. So this can be tracked, when you have code feel free to propose a PR too.

avatar rdeutz rdeutz - change - 22 Jul 2021
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-07-22 17:26:35
Closed_By rdeutz
avatar rdeutz rdeutz - close - 22 Jul 2021
avatar rdeutz rdeutz - merge - 22 Jul 2021

Add a Comment

Login with GitHub to post a comment