Improve access logic for Navigation blocks page

MR created

There was 1 failure:
1) Drupal\Tests\navigation\FunctionalJavascript\NavigationBlockUiTest::testNavigationBlockAdminUiPage
Invalid permission configure navigation layout.
/builds/issue/drupal-3447881/core/modules/user/tests/src/Traits/UserCreationTrait.php:298
/builds/issue/drupal-3447881/core/modules/user/tests/src/Traits/UserCreationTrait.php:253
/builds/issue/drupal-3447881/core/modules/user/tests/src/Traits/UserCreationTrait.php:156
/builds/issue/drupal-3447881/core/modules/navigation/tests/src/FunctionalJavascript/NavigationBlockUiTest.php:61
FAILURES!
Tests: 1, Assertions: 3, Failures: 1.

Shows the test coverage.

I believe since this is experimental changing permissions should be fine without needing any kind of upgrade

Manually testing on 11.x the permission is working as advertised.

Believe this to be good.

Because navigation is beta stability, this does need an upgrade path, although it's only been in a beta release of 10.3, not any stable releases yet, so I have double checked with the other release managers - not sure this specific situation has come up before. Upgrade paths are 100% needed when beta stability modules are in release candidates or stable releases at least, and it probably applies here too.

Thank you for the heads-up. I was not completely sure whether this change might require an upgrade path or not.

Added post_update hook that assigns configure navigation layout permission to roles with configure any layout permission. Those were the roles with access to modify the navigation layout, so the change will be transparent for them.

I did not add any replacement for administer navigation_block permission, given that it was not being used anywhere after the LB refactor.

I did not add any replacement for administer navigation_block permission, given that it was not being used anywhere after the LB refactor.

I think the post update would probably need to delete this permission from any roles that have it.

Added logic to get rid of the unused permission administer navigation_block.

Thank you for the guidance here.

Thanks I think this is fine. We would normally also need hook_role_presave() bc layer to update the config for module-shipped roles as well, but that is really not going to have happened one week after beta and it would be runtime code that would need to stay in navigation until 12.x if we added it, so I think it's better to skip that step here and just support any actual sites that potentially could have installed the beta.

Thanks for fixing this, just one minor question on the MR about the update path

Resolved the question opened by @larowlan.

I think it is safe to move it back to RTBC now.

Thank you!

catch and I agreed that a change record would be helpful here. I have added one but it needs to be reviewed. So, setting this to NR for that.

Thank you for taking care of the initial CR.

Took a look into that and made some minor changes to add some extra information.

Back to RTBC now.

All looks good to me now, thanks for the changes. We might want to tweak the experimental module allowed changes to make upgrades optional in the 'beta-in-beta' situation but at least this one is pretty straightforward.

Committed/pushed to 11.x and cherry-picked to back through to 10.3.x, thanks!

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