4.13 Regression: GridFieldFilterHeader now assumes that records are "searchable"

[chrispenny]

An issue has been raised on another module: silverstripe/silverstripe-search-service#82

The error is:

[Emergency] Uncaught BadMethodCallException: Object->__call(): the method 'getDefaultSearchContext' does not exist on 'SilverStripe\SearchService\Admin\IndexedDocumentsResult'

SilverStripe\View\ViewableData->__call
GridFieldFilterHeader.php:289
SilverStripe\Forms\GridField\GridFieldFilterHeader->getSearchContext
GridFieldFilterHeader.php:309
SilverStripe\Forms\GridField\GridFieldFilterHeader->getSearchFieldSchema
GridFieldFilterHeader.php:540
SilverStripe\Forms\GridField\GridFieldFilterHeader->getHTMLFragments
GridField.php:527
SilverStripe\Forms\GridField\GridField->FieldHolder
call_user_func_array
ViewableData.php:488

The class triggering this error is SearchAdmin.php, and this class displays IndexedDocumentsResult, which are ViewableData records (not DataObjects) in a GridField.

In 4.12 it looks like the GridFieldFilterHeader::canFilterAnyColumns() method didn't attempt to access this searchableFields() method (in fact, no part of this class does):
https://github.com/silverstripe/silverstripe-framework/blob/4.12/src/Forms/GridField/GridFieldFilterHeader.php#L251

In 4.13, it now does:
https://github.com/silverstripe/silverstripe-framework/blob/4.13/src/Forms/GridField/GridFieldFilterHeader.php#L254

I attempted to add this searchableFields() method to IndexedDocumentsResult. I tried to return null, an empty array, and tried returning the same fields as its summaryFields() method. All of these attempts resulted in different new errors.

searchableFields() returns null:

[Emergency] Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given

searchableFields() returns an empty array:

[Emergency] Uncaught BadMethodCallException: Object->__call(): the method 'getDefaultSearchContext' does not exist on 'SilverStripe\SearchService\Admin\IndexedDocumentsResult'

searchableFields() returns the same fields as summaryFields():

[Emergency] Uncaught BadMethodCallException: Object->__call(): the method 'getDefaultSearchContext' does not exist on 'SilverStripe\SearchService\Admin\IndexedDocumentsResult'

Resolution one

Could GridFieldFilterHeader be updated to no longer assume searchable fields are available?

Resolution two

Could you please help me figure out what code I need to add to IndexedDocumentsResult to add support for searchable fields?

PR

• FIX Don't assume searchableFields() exists in gridfield filter #10772

[GuySartorelli]

We'll go with resolution one - requiring those methods was an unintended breaking change

[GuySartorelli]

I've also created #10771 as a follow-up to this.

[chrispenny]

I'm not sure if there are impact tags on this module, but if there are, could we please also get an impact/high on this ticket?

[GuySartorelli]

@chrispenny this isn't high impact according to the definition in the docs since it can be worked around pretty easily by implementing some extra methods.
But this issue is already in our peer review column, so changing labels won't affect how soon it's merged at this point.

[michalkleiner]

Lowered the impact, however I had a look at the PR already and merged it now. Released 4.13.1.

[chrispenny]

@GuySartorelli @michalkleiner @sabina-talipova can this please be re-opened? The issue has now just moved to the next error (which I mentioned in the description when I attempted to implement a searchableFields() method).

[Emergency] Uncaught BadMethodCallException: Object->__call(): the method 'getDefaultSearchContext' does not exist on 'SilverStripe\SearchService\Admin\IndexedDocumentsResult'
Line 54 in /var/www/mysite/www/vendor/silverstripe/framework/src/Core/CustomMethods.php

SilverStripe\View\ViewableData->__call
GridFieldFilterHeader.php:306
SilverStripe\Forms\GridField\GridFieldFilterHeader->getSearchContext
GridFieldFilterHeader.php:326
SilverStripe\Forms\GridField\GridFieldFilterHeader->getSearchFieldSchema
GridFieldFilterHeader.php:557
SilverStripe\Forms\GridField\GridFieldFilterHeader->getHTMLFragments
GridField.php:527
SilverStripe\Forms\GridField\GridField->FieldHolder

If this isn't considered impact/high, can you please provide some guidance on what is expected to be added to our ViewableData records?

I tried adding getDefaultSearchContext() with empty data:

public function getDefaultSearchContext()
{
    return SearchContext::create(
        static::class,
        FieldList::create(),
        []
    );
}

But that (again) just kicks the can down the road to the next error:

[Emergency] Uncaught BadMethodCallException: Object->__call(): the method 'i18n_plural_name' does not exist on 'SilverStripe\SearchService\Admin\IndexedDocumentsResult'
Line 54 in /var/www/mysite/www/vendor/silverstripe/framework/src/Core/CustomMethods.php

SilverStripe\View\ViewableData->__call
GridFieldFilterHeader.php:342
SilverStripe\Forms\GridField\GridFieldFilterHeader->getSearchFieldSchema
GridFieldFilterHeader.php:557
SilverStripe\Forms\GridField\GridFieldFilterHeader->getHTMLFragments
GridField.php:527
SilverStripe\Forms\GridField\GridField->FieldHolder
call_user_func_array
ViewableData.php:488

[GuySartorelli]

The error you're indicating doesn't happen with the test class I created to mimic the IndexedDocumentResult class, so I'm not sure what's going on there. Have you tried reproducing this in a more simplified scenario?

Regardless, I'll try installing that module and reproducing with that.

can you please provide some guidance on what is expected to be added to our ViewableData records?

It's not expected that you'll need these methods. But to work around this bug, you can implement each of the methods that it complains don't exist until it doesn't complain. Note that that's a workaround, not an expected requirement.

[GuySartorelli]

It turns out these errors are actually a result of #10725 - basically, ArrayList was never returning the correct value for canFilterBy() in the past - so it was always saying "No, we can't filter by these fields" even when it could. Which meant GridFieldFilterHeader wasn't actually doing anything - removing it from the GridFieldConfig would have changed nothing.

Now, ArrayList is correctly reporting when it can filter by some fields, which means GridFieldFilterHeader tries to give you a form for filtering by those fields - which it needs extra methods to do. Which means this isn't a bug, even though it is in some ways a regression. I don't think we should try to resolve this regression from here.

I think for your use case, simply removing the GridFieldFilterHeader is the most appropriate course of action - it wasn't doing anything before anyway. Can you please confirm that this makes sense, and whether it resolves the problem without causing additional regressions for you?

[chrispenny]

@GuySartorelli it looks like removing GridFieldFilterHeader has sorted the issue! Thank you for the help!

[GuySartorelli]

Awesome! I'll close this issue then.