Submitting empty block layout form results in breakage for all block entities

Re-rolled for 9.3

fixed the fails.

Verified the issue when deleting all blocks you can't save the form.

Though I did not see the blocks from other themes go away.

This will 100% need a test case but it should be super simple

Adding a test case

Patch #21 Tested and Applied successfully on Drupal 9.5.x.

Steps:
1. Install Drupal
2. Enable the "Stark" theme, setting it as default.
3. At Admin > Structure > Block layout (admin/structure/block), delete all blocks for the Stark theme. After deleting the blocks, click the "Save blocks" button.
4. Getting the PHP type Error.
5. Apply the patch
6. Click the "save blocks button" its redirect blocks listing page.

Not sure why this was moved to 9.5 in #25. Note that the patch in #21 applies (with fuzz) to 10.0.x and 10.1.x.

kristens-mbp:drupal10 kristenpol$ patch -p1 < 2779321-21.patch 
patching file core/modules/block/src/BlockListBuilder.php
patching file core/modules/block/tests/src/Functional/BlockUiTest.php
Hunk #1 succeeded at 107 with fuzz 2 (offset 3 lines).

+++ b/core/modules/block/src/BlockListBuilder.php
@@ -379,13 +379,18 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
+    // Passing an empty value to ::loadMultiple would load all items from the
+    // storage.

Nitpick: I would make this comment one line and not note the method (loadMultiple) in it.

Addressed the comment #28
Attached patch against Drupal 10.1.x

Reproduced problem according to steps given in problem statement. Tested on 10.1 referring to #26 #27 #28 #29.
Getting error while saving changes to blocks in stark theme(attached screenshot)
After installing #29 patch error which was generating while saving changes resolved.
Also the breakdown of all other themes block also resolved and looks good with 10.1
Moving this issue to RTBC.
Attached screenshot for reference

I think we should fix this slightly differently. If we press save and there is nothing to do I think we should tell the user. With the current fix we're going to tell the user 'The block settings have been updated.' when we've done nothing. Is that correct? I think we could consider changing the ::validateForm() method to tell the user it's not going to do anything. Something like this:

    if (empty($form_state->getValue('blocks'))) {
      $form_state->setErrorByName('blocks', 'No blocks settings to update.');
    }

Adding the patch by addressing the comment #31

Address #31

Updated patch and address #31 fixed code of #30.
Fixing CCF #33 Needs Review

Patch #34 Tested and Applied successfully on Drupal 10.1.x.

Tested #34 and it worked.

See that feedback from #31 has been addressed also.

Committed/pushed to 10.1.x, cherry-picked to 10.0.x and 9.5.x.

Note I didn't add commit credit for duplicated manual testing, once is plenty.

After-the-commit review, coming here because this requires a reroll of #2910353: Prevent saving config entities when configuration overrides are applied

The committed patch is a little bit strange, because I don't think the submitForm() changes are necessary now that the validation is in place, because you can't end up there with an empty block list?

Additionally, loadMultiple() only returns all entities if you explicitly pass NULL. An empty array is a no-op and returns nothing. I suppose the form value really is null. FWIW, in PHP 8, array_keys(NULL) is actually a type error, so in Drupal 10, the worst that could happen there is a fatal error and not breaking all your config (yay stricter type handling).

I don't think it's worth reopening this to revert the submitForm() changes, but happy to do so if others/catch think that makes sense.

That's a good point with the submitForm changes. Here's a patch that just reverts that hunk, if that passes I may commit as-is since it's just an unnecessary hunk that we otherwise could have removed from the original patch on commit.

+++ b/core/modules/block/src/BlockListBuilder.php
@@ -372,20 +372,27 @@ public function getDefaultOperations(EntityInterface $entity) {
+    if (empty($form_state->getValue('blocks'))) {
+      $form_state->setErrorByName('blocks', 'No blocks settings to update.');
+    }

We also need a follow-up here to wrap the UI text in $this->t().

Plus we could add testing that this message appears.

Failed but that test looks unrelated?

Setting to needs work for #42. The test comment should then also be updated to that you actually can not save?

This is tipping into follow-up issue territory now, but here's those changes.

Looks good to me.

Let's do a quick commit here and forget it ever happened :)

Committed and pushed b5b2951668 to 10.1.x and 60ce4e1407 to 10.0.x and 9b61f274e7 to 9.5.x. Thanks!