Allow adding indirect fields/properties in aggregate_fields processor

Thanks for creating this issue!
As this can be solved at any time after a beta or a stable release, though, I don't think I'll be able to work on this in the near future.
Also, I'm still not sure which option would be best here, they all have their drawbacks. But probably including all "direct" properties and all indexed related properties, while potentially a bit confusing, really is the user-friendliest solution here. (Especially if we can have a good description.)

Does anyone else want to weigh in here?

I'm not entirely sure what's meant here, but I think it might be what I'm seeing:

I have an entity reference field to taxonomy terms on the entity types in my index.
I've added the term name field to the index, so under 'Content' in the index field list, I see 'VOCABULARY » Taxonomy term » Name'.

When I add an aggregated field, I see 'Content > VOCABULARY' as a field I can select, but not 'Content > VOCABULARY » Taxonomy term » Name'.

Is that the problem described here?

I assumed 'Content > VOCABULARY' was a shorthand, but the value that is stored for the aggregated field is the term ID, not the term name, which suggests that it's not.

If that's the case, I'd say this is a bug rather than a feature request.

Looked at this a bit more, and yes, I think my problem and the OPs are the same.

I think it would make more sense for the fields available for aggregation to be only (but all!) those added to the index already.

On user-friendlyness, I've just been playing around with reusing the selection code for fields in the first instance on the field configuration page. This as I was wondering about how to select fields to extract metadata from for search_api_attachments. It means squirrelling away the RequestStack and FieldsHelper to get it from FieldConfigurationForm to the property::buildConfigurationForm() but seems fine and 'understandable' otherwise. If people think there is merit in doing it so I can continue to look.

Testing with the attachment field I specifically I required to pass RequestStack and FieldHelper to the AggregateFieldProperty subform to make the list, and then the field itself so that it could save the configuration on adding fields while not doing the full submission. I'm sure this can be done better, I was just hacking around to see if it was possible. Can we write for aggregation field if it explains what I mean clearer.

Can we write for aggregation field if it explains what I mean clearer.

Sorry, but I don't understand what you mean?
As I understand, you've got this basically working (at least for the Attachments processor) but it's still very hacky?

Should have said can re-write it for the aggregation field.

Anyway chatting to Borisson I understand the/an idea for the current field selection page is to re-use the token system, rather than the rather clunky thing at the moment?

I don't actually think it will change the additional data that needs to be passed to the 'subform', but I should have a look.

Anyway chatting to Borisson I understand the/an idea for the current field selection page is to re-use the token system, rather than the rather clunky thing at the moment?

The clunky thing at the moment will definitely change, yes. We'll have AJAX, probably some modal and maybe other helpful additions that provide some context while doing those changes. I don't know if we'll be able to re-use the token system exactly, but something like that is the idea, yes. (Although the current UI already follows this – just minus the AJAX that lets you avoid the additional (full) page requests.)

I don't think this will change any other part of the framework significantly, though – e.g., a field's config might be displayed in a modal then, but it will still work the same internally. So, probably nothing would have to change there.

I think first we need to allow to do this programmatically and through config, and when it works we can think of the UI.

> I think first we need to allow to do this programmatically and through config, and when it works we can think of the UI.

I have successfully done some config-fu (cex/edit/cim) to add an indirect field and checked it is populated correctly.

If we work on the UI, we might also consider that a sitebuilder might need a different field order (for fallbacks) than the UI presents.

> I have successfully done some config-fu (cex/edit/cim) to add an indirect field and checked it is populated correctly.

I've been PMed what i did here so i share. TBH, i don't remember exactly, but i suppose...

* add the some/relation/field_foo to the index, and some other aggregated_fields
* edit the config directly (or do drush cex/edit/cim)
* ...and copy over the internal name of the some/relation/field_foo from index to aggregated_fields

HTH!

Thanks a lot for posting this, might indeed help others!
Please also see #3018079: Aggregated fields for paragraphs, entity references?, where the same solution is explained. (Also note that you don’t actually need the some/relation/field_foo field present on the index, as long as you know its combined property path. If adding it to the index first helps you with that, sure – but you can then remove it again afterwards.)

It indeed works from the config, but once you resave the UI it removes everything you done and this can't be a production feature for sure.

ShaxA,

I don't lose the aggregate fields setup when I use the UI. I can't view all the details of my aggregate fields in the solr fields config form, because the UI doesn't support that yet, but I can use the UI to add and edit other solr search fields, and save, and export to config - and all these operations preserve the aggregate field settings that I set up using the instructions in #12.

It indeed works from the config, but once you resave the UI it removes everything you done and this can't be a production feature for sure.

Well, at least that should be easy enough to fix.
Patch attached, please test/review!

@ LDG27182: I think ShaxA is talking about editing the aggregated field config itself, not just other fields.

@drunken-monkey it doesn't work for me. Still, if I resave the fields in the UI and export configs aggregated fields are removed. Are you using the latest dev version of search_api or the stable one?

> the latest dev version of search_api or the stable one?

If not stated otherwise, all patches are against dev.

Sorry, but the patch doesn't work for me on the latest dev version. Once I submit the fields from through the UI aggregated fields are gone.

Hello,

I just tested the patch from #16 with the lastest dev and it seems to work: the fields are kept.
But if all fields are nested we get an error when trying to save the form -

Contained fields field is required

e.g:

aggregated_field_test:
    label: 'Aggregated field test'
    property_path: aggregated_field
    type: date
    configuration:
      type: union
      fields:
        - 'entity:node/uid:entity:changed'
        - 'entity:node/uid:entity:created'

So editing an aggregated field from config works but from the UI it's rather sketchy/risky.
Because of this I'm not sure #16 really makes sense to commit since it's better to avoid editing those aggregated fields through the UI altogether.

