Enable conditional migrations through when callback

[clarkwinkelmann]

Fixes #0000

Changes proposed in this pull request:
A new feature that allows extensions to skip database migrations while keeping the ability to execute them again later.

Since Flarum always attempts running all migrations for all extensions on each extension activation, this works well to add tables, columns and constraints that are only needed with optional dependencies.

This cannot be achieved with regular migrations because not doing anything in the up callback will still get the migration logged as run and will never be attempted again. And extensions can't easily affect the migration repository to invalidate the recent migration since it's only updated at a later point after the migration callback has run.

This feature only applies to up migrations. down migrations don't need the ability to be skipped. If the thing to roll down no longer exists, migrations are already expected to become a no-op and will then successfully be marked as run down. Also, they will only run down if they successfully ran up before.

This feature has been successfully implemented in flamarkt/backoffice and used by flamarkt/taxonomies since March 2022. Implementing it in core will enable more extensions to use this feature without the need to add flamarkt/backoffice as a dependency.

Since there is intention to use this for the mentions-tags optional integration, it makes sense to put it in core.

Example implementation in a migration https://github.com/flamarkt/taxonomies/blob/main/migrations/20210401_000400_create_product_term_table.php

Reviewers should focus on:
Other options considered were to use a special return value in the up callback or throwing a special exception from up. But the dedicated when callback is the option I find most elegant.

I have dropped the protected method resolveAndRunClosureMigration that was introduced very recently in #3664 because it was making things more complex than necessary since we need the ability to return early in the runUp method.

Screenshot
The only part visible to the user is that the "Skipped" message will be shown every time they run migrate if the migration conditions are not met:

Necessity

• Has the problem that is being solved here been clearly explained?
• If applicable, have various options for solving this problem been considered?
• For core PRs, does this need to be in core, or could it be in an extension?
• Are we willing to maintain this for years / potentially forever?

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).
• Core developer confirmed locally this works as intended.
• Tests have been added, or are not appropriate here.

Required changes:

• Related documentation PR: Add documentation for conditional migrations docs#442

[imorland]

Checked this out locally, works a treat, thank you very much for this Clark!

[clarkwinkelmann]

PR for the documentation: flarum/docs#442

While working on the docs, I realized 2 potential issues:

• I don't actually think Flarum runs migrations for other extensions when you enable an extension, so you are still required to run php flarum migrate or disable+re-enable the other extension for the conditional migration to apply.
• If you rollback the other extension, 2 things could happen:
  • Your changes are lost and the conditional migration will never run again even if the other extension is re-installed (for example, if you added a column to a table that has now been deleted and re-created).
  • If you added a database constraint (like a foreign key), the other extension might fail to rollback and result in a broken state.

EDIT:

And yet another thing I haven't really tested, what order do the migrations run during forum installation? Could it cause an issue?

I think php flarum migrate order is fine because it's affected by the optional dependencies priority.

[luceos]

Do we need backend tests for this?

[SychO9]

Let's not merge yet

[SychO9]

While working on the docs, I realized 2 potential issues:

• I don't actually think Flarum runs migrations for other extensions when you enable an extension, so you are still required to run php flarum migrate or disable+re-enable the other extension for the conditional migration to apply.
• If you rollback the other extension, 2 things could happen:
  Your changes are lost and the conditional migration will never run again even if the other extension is re-installed (for example,
  • if you added a column to a table that has now been deleted and re-created).
  • If you added a database constraint (like a foreign key), the other extension might fail to rollback and result in a broken state.

Just got around to reading this. It seems like it might make things more difficult then 🤔. We should come up with solutions to these before implementing it.

• I don't actually think Flarum runs migrations for other extensions when you enable an extension, so you are still required to run php flarum migrate or disable+re-enable the other extension for the conditional migration to apply.

For this, we could switch to running all migrations instead, which would pick up new migrations for the current ext and dependent ones.

• if you added a column to a table that has now been deleted and re-created).
• If you added a database constraint (like a foreign key), the other extension might fail to rollback and result in a broken state.

I'm not sure if we can selectively rollback dependent migrations with the current logic 🤔

And yet another thing I haven't really tested, what order do the migrations run during forum installation? Could it cause an issue?

Since 1.6 they run in the same order as php flarum migrate. See #3655.

[SychO9]

Closing until we can look into it in 2.0