Problem/Motivation
If you try to filter a Comment view by language, you only get an original language field/filter. The translation language is not exposed. Same as was for nodes in #2217943: Views cannot be filtered by language of translation.
The reason is that in comment_views_data(), the language field/filter is only added for original language.
if (\Drupal::moduleHandler()->moduleExists('language')) {
$data['comment']['langcode'] = array(
'title' => t('Language'),
'help' => t('The language the comment is in.'),
'field' => array(
'id' => 'language',
),
'filter' => array(
'id' => 'language',
),
...
Proposed resolution
- Rename filter/field/sort to original language.
- Add translation language field/filter/sort.
- We should also take out the if (module exists ( language )) in both filters/fields. See notes in #2313159-12: [meta] Make multilingual views work
Remaining tasks
- Do it.
- Tests.
- Review.
User interface changes
Language changed to original language.
New translation language filter/field/sort.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#22 | interdiff.txt | 9.23 KB | jhodgdon |
#22 | 2320277-convert-comment-views-data-22.patch | 22.19 KB | jhodgdon |
Comments
Comment #1
jhodgdonWe should also take out the if (module exists ( language )) in this. See notes in #2313159-12: [meta] Make multilingual views work
And we need to postpone this on #2319671: ViewsDataController: Step1: Move entity views data into controllers and #1740492: Implement a default entity views data handler
Comment #2
Gábor HojtsyI'm not sure we should postpone fixing an existing bug on a generalisation. Would rather fix it now with tests, that would ensure the generalisation works best.
Comment #3
Gábor HojtsyLooking at the comment tables, the answer may not be that simple. The base comment table has a langcode field (unlike nodes) while the comment_field_data table also has a langcode and a default_langcode field (this later layout similar to nodes). Comparison:
Thats *confusing*.
Edited: moved menu link from second list to first.
Comment #4
jhodgdonHm. The filters and fields do not work as they are currently... not sure exactly what is going on?
Comment #5
Gábor HojtsyI created a comment view with an exposed filter on language. It filters fine based on the original comment language. (I could not for some reason actually translate comments to other languages to test translations, but this filter looks like it will work as an original language filter). #1740492: Implement a default entity views data handler does not seem to be dealing with original/translation language filters / fields.
(PS the error I'm getting when attempting to translate comments is
Uncaught PHP Exception InvalidArgumentException: "Invalid translation language (af) specified." at ....../drupal/core/lib/Drupal/Core/Entity/ContentEntityBase.php line 746, referer: http://drupal8.local:8083/comment/1/translations
, yet another problem :)Comment #6
Gábor HojtsyFor the record, I don't think the langcode field on the comment base table is a good idea, not sure why some entities have it there as well as in their field data table.
Comment #7
jhodgdonOK I am not seeing this problem now. Hm. I guess we should just close this as "cannot reproduce".
Comment #8
jhodgdonOK.
So the problem here is that the langcode column in the comment base table is the "original language of the comment". What we really need is field/filter of the translation, which is in the field data table.
So we need both, like we did for Node on #2217943: Views cannot be filtered by language of translation -- the ones that are there now should say "Original" and the new ones should say "Translation".
Comment #9
Gábor HojtsySo the answer to #3 is that those entities, that have revisions have their langcode in the revisions table (only content blocks and nodes have revisions in core). So those have the langcode NOT in the base table. Then those which do not have revisions, so the rest of the core content entities, have their original langcode in the base table, like comment.
And then the translation langcodes in the data table apply to both revisionable and non-revisionable entities.
So that should clear up the confusion on #3. For comment language filters (and all other entities), we need both an original language and a translation language filter. This issue is for comments :)
Comment #10
plachThe entity views data handlers will be useful because if the default table layout changes, the data definition changes as well and those differences need to be accomodated. The generic entity views data handler will (have to) do it transparently for all the content entity types having (core) SQL storage.
Comment #11
Gábor HojtsyRetitled, updated issue summary.
Comment #12
jhodgdonComment #13
jhodgdonThe issue that this was at one point postponed on has been done (see comment #1). Comment is using a class instead of hook_views_data() now.... This is ready to work on.
Comment #14
Gábor HojtsyFix D8MI tag.
Comment #15
jhodgdonThe proposed patch on #1740492: Implement a default entity views data handler would take care of this... so we may want to postpone until that is done and then hopefully close this issue as a duplicate. But if that issue isn't moving forward, we can do this here.
Comment #16
Gábor HojtsyComment #17
jhodgdon#1740492: Implement a default entity views data handler has been committed, so it's time to get back to this issue.
What needs to be done for Comment is something similar to what that issue's patch did for Node: convert Comment's views data handler to derive from the new default Views data handler. Which will automatically add the necessary field/filter on the translation language.
Comment #18
jhodgdonComment #19
jhodgdonHere's a first pass. Let's see if it works...
Comment #20
jhodgdonWow, it actually passed the tests, first try!
I also tested this manually in the UI on simplytest.me:
- Standard profile install
- Enable Language, Content Translation
- Add Spanish language
- Make comments translatable
- Add an Article node and a comment
- Add a new view with base table Comment. Try out the language fields and filters.
This is all working OK, but the labels for the fields are confusing due to the overrides I left in for $data['comment']['langcode']. The defaults for EntityViewsData are better. So, removing these overrides, and I think this is ready for review.
And as we have tests for the default entity views data... I'm not sure we need additional tests here?
Comment #21
dawehnerAwesome!!!
Nitpick: single quotes
Can we have some test coverage, please?
Comment #22
jhodgdonGood ideas. That double quote was left over from the original; good to get rid of it. Added test coverage to the base Views class test.
Comment #23
dawehnercool, I did not remembered that we have a url field.
Comment #24
jhodgdonRE #23 -- me neither, but Comment's views data was previously using it, which is why I found about it during the conversion and added it to the base Views entity data class. Next up: User views entity data... I think I won't have extra stuff to add to the base class this time, but we'll see!
Comment #25
jhodgdonComment #26
alexpottCommitted 412e733 and pushed to 8.0.x. Thanks!
Comment #28
Gábor HojtsyAmazing, thanks!