[D9] Field Visibility Manager

Please check http://pareview.net/r/361. These issues are related to are coding standards. For recheck you can use http://pareview.net/.

Thank you for applying! Remember to change status, as the currently used one means the project is not yet ready to be reviewed.

Automated tests fail
http://pareview.net/r/361

FILE: ...w/pareviewd/pareview_temp/tuzgplsy/field_visibility_manager.info.yml
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
5 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


DrupalPractice has found some issues with your code, but could be false positives.
FILE: ...viewd/pareview_temp/tuzgplsy/src/Form/FieldVisibilityManagerForm.php
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------
35 | WARNING | NodeType::loadMultiple calls should be avoided in
| | classes, use dependency injection instead
37 | WARNING | Role::loadMultiple calls should be avoided in classes,
| | use dependency injection instead
48 | WARNING | \Drupal calls should be avoided in classes, use
| | dependency injection instead
--------------------------------------------------------------------------

The server at pareview.net takes too long to respond. I would not count on it for reviewing project, for the next days.

• What follows is a quick review of the project; it doesn't mean to be complete
• For every point, the review doesn't make a complete list of lines that should be fixed, but an example of what is wrong in the code
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; if a point isn't about that, it makes it clear

The code needs to follow the Drupal coding standards for indentation and formatting.

   /**
   * Constructs object
   *
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager service.
   */
  public function __construct(EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager) {
      $this->entityTypeManager = $entity_type_manager;
      $this->entityFieldManager = $entity_field_manager;
  } 

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container) {
    return new static(
      $container->get('entity_type.manager'),
      $container->get('entity_field.manager')
    );
  }

Classes that extend ConfigFormBase should implement those methods with code similar to the following one (taken from AccountSettingsForm).

  public function __construct(ConfigFactoryInterface $config_factory, ModuleHandlerInterface $module_handler, RoleStorageInterface $role_storage) {
    parent::__construct($config_factory);
    $this->moduleHandler = $module_handler;
    $this->roleStorage = $role_storage;
  }

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container) {
    return new static($container
      ->get('config.factory'), $container
      ->get('module_handler'), $container
      ->get('entity_type.manager')
      ->getStorage('user_role'));
  }

  public function buildForm(array $form, FormStateInterface $form_state) {
    $node_types  = $this->entityTypeManager->getStorage('node_type')->loadMultiple();
    // List out all roles.
    $roles = $this->entityTypeManager->getStorage('user_role')->loadMultiple();
    $role_list = [];
    foreach ($roles as $role => $rolesObj) {
      $role_list[] = $role;
    }
    // List of all fields in system.
    $values = [];
    foreach ($node_types as $node_type) {
      $values[$node_type->id()] = $node_type->label();
      $entity_type_id = 'node';
      $bundle = $node_type->id();
      //$definitions = (\Drupal::service('entity_field.manager')->getFieldDefinitions($entity_type_id, $bundle));
      $definitions = $this->entityFieldManager->getFieldDefinitions($entity_type_id, $bundle);
      foreach ($definitions as $key => $val) {
        if (strpos($key, 'field_') !== FALSE) {
          $fieldNames[ucwords($bundle) . " - " .$val->getLabel()] = $key;
        }
      }
    }

    $bundleFields = $fieldNames;
    $config = $this->config('field_visibility_manager.adminsettings');
    // The permissions table.
    // For the initial loading.
    if (!$config->get('permissions')) {
      $form['permissions'] = [
        '#type' => 'table',
        '#header' => [$this->t('Field Label'), $this->t('Fields')],
        '#id' => 'permissions',
        '#attributes' => ['class' => ['permissions', 'js-permissions']],
        '#sticky' => TRUE,
      ];
      // Table columns.
      foreach ($role_list as $role) {
        $form['permissions']['#header'][] = [
          'data' => $role,
        ];
      }
      $form['permissions']['#header'][] = [
          'data' => "",
      ];
      // Table row values.
      foreach ($bundleFields as $key => $field) {
        foreach ($role_list as $role) {
           $form['permissions'][$key]['field_label'] = [
            '#type' => 'label',
            '#title' => $key,
          ];
          $form['permissions'][$key]['Field'] = [
            '#type' => 'textfield',
            '#default_value' => $field,
            '#attributes' => ['readonly' => 'readonly'],
          ];
          $form['permissions'][$key][$role] = [
            '#type' => 'checkbox',
            'data' => $field,
          ];
        }
        $form['permissions'][$key]['field_name'] = array(
            '#type' => 'value',
            '#value' => $key ,
        );
      }
    }
    $existingarray = [];
    $value = $config->get('permissions');
    foreach ($value as $field) {
      $existingarray[] = $field['Field'];
    }
    foreach ($definitions as $key => $val) {
      if (strpos($key, 'field_') !== FALSE) {
        $fieldNames[$val->getLabel()] = $key;
      }
    }
    // Newly added fields after configuration save.
    $result = array_diff($bundleFields, $existingarray);
    // Removed fields after configuration save.
    $oldresult = array_diff($existingarray, $bundleFields);
    // Removing the removed fields from listing.
    foreach ($value as $subKey => $subArray) {
      foreach ($oldresult as $exvalue) {
        if ($subArray['Field'] == $exvalue) {
          unset($value[$subKey]);
        }
      }
    }
    // Including the newly added fields for listing.
    foreach ($result as $label => $field) {
      $value[$label] = ["Field" => $field, "field_name" => $label];
    }
    // To bind saved values from config.
    if ($config->get('permissions')) {
      $form['permissions'] = [
        '#type' => 'table',
        '#header' => [$this->t('Field Label'), $this->t('Fields')],
        '#id' => 'permissions',
        '#attributes' => ['class' => ['permissions', 'js-permissions']],
        '#sticky' => TRUE,
      ];
      // Table columns.
      foreach ($role_list as $role) {
        $form['permissions']['#header'][] = [
          'data' => $role,
        ];
      }
       $form['permissions']['#header'][] = [
          'data' => "",
        ];
      // Table row values.
      foreach ($value as $key => $field) {
        foreach ($role_list as $role) {
           $form['permissions'][$key]['field_label'] = [
            '#type' => 'label',
            '#title' => $field['field_name'],
          ];
          $form['permissions'][$key]['Field'] = [
            '#type' => 'textfield',
            '#default_value' => $field['Field'],
            '#attributes' => ['readonly' => 'readonly'],
            'class' => ['checkbox'],
          ];
          $form['permissions'][$key][$role] = [
            '#type' => 'checkbox',
            'class' => ['checkbox'],
            '#default_value' => $field[$role],
          ];        
        }
          $form['permissions'][$key]['field_name'] = [
            '#type' => 'hidden',
            '#value' => $field['field_name'] ,
          ];
      }
    }
    return parent::buildForm($form, $form_state);

  }

parent::buildForm($form, $form_state) needs to be called at the beginning of the method.

  public function submitForm(array &$form, FormStateInterface $form_state) {
    parent::submitForm($form, $form_state);
    $this->config('field_visibility_manager.adminsettings')
      ->set('permissions', $form_state->getValue('permissions'))
      ->save();
    parent::submitForm($form, $form_state);

  }

parent::submitForm($form, $form_state) is called twice, when it should be called at the beginning of the method.

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the dedicated reviewers as well.