[2.1.x] Entity Extra Field

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 run phpcs --standard=Drupal,DrupalPractice on the project, which alone fixes most of what reviewers would report.
• 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 won't be changed by this application and no other user will be able to opt projects into security advisory policy.
• 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 Code Review Administrator before commenting on newly created applications. Code Review Administrators 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.

Hi all, I reviewed the module code some days ago already as we planned using the module in several projects and just did a review again using https://www.drupal.org/node/1587704

I think the module is a great and couldn't find any major issues, just a little potential bug: #3468262: Wrong theme hook suggestion?

And some code style issues: #3466812: Write (basic) automated tests (I think it would be great to have these tests and further tests added once the application here as been resolved)

The module uses several Drupal API functionalities like

• Theme System
• Plugin System & Services
• Form API
• Field API
• Submodules
• ...

and is a super useful contrib extension, so clear +1 on RTBC once the code style issues have been resolved. ✅

Thank you @droath! GREAT WORK!

1. Fix phpcs issues.

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

FILE: entity_extra_field/entity_extra_field.module
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
--------------------------------------------------------------------------------
  11 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityStorageInterface.
 128 | WARNING | [x] A comma should follow the last multiline array item. Found: )
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/modules/entity_extra_field_ui/src/Plugin/Derivative/MenuLinksTask.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 20 | ERROR | Missing short description in doc comment
--------------------------------------------------------------------------------

FILE: entity_extra_field/modules/entity_extra_field_ui/src/Plugin/Derivative/MenuLinksAction.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 20 | ERROR | Missing short description in doc comment
--------------------------------------------------------------------------------

FILE: entity_extra_field/modules/entity_extra_field_ui/src/Routing/RouteSubscriber.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 17 | ERROR | Missing short description in doc comment
--------------------------------------------------------------------------------

FILE: entity_extra_field/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 11 WARNINGS AFFECTING 11 LINES
----------------------------------------------------------------------
  4 | WARNING | Line exceeds 80 characters; contains 222 characters
  6 | WARNING | Line exceeds 80 characters; contains 264 characters
 14 | WARNING | Line exceeds 80 characters; contains 180 characters
 16 | WARNING | Line exceeds 80 characters; contains 168 characters
 21 | WARNING | Line exceeds 80 characters; contains 249 characters
 25 | WARNING | Line exceeds 80 characters; contains 116 characters
 27 | WARNING | Line exceeds 80 characters; contains 101 characters
 29 | WARNING | Line exceeds 80 characters; contains 109 characters
 35 | WARNING | Line exceeds 80 characters; contains 156 characters
 39 | WARNING | Line exceeds 80 characters; contains 234 characters
 41 | WARNING | Line exceeds 80 characters; contains 138 characters
----------------------------------------------------------------------

