[1.x] Json Users Import

Hi @vishal.kadam & @kishan@lnwebworks,

Thank you for your review and the feedback.

All the feedback's are done and moved to the 1.x branch.

1. Fix PHPCS issues. See details in attached file → .

2. json_users_import.info.yml

version: '8.x-1.1'
datestamp: 1667820989

Remove add lines. These will be added automatically by the packaging script.

Manual Review

Hi @vishal.kadam,

Thanks for your review. I have worked for PHPCS issues and changes are moved to the 1.x branch.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

src/Controller/JsonUsersImportController.php

    if ($userId > 0) {
      return ['id' => $userId, 'returnMsg' => $email . ' is already exists!'];
    }

Strings shown to users should be translated.
As a side note, the correct phrase is already exists, not is already exists.

    $userId = \Drupal::entityQuery('user')->condition('name', $username)->range(0, 1)
      ->execute();
    $userId = array_shift($userId);
    if ($userId > 0) {
      return ['id' => $userId, 'returnMsg' => $username . ' is already exists!'];
    }

Dependency injection must be used in controller classes.

    if (substr($username, 0, 1) == ' ') {
      $valid = FALSE;
      $returnMsg = $username . ' cannot begin with a space.';
    }

As a side note, to check the first character of a string, it is sufficient to use $username[0] == ' '.

  /**
   * Create user data if user not exists.
   *
   * @param array| $value
   *   An array of user data from the currently uploaded json data.
   * @param string| $emailKey
   *   Email key from the currently uploaded json data.
   * @param string| $usernameKey
   *   Username key from the currently uploaded json data.
   * @param array| $fields
   *   An array of fields from the user import configuration.
   *
   * @return string
   *   Status of the created user record.
   */

The verb used in the short description should use the third person singular.
| is not using on type hinting using a single type.

      $statusMsg = 'User ' . $value[$usernameKey] . ' is created successfully!';
      \Drupal::logger('json_users_import')->notice($statusMsg);

Logged strings should use placeholders.

      $message = t('There was a problem sending your email notification to @email.', ['@email' => $to]);
      \Drupal::messenger()->addError($message);
      \Drupal::logger('mail-log')->error($message);

Strings passed to the logger are not translated.

    $message = t('An email notification has been sent to @email', ['@email' => $to]);
    \Drupal::messenger()->addStatus($message);
    \Drupal::logger('mail-log')->notice($message);

A message saying that an email has been sent are not notice messages, but informative messages.

src/Form/JsonUsersImportConfig.php

    $form['user_import_fieldmap'] = [
      '#type' => 'details',
      '#title' => t('User Import Field Configuration'),
      '#attributes' => ['id' => ['user_import_fieldmap']],
      '#open' => TRUE,
    ];

Forms and controllers should use $this->t() or the TranslatableMarkup class.

Still there are PHPCS issues.

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml json_users_import/

