Incorrect value for aggregated reverse entity reference

Created on 10 August 2023, over 1 year ago
Updated 7 October 2023, about 1 year ago

Problem/Motivation

I'm attempting to create an aggregated field commerce product entities references. The goal is for the product_id search field to contain ids of related products via the reverse entity reference processor. Everything works until `Drupal\search_api\Item::addValue` is called. Since the search field is configured as an integer, the data type plugin converts it to a 1 instead of the entity id.

For example:

Product 5 references media entities 1,2,3
Product 6 references media entities 2,4

When indexing media entity 2, it should contain references to Product 5 & 6.

Expected:

product_id = [
  5,
  6,
]

Actual:

product_id = [
  1,
  1,
]

Steps to reproduce

Enable reverse entities, add an aggregated field to try and populate the reverse referenced entities.

Proposed resolution

I'm not sure the solution, but resolving the entity id in the AggregatedFields processor fixes the issue. Patch attached as an example.

Remaining tasks

1) Provide recommended place in code to handle converting the reference to an id or label depending on the search field data type.
2) Fix issue
3) Write test
4) Create Merge Request

πŸ› Bug report
Status

Needs work

Version

1.0

Component

Plugins

Created by

πŸ‡ΊπŸ‡ΈUnited States andyg5000 North Carolina, USA

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 @andyg5000
  • Status changed to Needs review over 1 year ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.1 & sqlite-3.27
    last update over 1 year ago
    544 pass
  • πŸ‡ΊπŸ‡ΈUnited States andyg5000 North Carolina, USA

    I've uncovered an interesting "bug" or misunderstanding in PHP with my first patch (https://twitter.com/AndyG5000/status/1699833476701450692)

    Passing items by reference in a foreach loop are incorrectly read when looping again with the same variable name,

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.1 & sqlite-3.27
    last update about 1 year ago
    544 pass
  • πŸ‡¦πŸ‡ΉAustria drunken monkey Vienna, Austria

    Thanks for reporting this problem and already supplying a patch to fix it!
    The location for the fix looks pretty good, and just using the field type to decide whether we use the entity ID or label also makes sense to me. I don’t think this needs to be any more complicated than that (for now, until someone finds other scenarios that don’t work). Please see/test/review the attached patch revision.

    However, what this definitely still needs is a regression test that demonstrates the problem and that verifies that this patch resolves it. Would you be able to add such a test to AggregatedFieldTest?

  • Status changed to Needs work about 1 year ago
  • πŸ‡¦πŸ‡ΉAustria drunken monkey Vienna, Austria

    NW for the tests.

Production build 0.71.5 2024