Since 8.7.0, EntityViewBuilder::__construct() takes an optional fifth argument (non-optional after 9.0.0), which is an EntityDisplayRepositoryInterface implementation.

It then checks whether that argument was indeed passed and triggers a warning if it wasn't. However the message in the warning is incorrect: it mentions Calling EntityViewBuilder::__construct() with the $entity_repository argument [...], but:

  • there is already an EntityRepositoryInterface implementation as the second argument
  • the warning should actually refer to the fifth argument which is an EntityDisplayRepositoryInterface instead of EntityRepositoryInterface.

Fix: change the wording to Calling EntityViewBuilder::__construct() with the $entity_display_repository argument [...].

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fgm created an issue. See original summary.

jyotimishra-developer’s picture

Assigned: Unassigned » jyotimishra-developer
jyotimishra-developer’s picture

@fgm
Hi,can you kindly tell me how did you produce or encounter this error?

fgm’s picture

It happened when writing the D9 patch for https://drupal.org/project/dfp which only passes the required parameters to this builder from a inherited builder in the module...using the legacy EntityManagerInterface instead of the Repository as second parameter. So my patch changed the second parameter to Repository and it didn't make sense to get this incorrect error although I had just added the Repository. That's when I noticed it was in fact about the DisplayRepository, not the Repository.

atul4drupal’s picture

atul4drupal’s picture

Issue tags: +error message

Thanks jyotimishra123, for the patch at #5 it is good to fix the issue and returns expected message

jyotimishra-developer’s picture

Assigned: jyotimishra-developer » Unassigned
jyotimishra-developer’s picture

jyotimishra-developer’s picture

Hi @fgm, please review

ju.vanderw’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed the trigger error and it communicates the correct parameter name and the fact that it will be required before 9.0.0.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/lib/Drupal/Core/Entity/EntityViewBuilder.php
@@ -108,7 +108,7 @@ public function __construct(EntityTypeInterface $entity_type, EntityRepositoryIn
-      @trigger_error('Calling EntityViewBuilder::__construct() with the $entity_repository argument is supported in drupal:8.7.0 and will be required before drupal:9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
+      @trigger_error('Calling EntityViewBuilder::__construct() without the $entity_display_repository argument is supported in drupal:8.7.0 and will be required before drupal:9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);

The text "__construct() with the $argument" is standardised across core. See the \Drupal\views\Plugin\views\row\EntityRow::__construct()

  public function __construct(array $configuration, $plugin_id, array $plugin_definition, EntityTypeManagerInterface $entity_type_manager, LanguageManagerInterface $language_manager, EntityRepositoryInterface $entity_repository = NULL, EntityDisplayRepositoryInterface $entity_display_repository = NULL) {
    parent::__construct($configuration, $plugin_id, $plugin_definition);

    $this->entityTypeManager = $entity_type_manager;
    $this->languageManager = $language_manager;

    if (!$entity_repository) {
      @trigger_error('Calling EntityRow::__construct() with the $entity_repository argument is supported in drupal:8.7.0 and will be required before drupal:9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
      $entity_repository = \Drupal::service('entity.repository');
    }
    $this->entityRepository = $entity_repository;

    if (!$entity_display_repository) {
      @trigger_error('Calling EntityRow::__construct() with the $entity_display_repository argument is supported in drupal:8.7.0 and will be required before drupal:9.0.0. See https://www.drupal.org/node/2549139.', E_USER_DEPRECATED);
      $entity_display_repository = \Drupal::service('entity_display.repository');
    }
    $this->entityDisplayRepository = $entity_display_repository;
  }

So that should not change here but +1 to detailing the correct argument in the message.

jyotimishra-developer’s picture

Assigned: Unassigned » jyotimishra-developer
jyotimishra-developer’s picture

Assigned: jyotimishra-developer » Unassigned
Status: Needs work » Needs review
fgm’s picture

Status: Needs review » Reviewed & tested by the community

Addressed alexpott's remarks, so LGTM.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: incorrect_error_EntityViewBuilder-3133386-14.patch, failed testing. View results

adityasingh’s picture

Status: Needs work » Reviewed & tested by the community

Marking RTBC based on #16 and tests are green.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Even though we only commit major bugs to 8.9.x I think this is worth it because it helps people prep for Drupal 9 so it's worth fixing - and committing has no risk.

Committed fca86a9 and pushed to 8.9.x. Thanks!

  • alexpott committed fca86a9 on 8.9.x
    Issue #3133386 by jyotimishra123, fgm: Incorrect error text in...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.