Add validation constraints to contact.form.*

😬 Forgot to change status while cloning

#2: Hah! :D

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

I see no reason to allow weight to be nullable

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

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

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

Moved to NW for fixing deprecation errors in better way

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.

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

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:

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.

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.

That was it. Thank you @smustgrave!

FWIW, this looks good to me, too.

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.

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

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.

I missed this issue when it was originally active, we should start by rerolling this.
Next we should look at https://git.drupalcode.org/project/drupal/-/merge_requests/7962#note_333039, to figure out how the schema should look.

Merged 11.x into this, lets see what happens.

Well, 2 test failures:

1. core/recipes/feedback_contact_form/recipe.yml -> The receipient input is not set when installing in core/tests/Drupal/Tests/Core/Recipe/RecipeQuickStartTest.php, so it fails since ${resipient} is not a vaild email adress.
2. core/tests/Drupal/FunctionalTests/Core/Recipe/StandardRecipeInstallTest.phphas the same issue, no recipient set.

Opened 📌 #3303126 was fixed, but a todo was not removed. Active since that should fix one of the tests. The references issue in the failing test tells us it was fixed and some code could be remove.d.

Some more digging into the feedback form recipe.

It seems the actions are applied, BUT InputConfigurator::collectAll is never called, so while installing it seems like it doesn't really collect anything from config, so the placeholder ${recipient} is never replaced, therefor we get install errors.

FOund when debuging the failing test, stepping through RecipeRunner and seeing the $config_action_manager->applyAction($action_id, $config_name, static::replaceInputValues($data, $replace)); didnt really have any input values.

So basically this part of therecipe:

input:
  recipient:
    data_type: email
    description: 'The email address that should receive submissions from the feedback form.'
    constraints:
      NotBlank: []
    prompt:
      method: ask
      arguments:
        question: 'What email address should receive website feedback?'
    form:
      '#type': email
      '#title': 'Feedback form email address'
    default:
      source: config
      config: ['system.site', 'mail']

Is never looking up the default value, therefor we enter a invalid value when the config is installed.

Some discussion on the issue with recipes here: https://drupal.slack.com/archives/C2THUBAVA/p1743182816947909

In order to load default values for recipes when quickstart is used we need to make sure default values that use config are actually found. We do this using a lookup array for the config path to $install_state.

Unfortunately the default email address for an install needed changing also since drupal@localhost is not a valid email address. I changes this to @drupal.local

Still failting on the standard install. Which has pretty much the. same problem, but unfortunately, does not configure system.site.mail at all.
In the test:

    // Standard expects to set the contact form's recipient email to the
    // system's email address, but our feedback_contact_form recipe hard-codes
    // it to another value.
    // @todo This can be removed after https://drupal.org/i/3303126, which
    //   should make it possible for a recipe to reuse an already-set config
    //   value.
    ContactForm::load('feedback')?->setRecipients(['simpletest@example.com'])
      ->save();

Which i alerady opened an issue for earlier. 📌 #3303126 was fixed, but a todo was not removed. Active . So guessing this might need to use another angle. Not sure what.

Somewhat related: 📌 Add validation constraints to system.site Needs work

Wonder, maybe 📌 Add NotBlankAfterInstall constraint and use it for system.date:timezone.default Active is relevant for this.