Allow packages to declare that their child extensions cannot be uninstalled

[mbabker]

Pull Request for Issue #12976 and #13151

Summary of Changes

This will allow a package to declare in its XML manifest that child extensions cannot be uninstalled separately and implements the checks to block this behavior. It also removes the check in the extension manager which prevents language extensions from being uninstalled separately.

To accomplish this, a new <blockChildUninstall> tag is supported and should provide a value that translates to a PHP boolean. The default behavior is <blockChildUninstall>0</blockChildUninstall> which is consistent with the existing behavior since there is no lookup in place. When set to 1 or true, this will block child elements from being removed.

Testing Instructions

Alter a package extension's manifest to include the new tag and test the uninstall behaviors of its individual extensions. This will rely on a successful install that registers the parent package ID to the database as implemented earlier.

Documentation Changes Required

• Document the new tag in docs covering package extension manifests
• Update https://github.com/joomla/schemas with the new tag if we are still maintaining these schemas

[andrepereiradasilva]

one general comment shouldn't the pkg_en-GB manifest have this new xml tag <allowChildUninstall>0</allowChildUninstall>?
https://github.com/joomla/joomla-cms/blob/staging/administrator/manifests/packages/pkg_en-GB.xml

[andrepereiradasilva]

small thing, but couldn't this be reduced to just one if?

if ($this->extension->package_id && !$this->canUninstallPackageChild($this->extension->package_id))

the same for the others.

[andrepereiradasilva]

hum tested with weblinks latest beta (just adding <allowChildUninstall>0</allowChildUninstall> to pkg_weblinks.xml file and ziping again) and didn't seem to work. i was able to uninstall every extensions of the component without uninstalling the package.

[infograf768]

Did not add to a lang pack the new tag <allowChildUninstall>
Tested both languages which have been installed as packs before #12977 and after.
In both cases, trying to delete a site or an admin language is not permitted and instead of the clear message we got before or the new string, we now get Error uninstalling language.

Then I added <allowChildUninstall>1</allowChildUninstall> in the lang pkg, zipped it and installed.
Tried to delete a site or admin language and could not. Same message.

Changed to <allowChildUninstall>0</allowChildUninstall> and same result.

[infograf768]

Concerning language packs, another change as to be done in the model:
See https://github.com/joomla/joomla-cms/blob/staging/administrator/components/com_installer/models/manage.php#L234
By taking off && $row->type != 'language', I now can uninstall a language.

But <allowChildUninstall>0</allowChildUninstall> still does not work.

[Bakual]

I think the whole approach is flawed here.
First I think using an allowChildUninstall tag is the wrong way. Since that is the default behavior I would expect a blockChildUninstall tag instead. It's a bit counterintuitive when you have to add allowChildUninstall="0" to change the behaviour.

But more importantely I don't think it makes sense to have that tag in the package. I think it's a very rare case that you don't want to allow any subpackage to be uninstalled. More often you just want to prevent eg the library or the component to be uninstalled. But the modules and plugins for example are fine to be uninstalled in most cases.

As for the language packs (where this request likely comes from), we're trying to solve the issue from the wrong end. It origins from the fact that you can't reinstall an already installed language from the language installer. Allow that (instead of hiding, show the language as installed and show a "reinstall" button) and you no longer need to block uninstalling language subpacks.
I can do that PR.

[infograf768]

In the mean while my test show here that the culprit comes from

public $allowChildUninstall = true;

if I take off this, then I can prevent an extension from being uninstalled.

[infograf768]

As for the language packs (where this request likely comes from), we're trying to solve the issue from the wrong end. It origins from the fact that you can't reinstall an already installed language from the language installer. Allow that (instead of hiding, show the language as installed and show a "reinstall" button) and you no longer need to block uninstalling language subpacks.

Agree. I still think that one should not be able to uninstall a language pack site or admin lang without the possibility to reinstall.
I am also concerned about multilingual sites where one would, by mistake, uninstall the site lang and thus breaking the site without knowing it, except by the multilangstatus module.

[andrepereiradasilva]

@Bakual we could use the locked as proposed in #13037 for that.

But this pr is about child parent relation so imo a diferent thing.

[Bakual]

Agree. I still think that one should not be able to uninstall a language pack site or admin lang without the possibility to reinstall.

Exactly that. As long as you can not reinstall the language from the language manager, we have to prevent uninstalling it partially. I haven't looked at it yet but I think it should be simple to add a reinstall button in that list. Just need time to do it 😄

I am also concerned about multilingual sites where one would, by mistake, uninstall the site lang and thus breaking the site without knowing it, except by the multilangstatus module.

