Follow up for #1953404: Add config schema to field and instance config entities

Problem/motivation

#1866610: Introduce Kwalify-inspired schema format for configuration introduced the idea of config schema. The changelog leads to (hopefully extensive) documentation on the format at http://drupal.org/node/1905070. As a part of schema addition to field module(#1953404: Add config schema to field and instance config entities), found that we have to provide schema for field types, field widgets and field storage.

Proposed solution

Create a configuration schema for field types, field widget and field storage (if applicable) defined in taxonomy module.
Schema in place

Schema not yet in place
field.taxonomy_term_reference.settings
field.taxonomy_term_reference.instance_settings
field.taxonomy_term_reference.value

field_widget.taxonomy_autocomplete.settings

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vijaycs85’s picture

interesting part is that when we try to save an instance with default term, it is getting saved as an entity with some dynamic keys. Here is an example:

id: node.article.field_tags
uuid: 778c8e07-b936-4952-bd14-1b7c60ddbb2f
status: '1'
langcode: en
field_uuid: ba877694-9ee3-4e01-95aa-2ed3dbff5f1c
entity_type: node
bundle: article
label: Tags
description: 'Enter a comma-separated list of words to describe your content.'
required: '1'
default_value:
  -
    tid: '0'
    entity:
      tid: ''
      uuid: 15a343b1-676a-48c1-b460-7fa9a9ca5948
      vid: tags
      name: Testig
      description: ''
      format: ''
      weight: '0'
      parent:
        - '0'
      langcode: und
      "\0*\0entityType": taxonomy_term
      "\0*\0enforceIsNew": ''
      "\0*\0newRevision": '0'
      "\0*\0isDefaultRevision": '1'
default_value_function: ''
settings:
  user_register_form: '0'
widget:
  weight: '-4'
  type: options_select
  settings:
    placeholder: working
    size: '60'
    autocomplete_path: taxonomy/autocomplete
  module: options
field_type: taxonomy_term_reference

UPDATE: created #1975156: Entering non-existing term as default to taxonomy reference field leads to wired data in field.instance config file. to fix default value issue.

vijaycs85’s picture

Initial patch with config_inspector screenshots...

vijaycs85’s picture

Minor syntax fix...

vijaycs85’s picture

Ignore the diff file.. it is just

-  type: mapping:
+  type: mapping

on taxonomy.schema.yml:62

yched’s picture

Aw. That looks like an unintended side effect of EntityNG. We should not save a whole entity in there, just a tid.
That should be fixed upstream, meanwhile we shouldn't try to generate schema info for this.

swentel’s picture

Title: Provide config schema to field types, widgets and storage in taxonomy module » Provide config schema to field types and storage in taxonomy module
Status: Needs review » Needs work
saki007ster’s picture

Assigned: Unassigned » saki007ster

Assigning it to myself, to work on it.

saki007ster’s picture

Removed the autocomplete widget from taxonomy.schema.yml

saki007ster’s picture

Status: Needs work » Needs review

Changing the status to needs review.

prateek479’s picture

Manually Tested This patch .
Working fine with configuration inspector module

star-szr’s picture

vijaycs85’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

yched’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/taxonomy/config/schema/taxonomy.schema.ymlundefined
@@ -39,3 +39,41 @@ taxonomy.vocabulary.*:
+field.taxonomy_term_reference.settings:

Looking at taxonomy_field_info(), this is missing 'options_list_callback' (type: string)

+++ b/core/modules/taxonomy/config/schema/taxonomy.schema.ymlundefined
@@ -39,3 +39,41 @@ taxonomy.vocabulary.*:
+field.taxonomy_term_reference.instance_settings:
+  type: mapping
+  label: 'Taxonomy term reference settings'
+  mapping:
+    user_register_form:
+      type: boolean
+      label: 'Display on user registration form.'

user_registration_form is gone, this should now be

 field.list_float.instance_settings:
   type: mapping
   label: 'Taxonomy term reference settings'
   sequence:
     - type: string
       label: 'setting'
vijaycs85’s picture

Status: Needs work » Needs review
FileSize
1021 bytes
1.31 KB

Updating changes for #13.

vijaycs85’s picture

Issue tags: +LONDON_2013_JULY

Updating tag...

yched’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
477 bytes
1.31 KB

Thanks @vijaycs85.
Just adjusted the label for 'options_list_callback'.

yched’s picture

Oops, sorry. Forgot that #1965208: Convert TaxonomyTermReferenceItem to extend EntityReferenceItem changed the column name from tid to target_id.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/taxonomy/config/schema/taxonomy.schema.ymlundefined
@@ -39,3 +39,43 @@ taxonomy.vocabulary.*:
+    options_list_callback:
+      type: string
+      label: 'Options list callback'
+    allowed_values:
+      type: sequence
+      label: 'Allowed values'
+      sequence:
+        - type: mapping
+          label: 'Allowed values'
+          mapping:
+            vocabulary:
+              type: string
+              label: 'Vocabulary'
+            parent:
+              type: string
+              value: 'Parent'

options_list_callback and allowed_values should be the other way around

+++ b/core/modules/taxonomy/config/schema/taxonomy.schema.ymlundefined
@@ -39,3 +39,43 @@ taxonomy.vocabulary.*:
+field.taxonomy_term_reference.value:
+  type: sequence
+  label: 'Default values'
+  sequence:
+    - type: mapping
+      label: 'Default value'
+      mapping:
+        target_id:
+          type: integer
+          label: 'Term ID'

Below is the structure of a default value in a field instance config file for a taxonomy term reference...

default_value:
  -
    target_id: '2'
    revision_id: ''
yched’s picture

Status: Needs work » Reviewed & tested by the community

We don't guarantee the ordering of entries under 'settings' in actual CMI files, and the order itself doesn't matter. The order in the current patch is the same as the one in taxonomy_field_info(), so I'd suggest we keep that one.

revision_id:
Hm. That's a result of having TaxonomyTermReferenceItem extend ConfigEntityReferenceItemBase, which defines a 'revision_id' property.
But, unlike entity_reference_field, taxonomy fields do not support this 'revision_id' property - see taxonomy_field_schema(). And this only makes sense, taxo terms are not revisioned.
So, that 'revision_id' has nothing to do here - but it's saved because it's a property of the FieldItem.

Not sure what to do about this.
- This entry will never have an actual value (other than '') in saved CMI files, and I don't really know how we'd document it in the first place since it has no meaning whatsoever here.
- But getting rid of it is going to be tricky in the current architecture around default values - unless TaxonomyTermReferenceItem overrides getPropertyDefinitions() and removes 'revision_id', but I'm not sure what that would break.

We do know that "default values' will need some more work ('default_value' for entity_ref-like fields need to be written in CMI as UUIDs, not as numeric IDs, there's an issue for that), I have a blurry plan for that now that field types are classes, I didn't get the opportunity to really sit down and hash it out so far, but it will need to allow this kind of "pre-CMI-save massaging" manipulations.

So I guess right now my advice would be to not document something which has no meaning and whose presence can be considered as a bug ?

Setting back to RTBC for feedback on the above.

yched’s picture

Talked with @amateescu on IRC, the agreed fix is that TaxonomyTermReferenceItem should not extend ConfigEntityReferenceItemBase (entity_reference.module), but just the basic EntityReferenceItem

amateescu’s picture

The latest patch is correct, we do not need to care about the 'revision_id' property in this schema. Posted a comment about that in #2015687-5: Convert field type to FieldType plugin for taxonomy module .

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 0643e0a and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.