Migrate Drupal 7 image and file fields

Just tested this patch, and tried 7.39->8.0.0-rc3 upgrade. Still don't see image field values being saved in the new d8 site.

I just tested patch in #5 with a "fresh" migrate, and it works well.

To follow Migration API naming convention, I changed "Image" to "ImageField". Patch is in the attachment.

D7 file fields are similarly broken, so I borrowed the image field plugin here to make one for those.

Over at #2612914: Images transfer but are not linked in content @joergM reports:

I tried the patch and both, file fields and image fields, were migrated successfully.

Looks like the next step here is tests.

I attempted to rerun my migration with a clean database with the combined patch in #10 after I noticed files were missing and #5 had worked for images.

However, images were now no longer being associated correctly, I thus reverted ImageField to Image and my migration completed as expected. That trivial change is attached.

Patch (migrate_drupal_7_image-2604484-11.patch) failed with error "fatal: corrupt patch at line 144"

Also my images fields are still not linking after patch 10

Attached is a patch to fix #10 where change from image to imagefield didn't import the image fields. I also moved imagefield to d7 folder as I don't think this will be compatible with d6.

Something went weird with git diff... Reuploading

I am not sure what is going on here... I pulled latest dev head.. WITHOUT applying this patch. Installed a fresh install and cannot get drupal to pass Drupal\migrate_drupal\Tests\d6\EntityContentBaseTest.... Which should be passing.... right? I have no idea... I dunno how to debug drupal 8... lol.

Doing a drush based migration from 7.41 to 8.0.0, confirming with patch 10 the files are coming through but not the images. With patch 16 both files and images are importing! However this patch forgets to import the file descriptions, which can be done with the addition of a "description" field value in FileField.php processCckFieldValues() line 55:

  'target_id' => 'fid',
  'display' => 'display',
  'title' => 'title',
  'description' => 'description',

Very happy to have file and image fields finally associated to D8 nodes!

Here's a test for d7 file and image field values.

Still needs tests for d6. And doing should help confirm if ImageField.php needs a d6 and d7 version, as suggested in #15.

It isn't clear to me what to do about D6 testing. Aren't file fields contrib in D6?

Make the D7 file and image tests explicit. Added file field data to the D7 dump to do this.

Don't know what I was thinking about the D6 fields and contrib. That aside, the D6 MigrateNodeTest doesn't have any file field data. The D6 process plugin, d6_cck_file, tries to lookup the ID of the migrated file. If it isn't found an empty fid is returned. So, no data to test. The D7 migration does not do this lookup. Should it?

Anyone know why the D6 file process, d6_cck_file, does a lookup on the fid while the D7 process doesn't? See http://cgit.drupalcode.org/drupal/tree/core/modules/file/src/Plugin/migr...

Plus that process has a todo, waiting for #2487568: Process plugins should not be allowed to skip rows. Since that is fixed, this needs a bit of review itself to see what should be happening here.

Talked to chx on IRC about the fid lookup being done in the d6_cck_file. He said it wasn't necessary because the drupal-to-drupal migrations keeps the ids. So, I removed the lookup code from /d6/CckFile.php. With that gone it was straightforward to test the filefield data in MigrateNodeTest.

Still need to do the same for D6 image files.

Need to install the file entity schema for those tests. Lets see if I fixed them all.

Add an image field to a test node in the D6 dump. This will cause a fatal error, in the preSave method in ImageItem.php.

Call to a member function getFileUri() on a non-object PHP Fatal error ImageItem.php 321

The preSave method ensures that the width and height are not empty. If either are empty, which seems to be true for D6, preSave attempts to get the dimensions using the image factory service. And somewhere in getFileUri() it fails.

That is the error I expected, just occurring in more tests. However, this issue is to add D7 image and file fields, not improve D6 image and file field testing. I've become distracted. Lets return to solving this issue only.

The tests for this issue are resolved in patch #28 and that is the patch that should be reviewed. At some later time I'll make an issue about the changes made for D6 file fields which are in the later patches. It is too late in the day to do that now.

To any reviewers, please review patch #28, ignore the later ones.

Correct whitespace error.

Patch #28 tested for migrating D7 site to D8. Works fine. Image are now linked to their node. Note that i have to do the upgrade process from a new D8 site. In the previous D8 site, with the migrate-revert and migrate-import drush commands, I don't succeed to link images to their nodes.

Confirmed that this fixes images problems in a D7 migration. I used the last patch, #36.

Tested D7 image field migration with path #36 - Working.

I have the same problem, but with a D6 => D8 migration. Could this be added to this issue, or is there a separate one?

The D6 issue is #2640842: Make related file migration ID configurable in d6_cck_file. Back in #35 I removed the D6 work from this issue but forgot to report back here.
At that time I thought the issue just needed to add tests for the D6 image and file migration but if it is more than that, as suggested in #40, please change the IS as needed.

Looks like testbot made a mistake. The test is passing.

Sorry, I was rushing off to the airport and made a mistake. This should be RTBC as per #38.

Yicks, I did it again. This time resetting to RTBC as per #38. The tests are passing even though testbot thought otherwise.

Committed e66fbef and pushed to 8.0.x and 8.1.x. Thanks!

Any patch for Drupal core 8.1.7?

Looks like version was changed by mistake in #51