They can also break their site by uninstalling the whole language pack and by doing a lot of other stupid stuff. You can't prevent them from doing stupid things. As long as we don't prevent them from fixing it again, I have no issues with that.

we could use the locked as proposed in #13037 for that.

Nah, locked is a different thing. It applies to specific core extensions which should not be able to be uninstalled. This here would be for 3rd party stuff. You would have to make that locked state a conditional ("only allow uninstalling if all other extensions of this package are uninstalled as well").

[mbabker]

I'm not making a language package specific fix, I'm trying to create something generally usable for package extensions in general. If you want a language specific fix then feel free to implement it.

But more importantely I don't think it makes sense to have that tag in the package. I think it's a very rare case that you don't want to allow any subpackage to be uninstalled. More often you just want to prevent eg the library or the component to be uninstalled. But the modules and plugins for example are fine to be uninstalled in most cases.

It is up to the distributor to decide that behavior. The current behavior we have a need for is to disallow all children of a package to be uninstalled. If you feel you have a more suitable alternative please propose it.

[mbabker]

Fixed the bugged behavior with the manifest object's instantiation keeping things from working right and changed the tag name to blockChildUninstall. Added it to the English package manifest.

[Bakual]

I'm not making a language package specific fix, I'm trying to create something generally usable for package extensions in general.

If you have a usecase beside the language pack, then I'm fine with it.

changed the tag name to blockChildUninstall.

Thanks 👍

[mbabker]

Personally, I don't. However, we have a very explicitly hardcoded behavior in core right now which doesn't allow individual language packages to be uninstalled; you have to uninstall the package. So if you do something like a discover install (which is what the initial issue report was anyway), you won't have a package extension installed so you can't uninstall the discovered languages.

So, the hardcoded "special" handling for language extensions is removed from the extension manager's MVC layer and integrated into the install adapters as a configurable behavior, creating a side effect of allowing developers to also do the same with their package extensions if they so choose.

[infograf768]

It works OK now for that PR. We still need the reinstall possibility as @Bakual rightfully wrote as otherwise this is no use for languages, causing more issues than fixes.

Remains also to decide what will be our policy for lang packs:
Do we let the TTs individually choose or not to implement the new feature preventing from uninstalling uniquely site or admin lang?
I know I would prefer my French pack to have <blockChildUninstall>true</blockChildUninstall> if this is merged.

Question, unrelated to this PR:
It gives here :
The Frenchfr-FR extension is part of a package which does not allow individual extensions to be uninstalled.
The parenthesis have disappeared for my French site and admin extension name which are French (fr-FR), while they show well for en-GB.

Any idea about that?

[mbabker]

Never claimed this was a "complete" thing for languages, and actually that's a feature above and beyond what I've been working on.

As is, we are in a better shape overall for languages and have given some new features to extension developers. Rightfully the hardcoded handling of languages in the uninstall process has been removed and that logic incorporated into the install routine itself. Fixing the bug originally reported about language extensions not being able to be uninstalled unless you remove the package (which if your install process didn't include it for whatever reason meant it couldn't be removed without FTP and database access).

Please don't let that lack of reinstall be a block to this. IMO it's unrelated and the situation neither improves or worsens with this PR.

[infograf768]

@mbabker
i think this patch is real fine and would not block it at all ( I have anyway no power to block anything 😸 )

indeed allowing or not languages to prevent uninstalling site or admin parts is independant from this.

[Bakual]

@infograf768 If you can reinstall the language, then you don't need this PR at all (for languages) anymore. It would be absolutely fine to allow users to uninstall languages partially.

Please don't let that lack of reinstall be a block to this. IMO it's unrelated and the situation neither improves or worsens with this PR.

It's indeed unrelated.

[infograf768]

I see no reason for any user to uninstall partially a language which has been installed by a registered pack. If someone wants to play with discover and expect the same behavior (almost) as in 1.5, then it is fine grace to this PR. For usual core packs I recommend anyway to use the new tag <blockChildUninstall>true</blockChildUninstall>

[mahagr]

👍

Though... Wouldn't it make sense to add xml attribute which would allow to override the global setting:

<file type="plugin" group="system" id="gantry5" enabled="1" uninstall="0">plg_system_gantry5.zip</file>

I'm not sure if there's a way to enable a plugin by default either -- right now it looks like I'm doing it manually during postflight.

[mbabker]

Feel free to send a PR after this one to add attributes for a per-extension basis. I'm cutting back contributions after my current backlog has cleared.

[infograf768]

I have tested this item ✅ successfully on 9ac5354

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13154.}

[mahagr]

@mbabker Ack. I'll add it to my todo list.

[andrepereiradasilva]

I have tested this item ✅ successfully on 9ac5354

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13154.}

[jeckodevelopment]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13154.}