[1.0.x] Media Field Gallery

Thank you for applying!

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.

The important notes are the following.

• If you have not done it yet, you should enable GitLab CI for the project and fix the PHP_CodeSniffer errors/warnings it reports.
• For the time this application is open, only your commits are allowed.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status will not be changed by this application; once this application is closed, you will be able to change the project status from Not covered to Opt into security advisory coverage. This is possible only 14 days after the project is created.
  
  Keep in mind that once the project is opted into security advisory coverage, only Security Team members may change coverage.
• Only the person who created the application will get the permission to opt projects into security advisory coverage. No other person will get the same permission from the same application; that applies also to co-maintainers/maintainers of the project used for the application.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.

The important notes are the following.

• It is preferable to wait for a project moderator before posting the first comment on newly created applications. Project moderators will do some preliminary checks that are necessary before any change on the project files is suggested.
• Reviewers should show the output of a CLI tool only once per application.
• It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.

1. FILE: mediafield_gallery.info.yml

core_version_requirement: ^9 || ^10 || ^11

A new project should not declare itself compatible with a Drupal release that is no longer supported. No site should be using Drupal 8 nor Drupal 9, and people should not be encouraged to use those Drupal releases.

2. FILE: mediafield_gallery.module

For a new module that aims to be compatible with Drupal 10 and Drupal 11, I would rather implement hooks as class methods as described in Support for object oriented hook implementations using autowired services.
It would require increasing the minimum Drupal 10 version supported, but Drupal 10.1 is no longer supported.

/**
 * @file
 * Contains mediafield_gallery.module.
 */

The usual description for a .module file is “Hook implementations for the [module name] module”, where [module name] is the module name given in the .info.yml file.

3. FILE: templates/media-item.html.twig and templates/overlay.html.twig

Strings shown in the user interface must be translatable. That holds true also for strings used in template files.

4. FILE: templates/overlay.html.twig

Twig code needs to be correctly indented. Drupal uses two spaces for indentation, not four spaces or tabs.

5. FILE: src/Form/MediaFieldGallerySettingsForm.php

ConfigFormBase::__construct() requires two parameters. See the change record.

With Drupal 10 and Drupal 11, there is no longer need to use #default_value for each form element, when the parent class is ConfigFormBase: It is sufficient to use #config_target, as in the following code.

    $form['image_toolkit'] = [
      '#type' => 'radios',
      '#title' => $this->t('Select an image processing toolkit'),
      '#config_target' => 'system.image:toolkit',
      '#options' => [],
    ];

Using that code, it is no longer needed to save the configuration values in the form submission handler: The parent class will take care of that.
For this change, it is necessary to require at least Drupal 10.3, but that is not an issue, since Drupal 10.2.x is no longer supported.

  /**
   * The entity field manager.
   *
   * @var \Drupal\Core\Entity\EntityFieldManager
   */
  protected EntityFieldManager $entityFieldManager;

  /**
   * The entity type manager.
   *
   * @var \Drupal\Core\Entity\EntityTypeManagerInterface
   */
  protected EntityTypeManagerInterface $entityTypeManager;

  /**
   * Constructs a MediaFieldGallerySettingsForm object.
   *
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   *   The config factory.
   * @param \Drupal\Core\Entity\EntityFieldManager $entity_field_manager
   *   The entity field manager.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager.
   */
  public function __construct(
    ConfigFactoryInterface $config_factory,
    EntityFieldManager $entity_field_manager,
    EntityTypeManagerInterface $entity_type_manager,
  ) {
    parent::__construct($config_factory);
    $this->entityFieldManager = $entity_field_manager;
    $this->entityTypeManager = $entity_type_manager;
  }

New modules, which are compatible with Drupal 10 and higher versions are expected to include type declarations in property definitions, and use constructor property promotion.

6. Fix the warnings/errors reported by PHP_CodeSniffer.

Note: I would suggest enabling GitLab CI for the project, follow the Drupal Association .gitlab-ci.yml template and fix the PHP_CodeSniffer errors/warnings it reports.

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

