search

presseddigital / linkit

Linkit Plugin for Craft 4
Other
28 stars 19 forks source link

Craft 2 to 3.1 migration doesn't migrate matrix or super-table fields. #53

Closed roelvanhintum closed 5 years ago

roelvanhintum commented 5 years ago

Looping through the fields in the project.yml isn't sufficient. It only migrates the root fields.

Using Linkit 1.1.8 and Craft 3.1.10

roelvanhintum commented 5 years ago

I think this one was fine, just needed to use uid's. https://github.com/fruitstudios/craft-linkit/blob/1.1.5.1/src/migrations/m180423_175007_linkit_craft2.php#L43

roelvanhintum commented 5 years ago

On top of this, checking the schema version on install prevents the entire migration to run. https://github.com/fruitstudios/craft-linkit/blob/master/src/migrations/Install.php#L36

samhibberd commented 5 years ago

Thanks @roelvanhintum I will take a look at this, we've actioned the migration with projectConfig after talking through the best practice with Craft CMS. Also the Install migration was previously required for sites upgrading from 2 to 3, I will double check this.

roelvanhintum commented 5 years ago

@samhibberd The Install migration still is needed, but it was never firing. That's why i removed the schemaVersion check. I also extended the upgrade logic from install, so it just lives in one place.

bossanova808 commented 5 years ago

Can we get this looked at pretty pronto please?

I don't want to be a nag, but it's really been far too long for a working migration to come along - for a paid plugin, for the current version of Craft. Whilst migrating over a very large Craft 2 site I have had persistent issues with testing our data migrations involving linkit it - for literally months now.

I get that 3.1 had a lot of unintended consequences, but this really need to be fixed for users migrating from Craft 2 to 3.1. We have a lot of data in these fields and use them extensively, as I am sure others do.

Currently during install of the plugin (which I have to trigger manually as for some reason Craft comes to the conclusion this was not installed in our Craft 2 system) I get:

Exception: Undefined index: type (/var/www/vhosts/c3-dev/vendor/fruitstudios/linkit/src/migrations/Install.php:60)
#0 /var/www/vhosts/c3-dev/vendor/fruitstudios/linkit/src/migrations/Install.php(60): yii\base\ErrorHandler->handleError(8, 'Undefined index...', '/var/www/vhosts...', 60, Array)
#1 /var/www/vhosts/c3-dev/vendor/fruitstudios/linkit/src/migrations/Install.php(24): fruitstudios\linkit\migrations\Install->_upgradeFromCraft2()
#2 /var/www/vhosts/c3-dev/vendor/craftcms/cms/src/db/Migration.php(56): fruitstudios\linkit\migrations\Install->safeUp()
#3 /var/www/vhosts/c3-dev/vendor/craftcms/cms/src/db/MigrationManager.php(243): craft\db\Migration->up(true)
#4 /var/www/vhosts/c3-dev/vendor/craftcms/cms/src/base/Plugin.php(152): craft\db\MigrationManager->migrateUp(Object(fruitstudios\linkit\migrations\Install))
#5 /var/www/vhosts/c3-dev/vendor/craftcms/cms/src/services/Plugins.php(520): craft\base\Plugin->install()
#6 /var/www/vhosts/c3-dev/modules/imagescience/console/controllers/UpgradeController.php(288): craft\services\Plugins->installPlugin('linkit')
#7 [internal function]: modules\imagescience\console\controllers\UpgradeController->actionRun()
#8 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array(Array, Array)
#9 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/base/Controller.php(157): yii\base\InlineAction->runWithParams(Array)
#10 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/console/Controller.php(148): yii\base\Controller->runAction('run', Array)
#11 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/base/Module.php(528): yii\console\Controller->runAction('run', Array)
#12 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/console/Application.php(180): yii\base\Module->runAction('imagescience/up...', Array)
#13 /var/www/vhosts/c3-dev/vendor/craftcms/cms/src/console/Application.php(93): yii\console\Application->runAction('imagescience/up...', Array)
#14 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/console/Application.php(147): craft\console\Application->runAction('imagescience/up...', Array)
#15 /var/www/vhosts/c3-dev/vendor/yiisoft/yii2/base/Application.php(386): yii\console\Application->handleRequest(Object(craft\console\Request))
#16 /var/www/vhosts/c3-dev/craft(22): yii\base\Application->run()
#17 {main}
roelvanhintum commented 5 years ago

As of 1.1.9 this is still an issue because nested linkit fields aren't migrated.

samhibberd commented 5 years ago

@roelvanhintum the new support chap Oliver Bon @ Craft, identified the cause of this issue:

A craft user wrote to us about have a problem installing your plugin. Tracking down the issue, it trips on a supertable (verbb) field. I’m contacting the supertable guys too just wanted to share the issue with you too and see if you can help solving it for the customer also :) This guy has a supertable field which, in the project config didn’t save properly and hasn’t got a ‘type' attached to it. linkit checks for field types on install and refuses to install.

I'm hoping this is the same issue, are you able to check your database, make sure the type value is set on all fields before the update, or perhaps reach out to the folks @ Verbb.

If you run into anything else, fell free to open this up again.

jamesmacwhite commented 5 years ago

@samhibberd Throwing into the game on the matrix field side of things. We are using the Linkit field extensively in a Craft 2 project and testing the migration process for Craft 3 I am finding Linkit fields part of a Matrix field or Super Table aren't migrated. For now, lets put aside Super Table for potential further debugging with Verbb, but the matrix field scenario is slightly more important I feel.

So after a Craft 2 -> 3.1 migration, this is what any Linkit field in a Matrix looks like.

image

Updating the field type to Linkit and re-saving works fine and the original URL value seems to be in tact. Is there something on the Craft 2 side I should be doing prior to the migration to help this process?

daniellelecomte commented 5 years ago

Same issue here as @jamesmacwhite but when we change the field type and re-save, all data is lost.

roelvanhintum commented 5 years ago

Looping through the fields in the project.yml only migrates the fields that are not nested in a matrix or something else. The problem is identical for matrix and super table, each of those have inside their definition al list of fields they contain. That is why i choose the database approach, it migrates all fields.

jamesmacwhite commented 5 years ago

Hi @roelvanhintum. Are you saying there is a workaround to the lack of migration for Matrix/Super Table field definitions that contain a Linkit field within?

roelvanhintum commented 5 years ago

@jamesmacwhite Yes, i created a pull-request for it. Please, make a database backup before running! Just to be sure. It is pull https://github.com/fruitstudios/craft-linkit/pull/54

jamesmacwhite commented 5 years ago

@roelvanhintum Many thanks! Wasn't aware of this. I'm happy to test it out. I'd rather have some extra hoops to have the fields migrated than have to manually adjust the many Matrix fields!

bossanova808 commented 5 years ago

@samhibberd Do you ever actually plan on fixing this properly? There is an open PR with the solution, you really just have to merge it and cut a release. It's kind of infuriating at this point, and the first and only time I find myself wondering about how to get a refund for a plugin...

samhibberd commented 5 years ago

@bossanova808 sorry for the delay folks, i've been away and the agency is stacked, no excuse I know but back on everything today.

samhibberd commented 5 years ago

I have rewritten the migration again, see 1.1.10, which should address this in the Craft recommenced way (thanks @andris-sevcenko), and also addresses an edge case where Linkit is installed before SuperTable during the Craft 2 - 3 upgrade process.