Contrib.social

Add validation constraints to contact.form.*

Open on Drupal.org →
Created on 8 May 2024, 8 months ago
Updated 24 July 2024, 5 months ago

Problem/Motivation

Contact form entity is not yet fully validatable:

.vendor/bin/drush config:inspect --filter-keys=contact.form.feedback --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                                                                       
 -------------------------------------------------- --------- ------------- ------ --------------------------------------------------------------------------------------------- 
  contact.form.feedback                              Correct   87%           ✅❓   ValidKeys: '<infer>'                                                                         
   contact.form.feedback:                            Correct   Validatable   ✅✅   ValidKeys: '<infer>'                                                                         
   contact.form.feedback:_core                       Correct   Validatable   ✅✅   ValidKeys:                                                                                   
                                                                                      - default_config_hash                                                                      
   contact.form.feedback:_core.default_config_hash   Correct   Validatable   ✅✅   NotNull: {  }                                                                                
                                                                                    Regex: '/^[a-zA-Z0-9\-_]+$/'                                                                 
                                                                                    Length: 43                                                                                   
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:dependencies                Correct   Validatable   ✅✅   ValidKeys: '<infer>'                                                                         
   contact.form.feedback:id                          Correct   Validatable   ✅✅   Length:                                                                                      
                                                                                      max: 32                                                                                    
                                                                                    ↣ PrimitiveType: {  }                                                                        
                                                                                    ↣ Regex:                                                                                     
                                                                                      pattern: '/^[a-z0-9_]+$/'                                                                  
                                                                                      message: 'The %value machine name is not valid.'                                           
   contact.form.feedback:label                       Correct   Validatable   ✅✅   Regex:                                                                                       
                                                                                      pattern: '/([^\PC])/u'                                                                     
                                                                                      match: false                                                                               
                                                                                      message: 'Labels are not allowed to span multiple lines or contain control characters.'    
                                                                                    NotBlank: {  }                                                                               
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:langcode                    Correct   Validatable   ✅✅   NotNull: {  }                                                                                
                                                                                    Choice:                                                                                      
                                                                                      callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'  
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:message                     Correct   Validatable   ✅✅   Regex:                                                                                       
                                                                                      pattern: '/([^\PC\x09\x0a\x0d])/u'                                                         
                                                                                      match: false                                                                               
                                                                                      message: 'Text is not allowed to contain control characters, only visible characters.'     
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:recipients                  Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints to config entity type: contact.form.*                   
   contact.form.feedback:recipients.0                Correct   Validatable   ✅✅   Email:                                                                                       
                                                                                      message: '%value is not a valid email address.'                                            
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:redirect                    Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints to config entity type: contact.form.*                   
   contact.form.feedback:reply                       Correct   Validatable   ✅✅   Regex:                                                                                       
                                                                                      pattern: '/([^\PC\x09\x0a\x0d])/u'                                                         
                                                                                      match: false                                                                               
                                                                                      message: 'Text is not allowed to contain control characters, only visible characters.'     
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:status                      Correct   Validatable   ✅✅   ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:uuid                        Correct   Validatable   ✅✅   Uuid: {  }                                                                                   
                                                                                    ↣ PrimitiveType: {  }                                                                        
   contact.form.feedback:weight                      Correct   Validatable   ✅✅   Range:                                                                                       
                                                                                      min: -2147483648                                                                           
                                                                                      max: 2147483647                                                                            
                                                                                    FullyValidatable: null                                                                       
                                                                                    ↣ PrimitiveType: {  }                                                                        
 -------------------------------------------------- --------- ------------- ------ ---------------------------------------------------------------------------------------------

Steps to reproduce

  1. Get a local git clone of Drupal core 11.x.
  2. composer require drupal/config_inspector — or manually install https://www.drupal.org/project/config_inspector/releases/2.1.5 → or newer (which supports Drupal 11!)
  3. composer require drush/drush
  4. vendor/bin/drush config:inspect --filter-keys=contact.form.feedback --detail --list-constraints

Proposed resolution

Add validation constraints to missing properties.

This requires looking at the existing code and admin UI (if any) to understand which values could be considered valid. Eventually this needs to be reviewed by the relevant subsystem maintainer.

For examples, search *.schema.yml files for the string constraints: 😊

Reach out to @borisson_ or @wimleers in the #distributions-and-recipes.

Remaining tasks

MR

User interface changes

None.

API changes

None.

Data model changes

More validation 🚀

Release notes snippet

None.

📌 Task
Status

Needs work

Version

11.0 🔥

Component
Contact  →

Last updated about 1 month ago

Created by

🇮🇳India narendraR Jaipur, India

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

Merge Requests

