? ? Failure

User tests: Successful: Unsuccessful:

avatar pe7er
pe7er
3 Jun 2017

Pull Request to distinguish 3rd party extensions that do NOT support the Joomla Extension Update

Summary of Changes

The JED lists for all extensions if they support the Joomla Extension Update.
However, you cannot see if the extensions you've installed in your Joomla site support the Joomla Extension Update. Maybe the JED lists some extensions that they support the Update, but you are using older versions in your site that do not support it.

To make it easier to see which extensions support the Joomla Extension Update, I've added a warning for 3rd party extensions that do not support it.

Testing Instructions

Install a 3rd party extension that uses the Joomla Update, for example
https://db8.eu/download/component/db8-optimize-site

Install this patch.

Go to Extensions: Manage. You should see the installed 3rd party extension.

screen-1

Go to Extensions: Update Sites. Remove the reference for the installed 3rd party extension.

screen-2

screen-3

Go to Extensions: Manage. You should see the installed 3rd party extension plus a message that it does not support Joomla update

screen-4

The extension did support the Joomla Update, but the reference is missing in the Update Sites and not checked anymore. To enable it again:
Go to Extensions: Update Sites. Choose Rebuild.

Go to Extensions: Manage. You should see the installed 3rd party extension without the warning message.

avatar pe7er pe7er - open - 3 Jun 2017
avatar pe7er pe7er - change - 3 Jun 2017
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 3 Jun 2017
Category Administration com_installer Language & Strings
avatar pe7er pe7er - change - 3 Jun 2017
Labels Added: ? ?
avatar pe7er
pe7er - comment - 3 Jun 2017

Oops, I used tabs but my reinstalled IDE converted them to non visible spaces :-(
Thank you @Quy + @richard67 for your comments!
I've made all the changes that you've recommended.

avatar AlexRed
AlexRed - comment - 4 Jun 2017

Good idea, but if an extension is a Package the update sites is in the package for all the extension part (plugin, component, module ecc...) , but are reported with no update sites like the language package:
pack-update-sites
or JCE or Akeeba Backup

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 4 Jun 2017

@pe7er is this PR ready for test?

avatar richard67
richard67 - comment - 7 Jun 2017

@wilsonge What is correct now to check for core extensions? Item ID 999 like done in this PR here? Or 9999 like done in PR #16494 ?
I ask you because the other PR seems to be based on your input.


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

avatar richard67
richard67 - comment - 7 Jun 2017

@pe7er Maybe you should change the 999 to 9999 for check of the extension id, see #16494 (comment).


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

avatar richard67
richard67 - comment - 7 Jun 2017

The sidenote in the comment I mentioned above makes me think it is not really save to use the ID number to check if an extension is core or not. But what else way do we have beside a hard-coded list which would have to be maintained every time an extension is added (and so would be a shit)?


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

avatar richard67
richard67 - comment - 10 Jun 2017

Regarding the question on how to detect if an extension is core or not which I mentioned in my comment above (999 or 9999), this PR here should not wait for a solution.

Let it continue to use the 999.

Any later PR implementing a safe check will also have to look for such things and replace them by using the new way which it implements.

But this PR needs 2 changes to be good:

  1. Change text COM_INSTALLER_UPDATE_NOT_SUPPORTED from "Update Sites not unavailable" to "Update Sites not available".
  2. Change the database query so it excludes extensions where the package id is set and which are not of type package from the check, so for these no warning is shown, or alternatively use the update site of the package for checking those extensions so the status of the update site of their package is shown.
    This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16474.
avatar AndyGaskell
AndyGaskell - comment - 14 Jun 2017

This sounds a really nice feature, couple of very minor comments on the "Update Sites not unavailable" text....

  1. I would think perhaps the sites should not be capitalised, so "Update sites not unavailable".
  2. Are there usually multiple update sites? I've only ever set up a single update site, I thought that was the normal way to do it. In which case it should perhaps be "site" rather than "sites", so "Update site not unavailable".

These are very minor comments, and no problem at all if folk disagree.

Thanks :)

avatar richard67
richard67 - comment - 15 Jun 2017

