Port status

@dawehner provided a Drupal 8 port of the menu_per_role module, that implemented the majority of the functionality.

@richgerdes extended his port to provide additional features. That version can be found

https://github.com/roygoldman/menu_per_role

Credit for development of the port goes to dawehner, spheresh, richgerdes, grimreaper.

Providing a stable release

The major issue blocking a stable release, is there is no official maintainer of the Drupal 8 module. Once someone has agreed to maintain the project and access is given, a release will be made on Drupal.org.

CommentFileSizeAuthor
#9 innerdiff.txt15.51 KBspheresh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fago’s picture

As discussed your help is very welcome. I've just granted you access to the Git repository. As you do not have project access yet I'll do another review of the code and make sure it fits our standards. Once that is fine I'm happy to grant you full project permissions so you can roll the release and maintain it!

charos’s picture

Any update on this? Now that D8 RC is here, it is a good timing for a branch.

dawehner’s picture

I'm working on it for fun

dawehner’s picture

joshi.rohit100’s picture

forked :)

dawehner’s picture

You could have not contributed more.

charos’s picture

Why not the the module maintainers check and push dawehner's release at least as an alpha version?

fago’s picture

>Why not the the module maintainers check and push dawehner's release at least as an alpha version?

Because there is no one maintaining it. I'm not maintaining the project any more since Drupal 5, and AlexisWilkes seems to have left as well. Maintainer wanted!

The code of https://github.com/dawehner/menu_per_role seems solid though.

spheresh’s picture

FileSize
15.51 KB

@dawehner's code looks very well.
I've fiexed a few related issues into my fork and create new PR.

- a configuration menu item was added/moved to "Configuration > System"
- a submitForm function was added to MenuPermRoleAdminSettings class
- two checking condition (uid1_see_all & admin_see_all) were added to the menuLinkCheckAccess function.
- an autocomplite widget was changed to options_buttons widget.
- menu_per_role.admin.inc was removed as deprecated.
- code standard small fixes

P.S. I would like to help with the development of d8 version too if you still open to suggestions.

thomaschall’s picture

Code looks good but had to add weight' => 0 to the menu_per_role.module for the 2 forms, as it's now required.

charos’s picture

since spheresh volunteers for the work, why not add him as a co-maintainer?

ibuildit’s picture

Issue summary: View changes

It's not working for me to set what role can NOT view a link.

arunkumark’s picture

@ibuildit

I have ported this module for Drupal 8.x version. Can you please review code on git.

https://github.com/arunkumarkuppuswamy/menu_per_role

@fago I like to become co-maintainer for this module.

Grimreaper’s picture

Hello,

I made a pull request on @dawehner repository to fix an undefined index and I gave support on the other github issue.

I think @dawehner repository is better.

@arunkumark: in your version, if you select no restriction or authorization the links are hidden instead of applying the default access.

I was trying to debug and make a pull request but I found the code not easily readable (function, variable names not intuitive and misleading).

If a 8.x-something branch could be made on Drupal.org that would be great for issue tracking and stopping all the forks on github...

dawehner’s picture

I'm happy to be maintainer of the module. Does someone know how the procedure looks like for doing so?

Grimreaper’s picture

@dawehner: If no maintainer can make you co-maintainer, there is a procedure: https://www.drupal.org/node/251466

I used it for the protected_node module two years ago: https://www.drupal.org/node/2417835

The maintainer was not answering private messages where I proposed to become co-maintainer. And there was no activity in the issue queue.

So I opened an issue on https://www.drupal.org/project/projectownership.

dawehner’s picture

@Grimreaper
Do you want to be the maintainer yourself :)

Grimreaper’s picture

@dawehner thank you for the suggestion.

I have never co-maintained a project with such number of reported installs, I don't know when I would have another opportunity like this, but I am currently work overloaded, so I don't know if I will be able to help a lot.

At least committing RTBC patches, and applying Drupal good practices: Coding standards, removing master branch, etc. can be done quickly.

The Drupal 8 version works good, it would be good to add automated tests but I currently don't see how it can be improved otherwise :)

So... Yeah, why not :)

And thanks for the PR :)

@fago: if I understand your comment, you can't give permissions for someone else to become co-maintainer?

dawehner’s picture

Grimreaper’s picture

@dawehner: Thanks.

So I opened an issue on https://www.drupal.org/project/projectownership.

Sorry for the confusion. I meant an issue for the protected_node module when I used the procedure to be co-maintainer.

guilhermevm’s picture

Dear,

Do I urgently need a user who belongs to a role, create a page and can set the menu only for a specific menu, is it possible?

Grateful

Dubs’s picture

@dawehner - thanks for the code.

Once small issue - links are hidden when editing the menu in admin.

Just add these lines into the menuLinkCheckAccess function to make an additional check: -