FILE: json_users_import/json_users_import.module
--------------------------------------------------------------------------------
FOUND 89 ERRORS AND 2 WARNINGS AFFECTING 42 LINES
--------------------------------------------------------------------------------
   1 | ERROR   | [x] Missing file doc comment
  17 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  17 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  27 | ERROR   | [x] Expected 1 spaces after parameter type; 0 found
  29 | ERROR   | [x] Whitespace found at end of line
  30 | ERROR   | [ ] Parameter tags must be grouped together in a doc comment
  33 | ERROR   | [ ] Type hint "array" missing for $context
  33 | ERROR   | [ ] All functions defined in a module file must be prefixed with the module's name, found "creating_users_batch" but expected
     |         |     "json_users_import_creating_users_batch"
  33 | ERROR   | [x] Expected 1 space before opening brace; found 0
  36 | ERROR   | [x] Short array syntax must be used to define arrays
  38 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  38 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  39 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  39 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  40 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  40 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  41 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  41 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  42 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  42 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  42 | WARNING | [ ] Unused variable $key.
  43 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  43 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  43 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  44 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  44 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  45 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  45 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  45 | ERROR   | [x] Expected 1 space after IF keyword; 0 found
  46 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  46 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  46 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  47 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  47 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  48 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  48 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  48 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  49 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  49 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  50 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  50 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  50 | ERROR   | [x] Expected 1 space after IF keyword; 0 found
  51 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  51 | ERROR   | [x] Line indented incorrectly; expected 8 spaces, found 4
  52 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  52 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  52 | ERROR   | [x] Expected newline after closing brace
  52 | ERROR   | [x] Use "elseif" in place of "else if"
  52 | ERROR   | [x] Expected 1 space after IF keyword; 0 found
  53 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  53 | ERROR   | [x] Line indented incorrectly; expected 8 spaces, found 4
  54 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  54 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  54 | ERROR   | [x] Expected newline after closing brace
  55 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  55 | ERROR   | [x] Line indented incorrectly; expected 8 spaces, found 4
  56 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  56 | ERROR   | [x] Line indented incorrectly; expected 8 spaces, found 4
  57 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  57 | ERROR   | [x] Line indented incorrectly; expected 8 spaces, found 4
  57 | ERROR   | [x] Spaces must be used for alignment; tabs are not allowed
  57 | ERROR   | [x] Whitespace found at end of line
  58 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  58 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  59 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  59 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  59 | ERROR   | [x] Expected newline after closing brace
  60 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  60 | ERROR   | [x] Line indented incorrectly; expected 6 spaces, found 3
  61 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  61 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  62 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  62 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  63 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  63 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  64 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  64 | ERROR   | [x] Line indented incorrectly; expected 2 spaces, found 1
  77 | ERROR   | [ ] Type hint "array" missing for $results
  77 | ERROR   | [ ] All functions defined in a module file must be prefixed with the module's name, found "users_finished_batch" but expected
     |         |     "json_users_import_users_finished_batch"
  81 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  81 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  82 | ERROR   | [x] Object operator not indented correctly; expected 4 spaces but found 8
  85 | ERROR   | [x] Spaces must be used to indent lines; tabs are not allowed
  85 | ERROR   | [x] Line indented incorrectly; expected 4 spaces, found 2
  99 | ERROR   | [x] Short array syntax must be used to define arrays
 102 | ERROR   | [x] Namespaced classes/interfaces/traits should be referenced with use statements
 106 | WARNING | [ ] Only string literals should be passed to t() where possible
 107 | ERROR   | [x] Expected one space after the comma, 0 found
 107 | ERROR   | [ ] The array declaration extends to column 156 (the limit is 80). The array content should be split up over multiple lines
 107 | ERROR   | [x] Expected 1 space between comma and "'clear'"; 0 found
 107 | ERROR   | [x] Expected one space after the comma, 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 83 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: json_users_import/json_users_import.routing.yml
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 15 | ERROR | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: json_users_import/src/Controller/JsonUsersImportController.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 17 WARNINGS AFFECTING 17 LINES
--------------------------------------------------------------------------------
  22 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
  39 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
  56 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 116 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 117 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 139 | WARNING | Unused variable $result.
 147 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 151 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 167 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 168 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 182 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 186 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 187 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 188 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 191 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 192 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 193 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
--------------------------------------------------------------------------------


FILE: json_users_import/src/Form/JsonUsersImport.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 65 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
--------------------------------------------------------------------------------


FILE: json_users_import/src/Form/JsonUsersImportConfig.php
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 18 WARNINGS AFFECTING 15 LINES
--------------------------------------------------------------------------------
  39 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  51 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  58 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  73 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  80 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  83 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
  88 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 107 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 108 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 113 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 132 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 133 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 133 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 154 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 155 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 155 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
 208 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 208 | WARNING | t() calls should be avoided in classes, use \Drupal\Core\StringTranslation\StringTranslationTrait and $this->t() instead
--------------------------------------------------------------------------------

@kurinjiselvan, Please apply this patch to fix the PHPCS issue on 1.x-dev version.

@kurinjiselvan, Please try to use dependency injection for following.

 $userAccountFields = array_filter(\Drupal::service('entity_field.manager')->getFieldDefinitions('user', 'user'), function ($fieldDefinition) {
  

$userId = \Drupal::entityQuery('user')->condition('mail', $email)->execute();

 $config = \Drupal::config('json_users_import.import_configuration');
    $language = \Drupal::languageManager()->getCurrentLanguage()->getId();
   

 $config = \Drupal::config('json_users_import.import_configuration');
    $mailManager = \Drupal::service('plugin.manager.mail');
   

\Drupal::messenger()->addStatus($message);
    \Drupal::logger('mail-log')->notice($message);

@shashank5563 Reviewers don't provide patches to fix what reported in a review.

Yes, in this queue, we just review code; we do not write patches.

I am changing priority as per Issue priorities → .

I am closing this application, since there haven't been replies in more than six months and the application has been created eight months ago or more.
Feel free to re-open it, once the project has been changed basing on what reported in the last review.