[D9] Group Bulk Operations

Hello @sivakarthik229

Thank you for your contribution and for the application for security advisory coverage. You did a great job!

I've reviewed this module and here are some things to fix/look at:

• context file: src/Form/AssignGroupRoleMultiple.php
  issue: you have this construction:

  /** @var \Drupal\group\Entity\GroupInterface[] $nodes */
  $groups = $this->storage->loadMultiple(array_keys($this->groupInfo));
  

  I suppose that you need to change $nodes to $groups.

• context file: src/Form/AssignGroupRoleMultiple.php
  issue: the class file is named AssignGroupRoleMultiple but you named the constructor method like:

  /**
     * Constructs a DeleteMultiple form object.
     *
     * @param \Drupal\Core\TempStore\PrivateTempStoreFactory $temp_store_factory
     *   The tempstore factory.
     * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
     *   The entity type manager.
     * @param \Drupal\Core\Session\AccountInterface $current_user
     *   Current user.
     */
  

  Please, give it a proper description.

• context file: src/Form/AssignGroupRoleMultiple.php
  issue: you have this construction $this->storage = $this->entityTypemanager->getStorage('group');. It will be more informative to change the property to $this->groupStorage.
• context file: src/Form/AssignGroupRoleMultiple.php
  issue/suggestion: you have an implemented batch under the submitForm() method. I see that the batch operation and the finished callbacks group_bulk_operations_batch_add_member/group_bulk_operations_batch_add_member_finished are placed inside group_bulk_operations.module file, which is not the best practice. A suggestion will be to move both callbacks into a service class or the form itself:
• context file: group_bulk_operations.module
  issue: both functions group_bulk_operations_batch_add_member and group_bulk_operations_batch_add_member_finished have missing @params description and also the argument types. Please, provide them.
• context file: src/Plugin/Action/AssignGroupRole.php
  issue: the class name is AssignGroupRole but the constructor description is:

    /**
     * Constructs a new DeleteNode object.
     *
     * @param array $configuration
     *   A configuration array containing information about the plugin instance.
     * @param string $plugin_id
     *   The plugin ID for the plugin instance.
     * @param mixed $plugin_definition
     *   The plugin implementation definition.
     * @param \Drupal\Core\TempStore\PrivateTempStoreFactory $temp_store_factory
     *   The tempstore factory.
     * @param \Drupal\Core\Session\AccountInterface $current_user
     *   Current user.
     */
  

  Please, set a proper description.

• suggestion: Please, add a composer.json file. Take a look at this document on how to implement it.

There might be something else but this is what I've found on my first look.

Regards,
Andrei

Hello @sivakarthik229

Thx! yeah, now is much better!

Let's see who else will write feedback but overwise it's ok for me. Nice job!

Regards,
Andrei

Looking at the commit history it appears that prior to opening the application there was only a single commit by the applicant to the project of only 22 lines. This commit occurred shortly after the applicant was granted access through the process for adopting modules not compatible with D9.

The commit history has increased since the application was opened, however I would recommend care in ensuring that only code the applicant created is used for the review.

