Hello,
We can't use the same name for the module and a permission, It visually breaks the Extend UI.
- Create a module named 'Hello' with all necessary (hello.info.yml etc...),
- create a permission file 'hello.permissions.yml' with this
hello:
title: 'Access hello'
description: 'Permission to access Hello'
restrict access: 'true'
- Go to /admin/people/permissions and search your permission : The displayed table is broken
Add a prefix "module_" before the $provider in $form['permissions'][$provider] in order to avoid using the same name for the module and permission:
$form['permissions']['module_' . $provider]
- Add a test
- Review
- Commit
Existing UI:
Proposed UI:
| Comment | File | Size | Author |
|---|---|---|---|
| #36 | after.png | 158.14 KB | sivaji_ganesh_jojodae |
| #36 | before.png | 114.86 KB | sivaji_ganesh_jojodae |
| #36 | test_permission_bug.zip | 1.93 KB | sivaji_ganesh_jojodae |
| #15 | module-name-permission-machine-2845241-15.patch | 653 bytes | arunkumark |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !11492compare compare 
Comments
Comment #2
cilefen commentedIndeed, this is true.
Comment #3
cilefen commentedComment #12
larowlanComment #15
arunkumarkResolved the issue by creating the following patch. Patch used to resolve the issue on the 9.5.x version.
Comment #16
arunkumarkAlso the patch for the Drupal 10.0.X
Comment #17
abhijith s commentedApplied patch #15 on 9.5.x and it fixes the issue.
Before patch
:
After patch:
RTBC +1
Comment #18
aarti zikre commentedchanging the status of the issue based on the last comment.
Comment #20
ameymudras commentedThanks for the work on this issue, I came across this issue while working on some permissions.
- The patch #15 applies cleanly and works well to resolve the issue
- Issue summary is clear and describes the problem
- After applying the patch there are no warnings, and the permission save works as expected.
Comment #21
ameymudras commentedComment #22
quietone commentedThanks everyone for getting this to RTBC.
There are a few more things to do before this is ready for a committer. The Issue Summary contains out of date screenshots and does not say how this is being fixed. The "Proposed resolution" section should describe all the changes made in the issue. The "User interface changes" should show the existing UI and the proposed UI. Adding tag for an IS update.
There is no test here so and nearly all patches require a test. It look like the page is tested in \Drupal\Tests\system\Functional\Form\ModulesListFormWebTest. Perhaps one of the existing tests could be expanded to include a test for this fix?
@Abhijith S and @ameymudras, thank you for testing. Your comments state that you applied the patch and it works. For this patch other steps need to be taken and should be listed in what you did to review the patch. I presume a test module was made and a new permission? Did you do or try other things?
And a reminder that when adding screenshots, update the Issue Summary with the before and after screenshots. This will help reviewers and committers a lot.
Comment #23
lucasscHi!
IS updated with the changes suggested in #22.
Comment #24
lucasscI forgot to remove "TBD" from User interface changes and "Update issue summary" from the Remaining tasks, so I did it now.
Comment #26
nitin_lamaComment #27
nitin_lamaAdding patch for 10.1.x.
Comment #28
ravi kant commentedPatch #27 is working as accepted. Attaching screenshot.
Comment #29
ravi kant commentedComment #30
borisson_Still needs tests
Comment #35
dcam commentedThe patch in #27 was converted to an MR. I also added a test.
Comment #36
sivaji_ganesh_jojodae commentedInteresting issue. The bug indeed is still around. I can reproduce the issue with the attached test module.
Attached are the screenshots of before and after the MR.
The patch looks straightforward and pipeline looks good as well.
Comment #37
borisson_Test looks solid, I think that's good. I wonder if we need a change record for this, but I don't think we do. RTBC++
Comment #38
nod_Thanks small nit on the test documentation
Comment #39
sivaji_ganesh_jojodae commentedGood catch @nod_
Comment #40
dcam commentedI updated the docblock, but I put the reason why I wrote it the way that I did in an MR comment.
Comment #41
borisson_While I agree that adding more functional tests is not great for the total test time duration, having them well documented is important.
We can always change the documentation when we want to add more testing to this.
To me, this change seems like it answers the question by @nod_.
Comment #42
dcam commentedI copied and pasted the
$next_rowvariable name from elsewhere in the test class. I forgot to change it and that name isn't relevant to the new test. It bothered me, so I updated it. This is a minor change that doesn't really affect anything, so I'm leaving the issue at RTBC.Comment #45
nod_Committed 19a37c3 and pushed to 11.x. Thanks!