Saved menu is reset on node form. Menu editable even if user has no access to saved menu. Menu edit form is not visible on the node edit page.

Patch attached, or if you prefer a PR: https://github.com/dpi/menu_admin_per_menu/pull/1

This patch includes:

• A fix from #2792329: Menu settings on node forms missing where the menu edit form is not visible on the node edit page.
• Fixes submissions from the menu form being lost, this is due to the usage of reset(). There is also a problem because reset() returns the value. Where we actually want the key. This code has been removed nontheless.
• A refactoring of the helpers:
  • Improving documentation, phpdoc
  • Passing the account object around instead of using the global account on each helper call.
  • Fixed values of MenuAdminPerMenuAccess::getPerMenuPermissions being permissions, when the signature states the values should be the menu ID's.
  • Simplified menu_admin_per_menu_filter_parent_options()
• If there is a menu item already defined, and he does not have access to the menu to which the menu item belongs, then the user will be disallowed from editing the menu item. (it will be hidden)

Mostly seems like a really good idea, but I'm a bit unsure about this bit:

If there is a menu item already defined, and he does not have access to the menu to which the menu item belongs, then the user will be disallowed from editing the menu item. (it will be hidden)

I think it will be pretty confusing to people wondering why the option is not present. Other ideas:

• Show the existing value with a message that it cannot be changed
• Add the existing item to the list of available values.

Show the existing value with a message that it cannot be changed

Making the existing fields disabled or read only would only tease the user.

I think displaying a message saying the user cannot modify the menu link might be the way to go, perhaps showing what the full tree of the link is? Whilst hiding the menu form fields

Add the existing item to the list of available values.

I thought about doing this, but then it may be odd to be hijacking a menu link that belongs to a menu you are not permitted to use. Perhaps this can be done, but it would advise against it being default behaviour.

If a user has control of a different menu, then he can add another menu link manually using the menu_ui routes.

The patch breaks when the entity has no available menus to begin with, passing NULL into a function that expects an array.
I've tweaked it slightly to fix my use-case, but it's not very clean.
$options = menu_admin_per_menu_filter_parent_options($account, $options ? $options : []);

passing NULL into a function that expects an array.

Yeh I'm getting this issue on 8.2 (original patch developed on 8.1) on a different site. Not due to lack of menus, rather because the mapm form alter hook is getting called before menu_ui, so the form elements (such as $form['menu']['link']['menu_parent']['#options']) are not available yet. Some kind of module weight issue...

Edit: oddly, I cleared cache and mapm is now called after menu_ui.

I thought about doing this, but then it may be odd to be hijacking a menu link that belongs to a menu you are not permitted to use. Perhaps this can be done, but it would advise against it being default behaviour.

Yes I think you are right.

Making the existing fields disabled or read only would only tease the user.

I'm not sure if I agree - I think it is quite a standard behaviour. The user has permission to see the current value, but not to change it, so we can show it as disabled. My reason for suggesting this is to help the user understand why they are not allowed to change it (because it is already on another menu).

If a user has control of a different menu, then he can add another menu link manually using the menu_ui routes.

Completely agree. My motivation is to guide the user to choose that option and avoid the series of support calls "there is a bug that I can't edit this menu item".

However if you still prefer your original option, then I don't think it matters too much.

patch in #6 works for me, with #2 I also have the null error.

Patch from #6 works for Drupal 8.1.10.

Patch from #6 works for 8.2

However to get mapm called after menu_ui, clearing cache by admin menu didn't make the trick, I oddly had to truncate cache tables manually.

Moreover, if the content type is allowed to be published in several menus and the default menu parent set in the content type is not allowed for the mapm admin, the menu won't show on node/add page.
I added the following code line 51 to get the menu and menu parent set in the node creation page :

