Problem/Motivation

It would be great if we could have a hook_ENTITY_TYPE_view_mode_alter(&$view_mode, \Drupal\Core\Entity\EntityInterface $entity) in order to avoid a test on the entity type when we have to use hook_entity_view_mode_alter and better target the entity types on which we want to act.
That would make the code more SOLID and readable.

I've set the version to 10.0.x-dev but since it would be a new feature, feel free to change it to 11.x-dev (even if it woudn't break anything on D10)

Steps to reproduce

NA

Proposed resolution

Add a new hook

Remaining tasks

  1. Add documentation of new hook to entity.api.php
  2. Add test coverage/implementation of the hook - see \Drupal\Tests\node\Functional\NodeEntityViewModeAlterTest::testNodeViewModeChange for an example
  3. Update EntityViewBuilder and any sub-classes that override ::getBuildDefaults
  4. change record

User interface changes

NA

API changes

New entity hook hook_ENTITY_TYPE_view_mode_alter that will "Change the view mode of a particular entity type that is being displayed."

Data model changes

NA

Release notes snippet

TBD

Issue fork drupal-3407912

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

MacSim created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Version: 10.0.x-dev » 11.x-dev

This could only go into 10.3

Are you sure this doesn't already exist?

macsim’s picture

macsim commented

I was surprised that it didn't exist but I can't find such a hook in the core ; there are a lot of hook_ENTITY_TYPE_[something] in the entity.api.php but it seems that there's no hook_ENTITY_TYPE_view_mode_alter() declared at all.

Edit : Didn't checked on a Drupal 11 though.
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co... I can't find it in Drupal 11 as well (I only checked in entity.api.php - that's where it should be)

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Confirming \Drupal\Core\Entity\EntityViewBuilder::getBuildDefaults doesn't fire such a hook.

I do note that you can override that method on a per entity-type basis via changing the view builder.

macsim’s picture

macsim commented

Not sure about how to achieve that... Is that the right way?

/**
 * Implements hook_entity_type_alter().
 */
function mymodule_entity_type_alter(array &$entity_types): void {
  if (isset($entity_types['node'])) {
    $entity_types['node']->setViewBuilderClass(NodeViewBuilder::class);
  }
}

class NodeViewBuilder extends EntityViewBuilder {
  
  protected function getBuildDefaults(EntityInterface $entity, $view_mode) {
    // Can I call the parent protected method in order to alter its result?
    // or do I have to copy/paste/alter/maintain that parent method in here?
  }

}
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Issue summary: View changes

To be clear, yeah its not ideal - you'd have to probably duplicate a lot of the parent code.

FWIW I'm plus one for this idea, its fairly self contained and consistent with other entity API hooks.

Added remaining tasks to issue summary

Akhil Babu made their first commit to this issue’s fork.

Akhil Babu opened merge request !5775

akhil babu’s picture

akhil babu
he/him
commented
Issue tags: +Needs tests

Added the hook and documented in entity.api.php

macsim’s picture

macsim commented
Assigned: Unassigned » macsim
macsim’s picture

macsim commented
Assigned: macsim » Unassigned
Status: Active » Needs review

Added kernel tests
Do we have to also implements Functional tests for this?

macsim’s picture

macsim commented
Assigned: Unassigned » macsim
Status: Needs review » Needs work

My bad I was only running tests on EntityViewBuilderTest.php in my local env before pushing.

I guess I should have created new view_modes in my test instead of using "full" and "teaser" view_modes in order not to break any other test from another file.
Gonna work on this at the end of the week.

macsim’s picture

macsim commented
Assigned: macsim » Unassigned
Status: Needs work » Needs review

Pipelines have been fixed by adding/using 2 new view modes

Back to "Kernel tests added / Do we also have to provide functional tests?"

smustgrave’s picture

smustgrave commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs change record

"Kernel tests added / Do we also have to provide functional tests?"

Not sure I follow?

This will need a change record though,

Updated issue summary to the standard issue template and added missing sections.

arisen’s picture

arisen
Location Goa, GMT +5:30
commented
Issue summary: View changes

Added the change record.

macsim’s picture

macsim commented
Issue summary: View changes
macsim’s picture

macsim commented
Status: Needs work » Needs review
Issue tags: -Needs change record

Do we really need more tests than the kernel tests?

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: -Needs tests +Needs Review Queue Initiative

Not sure where the request for functional testing came from but believe the current coverage should be fine. Ran the test-only feature and get

1) Drupal\KernelTests\Core\Entity\EntityViewBuilderTest::testHookEntityTypeViewModeAlter
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'entity_test.vm_alter_full'
+'entity_test.vm_alter_test'
/builds/issue/drupal-3407912/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/drupal-3407912/core/tests/Drupal/KernelTests/Core/Entity/EntityViewBuilderTest.php:336
/builds/issue/drupal-3407912/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!

Small comments on MR, but overall good and like 99% there.

Prashant.c made their first commit to this issue’s fork.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Needs work » Needs review

Updated the docblock sentences in the hook documentation for clarity:

 /**
- * Change the view mode of a particular entity type that is being displayed.
+ * Change the view mode of a specific entity type currently being displayed.
  *
  * @param string $view_mode
- *   The view_mode that is to be used to display the entity.
+ *   The view_mode currently displaying the entity.
  * @param \Drupal\Core\Entity\EntityInterface $entity
  *   The entity that is being viewed.
  *
  * @ingroup entity_crud
  */

Additionally, incorporated review comments.
Thank you.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Feedback has been addressed.

  • catch committed cbe1d286 on 11.x 
    Issue #3407912 by Akhil Babu, MacSim, smustgrave, Prashant.c, larowlan,...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

This looks good to me, just brings it into line with other entity alter hooks.

Committed/pushed to 11.x, thanks!

Status: Fixed » Closed (fixed)

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