SearchableDropdownField breaks grid field filtering

UndefinedOffset commented 2 months ago

Module version(s) affected

2.2.2

Description

In Silverstripe Admin 2.2.0+ the new SearchableDropdownField breaks grid field filtering, if you select any option from the dropdown and click search [object Object] is sent as the value and displayed in the filter tags. This appears to be caused by onChange receiving an object ({value: 1, label: "Option Label", selected: false } for example) instead of a string/numeric value.

This is exaggerated because SearchableDropdownField now appears to be the scaffold default for relationships.

How to reproduce

Add ParentID to SilverStripe\Security\Group's searchable_fields:

SilverStripe\Security\Group:
 searchable_fields:
   - 'ParentID'

~Attempt to search for the default Administrators Group~
~Note the tag in the filter bar and the request made to the server has the same value~
Create a group and make it the child of a group
In /admin/security/groups click the search magnifying glass, then click the "search options" dropdown.
In the "Parent ID" dropdown, select the parent group (or any group really) and click "Search"
Note that there are no results, and both the tag in the filter bar and the value in the request say [object Object]

Possible Solution

A possible solution to this is to change the handleChange function to call onChange like SingleSelectField does passing the actual value as the second argument.

const handleChange = (val) => {
  setHasChanges(false);
  if (JSON.stringify(value) !== JSON.stringify(val)) {
    setHasChanges(true);
    setJustChanged(true);
  }
  onChange({}, val);
};

Additional Context

No response

Validations

[X] Check that there isn't already an issue that reports the same bug
[X] Double check that your reproduction steps work in a fresh installation of silverstripe/installer (with any code examples you've provided)

PRs

sabina-talipova commented 1 month ago

I've made several attempts to work around these changes, but due to the variability of how data is stored, how we send data to the server and how it is displayed, this requires more careful study than simply fixing a tag display problem.

Since, for example, in the current implementation of SearchableDropdownField component (without changes), the search, with the provided example (How to reproduce), does not work at all, since in the request we send Parent_ID = "[object Object]" to the server instead of a numeric ID, which accordingly does not return any result.

I will leave my PR open, but I think that SearchableDropdownField requires more in-depth development, taking into account what data it receives and its further processing, for example, simply saving data or searching for a specific value. You should pay attention to how FormBuilder processes and stores data in state, as well as subsequent work with this data on the server.

GuySartorelli commented 1 month ago

@sabina-talipova Can you please give me some steps that I can follow to see and reproduce the problems you were running into?

sabina-talipova commented 1 month ago

Yes, sure, @GuySartorelli

Repeat step from "How to reproduce" for configuring searchable_fields
Go to Security section
Open Groups tab
Click Add new Group
Set Name and add any existing Group as Parent Group
Save
Go back to Groups list
Click Magnified glass and then Search option
Choose Parent Group of your new Group and click Search
You should get "No items found"
Check Network for "groups" request payload and you see filter[groups][ParentID]: [object Object].

For multiple choice is better to test in Files section in file Permissions tab (Viewer Users). Update: If you pass payload as it was suggested in issue, then SearchableDropdownField with single or multiple choice cannot keep data.

GuySartorelli commented 1 month ago

I've updated the steps to reproduce in the issue description - this isn't about searching for things by name like the original steps implied. It's specifically about searching using the SearchableDropdownField.

Note that searching by name in the original instructions only doesn't work because the Group model doesn't explicitly define searchable_fields, so when you do define those, it removes both the title and description fields. You can add those back quick easily:

SilverStripe\Security\Group:
  searchable_fields:
    - 'Title'
    - 'Description'
    - 'ParentID'

I'll look into the problems you were facing now that I understand the core issue.

sabina-talipova commented 1 month ago

It's not quite correct conclusion. The problem is that we convert Object to String (it means Object.toString()) and get filter[groups][ParentID]: "[object Object]" instead of ID or object converted into json string (smth like: "{id: 1, value: Admin}"). And then we use this "[object Object]" string everywhere. This is the main problem for SearchableDropdownField.

GuySartorelli commented 1 month ago

That's true from a technical perspective, yes. I was just trying to get the user perspective so I can actually reproduce the problem in the first place, which is what I now have.

GuySartorelli commented 1 month ago

PR merged. This will be automatically tagged by GitHub actions. There's still some follow-up work to do, see https://github.com/silverstripe/silverstripe-admin/issues/1746 for details.

silverstripe / silverstripe-admin