Merge to HumHub core?

[marc-farre]

I think it's not obvious that updating HumHub core requires installing a module.

In other similar softwares, the updater is part of the core. On Nextcloud, it's even the landing page of the administration settings (it shows the core version, if there are updates and warnings regarding the setup):

I can see some sys admin not updating their HumHub instance because they are not aware of this module (which is not easy to find among the long list of existing modules in the Marketplace).

If this module was included into core, sys admin would automatically receive notifications when a new version is available.

[luke-]

@marc-farre Good point, In principle, I am also in favor of including this module in the core.

The only advantage I see with the module version is that we are a bit more flexible and can quickly publish a new Updater module version if necessary.

But let's merge!

[dreua]

Maybe you can have both? The arguments that @marc-farre describes are all UI/UX related if I understand correctly: Make the admin landing page recognize whether the updater is installed and link to the module installation if it isn't. Once it is installed the page can proceed to notify for updates.

That's just an idea, do whatever seems best and easiest to maintain ;)

[marc-farre]

@dreua I mentioned Nextcloud only to show the importance of having the Updater module included in the core.
But this issue is only about merging this module in HumHub core: https://github.com/humhub/humhub/
It won't change anything in term of UI/UX, the Updater will remain accessible from the Administration left menu.

If you think it's better to display available updates on the Administration landing page, you can open a dedicated issue on https://github.com/humhub/humhub/issues/

[marc-farre]

@luke- I can create 2 PRs

On the core repository:

• Adding updater directory to protected/humhub/modules
• Removing the assets and docs folders
• Updating the module.json file (removing the homepage and humhub sections)
• Adding public $isCoreModule = true; and public $isEnabled = true; in Module.php (allowing disabling the module, e.g. for a SaaS installation)

On this repository:

• Adding a warning on the README file about the migration of this module in the core of HumHub 1.16
• Adding a new line in InstallController::actionInstallFiles() after https://github.com/humhub/updater/blob/master/modules/packageinstaller/controllers/InstallController.php#L82 to delete this module (to avoid having a duplicate)
• Adding a requirements.php preventing from installing the module if HumHub version is 1.16 or later

I'm not quite sure about this last step.
I was thinking something like this:

public function actionInstallFiles()
    {
        /* @var Module $module */
        $module = Yii::$app->getModule('updater');
        $oldModulePath = $module->getBasePath();

        $this->updatePackage->install();

        if (version_compare(Yii::$app->version, '1.16', '>=')) {
            \yii\helpers\FileHelper::removeDirectory($oldModulePath);
            $this->flushCaches();
        }
        
        if (function_exists('opcache_reset')) {
            @opcache_reset();
        }

        return ['status' => 'ok'];
    }

[luke-]

Sounds good.

We should definitely make sure that the ModulManager, from 1.18, no longer loads the updater from the module folder.

Maybe we can then do a cleanup via the core migrations?

[marc-farre]

ModulManager, from 1.18, no longer loads the updater

You mean from HumHub 1.16?

Do you think public bool $preventDuplicatedModules = true; (https://github.com/humhub/humhub/blob/master/protected/humhub/components/ModuleManager.php#L94) is not enough?

cleanup via the core migrations

Yes, you're right, in the case of manual updates.

But we also need the \yii\helpers\FileHelper::removeDirectory($oldModulePath); I suggested adding to InstallController::actionInstallFiles() (see above) otherwise we will have a bug on InstallController::actionMigrate() if we don't, as at this stage the module will be duplicated.

[luke-]

ModulManager, from 1.18, no longer loads the updater

You mean from HumHub 1.16?

Yes, I would include the Upgrader module in either version 1.16 or 1.17. Depending on how reliable the solution is.

Do you think public bool $preventDuplicatedModules = true; (https://github.com/humhub/humhub/blob/master/protected/humhub/components/ModuleManager.php#L94) is not enough?

Not sure, need to look into the code. Ideally, there are no fatal errors or warnings when the "Updater" module exists twice for a while. In addition, the core module should always be loaded.

cleanup via the core migrations

Yes, you're right, in the case of manual updates.

But we also need the \yii\helpers\FileHelper::removeDirectory($oldModulePath); I suggested adding to InstallController::actionInstallFiles() (see above) otherwise we will have a bug on InstallController::actionMigrate() if we don't, as at this stage the module will be duplicated.

Ideally there is no big error, when this module exists twice. But maybe it's also a solution to delete it via updater.

---

I still have to think about the disadvantages of the integrated updater.

For example, we can currently simply replace the updater for all old versions, e.g. to cover special cases. This will then no longer be possible.

[marc-farre]

Another option would be to install it by default when installing HumHub and, if the HumHub version is outdated but the Updater module is not installed, we display a warning somewhere with a link to install the module.

[luke-]

Yes, the solution currently seems more suitable to me.

A separate installer step "Install Updater Module"