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
The are two reasons why a field would be in a separate table, multiple values or if it is shared across multiple types.
Right now, the code checks for multiple *and* per-field, but it should be checking for multiple or per-field, but just checking for per-field storage should really be enough I think as well.
The table structure is also different as there's no delta, so we can't use fetchAllKeyed()*
* using fetchAllKeyed() is also a bug, as that only supports two columns, but we could have more than that. Should possibly be fetchAllAssoc(), would however affect the structure of the columns below, and it would need to be renamed anyway... But separate issue...
Proposed resolution
See patch :)
Works fine or on a real d6 site but needs tests.
Remaining tasks
User interface changes
API changes
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | migrate-shared-fields-2262975-4-interdiff.txt | 3.28 KB | Berdir |
| #5 | migrate-shared-fields-2262975-4.patch | 12.98 KB | Berdir |
| #2 | migrate-shared-fields-2262975-2.patch | 9.71 KB | Berdir |
| #2 | migrate-shared-fields-2262975-2-tests-only.patch | 6.83 KB | Berdir |
| #1 | migrate-shared-fields-2262975-1.patch | 1.69 KB | Berdir |
Comments
Comment #1
BerdirAnd the patch.
Comment #2
BerdirThere shall be tests!
Had to fix a lot of stuff in the dumps, which were setting up things wrong/incomplete, field_test in fact already has two instances (but one for a non-existing node type), which means it should be using per-field storage, so changed that and using it as a test field for this, instead adding another field as a single field so that we also have test coverage for that part.
Also added support for multiple columns in per-field storage, it's a bit ugly but seems to be working, although there are a lot of variations that still don't have explicit test coverage, there are a lot of tricks for file fields but I don't think there are tests anywhere for that, for example..
Comment #5
BerdirFixing tests that relied on the field instance for the non-existing (in the source) article node type.
Comment #6
chx CreditAttribution: chx commentedah-ha. Exceptional work, thanks so much!
Comment #7
Dries CreditAttribution: Dries commentedCommitted to 8.x. Thanks.