Contrib.social

Add support for rendering computed fields to the "field" views field handler

Open on Drupal.org →
Created on 12 April 2023, about 1 year ago
Updated 7 May 2023, about 1 year ago

Problem/Motivation

Search API doesn't support letting computed fields use the EntityView handler for rendering.

@See `_search_api_views_get_field_handler_for_property` function in `search_api.views.inc` 

  // Since the Views EntityField handler class doesn't support computed fields,
  // neither can we (easily), so keep the fallback handler as the only
  // definition for those.
  if (isset($definition)
      && $property instanceof FieldItemDataDefinition
      && !$property->isComputed()
      && !$property->getFieldDefinition()->isComputed()) {

It seems like this may no longer be the case, however, issue #2852067: Add support for rendering computed fields to the "field" views field handler → has been fixed. Is there a reason this logic still exists in the search api handlers?

I can see in issue 📌 Allow adding computed bundle fields in Views Fixed that this has only been resolved for entity base fields and not bundle fields, but perhaps there is some better logic that we can perform to identify this?

This has been tested with datetime computed fields, and removing the 2 `isComputed()` checks doesn't pose any obvious issues so far in my testing.

Steps to reproduce

  1. Install Drupal 9.5.x
  2. Install Search API
  3. Add a computed datetime base field to any entity
  4. Build a search index from this entity using the computed field
  5. Try to render this field using Drupal's standard EntityView date formatters

Proposed resolution

Enable computed fields to use the EntityView handler for the given type (at least for base fields).

Remaining tasks

  • Work out which computed fields this can apply to
  • Apply the new logic
✨ Feature request
Status

Needs work

Version

1.0

Component

Views integration

Created by

🇬🇧United Kingdom kalpaitch

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

  • Issue created by @kalpaitch
  • @kalpaitch opened merge request.
  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    Thanks a lot for reporting this problem and providing a fix!

    To make sure that this is indeed an improvement, though, we’ll need test coverage for this functionality, to make sure this works correctly both for computed base and bundle fields.

    Also, please do not use issue forks/merge requests in this project, as testing doesn’t work for them. (See #3190024: Problem with test dependencies when testing issue forks → .)

    (Also, this is neither major nor a bug.)

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024