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

#53 Don't use craft's project.yml for migrations. #54

Closed
wants to merge 4 commits into from
Closed

Conversation

roelvanhintum
Copy link

Restores support for Matrix and Supertable.

@samhibberd
Copy link
Collaborator

Thanks very much for this @roelvanhintum appreciate the time you spent on it, i'm going to make the changes directly, there are a few situations not covered (there were a selection of different field / plugin handles for the craft 2 link it) and i'm also going to leave the project config logic as advised by @andris over at Craft.

@andris-sevcenko
Copy link

andris-sevcenko commented Mar 19, 2019

You definitely should not just be removing rows from the database (e207ea0#diff-eb4ab0584b5426ababdb84f2ffadf519R38)

The purpose of looping through project configuration and removing it from there as well because updating from Craft 2 to Craft 3.1 would automatically put the incompatible plugins in the project config as well. Just removing them from the database would cause Craft to choke when trying to load the corresponding rows from the plugin table for such installations (C2 -> C3.1)

Also, updating the settings straight in the database (https://github.com/born05/craft-linkit/blob/8bc9524c7c1a26136371ef65f5063e8f940a3176/src/migrations/Install.php#L52-L55) means that Project Config won't be aware to those changes, leading to project configuration migration problems down the road because the settings in the fields table don't reflect the settings in project config.


The strongly recommended approach is muting Project Config events and making the same changes in Project Config and database. One of the reasons for doing that over just using services that would populate the changes is to avoid conflicts in scenarios when you're running a migration AND applying project config changes at the same time.


To sum up: to fix #53, the migration that updates the fields should select all the Link It fields from the database, loop through those and look at the context. If the context is "global", then update them in the fields project config key, if it's something else, then figure out where to update that. And, of course, update it in the DB.

@samhibberd
Copy link
Collaborator

Thanks for the input @andris-sevcenko actioning this now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants