Contrib.social

[PP-1] Override Symfony's Choice Constraint

Open on Drupal.org →
Created on 9 May 2024, about 1 year ago
Updated 3 July 2024, 12 months ago

Problem/Motivation

In order to add validation constraints to all user role configuration 📌 Add validation constraints to user.role.* Needs review , it was necessary to add a new public static function (getAllValidPermissions() it was necessary to add a convenience callback to be used as a choice validator that only called one method on a Drupal service:

return array_keys(\Drupal::service('user.permissions')->getPermissions());

This isn't the only place where will need to create additional functions that only return valid choices. If we want to make all configuration in Drupal fully validatable then there are many places where we need to do something similar.

For instance, the same thing was done in /core/modules/editor/src/Entity/Editor.php:

 /**
  * Computes all valid choices for the "image_upload.scheme" setting.
  *
  * @see editor.schema.yml
  *
  * @return string[]
  *   All valid choices.
  *
  * @internal
  */
 public static function getValidStreamWrappers(): array {
   return array_keys(\Drupal::service('stream_wrapper_manager')->getNames(StreamWrapperInterface::WRITE_VISIBLE));
 }

We don't have many other examples at this point, but there is still of a lot of configuration to validate and there may be more places where we need to add these functions that might not be useful for anything other than constraint validation.

Proposed resolution

As discussed with @xjm, @wim-leers, @carsoncho, and @jcorrao, we thought it might be useful to override Symfony's Choice Constraint doing something like this pseudo code:


namespace Drupal\Core\Validation\Plugin\Validation\Constraint;

use Drupal\Core\StringTranslation\TranslatableMarkup;
use Symfony\Component\Validator\Constraints\Choice;

#[Constraint(
  id: 'Choice',
  label: new TranslatableMarkup('Choice', [], ['context' => 'Validation'])
)]
class ChoiceConstraint extends Choice {

  public $callbackArgs = [];

}

and extend the ChoiceValidator:


namespace Drupal\Core\Validation\Plugin\Validation\Constraint;

use Symfony\Component\Validator\Constraints\ChoiceValidator;

/**
 * Validates complex data.
 */
class ChoiceConstraintValidator extends ChoiceValidator {

  /**
   * {@inheritdoc}
   */
  public function validate(mixed $value, Constraint $constraint) {
    assert($constraint instanceof ChoiceConstraint);
    if ($constraint->callbackArgs) {
      $callback = function() {
        // @todo;
      };
    }
    $constraint->callback = $callback;
    parent::validate($value, $constraint);
  }

}

Remaining tasks

Decide if this would be useful.

User interface changes

None.

API changes

New Choice validator that would allow us to do something like this is /core/modules/user/config/schema/user.schema.yml:

user.role.*:
  ...
  mapping:
    ...
    permissions:
      sequence:
        type: string
        label: 'Permission'
+         constraints:
+           Choice:
+             callback: user.permissions:getPermissions
+             transform: array_keys
📌 Task
Status

Postponed

Version

11.0 🔥

Component
Configuration entity  →

Last updated 3 days ago

Created by

🇺🇸United States mtift Minnesota, USA

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

Merge Requests

Comments & Activities

  • Issue created by @mtift
  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA
  • Comment about 1 year ago →
    🇬🇧United Kingdom longwave UK

    This does seem like it would be useful but should this replace the Symfony constraint or just be a new Drupal specific one that extends Choice?

  • Comment about 1 year ago →
    🇺🇸United States carsoncho Kansas City, MO

    [#3] The idea would be to extend Symfony's Constraint rather than copy and duplicate. Extending it would then allow for us to have the generic functionality and would be BC.

  • Merge request !8016Issue #3446364: Override Symfony's Choice Constraint → (Open) created by mtift
  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA
  • Pipeline finished with Failed
    about 1 year ago
    Total: 190s
    #169350
  • Comment about 1 year ago →
    🇮🇳India narendraR Jaipur, India
  • Pipeline finished with Failed
    about 1 year ago
    Total: 201s
    #182762
  • Issue was unassigned.
  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA
  • Pipeline finished with Failed
    about 1 year ago
    Total: 218s
    #182822
  • Pipeline finished with Failed
    about 1 year ago
    Total: 225s
    #183659
  • Pipeline finished with Failed
    about 1 year ago
    Total: 189s
    #184497
  • Pipeline finished with Failed
    about 1 year ago
    Total: 554s
    #185068
  • Pipeline finished with Failed
    about 1 year ago
    Total: 206s
    #189381
  • Pipeline finished with Failed
    about 1 year ago
    Total: 280s
    #189920
  • Pipeline finished with Failed
    about 1 year ago
    Total: 170s
    #194362
  • Pipeline finished with Failed
    about 1 year ago
    Total: 172s
    #194365
  • Pipeline finished with Canceled
    about 1 year ago
    Total: 430s
    #194370
  • Pipeline finished with Failed
    about 1 year ago
    Total: 208s
    #194376
  • Pipeline finished with Failed
    about 1 year ago
    #194379
  • Pipeline finished with Failed
    about 1 year ago
    Total: 583s
    #194410
  • Pipeline finished with Failed
    about 1 year ago
    Total: 645s
    #194539
  • Pipeline finished with Failed
    about 1 year ago
    Total: 637s
    #194551
  • Pipeline finished with Canceled
    about 1 year ago
    Total: 476s
    #194794
  • Pipeline finished with Failed
    about 1 year ago
    Total: 168s
    #194801
  • Pipeline finished with Failed
    about 1 year ago
    Total: 694s
    #194805
  • Pipeline finished with Failed
    about 1 year ago
    Total: 606s
    #194813
  • Pipeline finished with Success
    about 1 year ago
    Total: 690s
    #194924
  • Status changed to Needs review about 1 year ago
  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA
  • Pipeline finished with Failed
    about 1 year ago
    Total: 567s
    #194949
  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA
  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    🇺🇸United States smustgrave

    Re-ran the failing test and it was random.

    Looking at the change though I imagine it would need a change record for contrib/custom modules to be able to leverage it. Tagging for that.

  • Status changed to Needs review about 1 year ago
  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA

    Good call. I added the change record.

  • Comment about 1 year ago →
    🇺🇸United States mtift Minnesota, USA
  • Status changed to RTBC 12 months ago
  • Comment 12 months ago →
    🇧🇪Belgium borisson_ Mechelen, 🇧🇪

    I think this looks great, this makes it easier for other issues to land. Marking as rtbc.

  • First commit to issue fork.
  • Comment 12 months ago →
    🇬🇧United Kingdom alexpott 🇪🇺🌍

    Oh I thought we had landed 📌 Allow parsing and writing PHP constants and enums in YAML files Needs work ... we really should get that done.

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

    We can update the user role constraint to use this.

  • Status changed to Postponed 12 months ago
  • Comment 12 months ago →
    🇮🇳India narendraR Jaipur, India
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024