Right now, the d6_filter_format and d7_filter_format migrations try to migrate the php_code filter during format migration. This filter doesn't exist in Drupal 8 (and rightfully so), but they try anyway, which results in PluginNotFoundExceptions being thrown.
Both migrations should explicitly map php_code to filter_null. This will cause any node/comment/text field which uses php_code to display nothing until the user installs the PHP module. But I think that's preferable to throwing exceptions and potentially causing unpredictable side effects further down the line.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | Screen Shot 2015-09-11 at 1.46.44 PM.png | 61.95 KB | webchick |
| #21 | Screen Shot 2015-09-11 at 1.45.33 PM.png | 21.16 KB | webchick |
| #18 | 2562073-18.patch | 253 KB | phenaproxima |
| #14 | 2562073-14.patch | 246.41 KB | phenaproxima |
| #7 | interdiff-2562073-4-7.txt | 3.54 KB | phenaproxima |
Comments
Comment #2
phenaproximaComment #3
phenaproximaInitial patch.
Comment #4
phenaproximaWith tests. Sorry about the size of the patch -- I had to add a php_code format to both the D6 and D7 dump files, so a lot of extra crap was generated.
Comment #7
phenaproximaDerp.
Comment #8
mikeryanSo, out-of-the-box someone using the php_code filter is going to run the upgrade process and not see their content - major WTF. There needs to be communication... somewhere... that they need to either install the contrib PHP module or (better but unlikely) stop using PHP in content. Most likely there should be an alert up front in the upgrade process - but should there also be something during the migration - "This format will not work without the contrib PHP module"?
Comment #9
phenaproximaI agree. Things were always broken with regard to php_code format handling -- it would just throw an exception in the migration and the user would later be greeted with the same WTF condition.
I propose that the Filter module should implement an event listener so that, before a row from either filter_format migration is saved, it sets a message or something explaining what happened. We can leverage the Migrate event system and keep everything nice and contained in the Filter module. But this should be a follow-up issue.
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. Either way, it's a follow-up.
Comment #10
mikeryan+1 to the process plugin approach.
Comment #11
phenaproximaCreated follow-up: #2565715: php_code filter should be changed to filter_null if PHP module is not present
Comment #12
mikeryanOne might quibble on extra cleanup in the role test, but fine with me...
Comment #13
alexpottNeeds a reroll.
Comment #14
phenaproximaHave at me, testbot.
Comment #15
mikeryanComment #17
phenaproximaComment #18
phenaproximaRe-rolled.
Comment #19
phenaproximaDrupalCI approves, so back to RTBC.
Comment #21
webchickOk, awesome. This patch looks a lot scarier than you'd think because it has to update the database 1400 times.
My one concern here was what would happen to an actual PHP node (like if PHP code would be suddenly visible to end users), and luckily I happen to have one: node/37.
The php_code notice that was previously appearing vanished. Here's what it ends up looking like:
When you go to edit:
Despite the truly TERRIFYING help text, nothing happens at all to the text when you save. It's just not visble on view. Could use minor novice issue.
With that, committed and pushed to 8.0.x. YEAH! :D