Migrate the D6/D7 actions table to D8

We have a d6_action source plugin currently, but no actual migration of that data is taking place. I doubt there's significant difference between D6 and D7 here, we should be able to do it in one patch.

First rough attempt at migrating D6 & D7 actions into D8.

Some things to discuss:
* Custom actions in D6 & D7 get a numeric action id (aid). Should we use this as the D8 action machine name as well, or generate the machine name from the label?
* system_block_ip_action from D6 / D7 doesn't seem to exist in D8. In my patch I just skip the migration of this action. Not sure if this is the desired behavior.

Updated version where numeric action IDs get turned into a machine name based on the label.
The action to block IPs doesn't get migrated from D6 & D7, because there is no action plugin in D8 (not in system module, nor ban module).

Adding my WIP changes for the tests

Might need some help here...

* D7 PHPUnit test doesn't run - getting "No test results to display." (same issue with unit tests as I have in #2500483: Upgrade path for Image 7.x
* D7 migration test: still some things that are failing:
- Something with a missing schema value - haven't figured out yet what's causing this
- Can't find a way to assert the "configuration" settings in the Action config entity. They don't seem to be exposed through a getter. How would I go about checking them?

Once those issues are fixed - I can pretty much copy the tests for D6.

Also: how will we migrate actions that are defined by contrib modules or custom code? Just skip them?

Updated patch:
* Fixed PHPUnit errors
* Fixed wrong assertions in D7 MigrateActionsTest

Still have to figure out:
* Why it gives the following schema error:
Schema errors for system.action.unpublish_content_containing_keyword_s_ with the following errors: system.action.unpublish_content_containing_keyword_s_:configuration.keywords missing schema
* Can't find a way to assert the "configuration" settings in the Action config entity. They don't seem to be exposed through a getter. How would I go about checking them?

Updated patch:
* Uses new database fixtures to generate test data
* D7 test updated with assertions for $action->get('configuration')
* D6 test added

* Fixed newline issues
* Consolidated D6 & D7 action source - the only difference is the description field which is called "label" in D7 and "description" in D6.
* Assertion that counts number of actions has been removed.
* Periods added to comments
* @TODO: check configuration was already fixed - guess I uploaded an older patch by accident or something.

I could probably also consolidate d6_action.yml and d7_action.yml into the same file, but then the source property for the description field would need to have the same key in both cases. What's the best approach to do that, since I don't want to mess too much with the source data.

Bollocks... still a Git newline missed...
Let's try again

Migrate source Unit test also needed updating. Trying again.

Adding this tag - hope that's ok

@svendecabooter, I too would like to know about merging the d6 and d7 yml files. Can someone answer that question, in comment #14?

And other that the two wee edits below, this looks good to me. Not even sure if the question above prevents RTBC.

1. +++ b/core/modules/action/tests/src/Unit/Plugin/migrate/source/ActionTest.php
   @@ -0,0 +1,73 @@
   +      'parameters' => null,
   +      'description' => null,
   

   I think these should be uppercase.

2. +++ b/core/modules/action/tests/src/Unit/Plugin/migrate/source/ActionTest.php
   @@ -0,0 +1,73 @@
   +      'parameters' => null,
   +      'description' => null,
   

   And these too.

Unfortunately, that one discrepancy in source field name does mean we can't use a common config file for both, d6_action and d7_action will need to stay separate.

@mikeryan, thank you.

@svendecabooter, I'm still getting used to reviewing, so I looked again and found 2 more items.

1. +++ b/core/modules/action/src/Tests/Migrate/d6/MigrateActionsTest.php
   @@ -0,0 +1,77 @@
   +  public static $modules = ['action', 'comment', 'node', 'user', 'system'];
   

   MigrateDrupalTestBase enables system and user, so they can be removed.

2. +++ b/core/modules/action/src/Tests/Migrate/d7/MigrateActionsTest.php
   @@ -0,0 +1,77 @@
   +  public static $modules = ['action', 'comment', 'node', 'user', 'system'];
   

   Same as above.

Thanks for the review quietone!
Attached an updated with your remarks covered.

One more!

1. +++ b/core/modules/action/src/Tests/Migrate/d7/MigrateActionsTest.php
   @@ -0,0 +1,77 @@
   + * @group action
   

   Should this be migrate_drupal_7?

2. +++ b/core/modules/action/tests/src/Unit/Plugin/migrate/source/ActionTest.php
   @@ -0,0 +1,73 @@
   + * @group action
   

   Same

The @group setting seems to be different for Drupal 6 Migration tests compared to Drupal 7 Migration tests.
Not sure why that's the case, but I based myself on other tests that already made it into core, to keep things consistent.

@svendecabooter, thx!

Looks like everything has been addressed.

Committed 395c724 and pushed to 8.0.x and 8.1.x. Thanks!

1. +++ b/core/modules/action/migration_templates/d6_action.yml
   @@ -0,0 +1,39 @@
   +        user_block_ip_action: 0
   
   +++ b/core/modules/action/migration_templates/d7_action.yml
   @@ -0,0 +1,36 @@
   +        system_block_ip_action: 0
   

   It is odd that the Ban module did not add this action.

2. +++ b/core/modules/action/src/Tests/Migrate/d6/MigrateActionsTest.php
   @@ -0,0 +1,77 @@
   +    /** @var \Drupal\system\Entity/Action $action */
   
   +++ b/core/modules/action/src/Tests/Migrate/d7/MigrateActionsTest.php
   @@ -0,0 +1,77 @@
   +    /** @var \Drupal\system\Entity/Action $action */
   

   Should be \Action not /Action. Fixed on commit.