- Issue created by @lolcode
- 🇮🇳India vishal.kadam Mumbai
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.
- 🇮🇳India vishal.kadam Mumbai
Usually, after reviewing a project, we allow the developer to opt projects into security advisory coverage.
This project is too small for us and it doesn't contain enough PHP code to really assess your skills as a developer.
Have you made any other contributions that we could instead review?
- Status changed to Needs work
almost 2 years ago 7:39am 13 February 2023 - Status changed to Needs review
almost 2 years ago 9:40am 13 February 2023 - 🇺🇸United States cmlara
Cursory review indicates this module has over 500 lines of PHP code and 20 methods.
This module appears be to exceeding the required minimum standard of 120 lines/5 functions.
- 🇮🇳India jitesh_1 Jaipur
Hello @cmlara,
"120 Lines/5 Functions"
In the documentation, I couldn't find this requirement. https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or-distribution-project/security-coverage → . If Git admin permits us to review, then we'll move on. - 🇮🇹Italy apaderno Brescia, 🇮🇹
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 smother 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, please update the issue summary to give the link to the correct project link and the issue title to contain that project name.
To the 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 → .
Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review. - If you have not done it yet, you should run
- Status changed to Needs work
almost 2 years ago 3:09pm 13 February 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
- 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/Plugin/search_api/processor/TargetBundleFilter.php
/** * {@inheritdoc} */ public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { /** @var static $processor */ $processor = parent::create($container, $configuration, $plugin_id, $plugin_definition); $processor->setEntityTypeManager($container->get('entity_type.manager')); $processor->setEntityTypeBundleInfo($container->get('entity_type.bundle.info')); $processor->setEntityFieldManager($container->get('entity_field.manager')); return $processor; }
The purpose of
ContainerInjectionInterface::create()
(or similar methods likeContainerFactoryPluginInterface::create()
) is passing the injected dependencies to the class constructor, non initializing the instance properties.The plugin does not seem to implement
ContainerFactoryPluginInterface
./** * Retrieves the entity type manager service. * * @return \Drupal\Core\Entity\EntityTypeManagerInterface * The entity type manager service. */ public function getEntityTypeManager() { return $this->entityTypeManager ?: \Drupal::entityTypeManager(); }
\Drupal
should not be used for the services a class uses.$processor = new static(['#index' => $index], 'target_bundle_filter', []); foreach ($processor->getReferenceFields() as $field_options) { if (count($field_options) > 0) { return TRUE; } }
That code is not calling the
create()
method; it is not correctly instantiating the plugin. - 🇺🇸United States cmlara
@jitesh_1:
The original discussion and consensus was here:
https://groups.drupal.org/node/195848It was on the review templates https://groups.drupal.org/node/427683 which are somewhat often used in this group, but never ported to the new documentation pages.
All documentation on 'too short' has generally since been in reference to those discussions.
More anecdotally: I've seen a project with only 71 lines of code and 1 method receive approval by the queue reviewers this year without need of an additionally longer project.
NOTE:
Project length doesn't guarantee that a project is acceptable.The ''too short' is just a minimal standard below which a project can be denied without need for discussion. A project could have thousands of lines of PHP code in length and receive a ruling of 'not complex enough' when a reviewer can provide a justification that not enough of the Drupal API or other PHP skillset has been included to demonstrate an ability to write secure code. - Status changed to Closed: won't fix
about 1 year ago 7:35am 12 October 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
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.