FILE: mediafield_gallery/mediafield_gallery.module
--------------------------------------------------------------------------------
FOUND 49 ERRORS AND 28 WARNINGS AFFECTING 53 LINES
--------------------------------------------------------------------------------
   8 | WARNING | [x] Unused use statement
   9 | WARNING | [x] Unused use statement
  10 | WARNING | [x] Unused use statement
  11 | WARNING | [x] Unused use statement
  12 | WARNING | [x] Unused use statement
  13 | WARNING | [x] Unused use statement
  19 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  23 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  25 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  26 | ERROR   | [x] Expected newline after closing brace
  27 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  30 | ERROR   | [x] Whitespace found at end of line
  37 | ERROR   | [x] Whitespace found at end of line
  38 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  45 | WARNING | [x] There must be no blank line following an inline comment
  45 | WARNING | [ ] There must be no blank line following an inline comment
  45 | ERROR   | [ ] Comment indentation error, expected only 1 spaces
  45 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  51 | ERROR   | [x] No space found before comment text; expected "// print_r($field->field);" but found "//print_r($field->field);"
  52 | ERROR   | [x] No space found before comment text; expected "// print_r("\n");" but found "//print_r("\n");"
  52 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  54 | WARNING | [x] There must be no blank line following an inline comment
  54 | WARNING | [ ] There must be no blank line following an inline comment
  56 | WARNING | [x] There must be no blank line following an inline comment
  56 | WARNING | [ ] There must be no blank line following an inline comment
  56 | ERROR   | [x] No space found before comment text; expected "// print_r($variables['row']->_entity);" but found "//print_r($variables['row']->_entity);"
  56 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
  68 | WARNING | [ ] Unused variable $key.
  70 | WARNING | [x] There must be no blank line following an inline comment
  70 | WARNING | [ ] There must be no blank line following an inline comment
  70 | ERROR   | [x] No space found before comment text; expected "// print_r($node->get('field_post_upload_media')->getValue());" but found
     |         |     "//print_r($node->get('field_post_upload_media')->getValue());"
  72 | WARNING | [x] There must be no blank line following an inline comment
  72 | WARNING | [ ] There must be no blank line following an inline comment
  72 | ERROR   | [x] No space found before comment text; expected "// print_r($value);" but found "//print_r($value);"
  76 | ERROR   | [x] No space found before comment text; expected "// 'alt' => $value['alt']," but found "//'alt' => $value['alt'],"
  77 | ERROR   | [x] No space found before comment text; expected "// 'width' => $value['width']," but found "//'width' => $value['width'],"
  78 | ERROR   | [x] No space found before comment text; expected "// 'height' => $value['height']," but found "//'height' => $value['height'],"
  81 | ERROR   | [x] Expected 0 spaces between "''" and comma; 1 found
  82 | WARNING | [x] A comma should follow the last multiline array item. Found: 0
  84 | WARNING | [x] There must be no blank line following an inline comment
  84 | WARNING | [ ] There must be no blank line following an inline comment
  90 | ERROR   | [x] Namespaced classes/interfaces/traits should be referenced with use statements
  93 | WARNING | [x] There must be no blank line following an inline comment
  93 | WARNING | [ ] There must be no blank line following an inline comment
 111 | WARNING | [ ] Line exceeds 80 characters; contains 95 characters
 111 | WARNING | [x] There must be no blank line following an inline comment
 111 | WARNING | [ ] There must be no blank line following an inline comment
 111 | ERROR   | [x] No space found before comment text; expected "// $image_data['url'] = \Drupal::service('file_system')->realpath($image_data['uri']);" but found
     |         |     "//$image_data['url'] = \Drupal::service('file_system')->realpath($image_data['uri']);"
 111 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 115 | WARNING | [ ] Line exceeds 80 characters; contains 86 characters
 125 | WARNING | [x] There must be no blank line following an inline comment
 125 | WARNING | [ ] There must be no blank line following an inline comment
 125 | ERROR   | [x] 6 spaces found before inline comment; expected "// print_r($image_data['url']);" but found "//      print_r($image_data['url']);"
 125 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 148 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 151 | ERROR   | [x] Expected newline after closing brace
 152 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 157 | ERROR   | [x] Whitespace found at end of line
 160 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 164 | ERROR   | [x] Whitespace found at end of line
 165 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 175 | ERROR   | [x] Whitespace found at end of line
 178 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 195 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 199 | ERROR   | [x] Whitespace found at end of line
 203 | ERROR   | [x] Whitespace found at end of line
 205 | ERROR   | [x] Whitespace found at end of line
 206 | ERROR   | [x] Inline comments must end in full-stops, exclamation marks, question marks, colons, or closing parentheses
 210 | ERROR   | [ ] Invalid function name, expected get_media_field but found getMediaField
 210 | ERROR   | [ ] All functions defined in a module file must be prefixed with the module's name, found "getMediaField" but expected
     |         |     "mediafield_gallery_getMediaField"
 210 | ERROR   | [x] Missing function doc comment
 210 | ERROR   | [x] Expected 1 space before opening brace; found 0
 214 | ERROR   | [x] Expected newline after closing brace
 216 | ERROR   | [x] Expected newline after closing brace
 218 | ERROR   | [x] Expected newline after closing brace
 220 | ERROR   | [x] Expected newline after closing brace
 224 | ERROR   | [x] Expected 1 newline at end of file; 2 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 62 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

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

FILE: mediafield_gallery/mediafield_gallery.info.yml
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 3 WARNINGS AFFECTING 2 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
 14 | ERROR   | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: mediafield_gallery/README.md
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 29 | ERROR | [x] Expected 1 newline at end of file; 2 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

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

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

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

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

I am changing priority as per Issue priorities.