if (!empty($current_menu) && substr($form['#form_id'],-10) == "_edit_form" && !in_array($current_menu, $allowed_menus)) {

What's the progress on this issue? How can I help to push this issue forward?

It would be great to get this issue moving -- the editors now have to recreate the menu link, since updating a page with a menu item causes the menu item to get deleted. Settings priority to Major, since the module doesn't fix the problem it set out to solve.

Patch in #6 still applies and seems to work fine. Will report back if I notice any issues in use. Otherwise I suggest this gets moved to RTBC.

Patch applies cleanly for me too, and fixes the issue of the 'Menu edit form is not visible on the node edit page' for me.
Shall we set this RTBC or does okin's suggestion in #13 also need to be accommodated?

Patch no longer applies on dev branch though as the following already exists:
use Drupal\Core\Session\AccountInterface;

I've updated the patch + added browser test to help avoid regression on this in the future (mkdok can you enable tests on the project please?).

+1 for the issue

Latest patch #18 did not apply cleanly for me against the latest dev branch. My console:

mac-admin:menu_admin_per_menu lxt$ curl -s -O https://www.drupal.org/files/issues/saved_menu_is_reset_on-2796537-18.patch
mac-admin:menu_admin_per_menu lxt$ patch -p1 < saved_menu_is_reset_on-2796537-18.patch
patching file menu_admin_per_menu.module
Hunk #1 FAILED at 21.
Hunk #2 FAILED at 41.
2 out of 2 hunks FAILED -- saving rejects to file menu_admin_per_menu.module.rej
can't find file to patch at input line 105
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/src/Access/MenuAdminPerMenuAccess.php b/src/Access/MenuAdminPerMenuAccess.php
|index 281c2ea..242cce2 100644
|--- a/src/Access/MenuAdminPerMenuAccess.php
|+++ b/src/Access/MenuAdminPerMenuAccess.php
--------------------------
File to patch: 

It seems like the patch by @scott_euser in comment #18 is not being picked up by the test-bot... Do tests still need to be enabled on the project by @mkdok?

@ #21:

MAPM does not have any tests.

Patch #18 works fine for me with the latest 8.x-1.x-dev

Patch #18 tested on 8.x-1.x-dev.

This patch resolves issues for users without permission "administer menu" and some permission on one menu used. And for users without the permission "administer menu" and any menu_admin_per_menu permissions.

Mark this to RTBC

.

Just ignore my comments..

Thanks for working on this. It works is my testing. +1 for committing

(A quick note to anyone testing - make sure you test #18 against 8.x-1.x and not 8.x-1.0-rc1. In rc1, the AccountInterface interface isn't loaded in menu_admin_per_menu.module, so you'll get a fatal error -- and it's not related to this patch.)

Sorry if it's obvious, but I don't know how to use this patch manually.
Should I apply all the patches #2 + #6 + #18 ? In which order ?
If I only apply #18, it does not fix the edit link in node form issue.
If I try to apply the other patches, I get errors while patching...
Thanks for support.
(I used 8.x-1.x as recommended and apply patches with : patch -p1 < file.patch)

+1 for committing

thanks :)

It's really great that this issue is fixed, thanks! Perhaps it's time for a fresh release of the module, to get the patch integrated in the official release?

The first official release of D8 version of the module is now out, thank you @mkdok!

I wonder why this issue has been marked as "Closed (fixed)"
I still don't see the "Menu Settings" area in the node creation/edit window if I only have the Menu Admin Per Menu permission to edit a certain menu, and not the "Administer menus and menu items" permission.
I'm using Drupal 8.5.3 and tried both module versions - production and dev. I thought that maybe I still need to apply the patch #18, but when I tried to apply it I got the message that this patch has already been applied.

It worked when it was released October 2017, but it seems like something has happened since then ... I suggest you create a new issue about it, so we can get it fixed.

I can certainly create a new issue, but maybe module maintainers prefer to re-open the same issue instead.

Thanks @gintass. A new issue is better, since the reason it doesn't work now isn't related to the problem in this issue. There is probably also a better chance that the module maintainers will notice a new issue.