The entity MenuLink defines a render controller. This does not makes sense to for menu links can not be viewed by thier own like this is the case for node etc.

CommentFileSizeAuthor
#6 2100467-6.patch641 bytesamateescu
#1 drupal-2100467-1.patch640 bytescorvus_ch

Comments

corvus_ch’s picture

corvus_ch commented
Status: Active » Needs review
StatusFileSize
new drupal-2100467-1.patch640 bytes

Without testing it locally, let's see if the test bot will be happy with it.

dawehner’s picture

dawehner
Primary language German
commented

Are you sure there is no usecase to define a render controller for an entity? You never know whether things want to be done in views.

amateescu’s picture

amateescu commented
Component: menu system » menu_link.module
Status: Needs review » Needs work

I wanted the render controller to output the result of l().. I agree that it's not used *now*, but a specialized render controller should be introduced for menu links, imo :)

andypost’s picture

andypost
he/him
Primary language Russian
commented
Title: Remove render controller from menu links » Implement MenuLinkViewBuild controller for menu links
Issue tags: +entity API

Related #2097903: Rename EntityRenderController to EntityViewBuilder

pwolanin’s picture

pwolanin commented

We need to stop using l().

Ideally this entity class would have a way to return a render array? that would be useful.

amateescu’s picture

amateescu commented
Title: Implement MenuLinkViewBuild controller for menu links » Menu links are not ready to use a view builder so remove it from the annotation for now
Category: Bug report » Task
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -entity API
StatusFileSize
new 2100467-6.patch641 bytes

Discussed with @Berdir on IRC and we agreed that we should remove the generic view builder reference after all, at least until we're ready to implement one tailored for menu links.

Here's a rerolled patch.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Performance

Thanks, see #2006434-16: [meta] Speed up web tests for why this should result in quite a performance improvement for web tests, having a view builder means that saving them deletes cache tags. Lots of those.

So, RTBC unless bot disagrees.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2100467-6.patch, failed testing.

amateescu’s picture

amateescu commented
Status: Needs work » Needs review

6: 2100467-6.patch queued for re-testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Arghh, ImageFieldDisplayTest again.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Trying again.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 6: 2100467-6.patch, failed testing.

amateescu’s picture

amateescu commented
Status: Needs work » Needs review

6: 2100467-6.patch queued for re-testing.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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