- Issue created by @larowlan
- First commit to issue fork.
- Merge request !11459Issue #3512116: \Drupal\system\Controller\DbUpdateController::helpfulLinks... → (Open) created by annmarysruthy
- 🇨🇴Colombia juandhr Manizales
I can confirm that the changes introduced in the dwwed branch work as expected.
While testing these changes in a local environment, I:
- Added the 'access administration pages', 'administer site configuration', and 'administer software updates' permissions to a test role.
- Logged in with that role and navigated to the /update.php/selection page in the default update process.
- Verified that the "Administration pages" and "Status report" links display correctly.
This confirms that the modification to use the access() method from Drupal\Core\Url is working as intended.
- 🇳🇿New Zealand quietone
@juandhr, thanks for testing this issue and reporting the steps you followed. That is good practice. Is also helps to provide confirmation of the problem, before the patch is applied. And you can save yourself some time and effort because in Drupal core there is no need to upload screenshots of your IDE. There is some information about that at https://www.drupal.org/about/core/policies/maintainers/how-is-credit-gra... → .
Can we add a test for this?
Not sure what I am doing wrong but I can't duplicate the problem.
- 🇦🇺Australia mstrelan
@quietone the problem is more hypothetical and the update here is for best practice. I'll try to explain the problem.
- In
\Drupal\system\Controller\DbUpdateController::helpfulLinks
we check if the user has the'administer site configuration'
permission before displaying the "Status report" link. - In
\Drupal\Tests\system\Functional\UpdateSystem\UpdateScriptTest::testSuccessfulMultilingualUpdateFunctionality
we create a user with a bunch of permissions. This includes both'access site reports'
and'administer site configuration'
. Then we assert that the "Status report" link exists. - Core later decides that the permission required to access the
'system.status'
route should actually be'access site reports'
and not'administer site configuration'
so the route is updated. The test still passes because that user has both permissions. But a user with'administer site configuration'
and not'access site reports'
will still see the link, even though they won't be able to access the route. - The same problem could arise if a contrib or custom module decides to alter the permissions required for the route. For example, they might override the status report page and provide a custom permission for it. The link will still appear for users with the original permission.
By changing to
Url::access
we don't need to worry about what permission the route requires, just whether the logged in user can access it or not.Can we add a test for this?
I think the test mentioned above is sufficient, not sure it's worth the effort to expand that further.
- In