Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Create a migration config and class for enable end user to migrate Taxonomy Vocabulary from Drupal 7 to Drupal 8
Comment | File | Size | Author |
---|---|---|---|
#69 | interdiff-2410875-65-69.txt | 657 bytes | phenaproxima |
#69 | 2410875-69.patch | 59.25 KB | phenaproxima |
#65 | interdiff-2410875-61-65.txt | 1 KB | phenaproxima |
#65 | 2410875-65.patch | 58.48 KB | phenaproxima |
#61 | interdiff-2410875-59-61.txt | 1.27 KB | phenaproxima |
Comments
Comment #1
miguelc303 CreditAttribution: miguelc303 commentedComment #2
miguelc303 CreditAttribution: miguelc303 commentedCreate a migration config and class for enable end user to migrate Taxonomy Vocabulary from Drupal 7 to Drupal 8
Comment #3
-enzo- CreditAttribution: -enzo- commentedPatch tested against Drupal 8.0.x because the IMP is not totally sync
Comment #4
-enzo- CreditAttribution: -enzo- commentedComment #5
benjy CreditAttribution: benjy at CodeDrop commentedComment #6
miguelc303 CreditAttribution: miguelc303 at Anexus commentedAdded organization support to Anexus IT
Comment #7
phenaproximaUpdated the patch and added a test.
Comment #9
phenaproximaRound and round we go!
Comment #10
phenaproximaMinor test cleanup.
Comment #12
phenaproximaBlocked by #2495755: Create MigrateDrupal7TestBase.
Comment #13
phenaproximaComment #14
phenaproximaMerged #2411657: Migration Files for Drupal 7 Taxonomy Term.
Comment #15
phenaproximaChanging title.
Comment #16
phenaproximaAdded unit tests of the d7_taxonomy_term and d7_taxonomy_vocabulary source plugins.
Comment #18
phenaproximaWhoops...let's try that again.
Comment #19
phenaproximaMerged #2353775: Variable to config: taxonomy.settings [d7] and added a test of it.
Comment #20
Anonymous (not verified) CreditAttribution: Anonymous commentedIf you worked on this issue you may be able to quickly help fix #2506001: Taxonomy term values referenced on nodes do not migrate (D6).
Comment #21
phenaproximaThis is blocking #2353703: Variable to config: forum.settings [d7].
Comment #22
phenaproximaRe-rolled.
Comment #24
phenaproximaFixed a few bugs revealed by the tests.
Comment #26
phenaproximaOK, this oughta work.
Comment #27
mikeryanHere are the diffs between the files in this patch and their D6 equivalents, it's really helpful to identify what changes are due to actual differences between D6 and D7.
Comment #28
mikeryanExample of a necessary difference, D7 had machine names for vocabularies but D6 did not.
Missing d7 directory.
D7 adds format to the term data table, should be added to fields().
Missing d7.
Just to be sure I understand this difference - D6 had the VocabularyBase class to deal with the term_node business, but D7 just needs a regular source class derived from DrupalSqlBase because term references are normal fields in D7?
Ditto - D6 needed prepareRow() but D7 doesn't because of the term_node business, right?
Why the different file names?
Touch of the copypasta here..
Missing docs.
Just to be clear, the assertEntity()/testTaxonomyTerms() combo here is doing the same thing as testTaxonomyTerms() in D6, this is a refactoring?
Ditto with above term test.
As above, I assume we no longer need the TermTestBase because no more term_node business to deal with?
Comment #29
phenaproximaBecause D7 adds the format column to {taxonomy_term_data}, this is blocked by #2384567: Migration Files for Drupal 7 Text Formats & Filters.
Comment #30
webchickSince this is one of the "big 4" content migrations, escalating to a Migrate critical.
Comment #31
phenaproximaThis is now unblocked.
Comment #35
webchickJust some house-keeping, ignore me.
Comment #36
phenaproximaWe can give this a try...
Comment #38
phenaproximaD'oh.
Comment #39
mikeryanRunning a test D7 migration:
Migration d7_vocabulary_entity_display did not meet the requirements. Missing migrations d6_vocabulary_field_instance. requirements: d6_vocabulary_field_instance.
Oops!
On the positive side, the vocabularies and themselves appear to be successfully migrated.
Will follow up with a code review shortly...
Comment #40
mikeryanOh, actually d7_vocabulary_entity_display should just go away, right? The D6 version is only needed because of the term_node stuff, in D7 we had proper fields...
Comment #41
mikeryanDiffs between D6 and D7 taxonomy handling, at least for the basics of terms and vocabularies themselves (since the term field handling is totally different).
Comment #42
mikeryanApart from d7_vocabulary_entity_display, looks good to me. Since I contributed to the taxonomy migration in the sprint sandbox, though, I'm not the one to RTBC this...
Comment #43
phenaproximaGot rid of the d7_vocabulary_entity_display thing. @mikeryan is correct -- because these are fields, we shouldn't need to do anything special here.
Comment #44
ultimikeI reviewed this while chatting with phenaproxima on IRC - I found a couple of things...
This looks wrong to me...
Will this work for hierarchical taxonomies where a parent term has a higher term id than one of its children?
-mike
Comment #45
ultimikeI just chatted with mikeryan on IRC regarding the hierarchical taxonomy issue:
-mike
Comment #46
phenaproximaMoved CckBuilder one level up.
Comment #49
phenaproximaLet's try that again.
Comment #50
benjy CreditAttribution: benjy at PreviousNext commentedThis looks good to me, few super small code standards things, that's about it.
Seems to be some weird spacing here.
Now you're removing new array syntax for old style :)
and here
Didn't this skip the migration call when the parent was 0? Eg, no hierarchy?
I think we're still fine but I wonder how far we go with this before we just go back to a d6 and a d7 source.
You shouldn't need this if ::load has the correct docs. Maybe an issue in core.
Comment #51
mikeryanFWIW, I always have to do that with Migration::load(). Ultimately Entity::load() is called, which has
@return static|null
. Maybe an issue in PhpStorm - can other tools work out what class is being returned from that "static"?Comment #52
benjy CreditAttribution: benjy at PreviousNext commentedPHPStorm can definitely work it out when you have return static, I just tested it and it seems it's the |null part throwing phpstorm off.
Comment #53
phenaproximaAddressed #50:
Comment #54
ultimikeAssuming all tests pass, I think we're good to go here.
-mike
Comment #56
webchickAwesomesauce!!
I tested this locally, and confirmed that vocabs and tags are being moved over (interestingly, I end up with two "Tags" vocabs; one that was already existing in D8 via Standard profile and the other that was moved in via the migration). However, the tags do not show up on article view, nor does the field itself show up in the form. Adam says that's because of #2563819: [D7] Field display settings do not migrate, so off to review that one next.
Nevertheless, this one is definitely a huge step forward, so...
Committed and pushed to 8.0.x. Thanks!
Comment #57
webchickOh, shucks, I spoke too soon. When I applied the patch in #2563819: [D7] Field display settings do not migrate and re-ran the migration, it becomes apparent that instead of doing term associations, the migration is doing node associations. D'oh!
So have to revert this for now. Hopefully an easy fix. And obviously we need to fix our test coverage, too. ;)
Comment #59
phenaproximaThis oughta fix it. And I've added test assertions to prove it.
EDIT: The problem here is that taxonomy term reference fields are migrated to entity reference fields by the d7_field migration, but the migration wasn't bothering to adjust the field settings (like such as, to set the target entity type). So the fields were referencing nodes, not terms, by default.
Comment #60
mikeryanNot worth rerolling for on its own, but could do [] instead of array() here.
BTW, someone promised he'd rewrite labels according to #2565791: Fix migration labels on this go-round...
Comment #61
phenaproximaFixed!
Comment #65
phenaproxima*pouts*
Comment #69
phenaproximaWhen I get an ulcer, I'm going to name it testbot.
Comment #72
ultimikeRock and roll - this looks good.
-mike
Comment #73
webchickRock! Committed and pushed to 8.0.x. Thanks!