NEW Handle WithinRangeFilter in searchable_fields and SearchContext

[GuySartorelli]

Important

This PR has multiple commits. Do not squash - or if you do, use the second commit's message

This PR enables using WithinRangeFilter to filter by numeric, date, and time fields in searchable_fields. It can also be used with other field types (see the varchar unit test scenario) if some additional config is provided.

This PR is required so that the current SiteTree filter on LastEdited can be performed using a SearchContext instead of using the current custom weird filter it uses.

Note that this PR does not include any updates to the css e.g. to make date fields look nicer - that will come in silverstripe/silverstripe-admin#1897. For now we're adding just the functionality itself.

See the docs or unit tests for configuration that you can use to test locally.

Issue

• Refactor tree filtering in CMSMain/HierarchyModelAdmin silverstripe-cms#2949

[GuySartorelli]

These are moved to DBBigInt so they can be used with the new getMinValue() and getMaxValue().

[GuySartorelli]

These are moved to DBInt so they can be used with the new getMinValue() and getMaxValue().

[GuySartorelli]

This was a lot of code to parse through just to see that it's all for this singular purpose. Made sense to abstract into its own method here to encapsulate this functionality, reducing cognitive load for anyone working in this area in the future.

[GuySartorelli]

The condition would cause the following problem:

You have config like this:

[
    'MyRelation' => [
        'filter' => 'ExactMatch',
    ],
]

Because this is an array, but the relObject returns null, this condition isn't met. So we go to the else clause. This turns the config into this:

[
    'MyRelation' => [
        'filter' => [
            'filter' => 'ExactMatch',
        ],
    ],
]

[GuySartorelli]

If there's no relObject, these will just default to null. If the $specOrName array doesn't fill them in, they'll get filled in further down the line.

[GuySartorelli]

We had no test coverage for DataObject::scaffoldSearchFields()! So I've done reasonably comprehensive test scenarios here.

[GuySartorelli]

Needed because you can't have a non-db field in searchable_fields but you can have one in this method directly. See #11602 (comment)

[GuySartorelli]

See #11602 (comment) and #11602 (comment)

[GuySartorelli]

Need these getters for the match_any in search context.

[GuySartorelli]

This was only being called if the spec is an array - but that means if you're trying to search against a field that isn't valid, you only get an exception if you're using complex config! Otherwise you have to filter to get the exception, which isn't very friendly.

Note that it is possible for this to return null

[emteknetnz]

I followed the example in the developer-docs PR and end up with this somewhat suboptimal UX - the validation errors showed on blur after making no changes to the fields, just clicking between them

I had a "MyDataObject" in a "MyModelAdmin", with things auto-scaffolded

I only had this PR installed - am I missing something?

[GuySartorelli]

Was the front-end errors on blur the only thing you encountered? I suspect you'll find that exact same thing happens with that field type when any other filter is used even without this PR (i.e. it's a pre-existing problem). I haven't touched JS for this PR.

It doesn't prevent you submitting the form with or without values in those fields.

[emteknetnz]

OK, looks like it's CurrencyField that has the onblur issue, also does it with other filters e.g. GreaterThanFilter, there may be others where their default value is considered invalid. Could you please raise a bug issue for this? You probably have more context than me about why it's breaking like it is

I also tried the WithinRangeFilter on an Int field and that mostly worked, there were no onblur validation issues, and the gridfield did filter, however after filtering and opening up the filter again, then values I had put in were no longer there.

[GuySartorelli]

Could you please raise a bug issue for this?

