While migrating a D6 site to D8, I discovered that several imagecache presets were not being migrated.
Somehow, the legacy database had entries in the imagecache_action table with with blank "action" values. I don't know how they got there; we didn't tamper with the database. But the imagecaches work correctly.

Screenshot of bad data

These rows caused an exception which aborted that row

The "" plugin does not exist.

Rather than let those - otherwise valid - entries fail, we can filter the bad data out of the query in the first place.

Patch to be attached in first comment.

Steps to reproduce:
1. Create a D6 site.
2. Install imagecache
3. Create one or more presets.
4. Cause an action to have a blank "action" field in the imagecache_action table
5. Migrate the site to D8

Expected result:
The imagecache preset will exist as an image style in the d8 site.

Actual result:
drush mmsg shows an error saying The "" plugin does not exist.
The preset is not migrated.

CommentFileSizeAuthor
#8 2890514-8.patch1.43 KBquietone
#8 2890514-8-fail.patch753 bytesquietone
#5 2890514-5.patch2.84 KBquietone
#2 image-upgrade_d6_imagecache_preset-fails-on-blank-action-2890514-1-8.3.4.diff645 bytesvomitHatSteve
imagecache.png26.73 KBvomitHatSteve

Comments

vomitHatSteve created an issue. See original summary.

vomitHatSteve’s picture

vomitHatSteve commented
StatusFileSize
new image-upgrade_d6_imagecache_preset-fails-on-blank-action-2890514-1-8.3.4.diff645 bytes

Attached is a patch to alter the actions query to exclude results with blank action values.

quietone’s picture

quietone commented
Issue tags: +migrate-d6-d8

add tag.

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Active » Needs work
Issue tags: +Needs tests

NW for adding tests.

quietone’s picture

quietone commented
Status: Needs work » Needs review
StatusFileSize
new 2890514-5.patch2.84 KB

On the principle that all the data should be retrieved how about we do this 'skipping' in the process plugin. In general a process plugin should be validating the data anyway, or perhaps a skip_on_empty in the pipeline. The latter option isn't used because the process plugin loops through the data.

The process plugin simply does not process the array of image cache information if the action is empty. An entry is added to the imagecache_action table where the action = '';

quietone’s picture

quietone commented
Version: 8.3.4 » 8.8.x-dev
heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Needs work 
+++ b/core/modules/image/src/Plugin/migrate/process/d6/ImageCacheActions.php
@@ -20,27 +20,29 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
+      if (!empty($action['action'])) {

So we don't have to shift everything out, let's do a continue if empty(). That would make the whitespace changes a lot cleaner. It also might be good to have a fixtures only change added so we can see how the tests fail.

quietone’s picture

quietone commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2890514-8-fail.patch753 bytes
new 2890514-8.patch1.43 KB

Fix for #7 and a fail patch.

The last submitted patch, 8: 2890514-8-fail.patch, failed testing. View results

heddn’s picture

heddn
Primary language English
Location Nicaragua
commented
Status: Needs review » Reviewed & tested by the community

We've got tests. The IS is in good shape. Let's move along here.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 820c3d83a3 to 8.8.x and 530642fa4f to 8.7.x. Thanks!

AS a bugfix with no BC concerns backporting to 8.7.x

  • alexpott committed 820c3d8 on 8.8.x 
    Issue #2890514 by quietone, vomitHatSteve, heddn:...

  • alexpott committed 530642f on 8.7.x 
    Issue #2890514 by quietone, vomitHatSteve, heddn:...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 8.8.x-dev » 8.7.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.