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.
Comment | File | Size | Author |
---|---|---|---|
#9 | 2604618-vdc-comment-list-builder-9.patch | 9.62 KB | andypost |
#9 | interdiff.txt | 655 bytes | andypost |
#6 | 2604618-vdc-comment-list-builder-6-fail.patch | 8.37 KB | andypost |
#6 | interdiff.txt | 8.9 KB | andypost |
Comments
Comment #2
xjmComment #3
xjmPromoting to major as a blocker for #1986606: Convert the comments administration screen to a view.
Comment #4
xjmAnswer is yes, we need the empty
entitycomment update hook. Attached. Tested by applying the patch to a site with an existing broken view, then running update.php. It resolves the issue.Comment #5
xjm"enttiy" could be fixed when tests are added or on ocmmit. Typo retained deliberately.
Comment #6
andypostHere's a test and fix for #5
Comment #8
andypostno need for empty comment...
Comment #9
andypostfix copy-pasta
Comment #10
larowlanLooks good to me
Comment #13
andypostbotflux
Comment #16
alexpottDiscussed 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.
Comment #18
alexpottCommitted 03de872 and pushed to 8.0.x. Thanks!