Add validation constraints to system.file

Created on 26 March 2024, 8 months ago
Updated 17 July 2024, 4 months ago

Problem/Motivation

System module settings has 2 property paths that are not yet validatable:

vendor/bin/drush config:inspect --filter-keys=system.file --detail --list-constraints
โžœ  ๐Ÿค– Analyzingโ€ฆ

 Legend for Data: 
  โœ…โ“  โ†’ Correct primitive type, detailed validation impossible.
  โœ…โœ…  โ†’ Correct primitive type, passed all validation constraints.
 ---------------------------------------- --------- ------------- ------ ------------------------------------------ 
  Key                                      Status    Validatable   Data   Validation constraints                    
 ---------------------------------------- --------- ------------- ------ ------------------------------------------ 
  system.file                              Correct   67%           โœ…โ“   ValidKeys: '<infer>'                      
   system.file:                            Correct   Validatable   โœ…โœ…   ValidKeys: '<infer>'                      
   system.file:_core                       Correct   Validatable   โœ…โœ…   ValidKeys:                                
                                                                            - default_config_hash                   
   system.file:_core.default_config_hash   Correct   Validatable   โœ…โœ…   NotNull: {  }                             
                                                                          Regex: '/^[a-zA-Z0-9\-_]+$/'              
                                                                          Length: 43                                
                                                                          โ†ฃ PrimitiveType: {  }                     
   system.file:allow_insecure_uploads      Correct   Validatable   โœ…โœ…   โ†ฃ PrimitiveType: {  }                     
   system.file:default_scheme              Correct   NOT           โœ…โ“   โš ๏ธ  @todo Add validation constraints here  
   system.file:temporary_maximum_age       Correct   NOT           โœ…โ“   โš ๏ธ  @todo Add validation constraints here  

Steps to reproduce

Proposed resolution

Add validation constraints to:

  1. system.file:default_scheme
  2. system.file:temporary_maximum_age

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

๐Ÿ“Œ Task
Status

Needs work

Version

11.0 ๐Ÿ”ฅ

Component
Systemย  โ†’

Last updated 2 days ago

No maintainer
Created by

๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

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

Merge Requests

Comments & Activities

  • Issue created by @srishtiiee
  • Merge request !7187add validation constraints โ†’ (Open) created by srishtiiee
  • Pipeline finished with Failed
    8 months ago
    Total: 182s
    #129396
  • First commit to issue fork.
  • Pipeline finished with Failed
    8 months ago
    Total: 477s
    #129456
  • Pipeline finished with Failed
    8 months ago
    Total: 508s
    #129506
  • Pipeline finished with Failed
    8 months ago
    Total: 180s
    #130305
  • Status changed to Closed: duplicate 8 months ago
  • Status changed to Needs work 7 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia srishtiiee

    srishtiiee โ†’ changed the visibility of the branch 3436096-add-validation-constraints to hidden.

  • Pipeline finished with Failed
    7 months ago
    Total: 506s
    #148036
  • Pipeline finished with Failed
    7 months ago
    Total: 513s
    #148068
  • Pipeline finished with Failed
    7 months ago
    Total: 544s
    #150101
  • Pipeline finished with Failed
    7 months ago
    Total: 1081s
    #150231
  • Pipeline finished with Failed
    7 months ago
    Total: 195s
    #162152
  • Pipeline finished with Failed
    7 months ago
    Total: 1151s
    #162153
  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Tests are failing and I think we can fix that by just pushing the latest origin/11.x to this issue fork ๐Ÿคž

  • ๐Ÿ‡ง๐Ÿ‡ชBelgium wim leers Ghent ๐Ÿ‡ง๐Ÿ‡ช๐Ÿ‡ช๐Ÿ‡บ

    Review posted on MR โ€” if I'd found nothing in my review, then I'd have done #9 for you โ€” but since I can't RTBC this yet anyway, I'll let you handle #9 too ๐Ÿค“

  • Pipeline finished with Failed
    7 months ago
    Total: 521s
    #166069
  • Pipeline finished with Failed
    7 months ago
    Total: 1558s
    #168648
  • Pipeline finished with Failed
    7 months ago
    Total: 631s
    #168663
  • Pipeline finished with Failed
    7 months ago
    Total: 674s
    #169499
  • Pipeline finished with Failed
    6 months ago
    Total: 610s
    #171500
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    yash.rode โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Failed
    5 months ago
    Total: 170s
    #210495
  • Pipeline finished with Failed
    5 months ago
    Total: 186s
    #210500
  • Pipeline finished with Failed
    5 months ago
    Total: 538s
    #210538
  • Pipeline finished with Canceled
    5 months ago
    Total: 320s
    #212794
  • Pipeline finished with Success
    5 months ago
    Total: 549s
    #212800
  • Status changed to Needs review 5 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune
  • Status changed to RTBC 5 months ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    On a standard profile install on 11.x
    Installed configuration inspector and applied the MR

    Shows that the system.file is fully validated.

    Checking the MR everything appears to have a return type

    Believe this one is good to go.

  • Status changed to Needs work 5 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom alexpott ๐Ÿ‡ช๐Ÿ‡บ๐ŸŒ

    If path is deprecated we should deprecate it. See https://www.drupal.org/about/core/policies/core-change-policies/drupal-d... โ†’

    Added some review comments.

  • Pipeline finished with Failed
    5 months ago
    Total: 517s
    #215530
  • Pipeline finished with Canceled
    5 months ago
    Total: 215s
    #215552
  • Pipeline finished with Success
    5 months ago
    Total: 506s
    #215557
  • Pipeline finished with Failed
    5 months ago
    Total: 552s
    #216492
  • Pipeline finished with Failed
    5 months ago
    Total: 581s
    #216574
  • Pipeline finished with Failed
    5 months ago
    Total: 570s
    #219494
  • Pipeline finished with Success
    5 months ago
    Total: 479s
    #219500
  • Status changed to Needs review 5 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune

    Addressed feedback from #14

  • Status changed to Needs work 5 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

    I think as per #14, a deprecation message needs to be added for path in system.schema.yml before removing it completely.

  • Pipeline finished with Failed
    5 months ago
    Total: 695s
    #219930
  • Pipeline finished with Failed
    5 months ago
    Total: 508s
    #220074
  • Pipeline finished with Success
    4 months ago
    Total: 583s
    #220365
  • Pipeline finished with Failed
    4 months ago
    Total: 532s
    #220427
  • Pipeline finished with Failed
    4 months ago
    Total: 490s
    #220444
  • Pipeline finished with Canceled
    4 months ago
    Total: 414s
    #220463
  • Pipeline finished with Success
    4 months ago
    Total: 449s
    #220466
  • Status changed to Needs review 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia yash.rode pune
  • Status changed to RTBC 4 months ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

    Changes looks good to me.

  • Status changed to Needs work 4 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom catch

    One question on the MR.

Production build 0.71.5 2024