Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOC Document new filter by range functionality #687

Merged

Conversation

GuySartorelli
Copy link
Member

@@ -25,6 +25,7 @@ use SilverStripe\ORM\DataObject;

class MyDataObject extends DataObject
{
// ...
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding these where appropriate in this file to indicate we're skipping some best practices for the sake of brevity - we tend to do this with new code blocks, so this just gives consistency across this file.

Comment on lines -438 to +450
'ProductCode' => NumericField::class,
'ProductCode' => [
'field' => NumericField::class,
],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this worked at some stage and broke, but it definitely doesn't work right now and shouldn't be documented as a thing that works.

Comment on lines -518 to +589
> If you don't specify a `FormField`, you must use the name of a real database field as the array key instead of a custom name so that a default field class can be determined.
> If you don't specify `field` or `dataType`, you must use the name of a real database field as the array key instead of a custom name so that a default field class can be determined.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dataType is new and was intended for use with the range stuff, but it also works here so makes sense to document this option.

@@ -596,6 +597,40 @@ SilverStripe\UserForms\Model\UserDefinedForm:

</details>

### Filter within a range with `searchable_fields` {#searchable-withinrange}

Using the `searchable_fields` configuration, you can now declare that a field should be filtered using a range. This works out of the box with the numeric, date, datetime, and time fields that come in Silverstripe framework.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Using the `searchable_fields` configuration, you can now declare that a field should be filtered using a range. This works out of the box with the numeric, date, datetime, and time fields that come in Silverstripe framework.
Using the `searchable_fields` configuration, you can now declare that a field should be filtered using a range. This works out of the box with the numeric, date, datetime, and time fields that come in the Silverstripe framework.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

{
// ...
private static array $db = [
'Price' => 'Currency',
Copy link
Member

@emteknetnz emteknetnz Feb 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The currency filter currently seems very busted, even with the framework PR. There is a card open to fix at least some of that issues there, though the PR change this example use an Int instead so when people copy it they don't think that they've done something wrong

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather just fix the field. If it's just the validation thing, I'll pull that card into in progress and fix it. If there are more problems, please open cards for them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted on silverstripe/silverstripe-framework#11602 (comment) it's related to the framework PR, so should be fixed there

@GuySartorelli GuySartorelli force-pushed the pulls/6.0/withinrange-filter branch from 7d59002 to a0e943c Compare February 17, 2025 02:52
@emteknetnz emteknetnz merged commit 2a70b1e into silverstripe:6.0 Feb 18, 2025
3 checks passed
@emteknetnz emteknetnz deleted the pulls/6.0/withinrange-filter branch February 18, 2025 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants