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
Drupal 7 vocabularies have the following fields:
- vid (numeric)
- machine_name (varchar 255)
- name
Currently the D8 vid, which is a unique machine readable string, is mapped from Drupal 7 'name'.
- This is most probably because Drupal 6 vocabularies only had 'vid' and 'name' and the approach in D6-D8 migration is that we transfrom 'name' to unique machine readable string from 'name'.
- So D6-D8 migration for D8 vocabulary vid is OK but D7-D8 is confusing because D7 vocabularies have a 'machine_name' field.
What increases the severity of this D7-D8 issue is that vocabulary 'name' is on the primary language of the D7 site. When we migrate the vocabulary from the 'name' field, the result is unexpected especially for the Tags vocabulary which is created in both D7 and D8 Standard installation profiles. See screenshot below.
What happens here is that D7 has a vocabulary with 'machine_name' tags and 'name' Tagit. When this is migrated to D8, a vocabulary with 'vid' tagit is created. However, D8 Standard already had a vocabulary with vid tags so we now have two vocabularies for tags.
Note regarding the Forum vocabulary. As illustrated in the screenshot above, also Forum vocabulary is duplicated. However, Forum needs special handling which is handled separately in #2914249: Translated forum vocabulary migration creates duplicate forum vocabularies. .
Proposed resolution
D7-D8 vocabulary migration should map D8 vid from D7 'machine_name' instead of 'name'. We still need to pass the D7 'machine_name' source property through make_unique_entity_field process plugin because the D7 'machine_name' can be 255 characters long and D8 'vid' can only have 32 characters.
Remaining tasks
Patch
Review
Commit
Dance all night, one bug less :)
User interface changes
None.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | 2914649-fail.patch | 2.07 KB | dipakmdhrm |
| #21 | 2914649-21.patch | 2.64 KB | dipakmdhrm |
| #8 | interdiff-2914649-6-8.txt | 2.06 KB | masipila |
| #8 | 2914649-fail.patch | 2.06 KB | masipila |
| #8 | 2914649-8.patch | 2.86 KB | masipila |
Comments
Comment #2
masipila CreditAttribution: masipila as a volunteer commentedComment #3
masipila CreditAttribution: masipila as a volunteer commentedThis should be straight forward, I'll try to patch this right away.
Comment #4
masipila CreditAttribution: masipila as a volunteer commentedLet's see what the testbot says for this.
I checked that this bug does not affect D6-D8 migration. The vocabularies in D6 have only 'vid' and 'name', they don't have machine_name. Hence: mapping from 'name' is OK for D6-D8.
Cheers,
Markus
Comment #5
masipila CreditAttribution: masipila as a volunteer commentedComment #6
masipila CreditAttribution: masipila as a volunteer commentedRemoved the machine_name process plugin as it will not be needed. We still need to pass the source value through make_unique_entity_field because machine_name is VARCHAR(255) in D7 and vid can't be more than 32 characters in D8.
Comment #7
masipila CreditAttribution: masipila as a volunteer commentedIssue summary updated to help reviewers.
Comment #8
masipila CreditAttribution: masipila as a volunteer commentedThis latest patch should also include the test coverage for this.
Cheers,
Markus
Comment #9
masipila CreditAttribution: masipila as a volunteer commentedComment #10
masipila CreditAttribution: masipila as a volunteer commentedComment #12
maxocub CreditAttribution: maxocub as a volunteer commentedI think this is right. The failing patch shows the current bug, and the fix is quite straight forward. RTBCed.
Comment #13
phenaproxima+1 for RTBC; the problem, and its fix, make sense to me. Nice work, @masipila.
My only problem with this is that the issue summary is confusing, since the first thing it talks about, and steps to reproduce, concern the translation snafu. So it's not obvious that this patch addresses a subset of that problem.
Can this be clarified in the IS? For even more clarity's sake, should we postpone #2914249: Translated forum vocabulary migration creates duplicate forum vocabularies on this, even though that issue is RTBC?
Comment #14
maxocub CreditAttribution: maxocub as a volunteer commentedRe #13: #2914249: Translated forum vocabulary migration creates duplicate forum vocabularies does not have to be postponed, they are independent.
I removed the translation wording in the IS since we can change the vocabulary names on a single language site.
Comment #15
maxocub CreditAttribution: maxocub as a volunteer commentedFix IS typos.
Comment #16
masipila CreditAttribution: masipila as a volunteer commentedComment #17
masipila CreditAttribution: masipila as a volunteer commentedI re-wrote the issue summary to help reviewers.
Comment #18
phenaproximaThe IS is nice and clear, like the patch. Back to RTBC.
Comment #19
catchThis needs a re-roll now the other two issues are in, but otherwise looks good.
Comment #20
dipakmdhrm CreditAttribution: dipakmdhrm as a volunteer commentedComment #21
dipakmdhrm CreditAttribution: dipakmdhrm as a volunteer commentedThere were merge conflicts.
After the patch in #8, another plugin was added by one of the forum issues to map the vid.
Though this patch passes the
MigrateTaxonomyVocabularyTest, I wonder if we need to make any changes to the plugin added by the form issue.Please review.
Comment #23
phenaproximaThe fail patch, she failed testing as expected. Back to NR!
Yep, this is why patches occasionally need rerolls :)
Comment #24
phenaproximaReroll looks good to me. Let us proceed.
Comment #25
catchCommitted/pushed to 8.5.x and cherry-picked to 8.4.x. Thanks!