Add validation constraints to all system.* simple config (except system.rss)

Created on 21 February 2024, 4 months ago
Updated 15 April 2024, 2 months ago

Problem/Motivation

Per 🌱 [meta] Add constraints to all simple configuration Active , the current state of validatable simple config in the System module is:

  • system.site
  • βœ…system.maintenance
  • system.cron
  • system.date
  • system.diff
  • system.logging
  • system.performance
  • system.rss
  • system.theme
  • system.file
  • βœ…system.image
  • βœ…system.image.gd
  • system.mail
  • system.advisories
  • βœ…system.feature_flags

All of these seem possible to make fully validatable. Sole exception: system.rss, because it A) depends on config entities, B) it depends on multiple or none at the same time. It probably makes more sense to remove that config πŸ˜…

P.S.: system.theme.global isn't in that list because it extends type: theme_settings, not type.config, but fortunately πŸ“Œ Add validation constraints to `type: theme_settings` Active will handle that πŸ‘

Proposed resolution

  1. Add validation constraints.
  2. Mark FullyValidatable.
  3. Use #config_target in config forms wherever possible

Remaining tasks

Review.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

N/A

πŸ“Œ Task
Status

Needs work

Version

11.0 πŸ”₯

Component
SystemΒ  β†’

Last updated 42 minutes ago

No maintainer
Created by

πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @Wim Leers
  • Merge request !6714Resolve #3422904 "System config" β†’ (Open) created by Wim Leers
  • Pipeline finished with Failed
    4 months ago
    Total: 483s
    #100367
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    system.rss.yml is mostly useless and should be removed in πŸ› Views RSS view mode settings are completely broken Needs work

  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Thank you, @longwave! That's a relief πŸ˜„ I'll push that forward too then πŸ‘

  • Pipeline finished with Failed
    4 months ago
    Total: 554s
    #100436
  • Pipeline finished with Failed
    4 months ago
    #100462
  • Pipeline finished with Failed
    4 months ago
    Total: 472s
    #100674
  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Down to a single failure, for AjaxTest::testAjaxFocus(). I'll let that one slide for now and move on to the next system.* simple config πŸ‘

  • Pipeline finished with Success
    4 months ago
    Total: 472s
    #100683
  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Aha, that passed β€” so AjaxTest must be prone to random failures…

  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    That "dear committer" note on the MR updates code introduced by #2870878: Add config validation for UUIDs β†’ 6.5 years ago that is now obsolete πŸ₯³

  • Pipeline finished with Failed
    4 months ago
    Total: 475s
    #100693
  • Pipeline finished with Failed
    4 months ago
    Total: 481s
    #100718
  • Pipeline finished with Success
    4 months ago
    Total: 482s
    #100778
  • Pipeline finished with Failed
    4 months ago
    Total: 509s
    #100809
  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Wim Leers β†’ changed the visibility of the branch 2300677-jsonapi-config-entities to hidden.

  • Pipeline finished with Failed
    4 months ago
    Total: 485s
    #101598
  • Pipeline finished with Failed
    4 months ago
    Total: 473s
    #104149
  • Status changed to Needs work 4 months ago
  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 491s
    #112085
  • Pipeline finished with Failed
    4 months ago
    Total: 515s
    #112102
  • πŸ‡§πŸ‡ͺBelgium Wim Leers Ghent πŸ‡§πŸ‡ͺπŸ‡ͺπŸ‡Ί

    Down to a single failure triggered by the validation now running for system.theme β€” quite a few test coverage bugs uncovered! πŸš€

  • Pipeline finished with Failed
    4 months ago
    Total: 545s
    #112111
  • πŸ‡ΊπŸ‡ΈUnited States phenaproxima Massachusetts
Production build 0.69.0 2024