Problem/Motivation

  1. Front page view currently shows results in all languages. Expectation is that content on the front page, would show for only one language, and "switching" language (via the language switcher block, or with the langcode in the url), would show content in that language.
  2. The content admin page has a language filter (yay), but it actually filters for the original language of the node, not the translations shown, so its VERY misleading. We should use the filter on the translation instead.

Proposed resolution

  1. Add a language filter to the front page view that will limit the results to only content in the current page's language.
  2. Swap the filter on the node admin page from a filter on original language to a filter on translation language.
  3. Add tests.

Remaining tasks

Commit.

User interface changes

1. The front page will now only show nodes in the language of the page, not all translations of promoted nodes.
2. The node admin page language filter will now filter by language of entries shown (all translations) instead of just language of the original nodes.

API changes

No.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

I was trying to add a filter to do this...
@dawehner noted that

2057401 removed the langcode filter on node directly and just allows to filter once you have added the node revision relationship

#2057401: Make the node entity database schema sensible

So.

I wasn't sure which one I wanted,
Content revision: Content, The revision ID of the content revision.
or
Content revision: Content, The revision NID of the content revision.

Then, when to add a filter, and was looking for something named like current user's language cause I know of #2037979: "Current user's language" views filter label is named very misleading
but didn't see that, just got
"Content revision: Language The language the content is in."

Note searched for "language" when adding filter gave me some warnings like
Warning: Creating default object from empty value in Drupal\views\Plugin\views\row\EntityRow->preRender() (line 187 of core/modules/views/lib/Drupal/views/Plugin/views/row/EntityRow.php).

oh, here it is.

selected a relationship
and current user (current page)

hm.. preview "No valid values found on filter: Content revision: Language.":

another warning on save:

Warning: Creating default object from empty value in Drupal\views\Plugin\views\row\EntityRow->preRender() (line 187 of core/modules/views/lib/Drupal/views/Plugin/views/row/EntityRow.php). 

And on front page, "No front page content has been created yet."
so I didn't make it right.

Berdir’s picture

Wondering, did you try this with or without the patch from #2006606: Views rendering ignores entity language, eg: Identical nodes rendered in views when nodes have translations? Try the same with the reversed situation, so if you had it applied remove it, if you hadn't, try it with the patch :)

It's also very likely that this is currently broken (and not you doing something wrong), I don't think anyone did a lot of testing with views and all the translation and revision stuff that we did.

dawehner’s picture

It's also very likely that this is currently broken (and not you doing something wrong), I don't think anyone did a lot of testing with views and all the translation and revision stuff that we did.

One example is for example that we cannot longer filter the nodes itself but just the revisions. Is this a hard requirement we cannot change?

klonos’s picture

Just filed #2162051: Decide if views should default to having a language filter. for the task mentioned in the issue summary. Also added it as a child issue to this one.

plach’s picture

Guys, as I was saying in #2006606: Views rendering ignores entity language, eg: Identical nodes rendered in views when nodes have translations, we need a new Translation language field (+ relationship?) here: if we filter on the node language we will get a front page where only nodes originally created with that language are selected, but the problem we would be facing would be exactly the same, just with (potentially) less nodes to display. Instead we need to filter on node_field_data.langcode, so that only one record per node will be selected.

webchick’s picture

Priority: Normal » Major

I would actually declare this as major due to the WTF-factor.

YesCT’s picture

Gábor Hojtsy’s picture

Not sure #2162051: Decide if views should default to having a language filter. should be a separate issue, then what will this issue do? :D

Berdir’s picture

The other issue I think is that if you create a new view, it should automatically add a language filter for you. That's probably highly questionable, this issue is not.

Anyway, as commented in #2006606: Views rendering ignores entity language, eg: Identical nodes rendered in views when nodes have translations, instead of adding a language filter, we could also make it distinct (or probably actually group on the nid, as we do select the language and so on), so that every node is only shown once, with a fallback language if the current language doesn't exist as a translation. I'm not sure what makes more sense, The advantage of that would be that the behavior/view doesn't need to change in any way when you enable language.module/add translations.

Gábor Hojtsy’s picture

Yeah a distinct may be the simplest solution that is compatible with monolingual and multilingual (with the assumption that you want your front page to be rendered with only one copy of each node). Then the next question level is whether to change the view to render in the current page language instead, so it works with the language switcher.

plach’s picture

I don't think distinct is the way to go, as it makes the query (potentially) slower and is a fragile solution: add the "wrong" sort or filter and you are screwed. IMO we should make the translation language (entity_field_data.language) available as a field (providing filter and sort handlers) and add it to the fron page view as an optional filter.

plach’s picture

With a translation filter we can also support language fallback in a cleaner way (without needing a follow-up): it's just matter of setting the filter value to the original language ('default_langcode' = 1) and set the current language renderer option and we are done.

jhodgdon’s picture

