I noticed a little blemish while working on #2546212: Entity view/form mode formatter/widget settings have no translation UI

EntityListBuilder is a root class which has many many children and creates a dependency on the deprecated entity.manager.

So the purpose of this issue is to fixup all children which copy this outdated practise 

  public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) {
    return new static(
      $entity_type,
      $container->get('entity.manager')->getStorage($entity_type->id())
    );
  }
CommentFileSizeAuthor
#9 3035383-9.patch20.59 KBandypost
#9 interdiff.txt991 bytesandypost
#8 3035383-8.patch20.47 KBandypost
#8 interdiff.txt3.19 KBandypost
#7 3035383-7.patch20.28 KBandypost
#7 interdiff.txt3.93 KBandypost
#4 interdiff-3035383-2-4.txt612 bytesmartin107
#4 3035383-4.patch21.17 KBmartin107
#2 3035383-2.patch21.17 KBmartin107
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107 created an issue. See original summary.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Assigned: martin107 » Unassigned
Status: Active » Needs review
FileSize
3035383-2.patch21.17 KB

In a few classes such as FieldConfigListBuilder.php

entity_manager, was also being using in the class.

If entity_manager goes away this is maybe a breaking change for a contrib module - core has a convention for this
DeprecatedServicePropertyTrait and where appropriate I have made use of that

Status: Needs review » Needs work

The last submitted patch, 2: 3035383-2.patch, failed testing. View results

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Needs work » Needs review
FileSize
3035383-4.patch21.17 KB
interdiff-3035383-2-4.txt612 bytes

My goof-up was $entity__type_manager

that well known double underscore convention for creating errors.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Parent issue: » #2886622: Deprecate all EntityManager methods with E_USER_DEPRECATED

Please always add the entity manager deprecation parent so the issues can be found easily. Also be careful to check the existing issues for overlaps, some are changing a lot of places.

+++ b/core/modules/config_translation/src/Controller/ConfigTranslationFieldListBuilder.php
@@ -37,19 +46,19 @@ class ConfigTranslationFieldListBuilder extends ConfigTranslationEntityListBuild
    * {@inheritdoc}
    */
   public static function createInstance(ContainerInterface $container, EntityTypeInterface $entity_type) {
-    $entity_manager = $container->get('entity.manager');
+    $entity_type_manager = $container->get('entity_type.manager');
     return new static(
       $entity_type,
-      $entity_manager->getStorage($entity_type->id()),
-      $entity_manager
+      $entity_type_manager->getStorage($entity_type->id()),
+      $entity_type_manager
     );
   }

This one is already in #3025427: Add @trigger_error() to deprecated EntityManager->EntityTypeBundleInfo methods I think.

You will notice that it's not always so easy, because often methods of entityManager were used that are now on different services.

That's why I'm currently working on deprecations grouped by the replacing service, like the issue above. But soon there's only EntityFieldManager and EntityTypeManager left, so in general +1 one updating all entity list builder classes together, but you might want to avoid overlaps with the above issue.

Status: Needs review » Needs work

The last submitted patch, 4: 3035383-4.patch, failed testing. View results

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Needs review
FileSize
interdiff.txt3.93 KB
3035383-7.patch20.28 KB

re-roll after #3025427: Add @trigger_error() to deprecated EntityManager->EntityTypeBundleInfo methods and fix most of failures

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
interdiff.txt3.19 KB
3035383-8.patch20.47 KB

A bit more fixes and clean-up

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
FileSize
interdiff.txt991 bytes
3035383-9.patch20.59 KB

and the last one

The last submitted patch, 7: 3035383-7.patch, failed testing. View results

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Reviewed & tested by the community

Looks good I think.

I think that injecting the entity storage is something we shouldn't do, and one case injects the list of entity type definitions, that's even weirder, but changing that is not in scope of this issue.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Title: EntityListBuilder::createInstance() remove entity.manager dependency » Replace deprecated usages of entityManager in list builder classes

Filed follow-up #3038302: Do not cache entity type definitions in EntityDisplayModeListBuilder

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.8.x-dev » 8.7.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 9c547baa7e to 8.8.x and 15332e965d to 8.7.x. Thanks!

  • catch committed 9d96532 on 8.8.x 
    Issue #3035383 by andypost, martin107, Berdir: Replace deprecated usages...

  • catch committed 15332e9 on 8.7.x 
    Issue #3035383 by andypost, martin107, Berdir: Replace deprecated usages...

Status: Fixed » Closed (fixed)

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