enum values in schema.json vs schema.ui.json (#4041)

[stefan-korn]

fixes #4041

• Test coverage exists
• Documentation exists

QA Steps

• Add the following to dataset.ui.json at "accessLevel" -> "ui:options"

      "widget": "list",
      "type": "select_other"

see that access level will now generate a "select other" widget with the options from the enum from dataset.json

[janette]

@stefan-korn can you make this: if (isset($spec->source)) {

[stefan-korn]

@janette : Thanks, change makes sense.

[dafeder]

Hi @stefan-korn sorry it took me so long to really take a look at this, for some reason every time I started I would get confused by the different possible scenarios and then put it down again 🙃

The thing is I think the current behavior is correct, and perhaps needs more documentation. When something is given enum values in the schema itself, you are saying those are the only values that are valid. Your code works in the sense that it lets me add a select or other widget for the license field, but if you actually use "other" your value won't validate and you won't be able to save.

This is easy to test - using your branch I try to add a value other than "public," "restricted public" or "non-public" and I get a validation error.

The way we implemented the enum stuff in the UI schema is I think a little confusing and could be improved. "Enum" is maybe the wrong term. Also there is a lot there to support select options with labels but allow people to add a custom value that is just a raw URL -- this is pretty specific to the license field and we would probably not need it in very many other use-cases?

It might make sense to look more closely at how the inspiration for all of this -- react-jsonschema-form -- handles different scenarios with option lists and examples and such. I don't think that tool would support this specific thing we're doing with license but there are other combinations/use-cases they do support that we don't.

[stefan-korn]

@dafeder : Thanks for looking into this and thanks for the explanation. Did not realize the behavior around "select other". My focus was really on using widget "list" with "select2", though in the issue I mentioned select_other.

The validation happens from https://github.com/m1x0n/opis-error-presenter/blob/master/src/Implementation/Formatters/Enum.php if the enum is in schema.json and this validation does not happen if the enum is defined in schema.ui.json.

So in schema.ui.json it's basically some kind of "fake Enum" that skips the validation, which then enables the "other" feature.

It still seems a bit inconsistent to me having enum values in schema.json and/or schema.ui.json.

And if you want to use the "select2" list widget you would need to move the enum options from schema.json to schema.ui.json (or have it in both places). With this PR this is not necessary.

And the PR is not breaking the schema.ui functionality on enum. You can still use the select_other if you configure it in schema.ui.

But I agree that there is some need of documentation of different possibilities here.

That said, is there even some more about this with regards to validation if the dataset is not created via form, but maybe via API or other measures? So for example: I want a select2 in the form and enum validation (maybe for API creation). Then I need to define the enum values in schema.json (for validation) and in schema.ui.json for the select2?

I still see a consistency issue about the enum values in schema.json vs schema.ui.json, but now understand the need of schema.ui.json enum values. As the MR does not break anything currently, maybe it would be useful in some cases. But yes, then documentation around this should be improved.

[dafeder]

OK, I guess I assumed that the reason you wanted to use select_other was to have that "other" option, which will always fail validation. That's what I meant by this "breaking," sorry maybe I should have said it a different way.

Now I see that using "type": "autocomplete" instead of select_other would only allow valid values, and that wasn't possible without this change. So could you just confirm - the real user story here would be more like:

I want to customize the widget for an enum field so that I can use autocomplete on a long list of enum values from my schema

right?

I think in the conversation about whether enum makes sense in the UI schema I was losing the real purpose here. I still think this is useful even if implemented questionably (maybe we should have called it "examples" or something instead of "enum" to make it clearer that this was not a basis for validation).

So, I would be comfortable merging this if it has a test, and will work on some better documentation for the ui schema.

[dafeder]

Needs test and one more rebase. Something like this should work in WidgetRouterTest::dataprovider():

      'selectFieldAutocomplete' => [
        (object) [
          'widget' => 'list',
          'type' => 'autocomplete',
        ],
        [
          '#type' => 'select',
          '#options' => ['option1', 'option2'],
          '#title' => 'selectField',
        ],
        [
          '#type' => 'select2',
          '#title' => 'selectField',
          '#options' => ['option1', 'option2'],
          '#other_option' => FALSE,
          '#multiple' => FALSE,
          '#autocreate' => FALSE,
          '#target_type' => 'node',
        ],
      ],