Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run migrations fully when reenabling an app #29780

Merged

Conversation

nickvergessen
Copy link
Member

Steps

  1. Enable an app via occ
  2. Disable it via occ
  3. Add a breakpoint in the patched place
  4. Enable it via occ

With the patch

The migrations would be executed with pre/post steps

Without the patch

The migrations are only run in schema-mode

Regression from #24039

@nickvergessen
Copy link
Member Author

/backport to stable23

@nickvergessen
Copy link
Member Author

/backport to stable22

@nickvergessen
Copy link
Member Author

/backport to stable21

$ms = new MigrationService($app, \OC::$server->get(Connection::class));
$ms->migrate('latest', true);
$previousVersion = $config->getAppValue($app, 'installed_version', false);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code scanning - InvalidArgument - Error
Argument 3 of OCP\IConfig::getAppValue cannot be false, string value expected

While that is right with the docs, it's wrong with the signatures of the methods and actually used like this in several places, including same file line 147. So I vote to dismiss the warning here

@nickvergessen
Copy link
Member Author

So how can I solve Code scanning results / Psalm Failing after 2s
I ran composer run psalm:update-baseline locally and there is no diff. Also composer run psalm passes.

Also as mentioned, the issue is already in the code, but the baseline does not have an InvalidArgument error for it:

  <file src="lib/private/Installer.php">
    <FalsableReturnStatement occurrences="1">
      <code>false</code>
    </FalsableReturnStatement>
    <InvalidArrayOffset occurrences="2">
      <code>$app['path']</code>
      <code>$app['path']</code>
    </InvalidArrayOffset>
    <NullArgument occurrences="1">
      <code>null</code>
    </NullArgument>
    <RedundantCondition occurrences="1">
      <code>$archive</code>
    </RedundantCondition>
  </file>

So I guess we just merge?

Signed-off-by: Joas Schilling <coding@schilljs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants