Migrate "edit link settings" permission

migration for "edit link settings" permission.

1. +++ b/linkchecker.module
   @@ -217,3 +217,13 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   + * as permission name is changed in Drupal9.
   

   s/Drupal9/Drupal 9/

2. +++ b/linkchecker.module
   @@ -217,3 +217,13 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +    $migrations['d7_user_role']['process']['permissions'][0]['map']['edit link settings'] = 'edit linkchecker link settings';
   

   Should have 2 leading spaces. There's 4 here.

migrate linkchecker settings from node to respective fields.

Getting error on applying patch because of missing use statements.

1. +++ b/linkchecker.module
   @@ -217,3 +217,60 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +function linkchecker_migrate_prepare_row(Row $row, MigrateSourceInterface $source, MigrationInterface $migration){
   

   Missing use statements 😊

2. +++ b/src/Plugin/migrate/process/LinkChecker.php
   @@ -0,0 +1,27 @@
   + * transform D7 linkchecker node settings to D9 fields.
   

   Coding standard issue. Description should start with capital letter

3. +++ b/src/Plugin/migrate/process/LinkChecker.php
   @@ -0,0 +1,27 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
   +    if($row->getSourceProperty('linkchecker_enabled')){
   +      return ['scan' => true, 'extractor' => 'html_link_extractor' ];
   +    }
   +    return NULL;
   +  }
   

   Coding standard issues:
   1. Line missing before function.
   2. 1 Space after if statement
   3. 1 Space after closing braces
   4. true => TRUE

follow up for #8.

Permissions and configurations seems to be migrating correctly.
It would be great if we can prove a test coverage for migration of node settings to field settings.

Also found some coding standard issues:

1. +++ b/src/Plugin/migrate/process/LinkChecker.php
   @@ -0,0 +1,28 @@
   +class LinkChecker extends ProcessPluginBase {
   +  /**
   

   Missing blank line.

2. +++ b/src/Plugin/migrate/process/LinkChecker.php
   @@ -0,0 +1,28 @@
   +
   

   Probably we want to add a comment here to explain what we are trying to achieve here.

3. +++ b/src/Plugin/migrate/process/LinkChecker.php
   @@ -0,0 +1,28 @@
   +    if($row->getSourceProperty('linkchecker_enabled')) {
   

   Coding standard issue. 1 Space after if is required.

Added test coverage and made suggested changes.

fixed coding standards.

Tested manually and found:

• Permissions are migrated properly
• Content type setting is migrated to fields
• Comment settings is not migrated to respective comments

1. +++ b/tests/fixtures/drupal7.php
   @@ -0,0 +1,980 @@
   diff --git a/tests/src/Kernel/LinkCheckerTest.php b/tests/src/Kernel/LinkCheckerTest.php
   

   Filename can be more descriptive eg: LinkcheckerMigrationTest.php

2. +++ b/tests/src/Kernel/LinkCheckerTest.php
   @@ -0,0 +1,64 @@
   +        'extractor' => 'html_link_extractor'
   +      ]
   

   Comma missing from end at 2 places.

fix to migrate comment settings and follow-up for #13.

Tested manually and issue raised in #13 is fixed.

Small textual change, but apart from that good for rtbc:

+++ b/linkchecker.module
@@ -217,3 +221,61 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
+ * Mapped "edit link settings" to edit "linkchecker link settings"

Mapped "edit link settings" to edit "linkchecker link settings" => Mapped "edit link settings" to edit "edit linkchecker link settings"

Only a few nits:

1. +++ b/linkchecker.module
   @@ -217,3 +221,61 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +  $migrations['d7_user_role']['process']['permissions'][0]['map']['edit link settings'] = 'edit linkchecker link settings';
   

   Before you do this, you should ensure that the d7_user_role migration exists and also verify that the first process plugin config in its permissions destination property is a static_map plugin.

2. +++ b/linkchecker.module
   @@ -217,3 +221,61 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +  foreach (array_keys($field_migrations) as $plugin_id) {
   +    $migrations[$plugin_id]['process']['third_party_settings/linkchecker']['plugin'] = 'linkchecker_config';
   +  }
   

   If you added a unique migration tag here, it would be enough to search for that tag in the beginning of the migrate prepare row hook (to do an early return).

3. +++ b/linkchecker.module
   @@ -217,3 +221,61 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +  $field_type = $row->getSourceProperty('type');
   +  if (!($field_type == 'text_with_summary' || $field_type == 'text_long' || $field_type == 'text')) {
   +    return;
   +  }
   

   I would prefer an in_array() function here:

   $supported_field_types = [
     'text',
     'text_long',
     'text_with_summary',
   ];
   if (!in_array($row->getSourceProperty('type'), $supported_field_types, TRUE)) {
     return;
   }
   

4. +++ b/linkchecker.module
   @@ -217,3 +221,61 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +  if ($entity_type !== 'node' && $entity_type !== 'comment') {
   +    return;
   +  }
   

   in_array()?

5. +++ b/linkchecker.module
   @@ -217,3 +221,61 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +  if ($linkchecker_enabled) {
   +    $row->setSourceProperty('linkchecker_enabled', (bool) $linkchecker_enabled);
   +  }
   

   If you set the expected ['scan' => TRUE, 'extractor' => 'html_link_extractor'] array here as value, you would not need the linkchecker_config process plugin.

6. +++ b/tests/src/Kernel/LinkCheckerMigrationTest.php
   @@ -0,0 +1,64 @@
   +    $var = $this->config('field.field.node.article.body');
   +    $this->assertEquals([
   +      'linkchecker' => [
   +        'scan' => TRUE,
   +        'extractor' => 'html_link_extractor',
   +      ],
   +    ], $var->getRawData()['third_party_settings']);
   

   A: It would be more straightforward checking the field config entity here:

   $article_body_field = FieldConfig::load('node.article.body');
   assert($article_body_field instanceof FieldConfigInterface);
   $this->assertEquals(
     [
       'scan' => TRUE,
       'extractor' => 'html_link_extractor',
     ],
     $article_body_field->getThirdPartySettings('linkchecker')
   );
   

   B: Since you already have a pretty neat test, it would be a good idea to check the text fields of the other entity types as well (the body field of the page node type, or the comment_body fields of the migrated comment types.

follow up#16.

1. +++ b/linkchecker.module
   @@ -217,3 +221,78 @@ function linkchecker_migration_plugins_alter(array &$migrations) {
   + * Adds third party setting.
   

   "Adds third party settings migration process pipeline to field migrations to migrate linkchecker settings."

2. +++ b/linkchecker.module
   @@ -217,3 +221,78 @@ function linkchecker_migration_plugins_alter(array &$migrations) {
   +  $linkchecker_migrations = array_filter(
   +    $migrations,
   +    function ($definition) {
   +      return $definition['id'] === 'd7_user_role';
   +    }
   +  );
   +  foreach (array_keys($linkchecker_migrations) as $plugin_id) {
   +    if ($migrations[$plugin_id]['process']['permissions'][0]['plugin'] === 'static_map') {
   +      $migrations[$plugin_id]['process']['permissions'][0]['map']['edit link settings'] = 'edit linkchecker link settings';
   +    }
   +  }
   

   I don't think there is a patch that would break up user role migrations (like #3097336: Improve the DX for migrating content entities: view modes, fields, formatters, widgets etc should be migrated per entity type + bundle), so it is safe to check and modify only d7_user_role:

   if (
     !empty($migrations['d7_user_role']['process']['permissions'][0]['plugin']) &&
     $migrations['d7_user_role']['process']['permissions'][0]['plugin'] === 'static_map'
   ) {
     $migrations['d7_user_role']['process']['permissions'][0]['map']['edit link settings'] = 'edit linkchecker link settings';
   }
   

3. +++ b/linkchecker.module
   @@ -217,3 +221,78 @@ function linkchecker_form_views_exposed_form_alter(&$form, FormStateInterface $f
   +    $row->setSourceProperty('linkchecker_config', ['scan' => TRUE, 'extractor' => 'html_link_extractor']);
   

   This violates coding standard. Could you please replace this with

   $row->setSourceProperty(
     'linkchecker_config',
     ['scan' => TRUE, 'extractor' => 'html_link_extractor'],
   );
   

4. +++ b/tests/src/Kernel/LinkCheckerMigrationTest.php
   @@ -0,0 +1,82 @@
   + * Tests linkchecker configuration.
   

   "Tests linkchecker migrations."

5. +++ b/tests/src/Kernel/LinkCheckerMigrationTest.php
   @@ -0,0 +1,82 @@
   +class LinkCheckerMigrationTest extends MigrateDrupal7TestBase {
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  protected static $modules = [
   +    'linkchecker',
   +    'node',
   +    'text',
   +    'filter',
   +    'menu_ui',
   +    'dynamic_entity_reference',
   +    'field',
   +    'system',
   +    'user',
   +    'comment',
   +  ];
   

   1. No need for dynamic_entity_reference.
   2. field, system and user modules are installed in the parent class MigrateDrupalTestBase.

6. +++ b/tests/src/Kernel/LinkCheckerMigrationTest.php
   @@ -0,0 +1,82 @@
   +    $this->executeMigrations(['d7_node_type', 'd7_comment_type']);
   +    $this->executeMigrations(['d7_filter_format']);
   +    $this->executeMigrations(['d7_field']);
   +    $this->executeMigrations(['d7_field_instance']);
   

   You can execute this migrations in one step:

   $this->executeMigrations([
     'd7_node_type',
     'd7_comment_type',
     'd7_filter_format',
     'd7_field',
     'd7_field_instance',
   ]);
   

7. +++ b/tests/src/Kernel/LinkCheckerMigrationTest.php
   @@ -0,0 +1,82 @@
   +    $this->assertEquals([
   +        'scan' => TRUE,
   +        'extractor' => 'html_link_extractor',
   +      ],
   +      $article_body_field->getThirdPartySettings('linkchecker')
   +    );
   ...
   +    $this->assertEquals([
   +        'scan' => TRUE,
   +        'extractor' => 'html_link_extractor',
   +      ],
   +      $page_body_field->getThirdPartySettings('linkchecker')
   +    );
   ...
   +    $this->assertEquals([
   +        'scan' => TRUE,
   +        'extractor' => 'html_link_extractor',
   +      ],
   +      $article_comment_field->getThirdPartySettings('linkchecker')
   +    );
   

   Could you please fix these array indentation errors? (My editor does the same thing if I copy-paste, but it's wrong.)

follow up for #18

Re #20:

1. +++ b/linkchecker.module
   @@ -127,6 +127,9 @@ function linkchecker_form_field_config_form_alter(&$form, FormStateInterface $fo
   +/**
   + *
   + */
   

   This should be removed. We shoudn't touch preexisiting CS violations, mostly because it is less likely that the actual patch conflits with an another one.

2. +++ b/linkchecker.module
   @@ -228,19 +231,15 @@ function linkchecker_migration_plugins_alter(array &$migrations) {
   +  !empty($migrations['d7_user_role']['process']['permissions'][0]['plugin']) &&
   +  $migrations['d7_user_role']['process']['permissions'][0]['plugin'] === 'static_map'
   

   Wrong indentation here.

#22 is perfect!

@elber, we're very sorry, we had to wrap up this ASAP.

Thanks. Just released a new beta right before this one, totally forgot I wanted to include it (it was kind of the point).

Just going to tag another beta with only this commit.

Thanks everyone for working on this!