MigrationFailed on `develop`

luke- commented 9 months ago

Seems the recently introduce MigrateService is breaking the REST Tests: https://github.com/humhub/humhub/commit/4d142b002b4e681da4885b75569077bf663bceb9

See: https://github.com/humhub/rest/actions/runs/7235023663/job/19711995483

luke- commented 9 months ago

@martin-rueegg Any idea?

martin-rueegg commented 9 months ago

Is it normal that the installation of the module has this Ø sign?

The test in the tasks module does not have that. Are you sure it is related?

martin-rueegg commented 9 months ago

@luke- how would I run those tests locally? Would I need to install the REST module?

luke- commented 9 months ago

The Checkout Rest Module Steps are required for modules, which implement an own REST API e.g. for tasks Module. The rest module itself doesn't need this. So these steps are skipped here.

I expect that the tests from the REST module can be executed like other tests.

The REST Module Tests seems to failing with:

So my first guess where the recent develop MigrationService changes.

If you want to execute the REST Tests of e.g. the tasks module, some additional steps are neccessary. e.g. https://github.com/humhub/actions/blob/main/.github/workflows/module-tests-develop.yml#L152

martin-rueegg commented 9 months ago

At least, that's another issue. Because if I search for "REST" in the marketplace (on my local installation) it fails.

martin-rueegg commented 9 months ago

Installation of the REST API module works locally. But then activation fails with the same error as in the test:

Invalid Configuration – [yii\base\InvalidConfigException](https://www.yiiframework.com/doc-2.0/yii-base-invalidconfigexception.html)
Migration failed!

    1. in /shared/httpd/humhub/htdocs/protected/humhub/services/MigrationService.php at line 265
    256257258259260261262263264265266267268269270271272273274

            }

            $this->trigger(self::EVENT_AFTER_MIGRATION, $result);

            /** @see \yii\console\controllers\BaseMigrateController::actionUp() */
            if ($result->result > ExitCode::OK) {
                $errorMessage = "Migration failed!";

                if (YII_DEBUG) {
                    throw new InvalidConfigException($errorMessage);
                }

                Yii::error($errorMessage, $this->module->id);

                return false;
            }

            return true;
        }

martin-rueegg commented 9 months ago

@luke- I might have found something in the MigrateController. But again, that's not the main issue here.

The following line causes an exception during migration: https://github.com/humhub/rest/blob/1c65540baaf732b7cd71d8a13d66251d3f47ca14/migrations/m230401_174208_add_allow_jwt_auth.php#L13

Exception: Trying to get property 'settings' of non-object

which kinda makes sense since the module is not yet enabled: https://github.com/humhub/rest/blob/1c65540baaf732b7cd71d8a13d66251d3f47ca14/migrations/m230401_174208_add_allow_jwt_auth.php#L11

I guess this has to do with

As previously, the module was temporarily "enabled" (for the Yii:$app->getModule() and the Yii::$app->moduleManager) and then disabled again. So this made the module available during migration.

So we should either

revert and enable the module first and disable it again upon migration errors
or adapt migrations accordingly.

martin-rueegg commented 9 months ago

Does not solve the problem yet, but improves error message:

https://github.com/humhub/humhub/pull/6770

Further adjustments possible as per my previous comment.

luke- commented 9 months ago

@martin-rueegg Thank you for your investigation.

For now, I have reintroduced the ModuleManager::disable() in case of error as you originally suggested. https://github.com/humhub/humhub/commit/7cd97f0376d65c14750138545a693b7d6c120ad7

Background for Hotfix: All module tests run automatically at the weekend and I want to see whether develop is green now.

humhub / rest

MigrationFailed on `develop` #148