- Issue created by @rfmarcelino
- 🇮🇳India vinaymahale
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.
- Status changed to Needs work
over 1 year ago 10:47am 25 June 2023 - 🇮🇳India vinaymahale
Please fix the below PHPCS issue
FILE: /entity_display_json/src/Controller/EntityDisplayJsonController.php -------------------------------------------------------------------------------------------------------------- FOUND 1 ERROR AFFECTING 1 LINE -------------------------------------------------------------------------------------------------------------- 101 | ERROR | Type hint "\Drupal\Core\Entity\EntityInterface" missing for $entity --------------------------------------------------------------------------------------------------------------
- Status changed to Needs review
over 1 year ago 11:26am 25 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
Is there anything else that needs to be changed? That alone is not an application blocker.
- 🇵🇹Portugal rfmarcelino
Thank you @vinaymahale for your review.
Just added the missing type hint. - Status changed to Needs work
over 1 year ago 2:44pm 26 June 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/Controller/EntityDisplayJsonController.php
/** * The entity type manager. * * @var \Drupal\Core\Entity\EntityTypeManagerInterface */ protected $entityTypeManager;
That property is already defined from the parent class. There is no need to declare it.
/** * The entity display repository. * * @var \Drupal\Core\Entity\EntityDisplayRepositoryInterface */ protected $entityDisplayRepository; /** * The entity display repository. * * @var Drupal\path_alias\AliasManagerInterface */ protected $aliasManager;
The description for $aliasManager is copied from the other property.
path: '/ejson/{entity_type}/{uuid}/{display_id}' defaults: _title: 'Build JSON' _controller: '\Drupal\entity_display_json\Controller\EntityDisplayJsonController::build' display_id: 'default' requirements: _permission: 'access content'
Since the controller gives access to every entity on the site, including unpublished nodes that users with the access content permission would not otherwise see, or blocked accounts, which only users with the administer users permission should see, the permission used for that route is not sufficient.
At least, the route cannot be used to enumerate user accounts, since it requires the UUID which is not easy to gather (differently from the ID which is incremented by one for each entity). Still, there should be a flood protection.Since the route requires a UUID and the entity type that matches that UUID, how would the module be used?
- Status changed to Needs review
over 1 year ago 5:42pm 26 June 2023 - 🇵🇹Portugal rfmarcelino
Thank you @apaderno for your review.
- Removed the duplicate property
- Corrected the comment description
- Added entity access verification in all entities
The module can be used in conjunction with others like decoupled_router to get the uuid.
- Assigned to apaderno
- Status changed to RTBC
over 1 year ago 5:44pm 26 June 2023 - 🇮🇹Italy apaderno Brescia, 🇮🇹
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.
- Status changed to Fixed
over 1 year ago 5:45pm 26 June 2023 Automatically closed - issue fixed for 2 weeks with no activity.