Follow-up to #916388: Convert menu links into entities.

Problem/Motivation

We need to split the acces control code from _menu_link_translate() to its proper place in the entity class.

Proposed resolution

Implement access controllers for menu and menu link entities.

#2012916: Implement access controller for the menu and menu link entity
#1882552: Deprecate menu_list_system_menus() and menu_ui_get_menus()

CommentFileSizeAuthor
#8 interdiff.txt1.64 KBandypost
#8 access-menu-link-1842850-8.patch10.39 KBandypost
#7 interdiff.txt4.14 KBandypost
#7 access-menu-link-1842850-7.patch9.71 KBandypost
#6 interdiff.txt3.01 KBandypost
#6 access-menu-link-1842850-6.patch6.22 KBandypost
#5 access-menu-link-1842850-5.patch3.76 KBandypost

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Postponed » Active

Any reason this is postponed?

Adding this to the list in #1346214: [meta] Unified Entity Field API

amateescu’s picture

amateescu commented
Component: entity system » menu_link.module

It was postponed on the initial conversion issue.

andypost’s picture

andypost
he/him
Primary language Russian
commented

is this tags still right?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue tags: -Entity system +Entity Field API

This is better.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new access-menu-link-1842850-5.patch3.76 KB

Initial patch

andypost’s picture

andypost
he/him
Primary language Russian
commented
Title: Implement access control for the menu link entity » Implement access control for the menu and menu link entity
StatusFileSize
new access-menu-link-1842850-6.patch6.22 KB
new interdiff.txt3.01 KB

Suppose better to make them both

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new access-menu-link-1842850-7.patch9.71 KB
new interdiff.txt4.14 KB

And a bit more

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new access-menu-link-1842850-8.patch10.39 KB
new interdiff.txt1.64 KB

Seems access should live near definition, also cleaned-up copy/paste errors

andypost’s picture

andypost
he/him
Primary language Russian
commented

Next round should be split of _menu_link_translate() into 3 methods:

Provides menu link access control, translation, and argument handling.

And then _menu_check_access()

amateescu’s picture

amateescu commented

Nice work @andypost, the patch looks great so far!

However, the initial problem/motivation from the OP is still about cleaning up _menu_link_translate(), as you mentioned above, so I don't really feel comfortable rtbc-ing this without adressing that part :(

andypost’s picture

andypost
he/him
Primary language Russian
commented

@amateescu so maybe better to split the issue in to Generic implementation of AccessController and leave this one about _menu_link_translate()?

amateescu’s picture

amateescu commented

Yep, I think that's the best thing to do.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Filed #2012916: Implement access controller for the menu and menu link entity

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes

summary

amateescu’s picture

amateescu commented
Title: Implement access control for the menu and menu link entity » Untangle menu_link access control from _menu_link_translate() and friends

Better title.

amateescu’s picture

amateescu commented
Status: Needs review » Needs work

There's no patch for the current scope of this issue.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Now #2012916: Implement access controller for the menu and menu link entity commited so let's split _menu_link_translate()

andypost’s picture

andypost
he/him
Primary language Russian
commented
Component: menu_link.module » menu system
Issue tags: +MenuSystemRevamp

Suppose this more related to menu system and proper tags

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes

Updated issue summary.

pwolanin’s picture

pwolanin commented
Issue summary: View changes
Status: Needs work » Postponed
Related issues: +#2047633: Move definition of menu links to hook_menu_link_defaults(), decouple key name from path, and make 'parent' explicit

This is almost certainly duplicate to changes in the patch at #2047633: Move definition of menu links to hook_menu_link_defaults(), decouple key name from path, and make 'parent' explicit, or at least needs to be postponed until that is done.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Postponed » Active

@pwolanin Can we close this one?

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Assigned: Unassigned » dawehner

Yeah, pretty sure this can be closed?

dawehner’s picture

dawehner
Primary language German
commented
Assigned: dawehner » Unassigned
Status: Active » Fixed

Yeah, this can be closed indeed.

Status: Fixed » Closed (fixed)

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