Does nobody here notice that "not unavailable" is a mistake and it should either be "unavailable" or "not available"??? Do you sleep while reading?

avatar AndyGaskell
AndyGaskell - comment - 15 Jun 2017

I think @richard67 that you're probably right, may have been sleep reading there.

I'd say, on reflection, that double negatives tend to be confusing, so you're right, "unavailable" or "not available" would be better wording.

Also, wording aside "not unavailable" is, I guess not the intended description.

avatar richard67
richard67 - comment - 15 Jun 2017

@AndyGaskell Well, at the beginning the text was "unavailable", but to me "not available" sounded better so I suggested that, and @pe7er wanted to follow my suggestion but maybe missunderstood me or understood me right but made a mistake, and so he changed it to "not unavailable", but this negates the original meaning and so displays wrong information.

avatar Quy
Quy - comment - 15 Jun 2017

@richard67 You can submit a PR to his branch to be merged.

avatar richard67
richard67 - comment - 15 Jun 2017

I know, if he wants I can do that later.

avatar richard67
richard67 - comment - 15 Jun 2017

I will have to do that anyway if my PR with the new extensions helper class will be merged before this one.

avatar richard67
richard67 - comment - 15 Jun 2017

@pe7er I created PR no. 1 in your repository for your branch "supportsupdate". Please check and accept.

avatar richard67
richard67 - comment - 1 Jul 2017

@pe7er What remains to be done in my point of view is what @AlexRed wrote in his comment above:
If the extensions belong to a package, which means the "Package ID" column is not empty, the status of the package's update site has to be shown and not the one of the extension. This can be done by modifying the SQL select statement in file administrator/components/com_installer/models/manage.php by joining the update_sites_extensions table a second time, this time to the package_id column, and then checking the update_site_id of the package if it is set, otherwise the one of the extension. I am working on a PR to your repo for that.


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

avatar richard67
richard67 - comment - 1 Jul 2017

@pe7er Another thing is the check to exclude core extensions. I think it is not necessary to exclude all core extensions because the only core extensions which have update sites are those which can be excluded like it is done here: https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_installer/models/updatesites.php#L382.

All other core extensions have update sites ID = NULL when joining it with the update sites table for the extension itself or the package they belong to.

So you can say in your query you exclude those (type/element): 'file'/'joomla', 'package'/'pkg_en-GB', 'component'/'com_joomlaupdate'.

I will try to do that in my PR to your repo which I mentioned in my previous comment.


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

avatar richard67
richard67 - comment - 1 Jul 2017

@pe7er Check PR pe7er#20 against your repo. It solves the problem mentioned by @AlexRed .

The other thing with core extensions check I will see what I can do.


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

avatar richard67
richard67 - comment - 1 Jul 2017

@pe7er Regarding the core extensions I think it is really necessary to have my PR #16724 merged an then use it instead of checking for extention_id > 999. But for now I think it would be sufficient if you just change the 999 to 9999 so it fits to the auto increment of the extensions table (which starts at 10000 so everything below is a core extension for sure, and only core extensions which have been discovered by the discovery installer function will have larger IDs). I will add that as a commit to my previously mentioned PR to your repo.


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

avatar richard67
richard67 - comment - 5 Jul 2017

@pe7er I added now a commit to my PR for your repository which replaces the unsafe check for extension_id > 999 by using the new JExtensionHelper class.

I haven't tested it yet here, but I will soon and let you know about the result. Of course if you test faster, you are welcome.

avatar richard67
richard67 - comment - 5 Jul 2017

@pe7er Meanwhile I have tested my changes in my PR for your repo, and I corrected a silly mistake. Now it is ok. Core extensions are excluded now also if they have extension ID greater than 999 (or more), and for extensions which have a parent package which has an update site (e.g. German language pack), the warning is not shown.

So if you accept and merge my PR into your repo, I can test this one here with success.

But you might have to fix code style of my PR: The line with the condition in the view for check if the warning has to be displayed is a bit long after my changes, so you may have to do some line break.


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

avatar brianteeman
brianteeman - comment - 9 Sep 2017

@pe7er have you merged the changes from @richard67