......
if (isset($entity->menu_per_role__show_role)) {
+        $arg = explode('/', \Drupal::request()->getpathInfo());
+        // If menu is being edited allow user to see it in full.
+        if ($arg[1] == 'admin' && $arg[2] == 'structure' && $arg[3] == 'menu') {
+          return $result;
+        }
.......
richgerdes’s picture

@dubs - Where in the function are you adding the extra check in #22?

I also found a few problems with the module myself a few weeks ago, and maintain a fork (and PR) of @dawehner project. The major issue I ran into was that the settings did not actually affect the display of the extra fields on the menu edit page. I also added a handy feature for (optionally) hiding the field for menu links that link directly to Nodes. You can find the for on github at https://github.com/roygoldman/menu_per_role

Dubs’s picture

@richgerdes - it's the menuLinkCheckAccess function in /src/MenuPerRoleLinkTreeManipulator.php. Line 32 :-)

richgerdes’s picture

@dubs

Thinking about this, i was wondering about your configuration. I think that there is value in the current implementation. By hiding items users can't see on the admin page, you prevent a bit of confusion, on sites where the menu structure is managed by multiple people who don't have access to the full site.

However, the module does provide a solution as is. The 'administer menu_per_role' permissions combined with the 'admin_see_all' setting allows administrators to see all items, but not delegated editors/moderators. Which means that all admins can see all menu items. Was there a reason you choose not to configure your site that way?

Thinking about usage, i do think having an option to show all, items to on the admin page is useful, but i think it would be better implemented behind a setting and/or priv. This would then provide a middle ground, where no user ever gets hit with any items they don't have the roles for while browsing the site, while still given trusted users the ability to update and control the display of all items. Any thoughts?

richgerdes’s picture

Issue summary: View changes

Updated summary to better show current status of the D8 port.

Dubs’s picture

@richgerdes - I agree totally. A permission would be a much better implementation for this functionality - that makes sense. Our use case didn't really take this into consideration.

Grimreaper’s picture

Hello,

@dawehner and I have been promoted co-maintainer of the project.

I have cleaned up the issue queue of issues about deprecated version of Drupal.

I will compare:
- https://github.com/dawehner/menu_per_role
and
- https://github.com/roygoldman/menu_per_role

and see the comments in this issue to commit on drupal.org. I will see that this weekend.

richgerdes’s picture

Great to see this moving forward.

The main difference between my (roygoldman on github) and @dawehner's version is Available in a pr on git hub.

https://github.com/dawehner/menu_per_role/pull/5

Let me know if their is anything you need help with on this.

For consolidation (From #2905102)

@Grimreaper plan:
- clean up the issue queue of issues on outdated version of Drupal (done)
- creating a Drupal 8 version to stop having all the forks on Github
- review issues in "needs review"
- clean the Drupal 8 version of coding standards
- publish a Drupal 8 alpha version to have more people using it
- clean the Drupal 7 version of coding standards
- publish a Drupal 7 alpha version to have more people using it
- clean the project page (started)

Grimreaper’s picture

Assigned: Unassigned » Grimreaper

Branch 8.x-1.x created from https://github.com/roygoldman/menu_per_role.

I am currently fixing some typo and improving some code parts.

  • Grimreaper committed 0953bf9 on 8.x-1.x
    Issue #2202293 by Grimreaper: Fix default values retrieved from config...
  • Grimreaper committed 36a97cd on 8.x-1.x
    Issue #2202293 by Grimreaper: Fix some typo.
    

  • Grimreaper committed e275db1 on 8.x-1.x
    Issue #2202293 by Grimreaper: Fix pareview.sh.
    
Grimreaper’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev

- clean up the issue queue of issues on outdated version of Drupal (done)
- creating a Drupal 8 version to stop having all the forks on Github (done)
- clean the Drupal 8 version of coding standards
- publish a Drupal 8 alpha version to have more people using it
- review issues in "needs review"
- clean the Drupal 7 version of coding standards
- publish a Drupal 7 alpha version to have more people using it
- clean the project page (started)

Working on the README.txt on the 8.x-1.x

  • Grimreaper committed 3f0a9c7 on 8.x-1.x
    Issue #2202293 by Grimreaper: Format README.txt
    
Grimreaper’s picture

Assigned: Grimreaper » Unassigned
Status: Active » Fixed

Hello,

Drupal 8 alpha version created: https://www.drupal.org/project/menu_per_role/releases/8.x-1.0-alpha1

@richgerdes and @Dubs, I have only read quickly your comments. If you still have issues or feature request, can you please open dedicated issue? Thanks.

Thanks all for testing and feedbacks.

I have written the roadmap on the project page.

Changing the status to fixed.

Status: Fixed » Closed (fixed)

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