getDisplayModeOptions() returns only full or teaser regardless of the status of the entity display

I think this is a bug. Currently you can't choose a custom block to render in "Full" mode because we are checking the status that is "Whether or not this form or view mode has custom settings by default."

As @olli said in #2, the EntityManager::getDisplayModeOptions() method has a $include_disabled parameter, described as:

   * @param bool $include_disabled
   *   Force to include disabled display modes. Defaults to FALSE.

This is not right, given that the meaning of the status property of the entity type is not related to the display mode status. It means "Whether or not this form or view mode has custom settings by default."

The desired status is not available at this level, given that display modes can be enabled or disabled at bundle level.

Here I'm proposing a new approach after reviewing all the places where EntityManagerInterface::getViewModeOptions() and EntityManagerInterface::getFormModeOptions() are invoked.

• Remove the $include_disabled prameter from both methods above, given that is useless
• Add a new method BlockContentBlock::getViewModeOptions() where available view modes are displayed in the edit block form. Other places where these methods were called before are now OK with the new implementation where $include_disabled is removed.

Other approach could be add new methods EntityManagerInterface::getViewModeOptionsByBundle($entity_type_id, $bundle_id, $include_disabled = FALSE) where we could return this list. We could reuse this method in some places in the core code where this list is loaded like in the Manage Display pages or in the new node preview page select list.

Any thoughts?

Regards

Here is the new patch following the new methods approach.

Regards.

Sorry, forgot to include Test file changes and NodePreviewForm specific changes.

Regards.

Renaming variables.

Patch re-rolled

New version of the patch.

Re-roll attached. There were a bunch of conflicts but I think they're properly resolved. The entire relevant part of /core/modules/field/src/Entity/FieldConfig.php has been removed in HEAD, so that part of the patch from #14 is gone.

Just tested the patch with simplytest.me

I can now select all available view modes in the entity reference field formatter and also have them available in views. (getDisplayModeOptions() is used over there)
Also a fresh created view mode appears in the dropdown. Looks good so far :)

The tests for the custom block view mode selection fails. I also could not see any view mode selection fields over there. Does anyone have more infos about that? If the feature is just not implemented yet, we might be RTBC here.

Changing priority to major because this affects views, custom block, entity reference and the node preview.

Patch wouldn't apply. Here is a quick reroll.

Patch rerolled.

Thank you for that quick reroll epari.siva

We still do not pass the same tests. Does anyone have any informations about the block view mode switch functionality? Imho this patch will not solve this problem, so we might be already RTBC here.

