search

enonic / xp

Enonic XP
https://enonic.com
GNU General Public License v3.0
202 stars 34 forks source link

Dump migration - Flat page - Migration not executed on all contents #6947

Closed GlennRicaud closed 5 years ago

GlennRicaud commented 5 years ago

There seems to be tests that make the migration not run on most contents:

I agree that it makes no sense to have this page index configs if there are no components but it has always been like this and we should try to have a similar result between a content migrated and a new content. But the main impact and bug is that old configs (page.regions, ...) will not be removed then.

Also it would be good to add a check that the node in question if of type "content" (To not apply this to other node of the CMS repo like issues).

GlennRicaud commented 5 years ago

One more difference now that the migration is executed on everything. It seems that the static index configs set by "PageRegionsProcessor" (components.image.id, ...) are only set if a page is set (same behaviour as in 6.Y) while the migration set them for all contents.

But this is more arguable. Because PageConfigProcessor sets the static ones for all contents to get a unique IndexConfigDocument. But PageRegionsConfigProcessor does not set the static ones for all contents.

IMO, the migration is right here and we should fix PageRegionsConfigProcessor.

GlennRicaud commented 5 years ago

After discussion with Slava.

There has always been a mismatch between the behaviour of PageConfigProcessor and PageRegionsConfigProcessor. One sets configs for all contents while the other checks if the content has a page to set its configs.

Solution: Both should have the same check: "If there are components" (OR "if there is a page" depending at what level you are working)

Update code in DumpUpgrader and PageConfigProcessor: https://github.com/enonic/xp/issues/6958