avatar pe7er
pe7er - comment - 28 May 2018

@brianteeman I just merged @richard67 his code. Sorry for my delay!

avatar brianteeman
brianteeman - comment - 28 May 2018

@pe7er thanks - lets hope we can get it tested and merged now

avatar richard67
richard67 - comment - 28 May 2018

I've just tested and noticed following:

  1. If an update site is existing but disabled, it is not counted as missing, i.e. the badge is not shown in this case. It is only shown if the update site is really missing in database. @pe7er Is this by purpose? The testing instructions read as if it was by purpose. If so, then this observation is not an issue.

  2. For components like e.g. OSMap which come with a component and a plugin and these do not belong to any parent package, this PR does not work. But I think that's by purpose, because I do not see any chance to find out in database that the OSMap content plugin is shipped with the OSMap component. The only chance would be to parse the component's manifest XML. Or do I miss something? If I don't miss something, then this 2nd observation is not an issue.

If both these observations turn out to be ok, i.e. not an issue with this PR, then I can mark my test result positive.

But because of the 2nd observation I think the badges intruduced with this PR could confuse the user, as it confused me at a first view.

@pe7er @brianteeman @Quy @AlexRed @AndyGaskell what do you think?


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

avatar wojsmol
wojsmol - comment - 28 May 2018

@richard67 Regarding your observation number 2. This can be determined by checking value of package_id in #__extensions table.

avatar richard67
richard67 - comment - 28 May 2018

@wojsmol No, as I wrote, the OSMap stuff is organised differently, it does not use the package_id for its content plugin. What you write, checking the package_id column, is already implemented in this PR here (and I was the one who suggested that change, see history above).

avatar richard67
richard67 - comment - 28 May 2018

@wojsmol Anyway, if there are not more components having the same problem as OSMap and its content plugin, then it is an OSMap issue, and this PR is ok regarding that. But maybe there are other components which do it the same way, I don't know.

avatar richard67
richard67 - comment - 28 May 2018

I have tested this item successfully on 72fe81b

This PR works as described.

What it does not do is to show the badge if an update site is present in database but is disabled. It only shows the badge if the update site is missing in database. But I think this is as intended by this PR, so I do not count this as an error.

What this PR also does not do is not show the badge e.g. for plugins or modules which are installed with a component and not with a package, so the package_id column is not set in the extensions table in database, but the plugin or module is updated together with the component by the online update, like it is e.g. with the OSMap Component and its content plugin. But this is also not within the scope of this PR as I understand it, and so I don't count this as an error.

On the other hand, always showing the badge for those modules or plugins could confuse people and so maybe cause support requests in forums and issues in the tracker. The only chance to proper handle such stuff would be to parse the component's manifest XML, as far as I see. Or we can make component creators make their stuff be delivered in a package or set the package_id column at least.

Maybe the PLT or some regular maintainers should have a look on it and decide if that is ok or not.


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

avatar richard67 richard67 - test_item - 28 May 2018 - Tested successfully
avatar mbabker
mbabker - comment - 28 May 2018

Or we can make component creators make their stuff be delivered in a package or set the package_id column at least.

Extension developers can't be forced to repackage their extensions because of us. We can't account for every possible distribution model nor should we, so this scenario can be thrown into a list of known issues with a "won't fix" label.

avatar mbabker
mbabker - comment - 21 Jul 2018

Rethinking this it actually feels like it could be problematic or send mixed signals for extensions which are one off custom solutions for a site or client. For the joomla.org backend we'd have several sites where this error badge would show up in error but it's not really a problem because these are our extensions and we aren't publishing update servers for them. There's a part of me that almost wants to suggest closing this without accepting it between that and the list of potential issues with how different extensions are packaged.

If there's an alternative that doesn't make it come across as an extension is broken or otherwise bad because it doesn't have an update site, that should be considered. But looking fresh on this I honestly don't feel like this is a good idea.

avatar pe7er pe7er - change - 25 Sep 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-09-25 13:46:16
Closed_By pe7er
avatar pe7er pe7er - close - 25 Sep 2018

Add a Comment

Login with GitHub to post a comment