Drupal 10.1 introduces naming change that affects content block restrictions

Looks like these failing tests are due to the name change in Drupal 10.1 from "Custom block" to "Content block." The attached patch is a partial fix, changing the naming on most of the test logic.

However, there may be a remaining functional change that needs to be made, which is why some tests are still failing. Changing priority to "Major".

Also: it appears that this will likely be B/C breaking for Drupal 10.0... , meaning, after this change, sites running < 10.1 can't update safely, so we'll need to create a new minor version release for 10.1+

The attached patch -- not for merging -- provides a demonstration of the naming changes needed to make this work with Drupal 10.1. However, it would not be backwards compatible with Drupal 9.x or 10.0.x. Therefore, we should explore whether we can "rewrite" the "Content block" label in a more targeted way to allow this to be compatible with prior versions.

The attached patch provides an affordance to retain the existing "Custom blocks" labelling that was present prior to Drupal 10.1. This will allow sites running on Drupal 9.5.x, 10.0.x, or 10.1.x to use this.

One consequence of this, however, is that the test logic will differ between Drupal 10.1 and Drupal 10.0, so we will not expect tests to pass anymore on Drupal 10.0 or lower.

The second attached patch omits the changes to the test logic in order to demonstrate that the business logic change will still work with Drupal 10.0 or lower.

Would it not be possible to try to check both of these labels though? And then change the logic conditionally, and pass the tests conditionally? Disclaimer: I did not actually look through the patches, just my thought that it seems like it should be possible? And if not: why not?

Hi Eirik! I appreciate a co-maintainer weighing in on this. Yes, you're absolutely right that it is possible to check for both the labels "Custom blocks" and "Content block" and group them in the same restrictions category, thereby making a single release of this module compatible with 9.5.x / 10.0.x / 10.1.x.

So, in effect, this is not a backwards-compatibility-breaking change as I suspected in a previous comment.

That's where the patch in #8 ended up. What I'm not sure about is whether the test logic can also be done conditional on the Drupal version (or if it's even worth the effort to make it conditional). Thoughts on that?

Thanks again!

I think that would be possible indeed. I see the patch is compatible indeed, but the test not.

So something like

$candidate_found = false;
Foreach $label_candidates
//Check for element
If (!$candidate_found)
Throw exception

Sorry for the pseudo non usable code. On vacation and I am writing on my phone ✌️🤓

Foreach $label_candidates

Yeah, I think that would work, but there are so many references in the test suite that I think a change like that would make the test code significantly more difficult to maintain. I was picturing a helper method in the test suite that checked the Drupal core version and switched the expectations. Something like

Before:

    $element = $page->find('xpath', '//*[@id="edit-layout-builder-restrictions-allowed-blocks-custom-blocks-restriction-all"]');
    $assert_session->checkboxChecked('edit-layout-builder-restrictions-allowed-blocks-custom-blocks-restriction-all');
    $assert_session->checkboxNotChecked('edit-layout-builder-restrictions-allowed-blocks-custom-blocks-restriction-allowlisted');
    $element = $page->find('xpath', '//*[@id="edit-layout-builder-restrictions-allowed-blocks-custom-blocks-restriction-allowlisted"]');

After:

    $element = $page->find('xpath', '//*[@id="edit-layout-builder-restrictions-allowed-blocks-' . $block_target . '-restriction-all"]');
    $assert_session->checkboxChecked('edit-layout-builder-restrictions-allowed-blocks-' . $block_target . '-restriction-all');
    $assert_session->checkboxNotChecked('edit-layout-builder-restrictions-allowed-blocks-' . $block_target . '-restriction-allowlisted');
    $element = $page->find('xpath', '//*[@id="edit-layout-builder-restrictions-allowed-blocks-' . $block_target . '-restriction-allowlisted"]');

Given Drupal 9's EOL in November, I'm just not sure if it's worth the effort...

That's a good point. Priority one would have to be to actually support this is both versions which the patch already does. But maybe we can open an issue about running the tests on d9 at least? I will do that and link it here

Otherwise looked through the patch which makes sense and maintain backwards compatibility ✌️

Agreed that we should make this change and follow-up with test compatibility for < 10.1.

Thanks, Eirik, and have a good rest of the vacation!

Automatically closed - issue fixed for 2 weeks with no activity.