FILE: entity_extra_field/src/ExtraFieldTypePluginInterface.php
--------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
--------------------------------------------------------------------------------
  8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Plugin\ConfigurableInterface.
 44 | ERROR | [x] Function comment short description must start with exactly one space
 50 | ERROR | [x] Return comment indentation must be 3 spaces, found 4 spaces
 52 | ERROR | [x] Expected 1 blank line after function; 0 found
 53 | ERROR | [x] The closing brace for the interface must have an empty line before it
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldBlockPlugin.php
--------------------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 4 LINES
--------------------------------------------------------------------------------
   8 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Plugin\Context\Context.
  43 | ERROR   | [ ] Missing short description in doc comment
  48 | ERROR   | [ ] Missing short description in doc comment
 271 | WARNING | [x] A comma should follow the last multiline array item. Found: )
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldEntityLinkPlugin.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Access\AccessResult.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldTokenPlugin.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 67 | ERROR | [x] Array indentation error, expected 10 spaces but found 8
 68 | ERROR | [x] Array indentation error, expected 10 spaces but found 8
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldComponentPlugin.php
--------------------------------------------------------------------------------
FOUND 8 ERRORS AND 3 WARNINGS AFFECTING 11 LINES
--------------------------------------------------------------------------------
   8 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\Unicode.
  32 | ERROR   | [ ] Missing short description in doc comment
 167 | WARNING | [ ] Only string literals should be passed to t() where possible
 180 | WARNING | [x] A comma should follow the last multiline array item. Found: NULL
 185 | ERROR   | [x] Expected newline after closing brace
 218 | ERROR   | [x] Expected 1 space after USE keyword; found 0
 243 | WARNING | [x] A comma should follow the last multiline array item. Found: ]
 281 | ERROR   | [x] Return comment indentation must be 3 spaces, found 4 spaces
 356 | ERROR   | [x] Expected newline after closing brace
 381 | ERROR   | [x] Expected 1 blank line after function; 0 found
 382 | ERROR   | [x] The closing brace for the class must have an empty line before it
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 9 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Plugin/ExtraFieldType/ExtraFieldViewsPlugin.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\views\ViewEntityInterface.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/ExtraFieldTypePluginManager.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Cache\CacheBackendInterface.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/EntityExtraFieldContextTrait.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Plugin\Context\ContextHandlerInterface.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Form/EntityExtraFieldFormDelete.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 8 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Cache\CacheBackendInterface.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Form/EntityExtraFieldForm.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityInterface.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Entity/EntityExtraField.php
--------------------------------------------------------------------------------
FOUND 16 ERRORS AND 4 WARNINGS AFFECTING 20 LINES
--------------------------------------------------------------------------------
   8 | ERROR   | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\NestedArray.
  64 | ERROR   | [ ] Missing short description in doc comment
  69 | ERROR   | [ ] Missing short description in doc comment
  74 | ERROR   | [ ] Missing short description in doc comment
  79 | ERROR   | [ ] Missing short description in doc comment
  84 | ERROR   | [ ] Missing short description in doc comment
  89 | ERROR   | [ ] Missing short description in doc comment
  94 | ERROR   | [ ] Missing short description in doc comment
  99 | ERROR   | [ ] Missing short description in doc comment
 104 | ERROR   | [ ] Missing short description in doc comment
 109 | ERROR   | [ ] Missing short description in doc comment
 114 | ERROR   | [ ] Missing short description in doc comment
 119 | ERROR   | [ ] Missing short description in doc comment
 124 | ERROR   | [ ] Missing short description in doc comment
 141 | WARNING | [x] A comma should follow the last multiline array item. Found: name
 288 | WARNING | [x] A comma should follow the last multiline array item. Found: )
 300 | WARNING | [x] A comma should follow the last multiline array item. Found: )
 356 | ERROR   | [x] Array indentation error, expected 10 spaces but found 8
 357 | ERROR   | [x] Array indentation error, expected 10 spaces but found 8
 424 | WARNING | [x] A comma should follow the last multiline array item. Found: )
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Entity/EntityExtraFieldInterface.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Config\Entity\ConfigEntityInterface.
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/ExtraFieldTypePluginBase.php
--------------------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
--------------------------------------------------------------------------------
   6 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Component\Utility\NestedArray.
  27 | ERROR | [ ] Missing short description in doc comment
  32 | ERROR | [ ] Missing short description in doc comment
  37 | ERROR | [ ] Missing short description in doc comment
  42 | ERROR | [ ] Missing short description in doc comment
  47 | ERROR | [ ] Missing short description in doc comment
 430 | ERROR | [x] Expected 1 blank line after function; 0 found
 431 | ERROR | [x] The closing brace for the class must have an empty line before it
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Annotation/ExtraFieldType.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 16 | ERROR | Missing short description in doc comment
 21 | ERROR | Missing short description in doc comment
--------------------------------------------------------------------------------

FILE: entity_extra_field/src/Controller/EntityExtraFieldListBuilder.php
--------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 7 LINES
--------------------------------------------------------------------------------
   9 | ERROR | [x] Use statements should be sorted alphabetically. The first wrong one is Drupal\Core\Entity\EntityInterface.
  22 | ERROR | [ ] Missing short description in doc comment
  27 | ERROR | [ ] Missing short description in doc comment
  39 | ERROR | [ ] Doc comment for parameter $current_route_match does not match actual variable name <undefined>
 111 | ERROR | [x] Expected newline after closing brace
 172 | ERROR | [x] Expected 1 blank line after function; 0 found
 173 | ERROR | [x] The closing brace for the class must have an empty line before it
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

2. FILE: README.md

The module uses a README.md file instead of a README.txt file. While the Drupal coding standards have not been yet updated about that, the Drupal.org community consider that positive.
Since there is a README.md file, that should follow the content and formatting described in README.md template.

