Add validation constraints to system.site

This all looks correct to me. Config inspector is reporting this at 100% now. Is the reason this is back to Needs work because of the pipelines failing?

So applied the MR locally.

Update hook ran just fine

I'm still able to update the settings without issue

The proposed schema changes believe make sense.

+1 from me.

I read the IS and the MR. There are new methods in the MR that should be documented.

I've added docblock info for those new methods. Ready for review.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. 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.

Fixed coding standards and rebased to see if the validation checker can resolve updated dependencies.

Left 1 question on the MR.

Feedback addressed.

Believe feedback has been addressed.

I've added some comments to the MR and 1 that I think should have a follow up created for.

Followup created: 📌 Add PHP_INT_MAX to integer schema configuration Active

Test https://git.drupalcode.org/issue/drupal-3443432/-/jobs/2012360#L1253 was failing due to change in https://git.drupalcode.org/project/drupal/-/merge_requests/7723/diffs?commit_id=bc72ebbe5abb48bc994fc4df237eda7c30f37b51#01036d2820f2e116fab98a13152a9294069e8f55_116_114, hence reverted.

Looks good. Just have one minor doubt.

removing the UUID constraint feels like a step in the wrong direction. NULL is never really a valid value for site UUID.

Thanks, @narendraR; that makes sense.

However, I agree with @alexpott that NULL is never valid for the site UUID, and so it shouldn't be nullable.

I think this issue points to us needing a new constraint which is something like NotBlankAfterInstall - so a very can be blank during site install but once the site is up and running it can not be. This is true for uuid, name and email.

I also think that none of these should be nullable. PHP 8 makes swapping between NULLs and strings hard.

@alexpott, I tried to create a new constraint as suggested in #23. But I don't understand how to bypass existingNotNullConstraint for UUID. https://git.drupalcode.org/project/drupal/-/merge_requests/7723/diffs?commit_id=4fcb5099b2b23f22643948c901321db04b89b0db#4d658554b6066a85600dfbe03dc34c3003f8a3e1_0_32

I don't understand having to bypass NotNullConstraint? I think we have to reimplement it in here?

We should probably start with a reroll here.

Rebased thsi issue.

@alexpott

> I think this issue points to us needing a new constraint which is something like NotBlankAfterInstall - so a very can be blank during site install but once the site is up and running it can not be. This is true for uuid, name and email.

You say this, which is kinda the same problem as 📌 Add validation constraints to contact.form.* Needs work comment #32. There it is a recipe which wants to use mail to replace a value, which is doesn't get since mail is not yet in config when installing from a recipe with the drupal command.

So there is also some sort of 'during install' issue, where during an install is appearantly possible to have invalid config.

Perhaps we even need to fix it ealier? Although i don't really feal like ignoring validation for config while installing is a good idea. Since it is pretty valuable, what if you are to install some sort of Drupal implementation (like a decoupled Drupal) and it comes with invalid config, would we want to ignore that?