Follow-up from #2562073: Filter format migrations need to handle php_code filter.
At the moment, the filter format migrations will mishandle the php_code filter -- when a format that uses said filter is migrated, the migration throws an exception and the format may not be imported. There is a more graceful way to handle this:
An alternate approach is to implement a process plugin which will either preserve the php_code filter (if the PHP module is installed), or set it to filter_null if not, with a message. Slightly more work, but also slightly less disruptive to the user.
Let's do that.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 2565715-6.patch | 3.96 KB | phenaproxima |
| #3 | 2565715-3.patch | 3.97 KB | phenaproxima |
Comments
Comment #2
webchickYes, it would be nice to support https://www.drupal.org/project/php if it's available (and/or recommend it in the UI?)
Comment #3
phenaproximaInitial stab...
Comment #6
phenaproximaAnd that, kids, is why you always check your namespaces.
Comment #7
dawehnerI think core should not support an upgrade path of a contrib module. They could swap out this particular plugin, if they actually like to.
Comment #8
phenaproximaThe PHP module occupies the same grey area as the Profile module. Both were in core until Drupal 8, which to me means that Migrate does need to make some effort to upgrade any data they stored. Otherwise people who used these core features will be in for breakages, confusion, and disillusionment. I don't think we should throw people to the proverbial lions if it's trivial for us to make at least an attempt to migrate their formerly-in-core data.
Comment #9
mikeryanJust to put in my two cents - I see dawehner's point, ideally the contrib PHP module (note: last commit August 30, 2013) would alter the migrations to get that filter through instead of defaulting to filter_null. I'm not sure there's a good mechanism for it to do that how - we could hook_entity_load() the migrations to alter them, but that might cause trouble for someone who's customized the migration configuration. Actually, this is not the first time I've imagined a hook_migrate_template_alter(), to futz around with migration configuration at template load time...
The argument to just go ahead and do it here is, despite best practices a *lot* of Drupal 6 sites are using php_code, and I think it is necessary that this migration be supported, either here or in the php module. So, I'd hold off on committing this but leave it open unless/until the PHP module does it.
I've created an issue in the PHP queue: #2570365: Support migration of php_code filters from Drupal 6/7
Comment #10
dawehnerThere is another point I want to make regarding the PHP module support. Do you really believe the code will work afterwards, when yo copy it from d6/d7 to d8? If it does, you had just about a FREAKING LOT of luck, well, and otherwise you result into a PHP fatal. I guess we wanna be honest and better not crash the site.
Comment #11
mikeryanYep, good point there...
Comment #12
phenaproxima*stunned silence*
@dawehner is 100% right. An empty body field may scare some people, but it's far, far better than a fatal or WSOD. It's highly unlikely that any random snippet of PHP code written for D6 or D7 will execute happily on D8. Even if there are no fatals or exceptions, it will likely not produce the expected output.
I'm convinced -- this is something we should not migrate. A Pandora's Box we should not open. I will add it to the list of known migration issues.
Comment #13
phenaproximaThis also needs to be fixed in the d7_block and d6_block migrations, which do try to support PHP visibility code if the PHP module is installed. PHP code for visibility should simply be dropped.