3. FILE: src/ExtraFieldTypePluginBase.php

  /**
   * Extra field type view constructor.
   *
   * @param array $configuration
   *   The plugin configuration.
   * @param string $plugin_id
   *   The plugin identifier.
   * @param array $plugin_definition
   *   The plugin definition.
   * @param \Drupal\Core\Utility\Token $token
   *   The token service.
   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   *   The module handler service.
   * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match
   *   The current route match service.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager service.
   * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager
   *   The entity field manager service.
   */
  public function __construct(

FILE: src/ExtraFieldTypePluginManager.php

  /**
   * {@inheritdoc}
   */
  public function __construct(

FILE: src/Form/EntityExtraFieldForm.php

 /**
   * Define the extra field type manager.
   *
   * @param \Drupal\Core\Cache\CacheBackendInterface $cache_discovery_backend
   *   The cache discovery backend service.
   * @param \Drupal\Core\Plugin\Context\ContextRepositoryInterface $context_repository
   *   The context repository service.
   * @param \Drupal\Component\Plugin\PluginManagerInterface $extra_field_type_manager
   *   The extra field type plugin manager.
   * @param \Drupal\Component\Plugin\PluginManagerInterface $condition_plugin_manager
   *   The condition plugin manager.
   * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator
   *   The cache tags invalidator service.
   * @param \Drupal\Core\Entity\EntityDisplayRepositoryInterface $entity_display_repository
   *   The entity display repository.
   */
  public function __construct(

FILE: src/Form/EntityExtraFieldFormDelete.php

  /**
   * Entity extra field form delete constructor.
   */
  public function __construct(CacheBackendInterface $cache_discovery) {

FILE: src/Plugin/ExtraFieldType/ExtraFieldBlockPlugin.php

 /**
   * Extra field block plugin constructor.
   *
   * @param array $configuration
   *   The plugin configuration.
   * @param string $plugin_id
   *   The plugin identifier.
   * @param array $plugin_definition
   *   The plugin definition.
   * @param \Drupal\Core\Utility\Token $token
   *   The token service.
   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   *   The module handler service.
   * @param \Drupal\Core\Session\AccountInterface $current_user
   *   The current logged-in user object.
   * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match
   *   The current route match service.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager service.
   * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager
   *   The entity field manager service.
   * @param \Drupal\Core\Block\BlockManagerInterface $block_manager
   *   The block manager service.
   */
  public function __construct(

FILE: src/Plugin/ExtraFieldType/ExtraFieldComponentPlugin.php

  /**
   * Define the class constructor.
   *
   * @param array $configuration
   *   The plugin configuration.
   * @param string $plugin_id
   *   The plugin identifier.
   * @param array $plugin_definition
   *   The plugin definition.
   * @param \Drupal\Core\Utility\Token $token
   *   The token service.
   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   *   The module handler service.
   * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match
   *   The current route match service.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager service.
   * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager
   *   The entity field manager service.
   * @param \Drupal\sdc\ComponentPluginManager $component_plugin_manager
   *   The component plugin manager service.
   */
  public function __construct(

FILE: src/Plugin/ExtraFieldType/ExtraFieldEntityLinkPlugin.php

  /**
   * Extra field type view constructor.
   *
   * @param array $configuration
   *   The plugin configuration.
   * @param string $plugin_id
   *   The plugin identifier.
   * @param mixed $plugin_definition
   *   The plugin definition.
   * @param \Drupal\Core\Utility\Token $token
   *   The token service.
   * @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
   *   The module handler service.
   * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match
   *   The current route match service.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
   *   The entity type manager service.
   * @param \Drupal\Core\Entity\EntityFieldManagerInterface $entity_field_manager
   *   The entity field manager service.
   * @param \Drupal\Core\Access\AccessManagerInterface $access_manager
   *   The access manager service.
   */
  public function __construct(

The documentation comment for constructors is not mandatory anymore, If it is given, the description must be Constructs a new [class name] object. where [class name] includes the class namespace.

The project page needs to mention the existing, similar projects, such as the Extra Field module, and explain the differences between this module and the other ones.

I am changing priority as per Issue priorities.

This thread has been idle, in the needs work state with no activity for several months. Therefore, I am assuming that you are no longer pursuing this application.

May you confirm you are still pursuing this application? If this is the case, and you made commits basing on what previously reported, or you can answer the questions previously asked, please change the status to Needs review.

This thread has been idle, in the Needs work state with no activity for about eight months or more; the application has been created about 11 months ago or more. Therefore, I marked it as Closed (won't fix).

If this is incorrect, and you are still pursuing this application, please feel free to re-open it and set the issue status to Needs work or Needs review, depending on the current status of your code.

I would like to reopen this ticket to enable the security advisory for the Entity Extra Field Drupal module.

Just a clarification: That project is already covered by the security advisory policy, but these applications do not opt projects into security advisory coverage; they give to the person who applies the permission to opt projects into security advisory coverage.
The project is used just to verify how much the person who applies understands about writing secure code which follows the Drupal coding standards and correctly uses the Drupal API. That is why one of the requirements is that most (but preferably all) the commits are done by the person who applies.

What reported in comment #6 has not been done: The project page still does not mention the existing, similar projects.

Thanks @droath - I recently checked the project into SA. Please follow the steps mentioned here for your personal permissions :)
Let me know if I can help.

See also comment #2: Until this application is not closed as Fixed, the only allowed commits need to be done by the person who created the application.
These applications do not aim to understand what the project maintainers as group understand about writing secure code which follows the Drupal coding standards and correctly uses the Drupal API. They aims to understand what the person who applied understands about those topics.

A review has been done in comment #5. I just pointed out that the project needs to be updated to contain the information I gave in comment #6.

Anyway, since there are commits from other maintainers, the project no longer can be used by droath for this application.

This thread has been idle, in the Needs work state with no activity for about six months or more; the application has been created about 11 months ago or more. Therefore, I marked it as Closed (won't fix).