singleton($step['class'])->extend('augmentSQL', $sql); in SearchIndex::getDirtyIDs() causes throwing Fatal Errors

[silverstripe]

singleton($step['class'])->extend('augmentSQL', $sql); in SearchIndex::getDirtyIDs() causes throwing Fatal Errors

[UndefinedOffset]

I think all you'll need to do is update that one line referenced in #239 to fix it there, the same fix is needed in #240 but you also need to replace DataObject::singleton() as the singleton method is not in SilverStripe 3.x :). Hope that helps :)

[michalkleiner]

Even with the updates the test getting the list of dirty IDs for has_many is failing. Will try to look into it more.

[tractorcow]

I think it does need to be fixed in fulltextsearch. That parameter is required in some cases.

[dhensby]

If Translatable's signature is $dataQuery = null then it really shouldn't blindly expect an object...

@tractorcow how should we obtain a valid DataQuery in SearchIndex::getDirtyIDs() seeing as it doesn't seem to be using the ORM at all...

[tractorcow]

It's more = null for legacy reasons, that is we couldn't change the method signature. We should have in 4.0 though to make it mandatory.

It should always provide it though; Versioned relies on it for example.

hm

$sql = new SQLSelect('"ID"', '"'.$tableName.'"', '"'.$step['foreignkey'].'" IN ('.implode(',', $ids).')');
singleton($step['class'])->extend('augmentSQL', $sql);
$ids = $sql->execute()->column();

Should probably be:

$ids = DataObject::get($step['class'])
  ->filter($step['foreignkey'], $ids)
  ->column('ID');

Not sure why it's written in such a convoluted way.

Similarly for the other relations... stop trying to do raw-sql, and just use the ORM the way it's meant to be called?

[tractorcow]

Had a guess at how the code would look all up:

foreach ($derivation['chain'] as $step) {
    if ($step['through'] == 'has_one') {
        $ids = DataObject::get($step['class'])
            ->filter($step['foreignkey'], $ids)
            ->column('ID');
    } elseif ($step['through'] == 'has_many') {
        // Get many ids by querying component with alternate set of foreign ids
        $ids = DataObject::singleton($step['otherclass'])
            ->getComponents($step['method'], $ids)
            ->column('ID');
    }

    if (empty($ids)) {
        break;
    }
}

This makes use of the fact that you can mock a HasManyList with a set of foreign $id keys (instead of a single parent id).

Good thing I added that second arg to getComponents() :)

[michalkleiner]

Affecting MBIE on a CWP project using 2.1 recipe

[UndefinedOffset]

This also seems to affect Versioned objects as well in at least SilverStripe 4.3.0, Versioned requires a DataQuery as the second argument even though it's defined as accepting null as a parameter (see here).

[michalkleiner]

The proposed PR works for us on the project, just need to sort the tests. Will try looking at it ASAP.

[michalkleiner]

Back on this again, only 4 years after the initial report.

[tractorcow]

@michalkleiner I haven't used SOLR for a few years, but revisiting, I think the code example I pasted above should still fix the issue. I suggest throwing it into a PR and testing it out on a few live SOLR indexes to see if it works.

[michalkleiner]

@tractorcow it works for us on a project, yes.
So you reckon it's rather about the failing test that might need adjusting?

[michalkleiner]

Phew, ok, two new PRs with the tests passing and using the simplified DataObject::get approach.