Migrations in point upgrades need to commit the UDA files when they run as part of an Auto Upgrade on Umbraco Cloud.

[marcemarc]

Issue description

During the recent automatic upgrades on our cloud sites from v16.2.0 to v16.3.0, the CMS upgrade ran the following two Migrations:

https://github.com/umbraco/Umbraco-CMS/blob/9ad4a7eeba2472cff192860f7f3064fa6ba0097d/src/Umbraco.Infrastructure/Migrations/Upgrade/V_16_3_0/MigrateMediaTypeLabelProperties.cs
https://github.com/umbraco/Umbraco-CMS/blob/9ad4a7eeba2472cff192860f7f3064fa6ba0097d/src/Umbraco.Infrastructure/Migrations/Upgrade/V_16_3_0/AddRichTextEditorCapabilities.cs

These migrations 'changed' the DataType configuration for MediaTypeLabel and RichTextEditor in the Database...

... all good ...

Except

The UDA files that Umbraco Deploy would create if you had manually made those changes, were not commited to the underlying git repository (likely Umbraco Deploy listens for notifications/events to create the UDA files when things change).

As this occurs in a Production environment, you then have differences between the database and the UDA files.

Therefore when you next deploy from staging or development, the UDA files deployed from the cloud git repo, revert the changes made in the migration!

The only way I can think to keep on top of these kinds of changes between point releases, is to put a diary entry into the calendar on point release day, to make sure you manually trigger a full export of uda files into git, to make sure any changes are harvested, before any other deployments between environments take place... however this does take the shine off of the key selling point of Cloud, of auto upgrades! :-P

Could Umbraco Cloud Auto Upgrader trigger an Export of UDA files in this scenario? - every time it autoupgrades? or perhaps only when a Migration has taken place??

Then the changes would be in the git repo, and prevent them from being overriden.

[github-actions]

Hi there @marcemarc!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[AndyButland]

Have moved this to the CMS tracker as probably we need something there before this could be resolved. Will add a few notes.

One problem here is that notifications are supressed during the migration plan, and looks like that is done for good reason (see MigrationPlanExecutor):

// We want to suppress scope (service, etc...) notifications during a migration plan
// execution. This is because if a package that doesn't have their migration plan
// executed is listening to service notifications to perform some persistence logic,
// that packages notification handlers may explode because that package isn't fully installed yet.

But that means Deploy isn't going to respond to, for example, a DataTypeSavedNotification in order to flush the changes to the .uda files.

I see we have an MigrationPlansExecutedNotification that perhaps Deploy could hook into and run a full extract of the .uda files if any migration plan has run. However I believe this only runs for packages, not core migrations.

So perhaps a solution is to:

• Ensure we run this too on core migrations.
• Allow migration steps to register entities they have updated - e.g. with a collection of Udis. That would allow Deploy to target refreshing just those entities that have been updated, and not have to refresh everything "just in case".

[Zeegaan]

@AndyButland Just to add my 2 cents here, I think deploy listening to the MigrationPlansExecutedNotification is the correct way to handle this. Adding a collection of changes UDI's seems unneccesarily complicated to me, and adds either a TON of refactoring needed for backwards compatibility, or adds more work when making new migrations in the future 🙈

So in summary, I honestly think this is more of a deploy issue than a CMS issue 😁 But if they need something to handle this case, we will of course have to fix it 😄