Problem/Motivation

This issue was discovered by myself and alexpott while investigating #2587551: Selecting Operation Links for File Type View throws an Exception/#2491875: EntityViewsData adds Operations links to all entities, which won't work if the entity type has no list builder, leading to WSOD on some views and was also already included in the patch for #1986606: Convert the comments administration screen to a view. Currently, the Comment entity does not specify a list_builder handler, and therefore Views cannot discover if it has operation links available. Before #2491875: EntityViewsData adds Operations links to all entities, which won't work if the entity type has no list builder, leading to WSOD on some views, adding the Operations dropbutton as a field on an admin view will result in an exception; after that issue, the handler will simply not be listed in the Views UI.

This issue has a broader scope than just #1986606: Convert the comments administration screen to a view because even without that conversion being done, comment admin views should be able to add operation links. It's quite a common usecase.

Proposed resolution

Specify the default list builder for the Comment module.

Ideally, for better DX, we would provide an API to get operation links without having a list builder. The coupling of these two things is sort of historical, because we originally created list builders to handle these lists with entity operation links. But fixing it in a more complete way would introduce more disruption and is less appropriate for a release candidate or patch release.

Remaining tasks

  • Patch needs tests for adding the operations link to a comment view and ensuring it functions properly.
  • Do we need an update hook to trigger the cache rebuild for the updated entity definition with its dependency on the default list builder? Done
  • Need to confirm that adding a handler to an entity type is fixed with at most a cache clear and has no other disruption.

User interface changes

N/A

API changes

The Comment entity now uses the default list builder.

Data model changes

N/A

Why this should be an RC target

It's not really possible to create an administrative view for comments without this change. There is no disruption from the patch.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm created an issue. See original summary.

xjm’s picture

Issue summary: View changes
xjm’s picture

Priority: Normal » Major
xjm’s picture

Answer is yes, we need the empty entity comment update hook. Attached. Tested by applying the patch to a site with an existing broken view, then running update.php. It resolves the issue.

xjm’s picture

+++ b/core/modules/comment/comment.install
@@ -110,3 +110,20 @@ function comment_schema() {
+ * Clear caches to fix Comment enttiy list builder and operations Views field.

"enttiy" could be fixed when tests are added or on ocmmit. Typo retained deliberately.

andypost’s picture

The last submitted patch, 6: 2604618-vdc-comment-list-builder-6-fail.patch, failed testing.

andypost’s picture

andypost’s picture

fix copy-pasta

larowlan’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me

The last submitted patch, 6: 2604618-vdc-comment-list-builder-6-fail.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2604618-vdc-comment-list-builder-9.patch, failed testing.

andypost’s picture

Status: Needs work » Reviewed & tested by the community

botflux

The last submitted patch, 6: 2604618-vdc-comment-list-builder-6-fail.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: 2604618-vdc-comment-list-builder-9.patch, failed testing.

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -rc target triage +rc target

Discussed with @catch and we agree that this should be an rc target so that people can build views on comments with operations buttons.

Setting back to rtbc as per #13.

The last submitted patch, 6: 2604618-vdc-comment-list-builder-6-fail.patch, failed testing.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 03de872 and pushed to 8.0.x. Thanks!

  • alexpott committed 03de872 on 8.0.x
    Issue #2604618 by andypost, xjm: Views operations dropbuttons do not...

Status: Fixed » Closed (fixed)

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