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 image module.
Schema in place
field.image.settings

Note: Part of main field schema patch at #1953404: Add config schema to field and instance config entities
Schema not yet in place
field.image.instance_settings
field.image.value
field_widget.image_image.settings

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

vijaycs85’s picture

Initial patch...

Status: Needs review » Needs work

The last submitted patch, 1973436-config-schema-field-image-1.patch, failed testing.

vijaycs85’s picture

Status: Needs work » Postponed
vijaycs85’s picture

Component: field system » image system
Status: Postponed » Needs review
FileSize
3.63 KB

Removing image module changes from main issue and adding here...

swentel’s picture

Title: Provide config schema to field types, widgets and storage for image module » Provide config schema to field types storage for image module
Status: Needs review » Needs work
claudiu.cristea’s picture

Any news here? Is this still needed?

claudiu.cristea’s picture

Assigned: Unassigned » claudiu.cristea

Let's rework this.

vijaycs85’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
3.43 KB

Updating with re-roll and removed widget...

vijaycs85’s picture

Issue tags: +LONDON_2013_DECEMBER

Updating with re-roll and removed widget...

claudiu.cristea’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/image/config/schema/image.schema.yml
    @@ -89,3 +83,96 @@ image.settings:
    +    default_image:
    +      type: string
    +      label: 'Default image'
    ...
    +    default_image:
    +      type: boolean
    +      label: 'Default image'
    

    Default image default_image is no more a simple value (anyway it was wrongly casted here as 'string' and 'bool'). It has been converted to associative array in #1443606: Alt, title, width and height for default images.

  2. +++ b/core/modules/image/config/schema/image.schema.yml
    @@ -89,3 +83,96 @@ image.settings:
    +    user_register_form:
    +      type: boolean
    +      label: 'Display on user registration form.'
    

    user_register_form has gone along with #2049485: Remove traces of the 'user_register_form' field setting and we shouldn't reintroduce it here.

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
61.59 KB
951 bytes
3.69 KB

Thanks for the review @claudiu.cristea. Addressing both issues in #10 and attaching verified config_inspector screenshot.

aspilicious’s picture

Status: Needs review » Needs work
+    default_image:
+      type: string
+      label: 'Default image'

BUSTED :D, needs to be a mapping

vijaycs85’s picture

Status: Needs work » Needs review
+++ b/core/modules/image/config/schema/image.schema.yml
@@ -89,3 +83,109 @@ image.settings:
+    default_image:
+      type: mapping
+      label: 'Default image'
+      mapping:
+        fid:
+          type: integer
+          label: 'Image'
+        alt:
+          type: label
+          label: 'Alternate text'
+        title:
+          type: label
+          label: 'Title'
+        width:
+          type: integer
+          label: 'Width'
+        height:
+          type: integer
+          label: 'Height'
+

here is mapping?

claudiu.cristea’s picture

aspilicious’s picture

Hmm isn't that mapping part of field.image.value ?
(so that we can reference it as image type)

Partly guessing here, not sure how config schemas relate to field defintions

claudiu.cristea’s picture

Ok, then this looks better

The last submitted patch, 14: 1973436-config-schema-field-image-14.patch, failed testing.

The last submitted patch, 16: 1973436-config-schema-field-image-16.patch, failed testing.

claudiu.cristea’s picture

Wrong generated patch. Here we go.

vijaycs85’s picture

+++ b/core/modules/image/config/schema/image.schema.yml
@@ -89,3 +83,105 @@ image.settings:
+field.image.default:

Can we make this as content type by adding in image.data_type.schema.yml with key as field_image_default

+++ b/core/modules/image/config/schema/image.schema.yml
@@ -92,24 +92,8 @@ field.image.settings:
+      type: field.image.default

Not sure we can use type like this. We always keep content type with underscore than schema definition...

claudiu.cristea’s picture

The last submitted patch, 19: 1973436-config-schema-field-image-19.patch, failed testing.

The last submitted patch, 19: 1973436-config-schema-field-image-19.patch, failed testing.

claudiu.cristea’s picture

FileSize
5.07 KB
301 bytes

The last submitted patch, 21: 1973436-config-schema-field-image-21.patch, failed testing.

vijaycs85’s picture

Looks good to me, except one minor suggestion...

+++ b/core/modules/image/config/schema/image.data_types.schema.yml
@@ -0,0 +1,30 @@
+field_image_default:

may be field_default_image?

claudiu.cristea’s picture

FileSize
1.17 KB
5.07 KB

OK :)

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Good to go...

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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