+++ b/core/tests/Drupal/Tests/Core/Entity/EntityManagerTest.php
@@ -177,7 +184,8 @@ protected function setUp() {
+    ¶

Spacing issues here still

Going to see if I can reroll this so we can move this along. Search API is blocked because this is not available in core. See issue #2471669: Views should show the view mode for each bundle as view modes are configured per bundle

rerolling and rerunning tests

I do confirm that by using this patch, we would be unblocked from our issue in Search API. Awaiting the tests and will try to work through them

The test had wrong assumptions, but the patch is actually fine. Uploading a fixed version. It took me ages before I realised the tests were in a bad shape instead of the patch...

Looking better here. If someone can review this at Drupal Dev Days and commit this I'd be super happy so it can unblock Search API Views integration.

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,62 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +   * @param bool $include_disabled
   +   *   Force to include disabled view modes. Defaults to FALSE.
   

   That phpdoc needs to be adjusted, this is not about disabled view modes anymore. This is about filtering out modes for which the *display* is disabled.

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,62 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +    $display_type_entity_name = ($display_type == 'form_mode') ? 'entity_form_display' : 'entity_view_display';
   +    $entity_type = $this->getDefinition($display_type_entity_name);
   +    $config_prefix = $entity_type->getConfigPrefix();
   +
   +    // Load all the entity's display modes.
   +    $display_modes = $this->getDisplayModesByEntityType($display_type, $entity_type_id);
   +
   +    // Get the list of available display modes for the current block's bundle.
   +    $ids = $this->configFactory->listAll($config_prefix . '.' . $entity_type_id . '.' . $bundle);
   +    foreach ($ids as $id) {
   +      $load_ids[] = str_replace($config_prefix . '.', '', $id);
   +    }
   

   Comments and var names here are confusing - this is loading entity displays (EntitiyViewDisplay / EntityFormDisplay), not modes (EntityViewMode / EntityFormMode).

   Also : you only need to load the displays if $include_disabled is FALSE, right ?

3. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,62 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +      if ($display->status() || $include_disabled) {
   

   The 'default' display is not supposed to ever have its 'status' set to false, but for safety, you should always inlude that one in the list without checking its status.
   It should always be in the list, because at display-time it's always active as a fallback.

Other than that, not a fan of adding two new methods, a new __construct() dependency and yet more code in the EntityManager that's already 1300 lines long, but doing otherwise would require a refactor that's probably outside the scope of this patch. Opened #2472013: Move view_mode / display handilng code out of EntityManager and into a new EntityDisplayManager

But you only actually need to check the status if you don't want disabled ones ☺

I think this is a bit cleaner in how the Default option is being set.

Uploading interdiff for patch #41

Cleaning up

I think we can still simplify getDisplayModeOptionsByBundle() quite a bit :-)

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getDisplayModeOptionsByBundle($display_type, $entity_type_id, $bundle, $include_disabled) {
   +    $load_ids = array();
   +    $display_mode_options = array();
   +    $display_type_entity_name = ($display_type == 'form_mode') ? 'entity_form_display' : 'entity_view_display';
   +    $entity_type = $this->getDefinition($display_type_entity_name);
   +    $config_prefix = $entity_type->getConfigPrefix();
   +
   ...
   +    // Get the list of available entity displays for the current bundle.
   +    $ids = $this->configFactory->listAll($config_prefix . '.' . $entity_type_id . '.' . $bundle);
   +    foreach ($ids as $id) {
   +      $load_ids[] = str_replace($config_prefix . '.', '', $id);
   +    }
   

   This code could move in the if (!$include_disabled) { code branch

   Also, minor : I'd suggest to simplify/adjust var names a bit :
   s/$display_mode_options/$options
   s/$display_type_entity_name/$display_entity_type_id
   s/$entity_displays/$displays
   + remove $entity_type, inline it within the getConfigPrefix() line ?

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getDisplayModeOptionsByBundle($display_type, $entity_type_id, $bundle, $include_disabled) {
   +    // Load all the entity's display modes.
   

   Nitpick : "Collect" rather than "Load" ?

3. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +    // Set it as a list and always include our default view mode.
   

   Comment needs to be adjusted, the "always include default" part is not done in this code block anymore.
   Meaning, that comment line could go away IMO :-)

4. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +    // Load all the entity's display modes.
   +    $display_modes = $this->getDisplayModesByEntityType($display_type, $entity_type_id);
   +    // Set it as a list and always include our default view mode.
   +    foreach ($display_modes as $display_mode_id => $display_mode) {
   +      $display_mode_options[$display_mode_id] = $display_mode['label'];
   +    }
   

   Actually, this could now directly do $options = $this->getDisplayModeOptions($display_type, $entity_type_id) ?

5. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +      $entity_displays = entity_load_multiple($display_type_entity_name, $load_ids);
   

   We are in the EntityManager, so it should be $this->getStorage($display_type_entity_name)->load(...)

6. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +          $display_mode_name = $display->get('mode');
   +          unset($display_mode_options[$display_mode_name]);
   

   could be inlined, no need for the $display_mode_name var ?

7. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,23 +1011,75 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +    // Generate the display options array. We don't need to load the displays
   +    // if we don't care about the status.
   

   Likewise, comment needs updating - the options are already generated, this code block removes some of them if needed.

All comments were incorporated except for number 4. If we use that one, we have to start fiddling with the default and unset it if there are only 2 in the end. That seems unnecessary and not very clean to do.

Needs reroll ?
Otherwise looks good, thanks @Nick_vh !

Oops, sorry :

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,19 +1011,71 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +      $load_ids = array();
   +      $display_entity_type_id = ($display_type == 'form_mode') ? 'entity_form_display' : 'entity_view_display';
   +      $config_prefix = $this->getDefinition($display_entity_type_id)->getConfigPrefix();
   +
   +      // Get the list of available entity displays for the current bundle.
   +      $ids = $this->configFactory->listAll($config_prefix . '.' . $entity_type_id . '.' . $bundle);
   +      foreach ($ids as $id) {
   +        $load_ids[] = str_replace($config_prefix . '.', '', $id);
          }
   +      // Load the available entity displays.
   +      $displays = $this->getStorage($display_entity_type_id)->loadMultiple($load_ids);
   

   Actually, since the code now collects all possible view modes just above, no need for $this->configFactory->listAll() and the prefix dance anymore, we can just do :

   // Collect candidate display IDs for all available modes.
   $load_ids = [];
   foreach (array_keys($display_modes) as $mode) {
     $load_ids[] = $entity_type_id . '.' . $bundle . '.' . $mode;
   }
   // Load the corresponding displays.
   $displays = $this->getStorage($display_type == 'form_mode' ? 'entity_form_display' : 'entity_view_display')
     ->loadMultiple($load_ids);
   

   And we can thus revert the added dependency on the @config.factory :-)

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,19 +1011,71 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +      foreach ($displays as $display) {
   +        // Unset the display mode from the list if it is disabled.
   +        if (!$display->status()) {
   +          unset($options[$display->get('mode')]);
   +        }
   +      }
   

   Then once we have the $displays, there's a catch I didn't spot earlier : the display for a given $mode might not exist yet, and in our case we have to treat that as if it was disabled.

   So the code should be :

   foreach (array_keys($display_modes) as $mode) {
     $display_id = $entity_type_id . '.' . $bundle '.' . $mode;
     if (!isset($displays[$display_id]) || !$displays[$display_id]->status()) {
       unset($options[$mode]);
     }
   }
   

Cleaned up the code and the default is now added at the top and makes the code even easier and more readable

Thanks @Nick_vh - almost there :-)

1. +++ b/core/core.services.yml
   @@ -435,7 +435,7 @@ services:
   -    arguments: ['@container.namespaces', '@module_handler', '@cache.discovery', '@language_manager', '@string_translation', '@class_resolver', '@typed_data_manager', '@entity.definitions.installed', '@event_dispatcher']
   +    arguments: ['@container.namespaces', '@module_handler', '@cache.discovery', '@language_manager', '@string_translation', '@class_resolver', '@typed_data_manager', '@entity.definitions.installed', '@event_dispatcher', '@config.factory']
   
   +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -134,6 +135,13 @@ class EntityManager extends DefaultPluginManager implements EntityManagerInterfa
   +   * The config factory.
   +   *
   +   * @var \Drupal\Core\Config\ConfigFactoryInterface
   +   */
   +  protected $configFactory;
   +
   +  /**
   
   @@ -196,8 +204,10 @@ class EntityManager extends DefaultPluginManager implements EntityManagerInterfa
   +   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   +   *   The config factory.
       */
   -  public function __construct(\Traversable $namespaces, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache, LanguageManagerInterface $language_manager, TranslationInterface $translation_manager, ClassResolverInterface $class_resolver, TypedDataManager $typed_data_manager, KeyValueStoreInterface $installed_definitions, EventDispatcherInterface $event_dispatcher) {
   +  public function __construct(\Traversable $namespaces, ModuleHandlerInterface $module_handler, CacheBackendInterface $cache, LanguageManagerInterface $language_manager, TranslationInterface $translation_manager, ClassResolverInterface $class_resolver, TypedDataManager $typed_data_manager, KeyValueStoreInterface $installed_definitions, EventDispatcherInterface $event_dispatcher, ConfigFactoryInterface $config_factory) {
   
   @@ -209,6 +219,7 @@ public function __construct(\Traversable $namespaces, ModuleHandlerInterface $mo
   +    $this->configFactory = $config_factory;
   
   +++ b/core/tests/Drupal/Tests/Core/Entity/EntityManagerTest.php
   @@ -135,6 +135,13 @@ class EntityManagerTest extends UnitTestCase {
   +   * The config factory.
   +   *
   +   * @var \Drupal\Core\Config\ConfigFactoryInterface
   +   */
   +  protected $configFactory;
   +
   +  /**
   
   @@ -184,6 +191,7 @@ protected function setUp() {
   +    $this->configFactory = $this->getConfigFactoryStub();
   
   @@ -239,7 +247,7 @@ protected function setUpEntityManager($definitions = array()) {
   -    $this->entityManager = new TestEntityManager(new \ArrayObject(), $this->moduleHandler, $this->cacheBackend, $this->languageManager, $this->translationManager, $this->getClassResolverStub(), $this->typedDataManager, $this->installedDefinitions, $this->eventDispatcher);
   +    $this->entityManager = new TestEntityManager(new \ArrayObject(), $this->moduleHandler, $this->cacheBackend, $this->languageManager, $this->translationManager, $this->getClassResolverStub(), $this->typedDataManager, $this->installedDefinitions, $this->eventDispatcher, $this->configFactory);
   

   These changes can be reverted now, we don't use the config factory anymore (yay !)

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,19 +1011,64 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +    $options = array('default' => t('Default'));
   +
   +    // Collect all the entity's display modes.
   +    $display_modes = $this->getDisplayModesByEntityType($display_type, $entity_type_id);
   +    foreach ($display_modes as $display_mode_id => $display_mode) {
   +      $options[$display_mode_id] = $display_mode['label'];
   +    }
   

   Then this really is just strictly equivalent to $this->getDisplayModeOptions() now :-)

3. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -986,19 +1011,64 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +      // Unset the display modes that are not active or do not exists.
   

   minor : s/exists/exist

Cool - I think we have it :-)

Not marking down from RTBC but...

+++ b/core/modules/node/src/Form/NodePreviewForm.php
@@ -86,7 +86,11 @@ public function buildForm(array $form, FormStateInterface $form_state, EntityInt
+    // Unset view modes that are not used in the front end.
+    unset($view_mode_options['rss']);
+    unset($view_mode_options['search_index']);

How does contrib hook into this, if they're providing a non-front-end view mode? For example, https://www.drupal.org/files/project-images/screenshot_17.png shows "Tokens" and "Test bundle."

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -1028,13 +1028,11 @@ protected function getDisplayModeOptions($display_type, $entity_type_id) {
   -    $options = array('default' => t('Default'));
   -
        // Collect all the entity's display modes.
        $display_modes = $this->getDisplayModeOptions($display_type, $entity_type_id);
   

   Sorry to be a pain, but could we keep $options as a name for this ? (consistent with the internal var name in getDisplayModeOptions())

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -1028,13 +1028,11 @@ protected function getDisplayModeOptions($display_type, $entity_type_id) {
   -    // Remove the displays that are not set to active for this bundle if they
   -    // should not be returned.
   +    // Remove the display modes that are not set to active for this bundle if
   +    // they should not be returned.
   

   A little less accurate :-) What about :
   "If needed, filter out modes for which the entity display is disabled (or inexistent)" ?

@webchick : true, but the hardcoded special treatment of 'rss' and 'search_index' is not introduced by this patch, it's just moved around (see the code removed from NodePreviewForm). IIRC, it was already baked in the initial "node preview for realz" patch - I even think there was a followup issue opened for that ?

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -968,15 +968,29 @@ protected function getDisplayModesByEntityType($display_type, $entity_type_id) {
   +  public function getFormModeOptions($entity_type_id) {
   +    return $this->getDisplayModeOptions('form_mode', $entity_type_id);
   +  }
   ...
   +  public function getFormModeOptionsByBundle($entity_type_id, $bundle, $include_disabled = FALSE) {
   +    return $this->getDisplayModeOptionsByBundle('form_mode', $entity_type_id, $bundle, $include_disabled);
      }
   

   There are no usages of these methods anywhere - can we add tests as I guess these are here for symmetry with the view mode methods.

2. Also the $include_disabled parameter is never used (or tested) on getViewModeOptionsByBundle and getFormModeOptionsByBundle.

This would still need fixing :-)
Anyone up for rerolling / adressing #65 ?
(sorry, no drupal time on my side before another couple days...)

Re-roll required after #2472147: Standardize getter docblocks in node module was committed.

I will attempt to address the feedback in #65 in a separate patch.

When the getFormOptions method was introduced in #2154711-63: Move entity_get_(form/view)_mode_options() functions to EntityManager and add get(Form/View)ModeOptions() methods there was one use of it in the code. I will try and track down when it was removed.

Is it possible that the in $include_disabled parameter for getViewModeOptionsByBundle and getFormModeOptionsByBundle is scope creep and we should remove it here and should open a new issue to make a case for it and introduce the tests or implement it in the codebase if appropriate in that new issue?

The last use of getFormModeOptions was removed in this patch #2416409-65: Delete dependent config entities that don't implement onDependencyRemoval() when a config entity is deleted.

It feels rather strange adding tests for code that is never used, I know of some cases where that might be necessary (such as testing a hook that has not invoked in core) but us this one of those cases? If all use of it in the codebase has been stripped out over time then is it needed at all?

Getting rid of redundant parameter $include_disabled.

All that needs doing now is to provide tests for getFormModeOptions and getFormModeOptionsByBundle.

I'm going to get the ball rolling on the tests.

I'm not sure how this patch is going to be received but I found a place in the codebase that we should be using the getFormModeOptions and getViewModeOptions methods as the code in the Drupal\field_ui\Form\EntityDisplayFormBase::form was preparing an array just like the output of getFormModeOptions or getViewModeOptions.

If we prepare a method that is useful for contrib but are unwilling to use it where it is appropriate in code then it probably will not get used or at least it doesn't send out the right message.

If this is accepted and removes the need to prepare a test then we would just need a test for getFormModeOptionsByBundle. I'm fairly confident that a test is possible with phpunit which I'm not proficient with but willing to give it a go with some support. A test should belong in the Drupal\Tests\Core\Entity\EntityManagerTest class.

Leaving as 'Needs review' to invite feedback on the latest patch. I will try and prepare tests this weekend otherwise I will unassign.

The phpunit test is too tricky for me. I will review whatever someone else can come up with so that I can learn from it. So far, I'm getting stuck preparing the mock storage handler. So far I have this:

class EntityManagerTest extends UnitTestCase {


  /**
   * @covers ::getFormModeOptions
   */
  public function testGetFormModeOptions() {
    $apple = $this->prophesize(EntityTypeInterface::class);

    $this->setUpEntityManager(array(
      'apple' => $apple,
    ));

    $language = new Language(['id' => 'en']);
    $this->languageManager->getCurrentLanguage(LanguageInterface::TYPE_INTERFACE)->willReturn($language);
    $form_modes = $this->entityManager->getFormModeOptions('apple');
  }

}

This is not intended to be complete but I just don't know how to progress from this point.

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -1088,15 +1088,29 @@ protected function getDisplayModesByEntityType($display_type, $entity_type_id) {
   +  public function getViewModeOptionsByBundle($entity_type_id, $bundle) {
   ...
   +  public function getFormModeOptionsByBundle($entity_type_id, $bundle) {
   
   @@ -1106,19 +1120,55 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +  protected function getDisplayModeOptionsByBundle($display_type, $entity_type_id, $bundle) {
   

   Unlike the rest of the "display methods" here, those additionally filter by "the displays that are enabled for this bundle" - enabled/disabled is by bundle, otherwise all bundles have the same available modes.

   IMO we should make that clear in the name [edit : and also in the phpodoc : this looks at the available modes, but additionally looks at the corresponding entity displays for the bundle]. getEnabledDisplayModeOptionsByBundle() ?

2. +++ b/core/modules/field_ui/src/Form/EntityDisplayFormBase.php
   @@ -201,26 +201,23 @@ public function form(array $form, FormStateInterface $form_state) {
   +        if ($display_statuses = array_filter($this->getDisplayStatuses())) {
   +          $default = array_keys(array_intersect_key($display_mode_options, $display_statuses));
            }
   

   minor : by that definition, $display_statuses should be named $enabled_displays.

3. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -1106,19 +1120,55 @@ protected function getDisplayModeOptionsByBundle($display_type, $entity_type_id, $bundle) {
   +    // Load the corresponding displays.
   +    $displays = $this->getStorage($display_type == 'form_mode' ? 'entity_form_display' : 'entity_view_display')
   +      ->loadMultiple($load_ids);
   +
   +    // Unset the display modes that are not active or do not exist.
   +    foreach (array_keys($options) as $mode) {
   +      $display_id = $entity_type_id . '.' . $bundle . '.' . $mode;
   +      if (!isset($displays[$display_id]) || !$displays[$display_id]->status()) {
   +        unset($options[$mode]);
   

   Wonder if we could use an EntityQuery here to filter on status = 1. This loads the same amount of config records, but avoids instanciating the Entities ? Probably no biggie given this only for a given bundle.

3 since that would add another param in the contruct and make EntityManager become ubergod ? :)

Oh, right, I though the EntityManager had direct access to the EntityQuery builder just like an entity handler, but that's in fact a separate service.
Yeah, nevermind 3, that's a micro-optim at best anyway, we're only loading the displays for a given bundle, shouldn't be that heavy. Sorry about that :-)

Looks good, thanks @swentel !

Committed 64d6415 and pushed to 8.0.x. Thanks!

diff --git a/core/modules/field_ui/src/Tests/EntityDisplayTest.php b/core/modules/field_ui/src/Tests/EntityDisplayTest.php
index 376e096..53a8639 100644
--- a/core/modules/field_ui/src/Tests/EntityDisplayTest.php
+++ b/core/modules/field_ui/src/Tests/EntityDisplayTest.php
@@ -9,7 +9,6 @@
 
 use Drupal\Core\Cache\Cache;
 use Drupal\Core\Entity\Entity\EntityFormDisplay;
-use Drupal\Core\Entity\Entity\EntityFormMode;
 use Drupal\Core\Entity\Entity\EntityViewDisplay;
 use Drupal\Core\Entity\Entity\EntityViewMode;
 use Drupal\field\Entity\FieldConfig;

Removed unused use on commit.

1. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -1088,15 +1088,29 @@ protected function getDisplayModesByEntityType($display_type, $entity_type_id) {
   +  public function getEnabledViewModeOptionsByBundle($entity_type_id, $bundle) {
   

   I am missing this function. Cannot find it in core. What was the reason to not commit this when it was in the patch? @alexpott

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -1088,15 +1088,29 @@ protected function getDisplayModesByEntityType($display_type, $entity_type_id) {
   +  public function getEnabledFormModeOptionsByBundle($entity_type_id, $bundle) {
   
   @@ -1106,19 +1120,55 @@ public function getFormModeOptions($entity_type, $include_disabled = FALSE) {
   +  protected function getEnabledDisplayModeOptionsByBundle($display_type, $entity_type_id, $bundle) {
   

   Same goes for these.

Opps, it looks like the patch that got committed was #82 instead of #84...

So @axe312, the methods are there, but they're called getViewModeOptionsByBundle() / getFormModeOptionsByBundle() (without "Enabled")

Sadly it's too late now to change the method names to the name that was intended. Opened #2590605: Followup for [#2322503] for the rest of interdiff #84.

Anyone up for revieweing the super-trivial folowup for #93 at #2590605: Followup for [#2322503] ? :-)