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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Gábor Hojtsy’s picture

Status: Postponed » Active

I'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.

Gábor Hojtsy’s picture

Looking 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:

  1. Have a langcode in the base table (as well): comment, shortcut, taxonomy term, user (even after current state of #1498664: Refactor user entity properties to multilingual), menu link
  2. Have a langcode in the field_data table (only): content block, node

Thats *confusing*.

Edited: moved menu link from second list to first.

jhodgdon’s picture

Hm. The filters and fields do not work as they are currently... not sure exactly what is going on?

Gábor Hojtsy’s picture

I 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 :)

Gábor Hojtsy’s picture

For 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.

jhodgdon’s picture

Status: Active » Closed (cannot reproduce)

OK I am not seeing this problem now. Hm. I guess we should just close this as "cannot reproduce".

jhodgdon’s picture

Status: Closed (cannot reproduce) » Active

OK.

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".

Gábor Hojtsy’s picture

So 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 :)

plach’s picture

The 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.

Gábor Hojtsy’s picture

Title: Views - Comment language filter does not work (wrong table) » Views comment language field/filter is for original language code, no translation language field/filter
Issue summary: View changes
Priority: Normal » Major
Issue tags: +sprint

Retitled, updated issue summary.

jhodgdon’s picture

Issue summary: View changes
jhodgdon’s picture

The 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.

Gábor Hojtsy’s picture

Fix D8MI tag.

jhodgdon’s picture

Status: Active » Postponed

The 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.

Gábor Hojtsy’s picture

Issue tags: -sprint
jhodgdon’s picture

Status: Postponed » Active

#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.

jhodgdon’s picture

Assigned: Unassigned » jhodgdon
jhodgdon’s picture

Status: Active » Needs review
FileSize
13.58 KB

Here's a first pass. Let's see if it works...

jhodgdon’s picture

Issue tags: -Needs tests
FileSize
13.44 KB
804 bytes

Wow, 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?

dawehner’s picture

Awesome!!!

  1. +++ b/core/modules/comment/src/CommentViewsData.php
    @@ -7,186 +7,44 @@
    +    $data['comment']['table']['base']['help'] = t("Comments are responses to content.");
    

    Nitpick: single quotes

  2. +++ b/core/modules/views/src/EntityViewsData.php
    @@ -306,6 +306,13 @@ protected function mapSingleFieldViewsData($table, $field_name, $field_type, $co
    +      case 'uri':
    +        $views_field['field']['id'] = 'url';
    +        $views_field['argument']['id'] = 'string';
    +        $views_field['filter']['id'] = 'string';
    +        $views_field['sort']['id'] = 'standard';
    +        break;
    

    Can we have some test coverage, please?

jhodgdon’s picture

FileSize
22.19 KB
9.23 KB

Good 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.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/views/tests/src/Unit/EntityViewsDataTest.php
@@ -556,6 +582,19 @@ protected function assertStringField($data) {
+    $this->assertEquals('url', $data['field']['id']);

cool, I did not remembered that we have a url field.

jhodgdon’s picture

RE #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!

jhodgdon’s picture

Assigned: jhodgdon » Unassigned
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 412e733 and pushed to 8.0.x. Thanks!

  • alexpott committed 412e733 on 8.0.x
    Issue #2320277 by jhodgdon: Fixed Views comment language field/filter is...
Gábor Hojtsy’s picture

Amazing, thanks!

Status: Fixed » Closed (fixed)

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