Problem/Motivation

Action entity is not yet validatable:

vendor/bin/drush config:inspect --filter-keys=system.action.user_cancel_user_action --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.action.user_cancel_user_action                              Correct   79%           ✅❓   ValidKeys: '<infer>'                                                                                
   system.action.user_cancel_user_action:                            Correct   Validatable   ✅✅   ValidKeys: '<infer>'                                                                                
   system.action.user_cancel_user_action:_core                       Correct   Validatable   ✅✅   ValidKeys:                                                                                          
                                                                                                      - default_config_hash                                                                             
   system.action.user_cancel_user_action:_core.default_config_hash   Correct   Validatable   ✅✅   NotNull: {  }                                                                                       
                                                                                                    Regex: '/^[a-zA-Z0-9\-_]+$/'                                                                        
                                                                                                    Length: 43                                                                                          
                                                                                                    ↣ PrimitiveType: {  }                                                                               
   system.action.user_cancel_user_action:configuration               Correct   NOT           ✅❓   ❌ @todo Add validation constraints to ancestor type: action.configuration.user_cancel_user_action  
   system.action.user_cancel_user_action:dependencies                Correct   Validatable   ✅✅   ValidKeys: '<infer>'                                                                                
   system.action.user_cancel_user_action:dependencies.module         Correct   NOT           ✅❓   ❌ @todo Add validation constraints to ancestor type: config_dependencies                           
   system.action.user_cancel_user_action:dependencies.module.0       Correct   Validatable   ✅✅   NotBlank: {  }                                                                                      
                                                                                                    ExtensionName: {  }                                                                                 
                                                                                                    ExtensionExists: module                                                                             
                                                                                                    ↣ PrimitiveType: {  }                                                                               
   system.action.user_cancel_user_action:id                          Correct   Validatable   ✅✅   Regex:                                                                                              
                                                                                                      pattern: '/^[a-z0-9_\.]+$/'                                                                       
                                                                                                      message: 'The %value machine name is not valid.'                                                  
                                                                                                    ↣ PrimitiveType: {  }                                                                               
                                                                                                    ↣ Length:                                                                                           
                                                                                                      max: 166                                                                                          
   system.action.user_cancel_user_action:label                       Correct   Validatable   ✅✅   Regex:                                                                                              
                                                                                                      pattern: '/([^\PC])/u'                                                                            
                                                                                                      match: false                                                                                      
                                                                                                      message: 'Labels are not allowed to span multiple lines or contain control characters.'           
                                                                                                    NotBlank: {  }                                                                                      
                                                                                                    ↣ PrimitiveType: {  }                                                                               
   system.action.user_cancel_user_action:langcode                    Correct   Validatable   ✅✅   NotNull: {  }                                                                                       
                                                                                                    Choice:                                                                                             
                                                                                                      callback: 'Drupal\Core\TypedData\Plugin\DataType\LanguageReference::getAllValidLangcodes'         
                                                                                                    ↣ PrimitiveType: {  }                                                                               
   system.action.user_cancel_user_action:plugin                      Correct   Validatable   ✅✅   PluginExists:                                                                                       
                                                                                                      manager: plugin.manager.action                                                                    
                                                                                                      interface: Drupal\Core\Action\ActionInterface                                                     
                                                                                                    ↣ PrimitiveType: {  }                                                                               
   system.action.user_cancel_user_action:status                      Correct   Validatable   ✅✅   ↣ PrimitiveType: {  }                                                                               
   system.action.user_cancel_user_action:type                        Correct   NOT           ✅❓   ⚠️  @todo Add validation constraints to config entity type: system.action.*                         
   system.action.user_cancel_user_action:uuid                        Correct   Validatable   ✅✅   Uuid: {  }                                                                                          
                                                                                                    ↣ 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=system.action.user_cancel_user_action --detail --list-constraints

Proposed resolution

Add validation constraints to:

  1. system.action.*:type

Many tests creates actions by using only id and plugin.
Eg.

    $action = Action::create([
      'id' => 'entity_save_action',
      'plugin' => 'entity:save_action:entity_test_mul_changed',
    ]);
    $action->save();

or

    $action = Action::create([
      'id' => 'entity_delete_action',
      'plugin' => 'entity:delete_action:entity_test_mulrevpub',
    ]);
    $action->save();

These tests starts failing if we don't make type: nullable.

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

User interface changes

None.

API changes

None.

Data model changes

More validation 🚀

Release notes snippet

None.

Issue fork drupal-3449259

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:

Comments

narendraR created an issue. See original summary.

wim leers’s picture

ActionValidationTest needs to be updated now that it is fully validatable.

narendrar’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

This one seems pretty straight forward.

I ran the test-only feature but everything passes but as a task am making the assumption the testing was previous just missing.

Will mark.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs issue summary update

It's not clear to me why type can be nullable, and this looks like a @todo in the issue summary. I checked a random site that has been through various updates and with some not exactly consistent contrib modules on it since early Drupal 8 days, and all the system.action config entities had type: set to something. This doesn't mean it absolutely shouldn't be nullable but it should be clear why (and maybe a comment in the YAML too if so).

narendrar’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Maybe a follow up to make it required?

Feedback here seems to be addressed though.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

I've added a comment to the MR - I think this change introduces some questions for \Drupal\system\ActionConfigEntityInterface::getType()

narendrar’s picture

Status: Needs work » Needs review

Feedback addressed

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

We now have specific test coverage for a type that is null. This looks good to me.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 9ddbc2c and pushed to 11.x. Thanks!

  • alexpott committed 9ddbc2c7 on 11.x
    Issue #3449259 by narendraR, alexpott, catch, Wim Leers: Add validation...

Status: Fixed » Closed (fixed)

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