Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Currently, image fields on a D7 upgrade don't actually get linked to their respective image files (although the images themselves are indeed migrate and in the file table).
Proposed resolution
Add a CckFieldPlugin
for image
fields to customize the process config for image fields and use the iterator plugin to map the D7 fid, alt, title, width, and height sub-fields to their d8 equivalents. Attached patch does just that.
Remaining tasks
Patch Review
Write Tests
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#36 | 2604484-36.patch | 7.13 KB | quietone |
#36 | interdiff-2604484-28-36.txt | 843 bytes | quietone |
#33 | 2604484-33.patch | 14.74 KB | quietone |
#33 | interdiff-2604484-32-33.txt | 1.06 KB | quietone |
#32 | interdiff-2604484-30-32.txt | 2.29 KB | quietone |
Comments
Comment #4
skyredwangJust 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.
Comment #5
drclaw CreditAttribution: drclaw commentedOh yeah, they changed the Annotation for CckFieldPlugins in 8.0.0-rc3. Here's an updated patch! (I also updated the version of the patch on the issue)
Comment #6
drclaw CreditAttribution: drclaw commentedAlso, I should mention that you need to re-run your upgrade in order for this patch to work. Under the hood, all it does is properly set the process plugin for your image field during the upgrade process. So, if you've already upgraded, the configuration is already entered into the database incorrectly.
You can manually fix any image field migrations, however, by updating the migration configuration. For example, if you had a D7 node type called Blog Post, you would go to the Single Item configuration Export page (admin/config/development/configuration/single/export) and select "Migration" for your Configuration Type and "Nodes (Blog Post)" for your Configuration Name. Then copy the configuration and edit it in a text editor. In the
process
section of the configuration, locate your image field name. It may look something like thisAnd replace it with:
Be sure to replace any occurances of
field_image_field
value with your image field name.Comment #7
skyredwangI just tested patch in #5 with a "fresh" migrate, and it works well.
Comment #9
skyredwangTo follow Migration API naming convention, I changed "Image" to "ImageField". Patch is in the attachment.
Comment #10
mikeryanD7 file fields are similarly broken, so I borrowed the image field plugin here to make one for those.
Comment #11
mikeryanOver at #2612914: Images transfer but are not linked in content @joergM reports:
Looks like the next step here is tests.
Comment #12
grahlI 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.
Comment #13
bloomt CreditAttribution: bloomt commentedPatch (migrate_drupal_7_image-2604484-11.patch) failed with error "fatal: corrupt patch at line 144"
Comment #14
bloomt CreditAttribution: bloomt commentedAlso my images fields are still not linking after patch 10
Comment #15
dobe CreditAttribution: dobe as a volunteer commentedAttached 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.
Comment #16
dobe CreditAttribution: dobe as a volunteer commentedComment #19
dobe CreditAttribution: dobe as a volunteer commentedSomething went weird with git diff... Reuploading
Comment #22
dobe CreditAttribution: dobe as a volunteer commentedI 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.
Comment #23
steinmb CreditAttribution: steinmb as a volunteer commentedComment #24
steveOR CreditAttribution: steveOR commentedDoing 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:
Very happy to have file and image fields finally associated to D8 nodes!
Comment #25
quietone CreditAttribution: quietone commentedHere'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.
Comment #26
quietone CreditAttribution: quietone commentedComment #27
quietone CreditAttribution: quietone commentedIt isn't clear to me what to do about D6 testing. Aren't file fields contrib in D6?
Comment #28
quietone CreditAttribution: quietone commentedMake 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?
Comment #29
quietone CreditAttribution: quietone commentedAnyone 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.
Comment #30
quietone CreditAttribution: quietone as a volunteer commentedTalked 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.
Comment #32
quietone CreditAttribution: quietone as a volunteer commentedNeed to install the file entity schema for those tests. Lets see if I fixed them all.
Comment #33
quietone CreditAttribution: quietone as a volunteer commentedAdd 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.
Comment #35
quietone CreditAttribution: quietone as a volunteer commentedThat 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.
Comment #36
quietone CreditAttribution: quietone as a volunteer commentedCorrect whitespace error.
Comment #37
flocondetoilePatch #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.
Comment #38
KarenS CreditAttribution: KarenS at Lullabot commentedConfirmed that this fixes images problems in a D7 migration. I used the last patch, #36.
Comment #39
ckngTested D7 image field migration with path #36 - Working.
Comment #40
joel_guesclin CreditAttribution: joel_guesclin commentedI have the same problem, but with a D6 => D8 migration. Could this be added to this issue, or is there a separate one?
Comment #41
quietone CreditAttribution: quietone as a volunteer commentedThe 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.
Comment #44
quietone CreditAttribution: quietone as a volunteer commentedLooks like testbot made a mistake. The test is passing.
Comment #45
quietone CreditAttribution: quietone as a volunteer commentedSorry, I was rushing off to the airport and made a mistake. This should be RTBC as per #38.
Comment #46
quietone CreditAttribution: quietone as a volunteer commentedYicks, I did it again. This time resetting to RTBC as per #38. The tests are passing even though testbot thought otherwise.
Comment #49
alexpottCommitted e66fbef and pushed to 8.0.x and 8.1.x. Thanks!
Comment #51
AniaMi CreditAttribution: AniaMi commentedAny patch for Drupal core 8.1.7?
Comment #52
AdamPS CreditAttribution: AdamPS at AlbanyWeb commentedLooks like version was changed by mistake in #51
Comment #53
yurg CreditAttribution: yurg as a volunteer commentedComment #54
yurg CreditAttribution: yurg as a volunteer commented