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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

corvus_ch’s picture

corvus_ch CreditAttribution: corvus_ch commented
Status: Active » Needs review
FileSize
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
CreditAttribution: dawehner 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 CreditAttribution: 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
CreditAttribution: andypost 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 CreditAttribution: 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 CreditAttribution: 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
FileSize
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
CreditAttribution: Berdir 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 CreditAttribution: amateescu commented
Status: Needs work » Needs review

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

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Arghh, ImageFieldDisplayTest again.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir 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 CreditAttribution: amateescu commented
Status: Needs work » Needs review

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

amateescu’s picture

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

Back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick 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.