Lack of interest addon.json "require" isn't checked on enable

[DragonByte Tech]

So I realise that this is somewhat working as intended just now, but I'm reporting it as a bug because it can have some significantly detrimental effects. At the moment, it is possible to disable an add-on that other add-ons depend on. The end user has absolutely no way of knowing this has happened.

Bad example because the title makes it semi-obvious here, but you get the idea.

Needed changes

• If you disable an add-on, every other add-on that has this add-on in the "require" param should also be disabled*
• If you enable an add-on, the \XF\AddOn\AddOn::checkRequirements() should be called, at the very least

* It may be worth creating some form of "add-on dependency" table. This would save having to loop through every single add-on and re-parse their addon.json file to check for dependencies.

A proposed table structure:

CREATE TABLE `xf_addon_dependency` (
  `addon_id` varbinary(50) NOT NULL DEFAULT '',
  `dependency` varbinary(50) NOT NULL DEFAULT '' COMMENT 'Could be another addon_id or anything else addon.json accepts as "require" key',
  `version_string` varchar(30) NOT NULL DEFAULT '',
  `version_id` int(10) NOT NULL DEFAULT 0,
  PRIMARY KEY (`addon_id`,`dependency`)
)

During installation, the only change necessary that I can think of would be to run REPLACE INTO on the parsed requirements, as dependencies are already checked during installation.

When an addon is disabled, you could check against this table for any other addons that has this listed as dependency. The popup could then say "The following addons depend on this addon. Disabling this addon will also disable these addons." or something to that effect.

When attempting to enable an add-on, you could check against this table for any dependencies this addon has listed. If every dependency is satisfied, the enable could go ahead as it does just now. If a dependent addon is installed but not enabled, the popup could then say "This addon depends on the following addons. Enabling this addon will also enable these addons." or something to that effect.


Fillip

 

[Chris D]

Yeah this isn't a bug at this stage, and some of the proposed changes are fairly significant so we'll consider it in the future.

The requirements system was only really designed with installation/upgrade requirements checking in mind. We'd still recommend ensuring your code is resilient against the requirements changing, where feasible.

Another tip is the AbstractSetup class has an onActiveChange method, so you may be able to do something there, e.g. if eCommerce is disabled its onActiveChange method could make the Tickets add-on disabled or something like that.

 

[Chris D]

If you enable an add-on, the \XF\AddOn\AddOn::checkRequirements() should be called, at the very least

On reflection, this is probably worth doing.