Replace deprecated EntityManager and add EntityFieldManager in GroupContentViewsData

The minimum supported version is still core 8.6.x so the tests on PHP5.6 need to pass.

Thanks for this patch, and doing it in a separate issue. Hope this can be committed soon (before 8.x-1.0).

Mostly, the patch looks good, and the testbot is happy. Almost RTBC. One question/concern:

+++ b/src/Entity/Views/GroupContentViewsData.php
@@ -3,6 +3,8 @@
+use Drupal\Core\Entity\EntityFieldManagerInterface;

@@ -16,6 +18,20 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
+  /**
+   * The entity field manager.
+   *
+   * @var \Drupal\Core\Entity\EntityFieldManagerInterface
+   */
+  protected $entityFieldManager;
+

@@ -35,9 +51,21 @@ class GroupContentViewsData extends EntityViewsData {
+   * @param \Drupal\Core\Field\EntityFieldManagerInterface $entity_field_manager
+   *   The entity field manager.
...
+      $this->entityFieldManager = $entity_field_manager;

@@ -51,7 +79,9 @@ class GroupContentViewsData extends EntityViewsData {
+      $container->get('entity_field.manager')

It doesn't look like the EntityFieldManager is used anywhere. I know if we're going through the hassle to change constructors and inject services, maybe it makes sense to do both, assuming we'll want it eventually. But perhaps it'd be better to reduce the scope of this issue and only inject the service we're actually using (EntityTypeManager). Edit: I was wrong. See #13.

Thoughts?

Thanks!
-Derek

Oh, I see. It's because EntityViewsData::__construct() is expecting both services. I skimmed that detail when just looking at the patch.

I retract concern #11. Let's get this in.

Thanks!
-Derek

I'd rather see this patch remain in the issue queue for a while until 8.8 is released. I'm not a fan of adding in version checks for core versions that aren't even supported yet. If, for some reason, you are running a D8.8 site already, you should be fine by applying the above patch yourself.

Postponing until 8.8 is released. Also marking as a task, rather than a bug report because there is arguably no bug here. Any objections?

Re: #14:

One concern with postponing this and waiting for 8.8.0 to be released is that the d.o testbot uses 8.8.x-dev as an obvious, almost default test environment to use. All patches that test against that will fail until this code is in.

Next concern is that group official releases don't necessarily need to be on the same schedule as core releases. You might want to ship 8.x-1.0 before 8.8.0 is out. Once 8.8.0 is out, it'd be unfortunate to have to ship a 8.x-1.1 release of group just to fix this code. If you commit this now, 8.x-1.0 will be compatible with 8.8.x. Err, maybe "should be compatible" -- we don't know what else they might decide to deprecate/break in the meanwhile. ;) But at least there will be no currently-known brokenness (like we have now).

I don't see core un-doing the changes that this issue is meant to address. That train has left the station. So, I see very little benefit in waiting, and some costs to delay.

Would you be willing to reconsider?

Thanks!
-Derek

Should be fixed by #3081176: \Drupal\views\EntityViewsData is missing constructor BC for entity field manager, verifying...

This should suffice after the latest core patch mentioned above.