Poll migrate support

Are there any guides/tutorials on writing destination plugins? All the guides/tutorials that I have come across seem to be limited to the node/taxonomy/file cases where plugins already exist.

I have attached a not working partial not working migration for the poll question and choices.

I cant get the migrations to show up in Migrate drupal UI and my local drush is playing up, so I dont know how many errors there are. (Reading the docs has been a frustrating experience)

My thinking has broken this down to 4 steps in migratio

1. Migrate poll without options
2. Migrate poll options
3. Migrate stored field data
4. Migrate an entity reference of the poll to the poll node

I have attempted the first 2.

slightly updated patch from before if anyone wants to look at it - I havent done much since then, but probably a better starting point to improve on.

Updated patch:

1. Migration of Poll Question works.
2. Migration of poll choices does not run and drush gives the following error: "Migration pollchoice did not meet the requirements. [error]"

I cannot seem to get anything more verbose (--debug has been useless) to show why or what requirements are not met.

I have not included my hacky attempts to migrate the vote data - I havent got my head around the data model (votes are not a field?) and I cannot test what I have until migration of poll choices is successfull.

Previous patches had preparerow functions in the source plugin such as the following, but this did not seem to work so using addfield in the query of both plugins instead:

  public function prepareRow(Row $row) {

    $nid = getSourceProperty('nid');
    $row->setSourceProperty('pid', $nid);

    return parent::prepareRow($row);
  }

Any pointers at fixing the migration for poll choices will be much appreciated.

Updated patch:

0. No poll entity reference field is created on the original node poll
1. Poll Questions migrated but no reference is created to the poll node.
2. Poll choices are migrated but no reference is created to the poll
3. poll votes migration currently fails.

I need to figure out how to update existing node types during migration to add more fields and how to create references to the newly created entities on existing entities. Any pointers will be welcome.

Slight progress:

0. No poll entity reference field is created on the original node poll
1. Poll Questions migrated but no reference is created to the poll node.
2. Poll choices are migrated
2.1. Reference is created to the poll, but current code overwrites itself so only last poll choice reference remains afterwards.
3. poll votes migration currently fails.

I have simplified some of the plugins from prevoius patches too.

Updated code.

Currently:

1. To test, I need to manually create a poll_field entity reference on the poll node. Need to create a migration to add a poll entity reference to the poll node type.
2. The migration to add the reference currently resaves the whole node with a new id. I need to fix that.
3. Creating references to choices on the poll deletes the previously saved choice leaving only one choice per poll. Need to fix.

Set to needs review because I need pointers on the two problems:

1. How to create additional field on an existing node type
2. How to make the migration pollchoice_reference not overwrite the previously saved entry.

New Patch.

Before carrying out poll migration, need to manually create field_poll entity reference field on the poll node type. I have asked a question on the forums for ideas on how to do this here: https://www.drupal.org/forum/support/module-development-and-code-questio...

Other than the configuration issue, polls are created correctly with correct options and votes and references to the polls are correctly added to the poll nodes.

Slightly updated patch.

Other than the manual step, the rest should work well for Drupal 7 anyway as one of the migrations references another Drupal 7 specific reference.

Other than manual step, current patch seems to be fully working.

I have not figured out yet how to create a migration which would add an entity reference to the poll node type but it seems that almost everyone else does that step manually.

OK, I have found some documentation in \Drupal\migrate\Plugin\migrate\destination\EntityFieldStorageConfig and \Drupal\migrate\Plugin\migrate\destination\EntityFieldInstance

With Latest patch - field storage is created, but the field instance is not with the following error in the logs:

Source ID : Attempt to create a field without a field_name. ( ... \core\modules\field\src\Entity\FieldConfig.php:112)

Once that above error has been fixed, the patch should be fully complete

The error turned to be an indentation problem which meant the process section of the migration was not being processed.

Full migration from Drupal 7. Marking as RTBC.

There were no database changes between Drupal 6 and Drupal 7 that would impact migration, but as I have not tested from Drupal 6, the tags limit it to Drupal 7 only. Extending it further and potentially adding tests to make sure migration does not break in the future following potential database changes can be done in followups if needed.

Moving to RTBC.

Thanks for the review!

