Clean up email validation constraint definition

Ready for review

Nick,

Sorry, it's not clear to me why you are removing the Email constraint definition from the core/lib/Drupal/Core/Validation/ConstraintManager.php.

Also, can you please provide steps or scenarios for the validation process?

Thanks,
Carol

It is 100% a duplication of data - I strongly feel a plugin definition should only exist in one spot *either*the PHP standalone file (necessary in this case, since it is setting default options for the Symfony Validator) or the ConstraintManager itself.
Scenario is any config which needs to be an email. Ex:

spambot.settings:
  type: config_object
  label: Settings
  mapping:
    spambot_whitelist_email_list:
      label: 'Allow list of emails'
      type: sequence
      orderby: value
      sequence:
        type: email
        constraints:
          NotBlank: {}
          Email: {}
[snip]

Seems removing it though caused some failures.

Looks like it doesn't like single quotes there... confirming.

wrong file and didn't mark it needs review

Seems like a simple feature.

Not even a feature, just clean-up!

The last submitted patch, 7: 3365449-email-constaint-cleanup-7.patch, failed testing. View results →

This was a random fail, back to rtbc.

The last submitted patch, 7: 3365449-email-constaint-cleanup-7.patch, failed testing. View results →

Back to rtbc, the same random fail that was not introduced in this issue.

The patch needed a reroll.

--- a/core/lib/Drupal/Core/Validation/ConstraintManager.php
+++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php

@@ -105,11 +104,6 @@ public function registerDefinitions() {
-      'type' => ['string'],

@@ -5,13 +5,14 @@
+ *   type = { "email" }

I'm wondering if this could break some existing code if someone is applying the constraint on a string type. It might be easier if we open a follow-up for this change, and scope this issue for removing the duplicate definition.

Made the change requested in #15.

Update looks good to me.

+++ b/core/lib/Drupal/Core/Validation/ConstraintManager.php
@@ -105,11 +104,6 @@ public function registerDefinitions() {
-      'type' => ['string'],

Couldn't we keep the type: string still in the definition?

@lauriii not sure would that require label and class?

To add back string

Have reverted changes in "core/lib/Drupal/Core/Validation/ConstraintManager.php" file to restore 'type' key per #18 📌 Clean up email validation constraint definition Needs work .
Of the 3 keys to set constraint definition label seemed optional, but left it as is for consistency and ease of understanding.

Kindly review and suggest.

Think the issue summary needs to be updated and agreed upon.

#15 talks about a follow up.

Then to remove the duplicate definition.
Then added back

So now all we are doing is updating the comment? Is that desired?

Added string back, this is not compared to #21.

Back to needs review to kick off drupalci

Triggered drupalci for you, only moving to NW about the issue summary update.