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
Comment | File | Size | Author |
---|---|---|---|
#17 | 1973498-config-schema-field-taxonomy-17.patch | 1.31 KB | yched |
#17 | interdiff.txt | 422 bytes | yched |
#16 | 1973498-config-schema-field-taxonomy-16.patch | 1.31 KB | yched |
#16 | interdiff.txt | 477 bytes | yched |
#14 | 1973498-config-schema-field-taxonomy-14.patch | 1.31 KB | vijaycs85 |
Comments
Comment #1
vijaycs85interesting 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:
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.
Comment #2
vijaycs85Initial patch with config_inspector screenshots...
Comment #3
vijaycs85Minor syntax fix...
Comment #4
vijaycs85Ignore the diff file.. it is just
on taxonomy.schema.yml:62
Comment #5
yched CreditAttribution: yched commentedAw. 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.
Comment #6
swentel CreditAttribution: swentel commentedWidget needs to go out after #1875992: Add EntityFormDisplay objects for entity forms got in.
Comment #7
saki007sterAssigning it to myself, to work on it.
Comment #8
saki007sterRemoved the autocomplete widget from taxonomy.schema.yml
Comment #9
saki007sterChanging the status to needs review.
Comment #10
prateek479 CreditAttribution: prateek479 commentedManually Tested This patch .
Working fine with configuration inspector module
Comment #11
star-szr#8: 1973498-config-schema-field-taxonomy-8.patch queued for re-testing.
Comment #12
vijaycs85Looks good to me.
Comment #13
yched CreditAttribution: yched commentedLooking at taxonomy_field_info(), this is missing 'options_list_callback' (type: string)
user_registration_form is gone, this should now be
Comment #14
vijaycs85Updating changes for #13.
Comment #15
vijaycs85Updating tag...
Comment #16
yched CreditAttribution: yched commentedThanks @vijaycs85.
Just adjusted the label for 'options_list_callback'.
Comment #17
yched CreditAttribution: yched commentedOops, sorry. Forgot that #1965208: Convert TaxonomyTermReferenceItem to extend EntityReferenceItem changed the column name from tid to target_id.
Comment #18
alexpottoptions_list_callback and allowed_values should be the other way around
Below is the structure of a default value in a field instance config file for a taxonomy term reference...
Comment #19
yched CreditAttribution: yched commentedWe 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.
Comment #20
yched CreditAttribution: yched commentedTalked with @amateescu on IRC, the agreed fix is that TaxonomyTermReferenceItem should not extend ConfigEntityReferenceItemBase (entity_reference.module), but just the basic EntityReferenceItem
Comment #21
amateescu CreditAttribution: amateescu commentedThe 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 .
Comment #22
alexpottCommitted 0643e0a and pushed to 8.x. Thanks!