FIX SearchableDropdownField pass incorrect value on onChange event

[sabina-talipova]

Parent issue

• https://github.com/silverstripe/silverstripe-admin/issues/1733

[sabina-talipova]

I left comment in the issue: https://github.com/silverstripe/silverstripe-admin/issues/1733#issuecomment-2097160219

[GuySartorelli]

This PR seems to work without any problems for me.

In https://github.com/silverstripe/silverstripe-admin/issues/1733#issuecomment-2097160219 you mentioned "this requires more careful study than simply fixing a tag display problem" - but I don't understand what problems are left? Does this PR introduce new regressions? Or are there other separate problems which also need to be fixed separately?

[sabina-talipova]

That's right, this PR solves the current problem and, as far as I was able to test, it does not cause regression problems.

I will try to slightly reformulate my doubts about the correct operation of SearchableDropdownField expressed in the comments.

SearchableDropdownField works great everywhere except for search or where we need to pass a specific value (number or string). Since react-select accepts an object or an array of objects as a “props.value” and return specific object onChange event, SearchableDropdownField through FormBuilder stores the object in redux state, from where we later retrieve this data before sending it to the server.

Everything works fine if we process the data before sending it to the server, as for example in the case of an asset-admin and users who are allowed to view. But not in the case of searching, since the data is not processed in any way before sending and we simply transfer the object.

The problem occurs in $.ajax, where I assume obj.toString() is applied to the object instead of JSON.stringify(obj). Therefore, we send "[objectObject]" to the server instead of "{id: 1, value: something}". But in any case, the server expects to receive some specific value (a number or a string), but not an object. Therefore, we should make changes to the search function on the backend.

Also, after the server returns the response, we will store the value “[objectObject]” in state, which in turn will later be displayed in the CMS elements.

So I believe the problem is a little deeper than just SearchableDropdownField. That is, I think that we should analyse where and how this field can be used and what data it will contain. You should also consider what type of data react-select accepts. Since in the future we plan to change the approach to working using Redux, this should also be given attention.

This PR simply stores in state only the value for a single value field and the entire array for a multiple choice field. That is, I think if someone decides to useSearchableDropdownField with multiple choice as a filter for searching, then it will not work.

[GuySartorelli]

Sorry, that went a bit too deep into the weeds (too much specific technical detail) for me to be able to easily understand what problems remain.

But not in the case of searching, since the data is not processed in any way before sending and we simply transfer the object.

The problem occurs in $.ajax, where I assume obj.toString() is applied to the object instead of JSON.stringify(obj). Therefore, we send "[objectObject]" to the server instead of "{id: 1, value: something}". But in any case, the server expects to receive some specific value (a number or a string), but not an object. Therefore, we should make changes to the search function on the backend.

The above suggests there's still some problem related to SearchableDropdownField in relation to searching - but that's exactly what this PR fixes, isn't it?

You should also consider what type of data ReactSelect accepts. Since in the future we plan to change the approach to working using Redux, this should also be given attention.

Are there any specific problems related to this that you're aware of?

This PR simply stores in state only the value for a single value field and the entire array for a multiple choice field. That is, I think if someone decides to use SearchableDropdownField with multiple choice as a filter for searching, then it will not work.

That's a good point. I've just tested that using the following code:

<?php

namespace App\Extension;

use SilverStripe\Core\Extension;
use SilverStripe\Forms\SearchableMultiDropdownField;
use SilverStripe\Security\Member;

class GroupSearchableFieldsExtension extends Extension
{
    public function updateSearchableFields(array &$fields)
    {
        $fields['Members'] = [
            'title' => 'Users',
            'field' => SearchableMultiDropdownField::create('Members', 'Users', Member::get()),
        ];
    }
}

SilverStripe\Security\Group:
  extensions:
    - App\Extension\GroupSearchableFieldsExtension

Searching with that field does indeed give a bad result - specifically if I select two records I get [object+Object],[object+Object] in the request, which results in an exception. That should be easy to remedy in this PR.

[GuySartorelli]

Are there any specific problems outstanding beyond that? As in specific problems that cause errors or bad UX, which can be easily reproduced?

[sabina-talipova]

@GuySartorelli , how I said before, the current PR fixes initial issue. For another problems that were discussed in comments I've opened new ticket https://github.com/silverstripe/silverstripe-admin/issues/1746

Are there any specific problems outstanding beyond that? As in specific problems that cause errors or bad UX, which can be easily reproduced?

No, I couldn't find any problems. I would be good to test this field created by FormBuilder.

[GuySartorelli]

I've updated that issue to reflect what we discussed in slack - the main problem is that many_many relations can't be searched against, and fixing that is necessary for using multiple values in SearchableMultiDropdownField to be useful.