#18-1 - this should be fixed.
#18-2 - this should be fixed.
#18-3 - this should be fixed. Do you have a special tool that can find these coding fixes?
#18-4 - I assume you did not mean the code comment only, but the whole section. I deleted:

-    // Always include this fragment at the beginning of every prepareRow()
-    // implementation, so parent classes can ignore rows.
-    if (parent::prepareRow($row) === FALSE) {
-      return FALSE;
-    }

#18-5 - fixed

#19-0 - I looked at this approach a long time ago, I struggled to understand how to use it at the time. I might have another look.
#19-1 - AFAIK votes are fields in the Drupal 8 version of the poll module, but using the tableplugin seemed the simplest path
#19-2 - adding the dependency in the composer or the info.yml file would make migrate_plus a necessity for everyday operation, when it is only used for the migration.

#21.1 - fixed. I have decided to go for the singular.
#21.2 - fixed.
#21.3 - fixed.
#21.4 - ignored as per your following comment.
#21.5 - fixed along with the src plugin.

#21.6 - I dont understand what you mean here. In Drupal 7 the votes were simply stored in a table. In Drupal 8 they are a field to an entity (poll_choice). Can this be done or did you mean ignore this aswell in the following comment?

#22 - added the source_module: poll to the two migrations

A small error that was not fixed in previous patch

This should be the full patch and interdiff from comment 20.

Still to address #25

Just list the entity name and drupal's migrate api will store the values. I don't recommend using table in D8 for an entity destination.

From the code I cannot see the vote is being attached to any entity. I had expected it to be a field on the poll_choice entity. Any pointers of where to look will be helpful.

Choices (well, their references) are already passed to poll (the question). What is remaining is the votes which are currently copied over via the table plugin.

For the votes I see 2 options if we do not go by what we have right now:

1. A custom destination. But this will probably be a simplified version of the table plugin.
2. Change the poll vote to an entity and use the standard entity plugin destination method.

If 2. is chosen, should it use the same table as now or should it have a new name?

Before re-rolling this my understanding is that we need to get #3042995: Convert votes into entities in.

Any comments on the patch over there is welcome.

Updated patch on top of #3042995: Convert votes into entities

Code sniffer fixes only.

Patch works well.

The only issue is that the entity reference instances added to the node poll is disabled in the node form and not in the correct position for display. There is no content loss, but a user will need to manually update these settings.

@Dippers - do you know where the issue was - was it with the creation of the polls (ie in teh database were all the polls being imported) or with the attachment to the node?

We do need to take the vid into account otherwise AFAIK, in nodes with multiple revisions, only one revision will have a poll attached. Maybe the same logic doesnt apply for the poll_question and the poll_reference migrations though.

Updated patch.

For some reason I am getting a bogus extra poll node being created with a title which is a random string of letters. I cant track down why. but on my new test setup I did have one poll node which had a revision to it to test how the migration handled them.

Other than that it works.

Any help in tracking down the bug(s) much appreciated.

Before this can be finished, we need a resolution on #3042995: Convert votes into entities or agreement/guidance for an alternative form of destination plugin for poll votes that will be acceptable.

(My comment in post 36 was wrong - we were not adding the poll to all the revisions anyway)

Adding for retest - nothing to do with this patch, just checking if the module works with php 7.3.

While reading Migrate related issues, I came across reference to the PathAlias migrate destination.

Using a similar approach, I have gotten the poll_vote migration to (mostly) work.

The only two issues I can see that are remaining are:

1. A bogus extra node is created during the migration. It seems to coincide with having 1 extra revision on my test database. I cannot seem to figure out why this is happening.
2. The poll_vote migrate mapping is not saved. The log shows the following message: "Could not save to map table due to missing destination id values"

Git-foo fail. Correct patch.

The fix for the poll_reference creating nodes was in comment 35 - thanks @Dippers.

The only remaining fix is figuring out why the migrate map for poll_vote is not being saved.

Code style fixes (interdiff includes diff in comment 43)

Beyond the remaining bug with poll votes note being added to the migrate map table, I think this module should have migrate tests.

I have created fixtures following the guide at https://www.drupal.org/docs/8/api/migrate-api/generating-database-fixtur... however the tests page at https://www.drupal.org/docs/8/api/migrate-api/migration-tests-in-drupal-8 is not too useful.

Any pointers/guides?

Relevant fixtures for anyone interested in working on tests.

Migrate Map should now also be working.

Is there a good link/guide that I can follow for relevant tests?

Updated patch.

I do not expect it to work - I cannot seem to get my head around testing...

I tried a migration with the patch on comment 47 on my test site hundreds of polls and thousands of choices/votes.

At some point during the migration the choices seem to be imported/attached twice to the poll (I havent checked which it is yet.) and at that same point it seems the votes are no longer attached to the choice.

For a small number of polls and votes the code seems to work well.

Now with the correct patches.

Small cleanups - mostly coding style and notes for what needs testing.

Still not functional.

Testing patch

More tests. Fixtures probably still dont load.

more updates. Hopefully the fixture will be found now.

(I cannot seem to figure out the local testing environment - the failures are cryptic with no details.)

Updating fixtures to account for new additions in drupal 8.9.

Is there any way to automate these values? I can see having to update the fixtures to keep up new with core tests being troublesome.

Most tests are in place now. Just need to make it work.

more improvements and fixes.

Fix errors from last test

Going further down the rabbit hole. Shouldnt these be done by MigrateDrupal7TestBase?

lets try this

One more time - enable the text module.

Adding comment module to be enabled. Might also need to load the schema specifically.

Add image module to test setup.

Add telephone module.

Thanks! I was starting to pull my hair out over getting the tests to run.

That above test doest seem to have run all the tests - the number of passes match before this patch.

Having a look at the interdiff, the nodes being tested have been changed to 1, 2 and 3. The fixtures have the nodes with nids 11, 12 and 13. I am reverting that change.

Another question - core has the tess/src/Kernel/Migrate folder with a capital M. Are contrib meant to be different in this case?

Testing to see if tests are run if the tests folder name is reverted to Migrate with a capital M.

EDIT - I accidentally changed the namespace to migrate without a capital m in this patch...

Lets test having them both with lower case m.

Finally the tests/failures are showing!

interdiff from comment 71 - 74 attached.

Hopefully this works better.

Try to fixed poll votes test - check specifically if vote count is 8 + make sure that pid check uses integers

The previous patch erroneously tests for 8 votes - the database only has 6.

Nodes do not seem to be loading correctly, so trying a different way.

Also testPollNodeType() didnt seem to run, so renamed to testNodeType()

Fix typo in last patch

Slight update - keeping test name to testNodeType() but I think it was actually passing before - reverting the debuginfo in the assert message as that caused the test to fail.

Still need to fix connection query/use to get the nids from the destination database

I still can't get the testing to work on my PC (using Acquia Dev Desktop), so testing here.

fixing use of fetchCol()

lets try this

Updating fixtures. Is there a way of making them less brittle? Right now we will have to update them for every update to node fixtures in core.

Can we start our test data at an arbitrarily high start value for nid and vid?

Call migrateContent(); before main migration. Lets see if this works.

In this patch I have tried to move to the d7_node_complete migration. It still doesnt work - failure is before the tests even start. The setup rabbit hole needs to be fixed.

I have also merged all 3 tests into 1 - since setup doesnt need to happen 3 times, the test runs are faster for me.

Some progress: Fixtures were not the problem, but the comemnt on slack about the grouping of the migration was helpful - Thanks!

The poll_vote migration gives an error in the tests - the following code gives an empty pid:

  pid:
    plugin: migration_lookup
    migration: poll_question
    source: vid

Deleting this migration from the test then allows the setup to complete. For me next failures were when loading Poll node type and again when comparing counts of polls in source and destination databases.

The fixtures needed fixing - not all had been updated when changing the Node ID's the last time.

Now the setup should all work. Next step: fix the actual tests!

I have added the poll/migrate_drupal.yml file. A question however - I have marked it as finished. Drupal 7 and earlier poll module had a feature to set an offset for votes. This feature does not exist in Drupal 8 but actual votes are all migrated. Should the migrations still be marked as complete?

The Note Type test assert checking that a field_poll is attached passes and the node is also loaded correctly. Currently I am having issues around line 163 of the MigratePollTest.php file:

      // Test Poll attached to node.
      $polls[] = $destination_poll_node->field_poll->referencedEntities();

      // There should only ever be one poll attached to the node following migration from Drupal 7.
      $this->assertCount('1', $polls, "Number of attached polls is incorrect");
      $poll = $polls[0];
      $this->assertInstanceOf(Poll::class, $poll);