So I was looking at some related issues, and tried to even add a language filter to my View.

On my site, I have an article, which I wrote in English and then translated into French and Spanish.

I created a view of Articles, and it showed me all 3 translations of the same node.

So I tried to filter by language. I clicked "Add" in the Filter section, and looked for filters with the word "Language" in them. It allowed me to filter by the node revision language (even though I do not have a revision relationship in the View).

However, if I chose French or Spanish, my results were empty, and if I chose English, I got all 3 translations again. So that filter doesn't work.

I looked at the node revision table and the node field data table, and it looks like all 3 translations are part of the same "revision" (there are 3 rows in the node field data table for the latest revision ID, with my 3 languages and the same revision vid), and in the revisions table, the language is set to "en" for all the revisions.

I also tried adding one of the two very cryptic relationships that looked like they were related to revisions, and using that relationship in the filter, but the behavior didn't change.

So how can you even filter by language? I don't even think it's possible if you use translations of the same node. It may be possible if you have a bunch of separate nodes with different languages, but not if you use the I think supposed to be standard tactic in D8 of having one node with multiple translations.

jhodgdon’s picture

FileSize
11.11 KB

Here is a screenshots from PHPMyAdmin of the node_field_data table -- columns shown are nid, vid (revision ID), type, langcode, default_langcode (TRUE or FALSE for "is this the default language), and title
node_field_data table view
So you see, one revision, 3 languages.

I would have put in a screen shot of node_revision too but it's boring. For vid of 4, there is one row, and the langcode is 'en'.

jhodgdon’s picture

After discussing this with Gábor Hojtsy and Berdir in IRC, I just spun #13/#14 off onto a separate issue:
#2217943: Views cannot be filtered by language of translation

I do not believe that the issue being discussed here can be resolved until that other one is fixed.

plach’s picture

I think adding a translation language filter is exactly the goal of this issue. The filter you need for your use case (and that is missing) is node_field_data.langcode.

jhodgdon’s picture

Well, there are two issues:

a) You cannot filter by translation language currently at all. That is what #2217943: Views cannot be filtered by language of translation is about, and it advocates making two filters available, plus making the labels of the existing language-aware filters clearer so people don't try to mistakenly use them when they are inappropriate.

b) The default front page view should be using one of those two filters (this issue).

In my opinion, splitting the issue into "Views should have these filters" and "This view should use the filters" makes sense, so that the generic Views issues can be tackled separately than the issues specific to this use case. If not, we can close that other issue as a dup (or part of it) and continue on here.

plach’s picture

Well, we can split this into two issues but all the discussion is here, the parent issue links to this and what's left of this issue once you strip out the meaty part is adding a filter to a view.

Technically speaking we have two issues. In practice I really don't see the need for having two. Anyway, I am fine with either, I just wanted to make clear that we are aware of the problem you reported and that the problem space is well understood (aside maybe from the misleading labels :).

jhodgdon’s picture

Either way is fine with me. Please update the issue summary though, because it really was not clear to me (I didn't read all the comments, just the summary, who has time for it all?).

And I think the other issue I filed has other things that need to be done that are not needed for this issue, so we should leave it open for the other parts. This issue I think just needs one type of translation filter proposed on the other issue, and that other issue also proposes to take care of the UI mess and confusion that is present currently for existing language filters.

plach’s picture

I'd be glad to update the issue summary, but the approach I'm suggesting has not received consensus yet, so it'd be a bit unfair to do it now, IMHO.

jhodgdon’s picture

Issue summary: View changes

I added a couple of lines to the issue summary to at least reflect what this issue is apparently working on, and that it is not yet decided upon.

plach’s picture

Looks good, thanks.

Gábor Hojtsy’s picture

Issue tags: -sprint
dawehner’s picture

What is needed here to move forward?

Gábor Hojtsy’s picture

@dawehner: looks like some manpower to implement #11/#12 :)

Gábor Hojtsy’s picture

Status: Active » Postponed

Postponing on #2217943: Views cannot be filtered by language of translation which would add the filter we would use here.

jhodgdon’s picture

FileSize
12.24 KB

That other issue has a patch... With that patch installed, I edited the Frontpage view, and tried to add a filter.

Here are the options I saw -- these are the options available in any Language filters, not from that patch specifically:
language filter options in Views

Do I need to configure something or turn on a module to make "current language" be one of the options?

Berdir’s picture

The first option says "Current user's language" ? :)

jhodgdon’s picture

Oh, you're right!

Because it was called "Current user's language", I thought it meant "the language the current user has chosen as their default language", not "the language that is currently detected by the language detection and selection process" (e.g., if using URL detection, the language set by the language switcher).

Anyway, if I choose the misnamed "Current user's language" filter option, it does indeed work for the front page view.

Should we file a separate issue on the name of that filter option? It seems wrong to me.