I was not able to reproduce the issue described by @ShaxA though.
Could you provide more info on your setup in config?

So editing an aggregated field from config works but from the UI it's rather sketchy/risky.
Because of this I'm not sure #16 really makes sense to commit since it's better to avoid editing those aggregated fields through the UI altogether.

Hm, also makes sense. However, I think producing a clear error (as in your example) is preferable to silently deleting configuration values – no matter whether this is a “pro” feature. So, unless there are more objections, I’d just commit this as a first step.

Works like a charm on the interface-level... we're gonna test it further :)

Thanks!

Alright, thanks for testing! Committed.
Back to “Active” for hopefully getting a UI for this in place some day.

As of today you MUST include entity relation in the field configuration as shown in the example in #20 otherwise it will not work.

I don't follow why we cannot get the same level of introspection on a field in the modal like you can for any other fields that can be indexed from the main screen. In the interim would it be possible to link the help dialog for an Aggregate Field to this issue explaining that it is possible?

If you use Aggregate Fields on reference fields for example, the end result is you store a list of entity IDs on the item in Solr.

In our case, a developer had started adding fields of IDs and titles from reference fields, added glue code behind the scenes, etc, which wound up mimicking this behavior. When I sat down to pick up the work, I realized you could use an Aggregate and just adjust the generated YAML to get what you need.

Here is an example.

Say I want to index referenced entity labels on a Solr item because I have a suggester and copyField (Solr XML) that uses these labels as facet options. Or maybe I have a particular need to store that data on this item.

I can only get the IDs by using Aggregate Field from the UI as normal - but, as others noted, if you edit the YAML you can do this:

  example_field:
    ...
      fields:
        - 'entity:node/field_myreference:entity:title'

Now it will store labels (titles) instead of the IDs. You can add as many fields as you want here.

This is also true for references on references:

  example_field:
    ...
      fields:
        - 'entity:node/field_myreference:entity:field_other_reference:entity:title'

One caveat to this is approach is the UI config for aggregate fields won't match, but also, if you change data about the referenced entities, it will not reflect in the solr item unless you mark it for reindexing. In our case this is fine because entity titles never change and we perform re-indexing daily, but others should be aware of that.

As stated by @kevinquillen it would be great if we could do this from UI, I have not yet tested this from the config but facing the same issue here.

FYI, the patch for #16 is in 8.x-1.12 (11 Mar 2019).

Getting this functionality into the UI would be really helpful for folks who want to aggregate entity reference field values (not IDs) for display in views or for use as facets. If editing the config file is the only way to do this, then can we create some new documentation that specifically outlines where to find the correct config file and how to edit it?

@drunken-monkey, I was just looking at this and most of the time I would imagine that the nested properties in question are defined as fields in their own right, so once the properties have been gathered in the first pass, something like this should do the trick:

<?php
    foreach ($index->getFields() as $field) {
      if (!isset($form['fields'][$field->getCombinedPropertyPath()])) {
        $field_options[$field->getCombinedPropertyPath()] = $field->getLabel();
      }
    }

?>

Then the hack to avoid removing nested fields can be dropped.

I am sure there are some subtleties to be observed but it seems to meet the immediate needs for my project.

Patch attached.

#31 patch works fine in my case for Product Default Variation Price & Variation Price union price (Product + Product Variation entities in one table).
Thank u!

Thanks for the patch, @alfaguru, and thanks for reporting back, @VasiliyRepin!

Adding explicit options for all indexed fields indeed seems like a relatively easy solution that might help some people. I would however doubt that most people have those properties they want to aggregate indexed as individual fields, too. In a lot of cases, this wouldn’t make much sense.
Anyways, it’s a very small addition, so as long as it helps some people I don’t really see a reason against it. However, it’s important we don’t cause a regression here by removing the code dealing with already selected fields.

I created an MR with your patch and added some changes by me (in a separate commit), adding comments and reverting the removal of the existing code avoiding silently deleting config.

Patch in #31 works, with one minor issue: If you're indexing a field more than once (same Path, different Machine Name and Type), since the options are keyed by path, only the label of the last duplicate is used.

The use case I have is that we use a single index for multiple search views.

A location-only search:
- A geofield field is indexed as a "Latitude/Longitude" type for proximity search.
- The same geofield field is also indexed as "Storage Only" for rendering in Leaflet maps (Leaflet doesn't render the previous type).

A multi-type site search with location search in the advanced section:
- Aggregates several geofields into one "Latitude/Longitude" type for proximity search.

When choosing which fields to aggregate, the aggregate options only show the "Storage Only" geofield because it's the last one found.

Could just be a presentation/help text thing. Otherwise, looks good to me.

Thanks for reporting back, good to hear!
The problem with multiple fields for the same property seems like a very niche case, and also entirely a matter of presentation. It’s only the property that’s actually used, the field of which the label is used is irrelevant. I’d therefore not try to fix this minor detail.

Does anyone else still want to give feedback?
Please do use the changes in the MR for testing, though, not the patch in #31, as that is what I’d merge. If you need a patch, you can get it by clicking “plain diff” at the top.

Something that might be of interest is that I've just made a module that combined identically-named fields from different datasources. Unlike aggregation, you pick just one field name in the UI.

The use case is that I have some content entity datasources but also some Date Recur occurrences from event nodes (another module I'm working on, based roughly on this sandbox: https://www.drupal.org/sandbox/sam/3200275)

I've released the module I mentioned: https://www.drupal.org/project/search_api_common_field

Same sort of idea as aggregated fields, but merges matching fields on different sources so it's a clearer UI for that use case.