I think I am using referencedEntities incorrectly as its not returning a result.

Many thanks for your replies - they are very helpful.

I am moving forward slowly but surely. I have avoided writing tests before, so its a slow process (and most of my prior experience with drupal/php was years ago when we were in a procedural world.)

103.1 - I will sugggest that for a project page update when this patch is ready.

103.2 - I havent gotten this to work so unchanged in this patch. The migration adds a field_poll instance to the poll node type and at this stage I want to 1, test that there is a poll attached to the node, and check that only 1 poll is attached. I can get around it here by loading a poll directly.

I need to get the referencedEntities() code to work, though, because while I can avoid using it here, the next use of it is around 10 lines later where I cannot see a different approach - choices are referenced entities on polls and there is no guarantee that there are a specific number of them (2 test migration has 2 polls with 3 options, 1 poll with 2).

1. changed
2. I need to look into this further. I cant remember why I chose one over the other or even if I was aware of the other at the time.
3. changed
4. I tried improving the comment, but I feel it is still terrible. Should I be pointing out what is being done in the subprococess plugin, or the purpose of the prepareRow() function?
5. changed.
6. changed.
7. changed.

Updated patch. The referencedEntities() code wasnt working because the migrations were all being skipped. Simplified migration code and it now runs.

Now only errors are lines 197/198: later poll vote testing already passes.

These migrations could work for Drupal 6 and 7, however are drupal 7 only because they depend on eg d7_node_complete:poll being completed first. Is there a way to define an or relationship in the yaml that would prevent the need to duplicate the full migration yaml file?

All tests pass locally with this patch (Drupal 9.0.1, phpunit 8.5)

I cannot reproduce the error on line 166 locally.

As for the other test, I have opened #3154918: Failing tests as from what I can see they are unrelated.

Question on fixtures - currently the numbering for the nodes and revisions immediately follows what is last item in core fixtures. however as new fixtures are added to core, these will need to be amended to keep up. Should I start at an arbitrarily high number such as 101 instead?

Included the fix to the failing FieldUI test and also removed hard coded migrate mapping to one that is loaded from the migrate tables - it will make the tests less brittle.

I still need to investigate why there was a failure on line 166 on CI - I cannot reproduce it on my local Drupal 9 setup so it may be Drupal 8 related or due to a lower versioned phpunit.

Updated comment and removed fix to failing FieldUI test that has been committed separately. (interdiff is just the comment change - I am unsure how to include the other change after rebasing.)

Will still (probably) fail on CI as i havent made any changes to address that.

Attempt at fixinf failing test.

w00t!

Only question (other than things found in future reviews) is the question on fixtures. Is it reasonable to start the nids and vids on arbitrarily high numbers instead of following on from what is in core fixtures?

Thanks for updating the patch Deepak - I have added the datetime and telephone modules back into the setup as their omission was causing a failure.

I tried looking into comment 96 above but I couldnt figure out how to do that or get around the need to calculate new values for the nid and vid after import of the main fixtures so I have kept the current method.

I have updated the patch to change the fixtures to start at 101 for nid and at 111 for revision ID's. The chid is also looked up with migrate_lookup now.

There is something wrong with the interdiff, it is showing that the new lines added by Deepak are deleted, but that is (AFAIK) not the case with the main patch.

Has any one else tested this yet?

It would be good to have another tester before setting to rtbc, but if not I will do that in a few days.

Setting to RTBC.

If this is committed the commit message and module front page should note that migrate suport is limited to Drupal 8.9+ only.

Updated issue summary with how the test fixtures were generated/modified.

I couldnt figure out how to get git to push my local changes so I have manually added them manually via gui with some trial and error. what is uploaded now should be idential to patch in comment 124

Changes following comment #128:

1. Reordered the use statements
2. Added the Content migration tag
3. Use DrupalSqlBase
4. shortened install config to $this->installConfig(static::$modules);

from comment #128 items 1 and 4 have been done.

I have tried to do 2. and 3. but using DrupalSqlBase has always failed for me. I dont know how to do that change or if it is required.

tests should be passing again - apologies for such a ling time to fix the tests, my test system was broken and I finally got around to updating the patch.

