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: Get rid of menu_list_system_menus()

Files: 
CommentFileSizeAuthor
#8 interdiff.txt1.64 KBandypost
#8 access-menu-link-1842850-8.patch10.39 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 55,180 pass(es).
[ View ]
#7 interdiff.txt4.14 KBandypost
#7 access-menu-link-1842850-7.patch9.71 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#6 interdiff.txt3.01 KBandypost
#6 access-menu-link-1842850-6.patch6.22 KBandypost
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#5 access-menu-link-1842850-5.patch3.76 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 56,060 pass(es).
[ View ]

Comments

Berdir’s picture

Status:Postponed» Active

Any reason this is postponed?

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

amateescu’s picture

Component:entity system» menu_link.module

It was postponed on the initial conversion issue.

andypost’s picture

is this tags still right?

Berdir’s picture

This is better.

andypost’s picture

Status:Active» Needs review
StatusFileSize
new3.76 KB
PASSED: [[SimpleTest]]: [MySQL] 56,060 pass(es).
[ View ]

Initial patch

andypost’s picture

Title:Implement access control for the menu link entity» Implement access control for the menu and menu link entity
StatusFileSize
new6.22 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new3.01 KB

Suppose better to make them both

andypost’s picture

StatusFileSize
new9.71 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
new4.14 KB

And a bit more

andypost’s picture

StatusFileSize
new10.39 KB
PASSED: [[SimpleTest]]: [MySQL] 55,180 pass(es).
[ View ]
new1.64 KB

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

andypost’s picture

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

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

@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

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

andypost’s picture

andypost’s picture

Issue summary:View changes

summary

amateescu’s picture

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

Status:Needs review» Needs work

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

andypost’s picture

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

andypost’s picture

Component:menu_link.module» menu system
Issue tags:+MenuSystemRevamp

Suppose this more related to menu system and proper tags

andypost’s picture

Issue summary:View changes

Updated issue summary.

pwolanin’s picture

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

Status:Postponed» Active

@pwolanin Can we close this one?

Berdir’s picture

Assigned:Unassigned» dawehner

Yeah, pretty sure this can be closed?

dawehner’s picture

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.