jhodgdon’s picture

So anyway, once that other issue's patch is reviewed and committed (hint! review!), we can make a few-line patch to the default Front Page view to add the filter, and this issue will be resolved. I guess it will need a test too? Maybe anyway. There is a test on the other issue for the filter itself.

Gábor Hojtsy’s picture

@jhodgon: as for the misleading name of that language see #2037979: "Current user's language" views filter label is named very misleading open for a year now.

Gábor Hojtsy’s picture

Status: Postponed » Active
jhodgdon’s picture

I will possibly have time to work on this next week, unless someone else gets to it first, but I have a deadline looming, sorry!

I had some thoughts on the test for this issue. There is a test in core/modules/node/src/Tests/Views/NodeLanguageTest.php that was added on #2217943: Views cannot be filtered by language of translation. This adds a node in several languages, and tests the filter that was added on that issue, with definite languages (we want to use this filter for the home page in this issue, but set it to "current user's language").

I think the easiest way to add a test for this issue would be to edit that existing test slightly:
- Promote the nodes in that test to the front page when they are created and translated.
- After the tests that exist in the test class, add some new lines that visit the site front page, and its variations like example.com/fr or example.com/es, and verify that the correct text is/isn't shown in each variation.

Gábor Hojtsy’s picture

Title: Change default front page view of content to have a language filter: current page's language, will eliminate "duplicates" » Node views (front page, admin) do not use the proper language filter
Category: Task » Bug report
Issue summary: View changes
Status: Active » Needs review
Issue tags: -Needs tests +sprint
FileSize
8.31 KB
2.89 KB

Retitled to be more accurate. Also I decided to fold the admin view under this issue since it needs essentially the same test and the modifications to the view itself are only a few lines.

The last submitted patch, 34: 2161845-front-page-and-admin-view-language.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 34: 2161845-test-only.patch, failed testing.

Gábor Hojtsy’s picture

Lol, this is quite funny. Both patches failed the same times, but on *totally* different things. Can someone help fix the config schema? I will not have time for this on the weekend. Thanks!

vijaycs85’s picture

Status: Needs work » Needs review
FileSize
8.97 KB
679 bytes

Fixed config schema fails...

Status: Needs review » Needs work

The last submitted patch, 38: 2161845-language-filter-38.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
12.82 KB
3.85 KB

Fixing two sets of fails, but still one remains.

1. LocalePathTest was broken because the front page does not display all node translations now. Instead used the admin/content page, which does. The test tests application of path aliases. Works well.
2. NodeTranslationUITest.php tested that all translations of a node appear on the front page, that is not true anymore. It also tests read more and comment links in different languages. We can just load the corresponding language version of the front page to test them.

So those are fixed, the remaining one looks much trickier. This is LocaleContentTest and on a first look it tests very similar stuff to what we have tests for but that is not true. It tests nodes with multiple original languages. The prior language filter worked on the admin page because node_revision table has a correct langcode for the Chinese node. However, node_field_data has und(!!!) for the base fields. I think that is a problem. I tried all kinds of fun things but could not figure out why is this the case. drupalCreateNode() would default to this langcode if we did not provide one, but we did.

Adding this little snippet in LocaleContentTest before the fails shows the crucial difference. Duh.

    $result = db_query('SELECT * FROM {node_field_data}')->fetchAllAssoc('vid');
    debug($result);
    $result = db_query('SELECT * FROM {node_revision}')->fetchAllAssoc('nid');
    debug($result);

Status: Needs review » Needs work

The last submitted patch, 40: 2161845-language-filter-40.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
14.25 KB
2.67 KB

So the problem is/was that the language was added via the web UI and the language list cache was not cleared locally so when creating the node via the API, it did not let the language apply to the data table. Why does it apply to the revision table anyway is a different question and out of scope here. We should either use the web UI both for adding the language and creating the node or use the API for both. Now using the API for both, that solves the fail.

Gábor Hojtsy’s picture

This can be further optimized by removing this test from LocaleContentTest and instead add a node case which is not in English by default and does not have translations either in NodeLanguageTest. That should cover the case that was in LocaleContentTest, and this test already covers a whole lot more cases.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Add a language filter to the front page view that will limit the results to only content in the current page's language.
Swap the filter on the node admin page from a filter on original language to a filter on translation language.

Both points are really sane. As an enduser you don't want to see multiple translations thought as admin you want to have the capability.

Gábor Hojtsy’s picture

Issue summary: View changes
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed fa9e8bb and pushed to 8.0.x. Thanks!

  • alexpott committed fa9e8bb on 8.0.x
    Issue #2161845 by Gábor Hojtsy, vijaycs85 | YesCT: Fixed Node views (...
Gábor Hojtsy’s picture

Issue tags: -sprint

Woot, amazing, thanks!

jhodgdon’s picture

Status: Fixed » Closed (fixed)

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