[1.0.x] Xray Audit

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

1. Remove below lines. It will be automatically added by the packaging script.

xray_audit.info.yml

version: "1.0.0"
project: xray_audit
datestamp: 1675243844

2. It seems you have missed working on the coding standards. You can use the PHPCS tool for checking and resolving issues.

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

FILE: xray_audit/assets/css/xray-audit.css
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 2 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found 4
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: xray_audit/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 19 WARNINGS AFFECTING 19 LINES
----------------------------------------------------------------------
   5 | WARNING | Line exceeds 80 characters; contains 126 characters
  12 | WARNING | Line exceeds 80 characters; contains 112 characters
  15 | WARNING | Line exceeds 80 characters; contains 117 characters
  25 | WARNING | Line exceeds 80 characters; contains 85 characters
  27 | WARNING | Line exceeds 80 characters; contains 109 characters
  30 | WARNING | Line exceeds 80 characters; contains 102 characters
  46 | WARNING | Line exceeds 80 characters; contains 123 characters
  49 | WARNING | Line exceeds 80 characters; contains 122 characters
  63 | WARNING | Line exceeds 80 characters; contains 123 characters
  66 | WARNING | Line exceeds 80 characters; contains 116 characters
  68 | WARNING | Line exceeds 80 characters; contains 82 characters
  72 | WARNING | Line exceeds 80 characters; contains 123 characters
  73 | WARNING | Line exceeds 80 characters; contains 129 characters
  76 | WARNING | Line exceeds 80 characters; contains 131 characters
  85 | WARNING | Line exceeds 80 characters; contains 96 characters
 106 | WARNING | Line exceeds 80 characters; contains 127 characters
 107 | WARNING | Line exceeds 80 characters; contains 102 characters
 123 | WARNING | Line exceeds 80 characters; contains 175 characters
 129 | WARNING | Line exceeds 80 characters; contains 81 characters
----------------------------------------------------------------------


FILE: xray_audit/src/Plugin/tasks/QueryGroup/XrayAuditQueryTaskMediaPlugin.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 44 | ERROR | [x] Expected 1 space before "="; 0 found
 45 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: xray_audit/src/Plugin/tasks/QueryGroup/XrayAuditQueryTaskNodePlugin.php
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------
  18 | ERROR | [x] Doc comment star missing
  76 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found
  82 | ERROR | [x] Expected 1 space after FOREACH keyword; 0 found
 144 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: xray_audit/src/Plugin/tasks/QueryGroup/XrayAuditQueryTaskParagraphsPlugin.php
--------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
--------------------------------------------------------------------------------
 50 | ERROR | [x] Expected 1 space after "="; 2 found
 51 | ERROR | [x] Expected 1 space after "="; 2 found
 85 | ERROR | [x] Expected 1 space after "="; 2 found
 86 | ERROR | [x] Expected 1 space after "="; 2 found
 96 | ERROR | [x] Expected 1 space after "="; 0 found
 97 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected "NULL" but found "null"
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: xray_audit/xray_audit.info.yml
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 4 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------
  1 | WARNING | [ ] Remove "project" from the info file, it will be added by drupal.org packaging automatically
  1 | WARNING | [ ] Remove "datestamp" from the info file, it will be added by drupal.org packaging automatically
  1 | WARNING | [ ] Remove "version" from the info file, it will be added by drupal.org packaging automatically
  7 | WARNING | [ ] All dependencies must be prefixed with the project name, for example "drupal:"
 11 | ERROR   | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

For these applications, we need a project where, in at least the branch used for the application, most of the commits (if not all the commits) have been done from the user who applies.
The purpose of these applications is reviewing a project to understand what the user who applies understands about writing secure code that follows the Drupal coding standards and correctly uses the Drupal API. A project without code committed by the user who applies cannot be used for these applications, especially when the user doesn't have the permission to commit code.

I see commits from Luis Ruiz.

Hello Luis,

I have reviewed the changes, and everything looks fine to me.

Let’s wait for other reviewers to take a look, and if everything goes fine, you will get the role.

Thanks

Hi @lpeidro,
This module works for me. I review this module and did not find any error.

Thanks.

• 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/XrayAuditGroupsController.php

    usort($groupDefinitions, fn($a, $b) => ($a['sort'] ?? 0) <=> ($b['sort'] ?? 0));

Drupal 9 still requires PHP 7.3. A module that uses PHP 7.4 or PHP 8 features should explicitly require that PHP version, or require Drupal 10.x, which requires PHP 8.1 or higher versions.

src/Plugin/tasks/DisplaysGroup/XrayAuditEntityDisplaysPlugin.php

  /**
   * Entity type manager.
   *
   * @var \Drupal\xray_audit\Services\DisplayModesEntityArchitectureInterface
   */
  protected $extractorDisplayModes;

  /**
   * Constructs a \Drupal\Component\Plugin\PluginBase 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\xray_audit\Services\DisplayModesEntityArchitectureInterface $extractorDisplayModes
   *   Entity type manager.
   */

Both those comments contain wrong information: What described as entity type manager is not the entity type manager. The constructor is not for the \Drupal\Component\Plugin\PluginBase class.

src/Plugin/tasks/DisplaysGroup/XrayAuditEntityDisplaysPlugin.php

 /**
   * {@inheritdoc}
   */
  public function buildDataPresentationForCli(array $data, string $operation = NULL) {
    // @todo Implement buildDataPresentationForCli() method.
    return '';
  }

The code used for these applications is supposed to be complete. It should not contain methods that returns an empty string because the method has not been yet implemented.

  /**
   * Create static method.
   *
   * @param \Symfony\Component\DependencyInjection\ContainerInterface $container
   *   Container.
   * @param array $configuration
   *   Configuration.
   * @param string $plugin_id
   *   Plugin id.
   * @param mixed $plugin_definition
   *   Plugin definition.
   *
   * @return static
   */
  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {

Methods defined in an interface or parent class are documented with {@inheritdoc}.

    return [
      '#theme' => 'table',
      '#header' => ['Bundle', 'Displays', 'Displays Count'],
      '#rows' => $rows_current_data,
      '#weight' => 20,
    ];

Strings shown in the user interface need to be translatable.

src/Plugin/XrayAuditGroupPluginManager.php

  /**
   * Constructs XrayAuditGroupPluginManager object.
   *

The class name needs to include the full namespace.

@lpeidro,

Reporter must not mark the status as RTBC. Only the reviewers is allowed to do so.

Thanks

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 Slack #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 reviewers.