Skip to content
Snippets Groups Projects

Add generateMultiple(); remove some methods.

13 unresolved threads

Closes #3440446

Merge request reports

Merged results pipeline #282474 skipped

Merged results pipeline skipped for f2c6f255

Merged by Roderik MuitRoderik Muit 8 months ago (Sep 13, 2024 3:32pm UTC)

Merge details

  • Changes merged into 3.x with 7ae0009f (commits were squashed).
  • Did not delete the source branch.

Pipeline #282475 passed with warnings

Pipeline passed with warnings for 7ae0009f on 3.x

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
169 169 * the current content language.
170 170 *
171 171 * @return \Drupal\custom_elements\CustomElement
172 * Extracted custom elements containing data attributes and slots.
172 * Custom element containing entity properties as attributes and slots.
173 173 */
174 174 public function generate(ContentEntityInterface $entity, string $viewMode, string $langcode = NULL) {
175 // Get desired entity translation.
176 $entity = $this->entityRepository->getTranslationFromContext($entity, $langcode);
177 $custom_element = $this->getEntityDefaults($entity, $viewMode);
178 $this->buildEntityContent($entity, $custom_element, $viewMode);
179 $this->moduleHandler->alter('custom_element_entity', $custom_element, $entity, $viewMode);
  • This pre-existing alter hook is moved into buildEntities(),

    • because that happened to be more convenient
    • and because it feels like generated custom elements should always be passed through an alter, also when called through another public function. (Since all the other public functions are new in 3.x, that's not a backward compatibility issue.)
  • Please register or sign in to reply
  • Roderik Muit
  • 332 * @param \Drupal\custom_elements\CustomElement[] $custom_elements
    333 * The custom elements to build into; these must have the same keys as the
    334 * array of entities.
    270 335 * @param string $viewMode
    271 336 * The view mode being built.
    272 337 * @param \Drupal\Core\Session\AccountInterface|null $account
    273 338 * (optional) The user for which to check access, or NULL to check access
    274 339 * for the current user. Defaults to NULL.
    275 340 */
    276 public function buildCeDisplay(ContentEntityInterface $entity, CustomElement $custom_element, EntityCeDisplayInterface $display, string $viewMode, AccountInterface $account = NULL) {
    341 public function buildEntities(array $entities, array $custom_elements, string $viewMode, AccountInterface $account = NULL): void {
    342 $ce_displays = $this->getEntityCeDisplaysByBundle($entities, $viewMode);
    343
    344 foreach ($entities as $id => $entity) {
    345 // Silently skip if no accompanying custom elements are set; these may
    346 // have been unset by prepareBuildEntities().
  • 347 if (isset($custom_elements[$id])) {
    348 $this->buildCeDisplay($entity, $custom_elements[$id], $ce_displays[$entity->bundle()], $viewMode, $account, TRUE);
    349 }
    350 }
    351 }
    352
    353 /**
    354 * Builds an entity's content as custom element using a CE display.
    355 *
    356 * @deprecated in 3.0.0-beta2 and is removed in 4.0.0. Use generate() or
    357 * prepareBuildEntities() + buildEntities() instead.
    358 *
    359 * @see \Drupal\custom_elements\CustomElementGenerator::prepareBuildEntities()
    360 * @see \Drupal\custom_elements\CustomElementGenerator::buildEntities()
    361 */
    362 public function buildCeDisplay(ContentEntityInterface $entity, CustomElement $custom_element, EntityCeDisplayInterface $display, string $viewMode, AccountInterface $account = NULL, $skipPrepare = FALSE) {
  • 455 *
    456 * @return \Drupal\custom_elements\Entity\EntityCeDisplayInterface[]
    457 * An array of Custom Elements displays, keyed by entity bundle.
    458 */
    459 protected function getEntityCeDisplaysByBundle(array $entities, string $viewMode) {
    460 $ce_displays = [];
    461 foreach ($entities as $entity) {
    462 $bundle = $entity->bundle();
    463 if (!isset($ce_displays[$bundle])) {
    464 $ce_displays[$bundle] = $this->getEntityCeDisplay($entity->getEntityTypeId(), $bundle, $viewMode);
    465 }
    466 }
    467
    468 return $ce_displays;
    469 }
    470
    • getEntityCeDisplaysByBundle() is called twice (from prepareBuild and Build), and we explicitly trust that it is fast / cached.

      I keep revolving back around to the opinion that we don't actually need to cache CE displays statically (in a class variable $ceDisplays[ENTITYTYPE][BUNDLE][VIEWMODE], and add a $reset parameter to getEntityCeDisplay()), because

      • We know config entities' load() is cached already;
      • If a default-view-mode CE display does not exist / is disabled, getEntityCeDisplay() creates it anew... but that 'never' happens;
      • If we cache things, this inconveniences tests: we need to add getEntityCeDisplay(,,,$reset=TRUE) in a lot of places, some of which are not very apparent.

      But if you don't agree, I'll add the static caching.

      Edited by Roderik Muit
    • I agree, this should be good enough. We can do some performance testing of large article-listings later and improve / check for bottlenecks then.

    • Roderik Muit changed this line in version 4 of the diff

      changed this line in version 4 of the diff

    • Please register or sign in to reply
  • Roderik Muit
  • Roderik Muit added 1 commit

    added 1 commit

    • 3d8caf09 - Remove $reset parameter (inadvertently committed without accompanying code)

    Compare with previous version

  • 260 }
    261
    262 $ce_displays = $this->getEntityCeDisplaysByBundle($entities, $viewMode);
    263
    264 foreach ($entities_by_bundle as $bundle => $entities) {
    265 foreach ($ce_displays[$bundle]->getComponents() as $component_name => $display_component) {
    266 if ($formatter = $ce_displays[$bundle]->getRenderer($component_name)) {
    267 $field_name = $display_component['field_name'];
    268 $grouped_items = [];
    269 foreach ($entities as $id => $entity) {
    270 if ($this->fieldIsAccessible($entity, $field_name, $custom_elements[$id], $account)) {
    271 $items = $entity->get($field_name);
    272 $items->filterEmptyItems();
    273 $grouped_items[$id] = $items;
    274 }
    275 }
    • I just spotted this part in EntityViewDisplay::buildMultiple() and copied it here. But something is wrong here.

      • If we filterEmptyItems() here in prepare(), then we should also filterEmptyItems() in the next step / the existing buildCeDisplay() code.

      • Maybe I made a wrong assumption, and there is no need for prepareBuildEntities() to be a separate (public) method from buildEntities()? Maybe I should just have one single method that does both steps, like EntityViewDisplay::buildMultiple() does?

      @fago can you comment on that?

      I now feel like

      • merging the code from this method and buildEntities() into a single method (so I also don't need to call getEntityCeDisplaysByBundle() twice)
      • adding $account as an extra optional parameter to generateMultiple(), and only keeping generate() + generateMultiple() as a public entry point.
    • Roderik Muit changed this line in version 4 of the diff

      changed this line in version 4 of the diff

    • Please register or sign in to reply
  • 172 * Extracted custom elements containing data attributes and slots.
    172 * Custom element containing entity properties as attributes and slots.
    173 173 */
    174 174 public function generate(ContentEntityInterface $entity, string $viewMode, string $langcode = NULL) {
    175 // Get desired entity translation.
    176 $entity = $this->entityRepository->getTranslationFromContext($entity, $langcode);
    177 $custom_element = $this->getEntityDefaults($entity, $viewMode);
    178 $this->buildEntityContent($entity, $custom_element, $viewMode);
    179 $this->moduleHandler->alter('custom_element_entity', $custom_element, $entity, $viewMode);
    180 return $custom_element;
    175 $custom_elements = $this->generateMultiple([$entity], $viewMode, $langcode);
    176 return current($custom_elements);
    177 }
    178
    179 /**
    180 * Generates a custom element for the given entities and view mode.
  • Wolfgang Ziegler approved this merge request

    approved this merge request

  • Roderik Muit added 1 commit

    added 1 commit

    • ab88faed - Move all code into generateMultiple(); remove other public methods.

    Compare with previous version

  • Roderik Muit reset approvals from @fago by pushing to the branch

    reset approvals from @fago by pushing to the branch

  • Roderik Muit changed title from Add generateMultiple(), prepareBuildEntities() and buildEntities(). to Add generateMultiple(); remove some methods.

    changed title from Add generateMultiple(), prepareBuildEntities() and buildEntities(). to Add generateMultiple(); remove some methods.

  • Roderik Muit added 1 commit
  • Roderik Muit
  • 206 foreach ($entities as $key => $entity) {
    207 if ($langcode !== '') {
    208 $entity = $this->entityRepository->getTranslationFromContext($entity, $langcode);
    209 }
    210 $entities_by_type_bundle_key[$entity->getEntityTypeId()][$entity->bundle()][$key] = $entity;
    211
    212 $custom_elements[$key] = $this->getEntityDefaults($entity, $viewMode);
    213 }
    214
    215 foreach ($entities_by_type_bundle_key as $entity_type_id => $entities_by_bundle_key) {
    216 foreach ($entities_by_bundle_key as $bundle => $entities) {
    217 $ce_display = $this->getEntityCeDisplay($entity_type_id, $bundle, $viewMode);
    218 $ce_display->setOriginalMode($viewMode);
    219
    220 $processed = FALSE;
    221 if ($ce_display->getUseLayoutBuilder()) {
  • 223 // display (for view mode or default), skip processing of display
    224 // components and let layout builder render everything. Otherwise,
    225 // fall through to next method.
    226 $entity_view_displays = EntityViewDisplay::collectRenderDisplays([current($entities)], $viewMode);
    227 $entity_view_display = $entity_view_displays[$bundle];
    228 if ($entity_view_display->getThirdPartySetting('layout_builder', 'enabled')) {
    229 assert($entity_view_display instanceof CustomElementsLayoutBuilderEntityViewDisplay);
    230 foreach ($entities as $key => $entity) {
    231 $custom_elements[$key]->addCacheableDependency($ce_display);
    232 $this->buildLayoutBuilderContent($entity, $custom_elements[$key], $entity_view_display);
    233 }
    234 $processed = TRUE;
    235 }
    236 }
    237
    238 if (!$processed) {
  • 227 $entity_view_display = $entity_view_displays[$bundle];
    228 if ($entity_view_display->getThirdPartySetting('layout_builder', 'enabled')) {
    229 assert($entity_view_display instanceof CustomElementsLayoutBuilderEntityViewDisplay);
    230 foreach ($entities as $key => $entity) {
    231 $custom_elements[$key]->addCacheableDependency($ce_display);
    232 $this->buildLayoutBuilderContent($entity, $custom_elements[$key], $entity_view_display);
    233 }
    234 $processed = TRUE;
    235 }
    236 }
    237
    238 if (!$processed) {
    239 $ce_name = $ce_display->getCustomElementName();
    240 $force_auto = $ce_display->getForceAutoProcessing();
    241 foreach ($entities as $key => $entity) {
    242 $custom_elements[$key]->addCacheableDependency($ce_display);
  • Roderik Muit added 1 commit

    added 1 commit

    • ff7bdb6b - Re-add builEntityContent() with different signature than before; improve code flow.

    Compare with previous version

  • 180 return $custom_element;
    178 public function generate(ContentEntityInterface $entity, string $viewMode, string $langcode = NULL, AccountInterface $account = NULL) {
    179 $custom_elements = $this->generateMultiple([$entity], $viewMode, $langcode, $account);
    180 return current($custom_elements);
    181 }
    182
    183 /**
    184 * Generates custom elements from entities, possibly after translation.
    185 *
    186 * @param \Drupal\Core\Entity\ContentEntityInterface[] $entities
    187 * Entities to process.
    188 * @param string $viewMode
    189 * View mode used for rendering field values into slots.
    190 * @param string|null $langcode
    191 * (optional) For which language the entities should be rendered. Defaults
    192 * to the current content language; empty string means do not try to
  • 158 158 }
    159 159
    160 160 /**
    161 * Generates a custom element tag for the given entity and view mode.
    161 * Generates a custom element from entity, possibly after translation.
    162 162 *
    163 163 * @param \Drupal\Core\Entity\ContentEntityInterface $entity
    164 164 * Entity to process.
    165 165 * @param string $viewMode
    166 166 * View mode used for rendering field values into slots.
    167 167 * @param string|null $langcode
    168 * (optional) For which language the entity should be rendered, defaults to
    169 * the current content language.
    168 * (optional) For which language the entities should be rendered. Defaults
    169 * to the current content language; empty string means do not try to
  • 160 160 /**
    161 * Generates a custom element tag for the given entity and view mode.
    161 * Generates a custom element from entity, possibly after translation.
    162 162 *
    163 163 * @param \Drupal\Core\Entity\ContentEntityInterface $entity
    164 164 * Entity to process.
    165 165 * @param string $viewMode
    166 166 * View mode used for rendering field values into slots.
    167 167 * @param string|null $langcode
    168 * (optional) For which language the entity should be rendered, defaults to
    169 * the current content language.
    168 * (optional) For which language the entities should be rendered. Defaults
    169 * to the current content language; empty string means do not try to
    170 * translate.
    171 * @param \Drupal\Core\Session\AccountInterface|null $account
    172 * (optional) The user for which to check access, or NULL to check access
  • 201 */
    202 public function generateMultiple(array $entities, string $viewMode, string $langcode = NULL, AccountInterface $account = NULL): array {
    203 foreach ($entities as &$entity) {
    204 $entity = $this->entityRepository->getTranslationFromContext($entity, $langcode);
    205 }
    206
    207 return $this->buildEntityContent($entities, $viewMode, $account);
    208 }
    209
    210 /**
    211 * Generates custom elements from entities.
    212 *
    213 * @param \Drupal\Core\Entity\ContentEntityInterface[] $entities
    214 * Entities to process.
    215 * @param string $viewMode
    216 * View mode used for rendering field values into slots.
  • Wolfgang Ziegler approved this merge request

    approved this merge request

  • Roderik Muit added 1 commit

    added 1 commit

    Compare with previous version

  • Roderik Muit reset approvals from @fago by pushing to the branch

    reset approvals from @fago by pushing to the branch

  • Roderik Muit added 11 commits

    added 11 commits

    • 7b84d43e...e32dfa2c - 4 commits from branch project:3.x
    • dcf5f581 - Add generateMultiple(), prepareBuildEntities() and buildEntities().
    • 459418b0 - phpcs
    • 12d26a64 - Remove $reset parameter (inadvertently committed without accompanying code)
    • 7e09ea40 - Move all code into generateMultiple(); remove other public methods.
    • 15eaa83a - phpcs
    • 583cda98 - Re-add builEntityContent() with different signature than before; improve code flow.
    • c801cbe6 - Code comment fixes.

    Compare with previous version

  • merged

  • Please register or sign in to reply
    Loading