Problem/Motivation

In #3428486: Fix invalid schema definition @Grevil found schema issues with Micon.

Looking into the schema I think the module is missing the schema definitions for its fields entirely!
The schema was first introduced in #3271002: Fix coding standards, add installation test and fix schema errors a while ago, but I think we didn't run into issues, because the fields were not set up and tested entirely combined with tests?

As Micon extends StringItem I guess we shouldn't define that part of the schema ourselves but inherit the string schema, if possible?
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/config/schema...

I think looking at the largest contrib field modules could speed things up:

Another option might be to unset these settings like written here: https://www.drupal.org/project/drowl_paragraphs_bs/issues/3428486#commen... if they are really not used and not useful!

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#11 2024-03-21 10_06_29-Manage fields _ Drush Site-Install.png32.62 KBgrevil

Issue fork micon-3428592

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Anybody created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Issue summary: View changes
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Priority: Normal » Major

Grevil made their first commit to this issue’s fork.

grevil’s picture

grevil commented

Interestingly enough, config inspector AND the tests both won't show any schema errors... interesting.

grevil’s picture

grevil commented

No idea, the schema definition seems to be fine on micon's site.

I tried redefining the storage definition using:

field.storage_settings.micon_string:
  type: mapping
  label: 'Micon string settings'
  mapping:
    max_length:
      type: integer
      label: 'Maximum length'
    case_sensitive:
      type: boolean
      label: 'Case sensitive'
    is_ascii:
      type: boolean
      label: 'Contains US ASCII characters only'

and "extending" it:

field.storage_settings.micon_string:
  type: field.storage_settings.string

similiar to https://git.drupalcode.org/project/entity_reference_revisions/-/blame/8....

Nothing seems to work and neither the tests nor the config inspector throws any schema errors before or after the changes.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@Grevil: Did you set up such a field like the one that shows the error in the tests?

My guess is, that this will first happen with a field set up?

grevil’s picture

grevil commented
Status: Active » Needs review

Fixed, please review!

Anybody opened merge request !31

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Needs work

@Grevil: Nice! I left two minor comments.
But we shouldn't tag a release too soon with this change, I think. We need to be sure this has no side-effects.

Also one hard part is left I guess: Removing the old config from existing installations?
Perhaps you can find a snippet for that?

grevil’s picture

grevil commented
Assigned: grevil » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2024-03-21 10_06_29-Manage fields _ Drush Site-Install.png32.62 KB

Alright all done! Added an update hook as well, and it runs well and as expected:
screenshot

After the update, the field still works as expected and displays the given icon. Furthermore, no default_value is overriden or any other important field storage values.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @Grevil! LGTM! Still it's a heavy, but important change, so I'd like to have a sign-off by another maintaner who tested this.

RTBC from my side.
We should then wait with a new tagged release, once signed off and merged into dev, to see if this causes trouble somewhere.
Thank you!

grevil’s picture

grevil commented
Assigned: Unassigned » jacobbell84
grevil’s picture

grevil commented
Assigned: jacobbell84 » Unassigned
Status: Needs review » Needs work

@Anybody found another schema error:

content.field_symbol.settings.packages Undefined

grevil’s picture

grevil commented
Assigned: Unassigned » jacobbell84
Status: Needs work » Needs review

Alright, should be fixed now, back to needs review.

grevil’s picture

grevil commented
Status: Needs review » Reviewed & tested by the community

I guess, @jacobbell84 is quite busy. I just ran into this issue again. As you RTBC'd this, I am just going to merge it. It was tested enough in my opinion.

grevil’s picture

grevil commented
Assigned: jacobbell84 » Unassigned
Status: Reviewed & tested by the community » Fixed

  • Grevil committed 3cf3d3be on 2.x 
    Hotfix: Adjust schema definition for "field.widget.settings.micon_string...

Status: Fixed » Closed (fixed)

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