Replace the entity manager with the entity type manager in the module.

CommentFileSizeAuthor
#5 2759243-5.patch13.48 KBrajeshwari10
#2 2759243-2.patch13.48 KBrajeshwari10

Comments

rajeshwari10 created an issue. See original summary.

rajeshwari10’s picture

Assigned: rajeshwari10 » Unassigned
Status: Active » Needs review
StatusFileSize
new13.48 KB

Replaced entityManager with entityTyepManager.

Status: Needs review » Needs work

The last submitted patch, 2: 2759243-2.patch, failed testing.

rajeshwari10’s picture

Status: Needs work » Needs review
rajeshwari10’s picture

StatusFileSize
new13.48 KB

Status: Needs review » Needs work

The last submitted patch, 5: 2759243-5.patch, failed testing.

rajeshwari10’s picture

Status: Needs work » Needs review

Dont know what is causing CI error.

Please review.

kylebrowning’s picture

Failing cause we have no tests written.

13:28:22 ERROR: No valid tests were specified.

Whats this benefit of this change?

rajeshwari10’s picture

@kylebrowning

entityManager is deprecated so we need to remove it from code base as said in

https://api.drupal.org/api/drupal/core!lib!Drupal.php/function/Drupal%3A...

I just replaced it with entityTypeManager.

kylebrowning’s picture

Status: Needs review » Reviewed & tested by the community
kylebrowning’s picture

Status: Reviewed & tested by the community » Fixed
nicola85’s picture

Status: Fixed » Needs review

There is no property 'entityTypeManager' in the class Drupal\ctools\Plugin\Deriver\EntityDeriverBase

rajeshwari10’s picture

@nicola85,

For entityTypeManager property i have added Drupa\Core\Entity\EntityTpyeManagerInterface.

kylebrowning’s picture

Status: Needs review » Fixed
therealssj’s picture

I still get the same error as pointed out by @nicolas85.
Even if Drupa\Core\Entity\EntityTpyeManagerInterface. has been used I don't see it getting injected anywhere.

If we take a look at Drupal\ctools\Plugin\Deriver\EntityDeriverBase
It still utilizes \Drupal\Core\Entity\EntityManagerInterface

 /**
   * The entity manager.
   *
   * @var \Drupal\Core\Entity\EntityManagerInterface
   */
  protected $entityManager;

  /**
   * Constructs new EntityViewDeriver.
   *
   * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
   *   The entity manager.
   * @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation
   *   The string translation service.
   */
  public function __construct(EntityManagerInterface $entity_manager, TranslationInterface $string_translation) {
    $this->entityManager = $entity_manager;
    $this->stringTranslation = $string_translation;
  }

So unless ctools is updated this commit will not work.

kylebrowning’s picture

Status: Fixed » Needs work

Looks like even with most up to date ctools, this still breaks.

kylebrowning’s picture

Version: 8.x-4.x-dev » 8.x-4.0-alpha4
Category: Task » Bug report

Its also now a bug since we introduced a breaking commit.

therealssj’s picture

I have posted a patch in ctools for this https://www.drupal.org/node/2761717
If that doesn't get merged anytime soon, one other way to fix this would be to replace $this->entityTypeManager with \Drupal::entityTypeManager though this is obviously not a good way to fix this.

lahoosascoots’s picture

In all the derivers we just need to change $this->entityTypeManager back to $this->entityManager in all instances. The parent class EntityDeriverBase still uses entityManager as the property name. Once ctools commits the above issue we can switch it.

kylebrowning’s picture

Category: Bug report » Task

I think this is a just a task, since everything works still.

  • kylebrowning committed c2e9a37 on 8.x-4.x
    Revert "Issue #2759243 by rajeshwari10: Replace the entity manager with...
jcnventura’s picture

Status: Needs work » Closed (duplicate)

This was done as part of the Drupal 9 readiness in #3154535: Drupal 9