Done (silverstripe/silverstripe-admin#1899) - though I haven't looked at all into a) what causes that or b) what the desired UX should be.

I also tried the WithinRangeFilter on an Int field and that mostly worked, there were no onblur validation issues, and the gridfield did filter, however after filtering and opening up the filter again, then values I had put in were no longer there.

Ahh yup I missed a fix that was originally included in my larger set of PRs. Raised #11609 since this is a bug that exists in CMS 5. Will need to be merged, then merged up, and I'll rebase on top.

[GuySartorelli]

CI failures are being fixed in #11615 and aren't related to this PR

[emteknetnz]

Tested locally, work good with Date and Int data types

With Currency, which I is what is listed in as an example in the docs, seems very broken e.g. validates default values as invalid, clear button doesn't set to a valid value, the actual filtering doesn't seem to work. I'm not sure how much of that is an existing issue, or how much of it relates to this PR. Could you have a look and let me know if it should be fixed here or in another issue

[GuySartorelli]

the actual filtering doesn't seem to work

Can you please be more specific about that? That sounds like something I need to fix in this PR but I can't without details about what you're doing, what you're expecting, and what's actually happening.

[emteknetnz]

OK it's specific to this PR with the WithinRangeFilter, not prior to this PR with the GreatThanFilter

Filtering a currency value between two currency values just doesn't work i.e. no results returned

<?php

use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Filters\WithinRangeFilter;

class MyDataObject extends DataObject
{
    private static $table_name = 'MyDataObject';

    private static $db = [
        'Title' => 'Varchar',
        'MyCurrency' => 'Currency',
    ];

    private static array $searchable_fields = [
        'MyCurrency' => [
            'filter' => WithinRangeFilter::class,
        ],
    ];
}

And create ModelAdmin to manage the model

[GuySartorelli]

Seems to work fine for me?

Expand to see the exact PHP I used for the model and admin

<?php

use SilverStripe\Admin\ModelAdmin;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Filters\WithinRangeFilter;

class MyDataObject extends DataObject
{
    private static $table_name = 'MyDataObject';

    private static $db = [
        'Title' => 'Varchar',
        'MyCurrency' => 'Currency',
    ];

    private static array $summary_fields = [
        'Title',
        'MyCurrency',
    ];

    private static array $searchable_fields = [
        'MyCurrency' => [
            'filter' => WithinRangeFilter::class,
        ],
    ];
}

class MyModelAdmin extends ModelAdmin
{
    private static string $url_segment = 'my-admin';

    private static string $menu_title = 'my admin';

    private static array $managed_models = [
        MyDataObject::class,
    ];
}

2025-02-17.15-56-07.mp4

What sort of values are you trying to add? Are you adding the $ sign before the values, or something like that?

[emteknetnz]

Ah right, it'll work if I remove the leading $ from the filter values. This is very confusing as the filter values start off with leading $

[GuySartorelli]

Turns out the values weren't being passed through the form first, which meant form fields like CurrencyField didn't get a chance to update the value - so the DB query was trying to check if $1.23 matched 1.23 and uhhhh no it doesn't lol.

Added a new commit to pass values through the form first, with a fallback to the original filter value if somehow there's no form field (which shouldn't ever happen but would have been the previous behaviour anyway)

[emteknetnz]

It's better now, though if I leave the filter values unchanged i.e. $0.00 and $0.00, then it will filter out values. When the values are left in their default state they should do any filtering

Have talked offline, best solution is probably to swap out CurrencyField for TextField in the method that scaffolds search fields. Have a look at other fields that should probably also have this to prevent formatting or validation that prevents partial searches e.g. MoneyField, EmailField, UrlField, IpField, etc

[GuySartorelli]

swap out CurrencyField for TextField

I've gone for NumericField since the value should never be e.g. "seven dollars" but instead 7.00

Updated DBEmail and DBUrl to use TextField. DBIp already uses that field.

I've left enums alone - they have concrete values, so a dropdown makes sense for them in general (and withinrange probably doesn't make sense for them so they're not really in scope anyway)

To use DBMoney you'd have to do something like this to use it (at which point you're not actually dealing with MoneyField directly anyway):

<?php

use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\Filters\WithinRangeFilter;

class MyDataObject extends DataObject
{
    private static $table_name = 'MyDataObject';

    private static $db = [
        'Title' => 'Varchar',
        'MyMoney' => 'Money',
    ];

    private static array $summary_fields = [
        'Title',
        'MyMoney',
    ];

    private static array $searchable_fields = [
        'MyMoney.Currency',
        'MyMoney.Amount' => [
            'filter' => WithinRangeFilter::class,
        ],
    ];
}

And that only works for building the form - the default search context can't handle that syntax because it expects . syntax to be used with relations, not composite fields. So tl;dr handling DBMoney isn't in scope for this and has its own pre-existing challenges in relation to filtering that go waaay beyond the scope of this card which is dealing with within range filter.