There is also an extra line in src/Plugin/migrate/destination/PollVote.php, but I cant see it in my vscode, so I cant push a fix from there. I am guessing there is a problem with my config here.

Please feel free to commit to the branch and get this over the line, especially as Drupal 7 is coming to EoL so this issue has greater importance.

Thank you for looking at this!

I have enabled the setting to allow commits from members in the merge request. I hope this is enough.

I have pushed a commit with the interdiff minus the mode to drupalSqlBase as that was causing the following error:

PHPUnit 9.5.4 by Sebastian Bergmann and contributors.

Warning: Your XML configuration validates against a deprecated schema.
Suggestion: Migrate your XML configuration using "--migrate-configuration"!

Testing Drupal\Tests\poll\Kernel\migrate\d7\MigratePollTest
E 1 / 1 (100%)R

Time: 00:28.839, Memory: 10.00 MB

There was 1 error:

1) Drupal\Tests\poll\Kernel\migrate\d7\MigratePollTest::testPoll
PHPUnit\Framework\Exception: PHP Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in Standard input code:89
Stack trace:
#0 Standard input code(89): serialize(Array)
#1 Standard input code(123): __phpunit_run_isolated_test()
#2 {main}
thrown in Standard input code on line 89
Fatal error: Uncaught Exception: Serialization of 'Closure' is not allowed in Standard input code:89
Stack trace:
#0 Standard input code(89): serialize(Array)
#1 Standard input code(123): __phpunit_run_isolated_test()
#2 {main}
thrown in Standard input code on line 89

/srv/drupaldev/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:270
/srv/drupaldev/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:187
/srv/drupaldev/vendor/phpunit/phpunit/src/Framework/TestSuite.php:677
/srv/drupaldev/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:667
/srv/drupaldev/vendor/phpunit/phpunit/src/TextUI/Command.php:143
/srv/drupaldev/vendor/phpunit/phpunit/src/TextUI/Command.php:96

--

There was 1 risky test:

1) Drupal\Tests\poll\Kernel\migrate\d7\MigratePollTest::testPoll
This test did not perform any assertions

/srv/drupaldev/web/core/tests/Drupal/Tests/Listeners/DrupalListener.php:127
/srv/drupaldev/vendor/phpunit/phpunit/src/Framework/TestResult.php:446
/srv/drupaldev/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:376
/srv/drupaldev/vendor/phpunit/phpunit/src/Util/PHP/AbstractPhpProcess.php:187
/srv/drupaldev/vendor/phpunit/phpunit/src/Framework/TestSuite.php:677
/srv/drupaldev/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:667
/srv/drupaldev/vendor/phpunit/phpunit/src/TextUI/Command.php:143
/srv/drupaldev/vendor/phpunit/phpunit/src/TextUI/Command.php:96

ERRORS!
Tests: 1, Assertions: 0, Errors: 1, Risky: 1.

...

In relation to the two questions, I will have to go back and check but from my recollection:
1. the reason was that the poll does not have revisions and from the manual testing I did, adding the field to the node displayed it on all revisions.
2. The source plugin works for both Drupal 6 and 7. if I depended on the node source plugins I would need to have 2 versions. (probably less of an issue now since Drupal 6 migrations are not in core.)

I have updated the merge request and I would be grateful for another review/pointers:

1. I switched to DrupalSqlBase from sqlBase. I also needed to change source_module to node instead of poll as otherwise the test complained that the source module was not installed. If this is the incorrect fix, I would be grateful for a pointer where I need to change it.

2. There are test failures but these seem to exist on Drupal 9.3 onwards on the main module too - testing on Drupal 9.2 shows no errors. Again, any pointers where I need to fix what I assume are deprecations would be much appreciated. Fixed main branch failires. if any remain past this point, they are from this branch (tests currently running)

ok, tests fixed. Just the one question from comment 137 over whether I did the right thing.

I managed to figure out why using DrupalSqlBase had resulted in errors previously: the fixtures had not included changes to the system table to mark the poll module as enabled.

I have manually added that to the fixtures now and the tests work properly without the previous workaround of having to set the source module to node.

If tests pass, this is ready. All previous feedback now dealt with

EDIT - Failing test does not seem to be due to this patch but due to change to testing on Drupal 9.4 and failing FieldUITest. Second test run will confirm.

rebased to fix unrelated failing test.

good catch @phjou - should be fixed now.