Sequence subtyping cannot override item type in config schema, views taxonomy term filter schema incorrect

Does that have a failing test? If yeah it may be easier to fix it there. It may be more painful to provide a standalone test here.

Right, it may be simpler to fix it there? No need for one more explicit test then?

So we can postpone on #2372855: Add content & config entity dependencies to views and just update that config.

#2372855: Add content & config entity dependencies to views is fixed, so unpostponing this one.

This also could probably be a novice issue!

Where the schema is incorrect, it will be a D8 upgrade path issue because we would need to write an update function for updating the config structure to the new correct expectation. At that point I am not sure this would be a novice.

Additionally to the incorrect value type on views.filter.taxonomy_index_tid (inherited from views.filter.many_to_one), the definition of views.filter.taxonomy_index_tid_depth was entirely incorrect. It inherits from views.filter.taxonomy_index_tid on the PHP side (class TaxonomyIndexTidDepth extends TaxonomyIndexTid) so should on the schema side. Then all the duplicate keys should be removed (including values). The only new field is depth. See TaxonomyIndexTidDepth::defineOptions().

Looks fine for me.

So this is a WAY bigger mess than expected. The problem is as follows:

1. views_filter defines a value key as a dynamic type based on the filter's plugin_id:

views_filter:
  type: views_handler
  mapping:
    ...
    value:
      type: views.filter_value.[%parent.plugin_id]
      label: 'Value'
    ...

So all types extending from views_filter theoretically need to define their own value via subtyping in the views.filter_value.X namespace. Here are some:

views.filter_value.boolean:
  type: boolean

views.filter_value.combine:
  type: string

That is all good, and these are actually single value filter types. BUT then there are multivalue field types, chiefly InOperator and its extension ManyToOne. The config schema for those defined the value as a sequence of strings.

views.filter.in_operator:
  type: views_filter
  label: 'IN operator'
  mapping:
    ...
    value:
      type: sequence
      label: 'Values'
      sequence:
        - type: string
          label: 'Value'
    ...

Due to how this is deepmerged, the sequence typing overrides the original dynamic type so all in_operator inheritors (such as many to one, taxonomy index tid, etc) take the string sequence instead of the dynamic type.

Now we naturally want to override that in views.filter.taxonomy_index_tid which turns out to be impossible. Why? If you look into TypedConfigManager::getDefinition(), the definition is merged in with the parent, giving the child precedent but NestedArray::mergeDeep() is used, which accumulates numerically indexed array items instead of replacing them with each other. Configuration schema uses numerically indexed items for typing only for sequences. It is a bit of an odd use of a list because it can only ever have one item, but well, that is how it is now. We could change that and incur the API change across all schemas or we need to fix the merging to not accumulate array items (having second and third type definitions for the sequence) but to overwrite it, so our type definition for the taxonomy item will prevail.

Definitely not novice and at least a major.

Additionally to the PHP bugfix, removing the elements from many_to_one that are already inherited and removing a never ever used views.filter_value.language since the parent filter definition is an in_operator, so as discussed above, the dynamic views.filter_value.language value would never be consulted anyway. (Its also incorrect, the in_operator is multivalued not single valued).

Updated issue summary and added beta evaluation as well.

Looks good.

1. +++ b/core/modules/views/config/schema/views.filter.schema.yml
   @@ -101,18 +101,9 @@ views.filter.many_to_one:
   -    operator:
   ...
   -    value:
   

   another cleanup?

2. +++ b/core/modules/views/config/schema/views.filter.schema.yml
   @@ -140,7 +131,3 @@ views.filter_value.combine:
   -views.filter_value.language:
   

   left over removal++

Committed a542a43 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation.