Migration Files for Drupal 7 Text Formats & Filters

Remove un necessary code in Filter format file for permissions and set the names to formats migrated from Drupal7

This patch fix an error in previous patch to enable import custom formats, but only the following plugins are accepted for now in Drupal 8

• filter_html
• filter_url
• filter_htmlcorrector
• filter_html_escape
• filter_autop
• php_code

Because if a format has other plugins these are import as a filter_null plugin disabling the input format in Drupal 8.

If you want enable others plugins you must to add in the yml file and in the FilterFormat class in array filters_allowed.

Moving to the IMP sandbox issue queue.

Added organization support to Anexus IT

Wrote a test, and greatly simplified the migration itself. It seems to me that using the static_map plugin to determine the filter IDs is pointless, since they're exactly the same between D7 and D8.

Postponing until MigrateDrupal7TestBase lands.

This is blocking migration of nodes, comments, and taxonomy terms (and probably others); escalating.

Re-rolled against HEAD and added a unit test of the d7_filter_format source.

This oughta work now.

Thanks to phenaproxima and mikeryan1 I'm reviewing this.

I don't have enough knowledge to know if the function assertEntity is correct and sufficient. But, personally, I find the name odd because it appears to be for one specific type of entity, not a generic test for all entities. But I can be pedantic, so don't know if that is a problem.

The only other thing I noticed was a typo, which is fixed in the attached patch.

So, still needs someone more experienced to have a look.

It looks like were not testing the migration of any custom filter settings for example changing the list of tags filtered by filter_html.

Good point. Added a couple of assertions for that.

Interdiff looks good, back to RTBC.

Ok, gave this a once-over...

The biggest thing is we intentionally skip plain text format in a couple of places:

+  public function query() {
+     return $this->select('filter_format', 'f')
+      ->fields('f')
+      // Ignore the plain text format.
+      ->condition('format', 'plain_text', '<>');

+  public function testFilterFormat() {
...
+    // The plain text format is skipped by the source plugin.
+    $this->assertNull(FilterFormat::load('plain_text'));

However, I don't understand why we do that. While it's not deletable, it is configurable, and you can even configure it like Full HTML if you really want to ruin someone's day. ;) So it seems like we need to account for it as well, unless I'm missing a big cluestick. Needs work for that.

Then I had a question about:

+  public function prepareRow(Row $row) {
+    // Find filters for this format.
+    $filters = $this->select('filter', 'f')
+      ->fields('f')
+      ->condition('format', $row->getSourceProperty('format'))
+      ->condition('name', [
+        'filter_html',
+        'filter_autop',
+        'filter_url',
+        'filter_htmlcorrector',
+        'filter_html_escape',
+        'php_code',
+      ], 'IN')
+      ->condition('status', 1)
+      ->execute()
+      ->fetchAllAssoc('name');

Is there a reason we get this specific in terms of what filters to move over? Wouldn't the migration path for, say, BBCode look exactly the same? If so, why not just take care of all filters at the same time here?

Then some other stuff that phenaproxima answered in IRC and are fine:

  */
 class FilterFormatTest extends MigrateSqlSourceTestCase {
 
-  // The plugin system is not working during unit testing so the source plugin
-  // class needs to be manually specified.
   const PLUGIN_CLASS = 'Drupal\filter\Plugin\migrate\source\d6\FilterFormat';
 
-  // The fake Migration configuration entity.
   protected $migrationConfiguration = array(

Why remove docs? Because this is already covered in parent class and this stuff is just boilerplate.

-      'settings' => 'a:3:{s:12:"allowed_html";s:74:"<a> <em> <strong> <cite> <blockquote> <code> <ul> <ol> <li> <dl> <dt> <dd>";s:16:"filter_html_help";i:1;s:20:"filter_html_nofollow";i:0;}',
+      'settings' => 'a:3:{s:12:"allowed_html";s:22:"<div> <span> <ul> 
...
-      'settings' => 'a:1:{s:17:"filter_url_length";i:72;}',
+      'settings' => 'a:1:{s:17:"filter_url_length";s:3:"128";}',

Why change default values? Trying to catch edge cases.

Those are very valid points. It's not at all clear why the source plugin skips the things it does, especially when you consider that the D6 version of the plugin doesn't skip those things. Fixed.

Couple of minor fixes.

Earlier we talked about benjy providing final review of this one, so assigning to him.

Looks good from my side, though!

Patch looks good, one nitpick and a question.

1. +++ b/core/modules/filter/src/Plugin/migrate/source/d7/FilterFormat.php
   @@ -0,0 +1,71 @@
   + * Drupal 7 role source from database.
   

   s/role/filter

2. +++ b/core/modules/filter/src/Plugin/migrate/source/d7/FilterFormat.php
   @@ -0,0 +1,71 @@
   +      $filters[$id]['settings'] = unserialize($filter['settings']);
   

   The d6 source specifically maps certain settings into the appropriate keys. Am I right in saying the setting arrays are identical between D7/D8?

Fixed a few minor nits that were annoying me, plus #28-1.

The d6 source specifically maps certain settings into the appropriate keys. Am I right in saying the setting arrays are identical between D7/D8?

As far as I know, yes. D6 doesn't have a serialized filters.settings column (filter settings seem to be stored in the variable table) but D7 does.

Here's a diff of the D6 and D7 versions of this migration and its plugins.

Looks good.

Silly testbot.

Ok, excellent. Looks like all feedback has been addressed.

Committed and pushed #29 to 8.0.x. Thanks!