Comments & Activities

  • Issue created by @narendraR
  • Status changed to Needs work 8 months ago
  • Comment 8 months ago →
    🇮🇳India narendraR Jaipur, India

    😬 Forgot to change status while cloning

  • Merge request !7962constraint added → (Open) created by narendraR
  • Pipeline finished with Failed
    8 months ago
    Total: 705s
    #167399
  • Comment 8 months ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #2: Hah! :D

    This is a good start, but there's a bunch left to do.

  • First commit to issue fork.
  • Pipeline finished with Failed
    8 months ago
    Total: 518s
    #168155
  • Comment 8 months ago →
    🇫🇷France andypost

    I see no reason to allow weight to be nullable

  • Pipeline finished with Failed
    8 months ago
    Total: 544s
    #171419
  • Pipeline finished with Failed
    8 months ago
    Total: 582s
    #171558
  • Pipeline finished with Failed
    8 months ago
    #171677
  • Pipeline finished with Failed
    8 months ago
    Total: 603s
    #172161
  • Pipeline finished with Success
    8 months ago
    Total: 697s
    #172367
  • Pipeline finished with Failed
    8 months ago
    Total: 700s
    #172647
  • Pipeline finished with Canceled
    8 months ago
    Total: 8s
    #173028
  • Pipeline finished with Success
    8 months ago
    Total: 574s
    #173029
  • Status changed to Needs review 8 months ago
  • Comment 8 months ago →
    🇮🇳India narendraR Jaipur, India
  • Status changed to Needs work 8 months ago
  • Comment 8 months ago →
    🇫🇷France andypost

    Left review on MR - we can add proper types with deprecation message as 11.x going beta

  • Pipeline finished with Canceled
    7 months ago
    Total: 77s
    #176820
  • Pipeline finished with Success
    7 months ago
    Total: 577s
    #176822
  • Pipeline finished with Failed
    7 months ago
    Total: 607s
    #176844
  • Comment 7 months ago →
    🇮🇳India narendraR Jaipur, India

    I tried to add deprecation message in previous commit, not sure if this is the correct way or not. But due to this change tests started failing. https://git.drupalcode.org/issue/drupal-3445976/-/jobs/1632410#L2068

  • Pipeline finished with Failed
    7 months ago
    Total: 652s
    #178386
  • Pipeline finished with Success
    7 months ago
    Total: 2539s
    #178955
  • Status changed to Needs review 7 months ago
  • Comment 7 months ago →
    🇮🇳India narendraR Jaipur, India
  • Status changed to RTBC 7 months ago
  • Comment 7 months ago →
    🇮🇳India srishtiiee

    The MR looks good, I think it just needs a CR for the deprecation. RTBC'd otherwise 👍🏼

  • Status changed to Needs work 7 months ago
  • Comment 7 months ago →
    🇮🇳India narendraR Jaipur, India

    Moved to NW for fixing deprecation errors in better way

  • Comment 7 months ago →
    🇮🇳India narendraR Jaipur, India

    I attempted to remove the @legacy from the tests, which I added in previous commit, but it seems something was done incorrectly in the MR, resulting in deprecation errors. I would appreciate any assistance.

  • First commit to issue fork.
  • Pipeline finished with Success
    7 months ago
    Total: 538s
    #185128
  • Pipeline finished with Failed
    7 months ago
    Total: 780s
    #185749
  • Pipeline finished with Success
    7 months ago
    Total: 670s
    #185793
  • Status changed to Needs review 7 months ago
  • Comment 7 months ago →
    🇮🇳India narendraR Jaipur, India
  • Status changed to Needs work 7 months ago
  • Comment 7 months ago →
    🇺🇸United States phenaproxima Massachusetts

    I noticed a few more things but this is looking very close to ready. It does still need a change record.

  • Pipeline finished with Canceled
    7 months ago
    Total: 139s
    #189487
  • Pipeline finished with Failed
    7 months ago
    Total: 613s
    #189488
  • Pipeline finished with Success
    7 months ago
    Total: 537s
    #191695
  • Status changed to Needs review 7 months ago
  • Comment 7 months ago →
    🇮🇳India narendraR Jaipur, India
  • Comment 7 months ago →
    🇺🇸United States mtift Minnesota, USA

    This one makes sense to me. All of the reply & redirect issues have been addressed per https://www.drupal.org/node/3452650 → . The deprecation messages and tests make sense. The feedback has been addressed.

    However, I applied the MR on a standard install for 11.x with config inspector and got all green checks, but it shows "1 errors" and I'm not quite sure what that is referring to:

  • Comment 7 months ago →
    🇺🇸United States phenaproxima Massachusetts

    I know nothing about config inspector, but I assume that it's flagging an error in the data in that config object, not any problem with the validation constraints themselves.

  • Status changed to RTBC 7 months ago
  • Comment 7 months ago →
    🇺🇸United States smustgrave

    Regarding #18 did you run updb? When I applied the MR, before running updb, I saw the error in configuration inspector.

    On a standard 11.x profile install
    Installed configuration inspector
    Applied the MR
    Ran drush updb

    Reviewing the code thanks @phenaproxima for pointing out scope issue.

    Searched for .feedback.yml files and all appear to be updated.

    Believe this one is good to go.

  • Comment 6 months ago →
    🇺🇸United States mtift Minnesota, USA

    That was it. Thank you @smustgrave!

    FWIW, this looks good to me, too.

  • Status changed to Needs work 6 months ago
  • Comment 6 months ago →
    🇬🇧United Kingdom alexpott 🇪🇺🌍

    This MR no prevents me from removing the message from a contact form. If I installed standard with this MR applied and go to admin/structure/contact/manage/feedback and remove all the text in the message field and press save the empty message field is not saved. If I remove the MR and a tried again I can empty the field and saves as expected.

  • Pipeline finished with Success
    6 months ago
    Total: 519s
    #213816
  • Pipeline finished with Success
    6 months ago
    Total: 629s
    #214632
  • Status changed to Needs review 6 months ago
  • Comment 6 months ago →
    🇮🇳India narendraR Jaipur, India

    Re #22, I updated the code to set values to NULL.
    Reverting changes related to message(message: '') will fail ContactFormValidationTest.

  • Pipeline finished with Success
    6 months ago
    Total: 514s
    #214688
  • Status changed to Needs work 5 months ago
  • Comment 5 months ago →
    needs-review-queue-bot

    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.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024