Hello @sivakarthik229,
1.I was view modules and use drupal-check bug report. Below are details when I ran scan

 ------ -------------------------------------------------------------------------
  Line   group_bulk_operations\src\Form\AssignGroupRoleMultiple.php
 ------ -------------------------------------------------------------------------
  75     Access to an undefined property
         Drupal\group_bulk_operations\Form\AssignGroupRoleMultiple::$groupStorag
         e.
  76     Access to an undefined property
         Drupal\group_bulk_operations\Form\AssignGroupRoleMultiple::$user.
  130    Access to an undefined property
         Drupal\group_bulk_operations\Form\AssignGroupRoleMultiple::$groupStorag
         e.
  154    Call to an undefined method Drupal\Core\Entity\EntityInterface::get().
  158    Call to an undefined method
         Drupal\Core\Entity\EntityInterface::getGroupType().
  244    Access to an undefined property
         Drupal\group_bulk_operations\Form\AssignGroupRoleMultiple::$groupStorag
         e.
  249    Access to an undefined property
         Drupal\group_bulk_operations\Form\AssignGroupRoleMultiple::$user.
 ------ -------------------------------------------------------------------------

 ------ -------------------------------------------------------------------------
  Line   group_bulk_operations\src\Form\RemoveGroupUserMultiple.php
 ------ -------------------------------------------------------------------------
  75     Access to an undefined property
         Drupal\group_bulk_operations\Form\RemoveGroupUserMultiple::$groupStorag
         e.
  76     Access to an undefined property
         Drupal\group_bulk_operations\Form\RemoveGroupUserMultiple::$user.
  130    Access to an undefined property
         Drupal\group_bulk_operations\Form\RemoveGroupUserMultiple::$groupStorag
         e.
  219    Access to an undefined property
         Drupal\group_bulk_operations\Form\RemoveGroupUserMultiple::$groupStorag
         e.
  221    Access to an undefined property
         Drupal\group_bulk_operations\Form\RemoveGroupUserMultiple::$user.
 ------ -------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------
  Line   group_bulk_operations\src\Plugin\Action\AssignGroupRole.php
 ------ ------------------------------------------------------------------------
  29     Property
         Drupal\group_bulk_operations\Plugin\Action\AssignGroupRole::$tempStore
         has unknown class Drupal\user\SharedTempStore as its type.
         � Learn more at https://phpstan.org/user-guide/discovering-symbols
  81     Call to method set() on an unknown class Drupal\user\SharedTempStore.
         � Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ ------------------------------------------------------------------------

 ------ ------------------------------------------------------------------------
  Line   group_bulk_operations\src\Plugin\Action\RemoveGroupUser.php
 ------ ------------------------------------------------------------------------
  29     Property
         Drupal\group_bulk_operations\Plugin\Action\RemoveGroupUser::$tempStore
         has unknown class Drupal\user\SharedTempStore as its type.
         � Learn more at https://phpstan.org/user-guide/discovering-symbols
  81     Call to method set() on an unknown class Drupal\user\SharedTempStore.
         � Learn more at https://phpstan.org/user-guide/discovering-symbols
 ------ ------------------------------------------------------------------------

 [ERROR] Found 16 errors

2.I ran Drupal coding standards check code

FILE: ...l-test\web\modules\custom\group_bulk_operations\group_bulk_operations.module
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: ...est\web\modules\custom\group_bulk_operations\group_bulk_operations.views.inc
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: ...eb\modules\custom\group_bulk_operations\src\Form\AssignGroupRoleMultiple.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: ...eb\modules\custom\group_bulk_operations\src\Form\RemoveGroupUserMultiple.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: ...b\modules\custom\group_bulk_operations\src\Plugin\Action\AssignGroupRole.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: ...b\modules\custom\group_bulk_operations\src\Plugin\Action\RemoveGroupUser.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------


FILE: ...odules\custom\group_bulk_operations\src\Plugin\views\field\GroupBulkForm.php
----------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------------------
 1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
----------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------

• What follows is a quick review of the project; it doesn't mean to be complete
• Each review point doesn't show all the lines that should be changed; it shows a single example of what is wrong in the code
• The review points are about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; they aren't listed in any particular order, not even in order of importance

dependencies:
  - drupal:group

The Group module isn't a Drupal core module.

    $form['user'] = [
      '#type' => 'entity_autocomplete',
      '#target_type' => 'user',
      '#title' => $this->t('User'),
      '#description' => $this->t('Enter a comma separated list of user names.'),
      '#tags' => TRUE,
      '#required' => TRUE,
    ];
    $group_roles = $this->entityTypemanager
      ->getStorage('group_role')
      ->loadMultiple();
    $group_roles_options = [];
    foreach ($group_roles as $group_role) {
      if (in_array($group_role->get('audience'), ['anonymous', 'outsider'])) {
        continue;
      }
      $group_roles_options[$group_role->id()] = '['
        . $group_role->getGroupType()->label() . '] ' . $group_role->label();
    }
    $form['group_roles'] = [
      '#type' => 'checkboxes',
      '#title' => 'Group Roles',
      '#options' => $group_roles_options,
      '#required' => TRUE,
    ];
    $form['groups'] = [
      '#title' => $this->t('Groups'),
      '#theme' => 'item_list',
      '#items' => $items,
    ];
    $form = parent::buildForm($form, $form_state);

A confirmation form just asks confirmation about an operation. Its task isn't asking users to submit values, or select values.

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.

Thank you to all the reviewers.

@apaderno thx for a quick reaction!