Aggregate fields of processed fields (AddHierarchy)

Created on 16 June 2017, about 7 years ago
Updated 12 April 2024, 3 months ago

At the moment, only the raw values of the vocabulary fields are aggregated. I think the aggregation should execute the specified field processors (in this case AddHierarchy) in the value extraction.

Use case:
- Multiple indexed vocabulary fields using field processor AddHierarchy (for hierarchical search)
- Aggregated field tax_ids (containing all vocabulary fields - including the parent terms for each items)
- View with contextual filter on tax_ids (input: tid) to display all media entities belonging to a specified term

I cannot say whether this is by design or a bug, but would appreciate some feedback.

✨ Feature request
Status

Needs work

Version

1.0

Component

Plugins

Created by

🇨🇭Switzerland kjauslin

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

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & sqlite-3.27
    last update about 1 year ago
    534 pass
  • 🇳🇱Netherlands Kingdutch

    Since the functionality that I needed and that Drunken Monkey describes in #13 without the per-subfield configuration already works as per #12 I've written an update for the hierarchy processor.

    The attached patch is a fresh rewrite and updates AddHierarchy to also look at the subfields of any aggregated fields. If those subfields can all be put into a hierarchy of the same type, then the aggregated field is added as an option with that type under the processor settings. This allows the normal form save functionality to kick in and update the configuration which leads to the scenario of #12 where everything else already just works (TM).

    I'm not sure whether this is acceptable as-is or needs tests specifically for hierarchies of aggregated fields?

  • 🇦🇹Austria drunken monkey Vienna, Austria

    Looks great, thanks a lot! This seems like a much simpler solution. Would be great if others could give it a try.

    Just some minor nit-picks, fixed in the attached revision:

    1. +++ b/src/Plugin/search_api/processor/AddHierarchy.php
      @@ -123,28 +124,80 @@
      +            [$datasource_id, $path] = Utility::splitCombinedId($aggregated_field);
      +            if ($datasource_id === NULL) {
      +              continue 2;
      +            }
      

      I don’t think this check is needed here. In theory, I think, it shouldn’t be a problem to aggregate datasource-independent fields, and they could (theoretically) also reference taxonomy terms (or other hierarchical fields).
      Or did you run into some specific problem without this?

    2. +++ b/src/Plugin/search_api/processor/AddHierarchy.php
      @@ -123,28 +124,80 @@
      +            $aggregated_type ??= $type;
      

      Thanks a lot, didn’t know about that operator!
      However, we (nominally) still support PHP 7.3 (even though this should soon change), so we probably should use the long version.

    And yes, it would be great if you could also add a test for this new functionality. There is specific code for it, so it should be covered by tests.

  • Open in Jenkins → Open on Drupal.org →
    Core: 9.5.x + Environment: PHP 8.1 & sqlite-3.27
    last update about 1 year ago
    534 pass
  • 🇦🇹Austria drunken monkey Vienna, Austria

    Forgot to attach.

  • Status changed to Needs work about 1 year ago
  • 🇦🇹Austria drunken monkey Vienna, Austria

    NW for the tests.

  • rerolled patch for latest version

Production build 0.69.0 2024