Problem/Motivation
Prior to #1740492: Implement a default entity views data handler, node views only had language fields/filters if the Language module was enabled. This meant that views with these filters/fields (including the default views shipped with Drupal Core for the /node home page) had broken/missing handlers if the Language module is not installed.
This has now been fixed, but the labels for the language filters/fields were a bit off. This issue was at one time about making sure the language filters/fields were included, but has now been repurposed to fix the labels.
Proposed resolution
Fix the labels so they are accurate.
Remaining tasks
Review and commit the patch. Before screen shot is in #14, analysis is in #17, first patch and discussion is in #18, see following comments for reviews and/or new patches.
User interface changes
Filters for node language will have more sensible labels.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#27 | interdiff.txt | 1.26 KB | jhodgdon |
#27 | 2320521-fix-labels-and-tests.patch | 3.05 KB | jhodgdon |
#18 | nodefiltersfixed.png | 19.14 KB | jhodgdon |
#14 | nodefilters.png | 20.05 KB | jhodgdon |
Comments
Comment #1
jhodgdonComment #2
jhodgdonDependent issue is done. Here is a patch.
Comment #4
jhodgdonHm. I guess this simple fix is not going to work.
The reason for all of these test failures is that the default front page view now has a filter that says "Only show translations in the currently negotiated language".
Without this patch, and without the Language module enabled, that filter, if you looked it in Views, would have shown you "Broken/missing handler", because the filter was only defined if the Language module is enabled. I believe that in Views 7.x, if you had a broken handler the view wouldn't display (not sure about that), but in 8.x, it just skips adding any WHERE clauses for the broken filter, so the view works and languages are not filtered.
But with this patch applied, the filter is enabled and is trying to filter to "translations in English". However, the nodes themselves have no language set, so nothing is being displayed.
So... I think that the actual right thing to do is to add to this patch a small change to the Front Page view (and any others that are breaking in these tests) so that they show both nodes in the currently negotiated language and without a defined language.
This way you will not see "broken/missing handler" in Views if you install Drupal and want to edit the default home page view (or these others), because the language filters will be defined, but (hopefully) the tests will also pass.
I'll see what I can do.
Comment #5
jhodgdonOK, here's a new patch. Fixes the test failures in the first Comment test at least, locally... let's see if it breaks anything else or leaves anything out.
Comment #6
jhodgdonThat did the trick -- all the tests passed. Review please?
Comment #7
Gábor HojtsyI don't think its the right fix to include those languages, while it looks like a sensible stop-gap, we don't want those nodes to show up on the front page. The problem you are seeing is a side effect of #1966436: Default *content* entity languages are not set for entities created with the API. We can postpone this issue on that one, if that helps. Once that is in, nodes should be created properly in English even if no language module. Which is the intention in the first place. Entities created with no language without the language module enabled is wrooooong!
Comment #8
jhodgdonOK, yes, let's postpone until that is done, because the tests will not pass without the stop-gap without a fix.
Comment #9
jhodgdonAdded summary of current status of this issue.
Comment #10
jhodgdonAlso, #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 #11
andypostComment #12
Gábor Hojtsy@andypost: #1740492: Implement a default entity views data handler was not yet committed? That is what this one was postponed on, right?
Comment #13
andypostok, let's [pp-1] maybe this one would be closed as duplicate as #10 said
Comment #14
jhodgdon#1740492: Implement a default entity views data handler has been committed, so it's time to get back to this issue.
I just installed the latest D8 and without the Language module enabled, there are currently 3 filters showing in Views UI for a node view:
So, something probably needs to be adjusted here and investigated.
Comment #15
Gábor HojtsyThat looks like a regression most likely introduced by #1740492: Implement a default entity views data handler to me :/
Comment #16
jhodgdonYes, probably easy to fix too. I'd rather do it here than go back to that issue...
Comment #17
Gábor HojtsyLooking for sources of the two:
NodeViewsData.php has these for languages:
Looking at the form IDs for the 3 languages you listed, they are (in order of appearance):
Indeed both node_field_revision and node_revision tables have a langcode. Looks like based on the code the node_revision one gets the nicer description, while the node_field_revision one does not. The node_field_revision value however is the ORIGINAL language code of prior revisions, no? Not sure whether we should expose that, it sounds very confusing with all the rest :D
Comment #18
jhodgdonI discussed this with Gabor in IRC. There really do need to be 3 filters/fields here:
- node_revision.langcode - this is the original language of this particular revision's translations.
- node_field_revision.langcode - this is the language of this particular revision, in this particular translation
- node_field_data.langcode - this is the language of the active revision, in this particular translation.
Note that the node base table does not have langcode on it, so there is no shortcut for "the original language of this particular revision".
Here's a patch that fixes up the labels. And a screenshot showing what it looks like:
Note that I fixed this on the base Views entity language controller. We inadvertently put the wrong labels on those fields -- base/revision tables, if they have language codes, are the language codes of the original language, for the revision/current revision, while the data and revision data tables are language codes for translations.
And the fix on the Node class is for the longer help, as is already being done for the other langcode fields.
Comment #19
jhodgdonAnd just as a note, every other base field also has this duplication between Revision and Base tables, such as publication status, title, etc. So that is I guess by design and is not specific to langcode...
Comment #20
jhodgdonUpdated title and summary to reflect having repurposed this issue.
Comment #21
Gábor HojtsyThe changes look good.
That leaves the question though why there is no content original language (as opposed to revision)? Is that because its not in out data model? Or not yet exposed? Sorry not at computer, could not check.
Comment #22
tstoecklerRe
and
In terms of our data model the default language of the default revision can be accessed by filtering on
node_field_data.default_language = 1
. Not sure how to get views to do that in a field or a filter.Comment #23
jhodgdonIf you used "Revision: Original Language" in a Node based view, you would be filtering on the current revision, because of the way the joins are done. There isn't another filter/field available, simply because the node table doesn't have a langcode field in it.
Comment #24
Gábor HojtsyYeah do you can get the translation language either way, but the original language is only on the revision. It is unfortunate we need to expose so much about the particularities of the data model in this way. Anyway I think the fixes are fine. The "missing" filter may not resolvable anyway. Certainly should not be here. Thought a committer would ask.
Comment #26
jhodgdonAh, need to adjust the EntityViewsTest, which another issue has now added to the code base (it was in the wrong namespace and wasn't being run).
Comment #27
jhodgdonHere's a new patch, which also makes corresponding changes in the test.
Comment #28
Gábor HojtsyLooks great. I am also upping this to major bug, since the labels are swapped for original language vs. translation language so whoever wants to build a view would build the wrong kind of view vs. what they wanted.
Comment #29
dawehnerHa, there was even a test for it.
+1 in general
Comment #30
alexpottCommitted 8ff6a61 and pushed to 8.0.x. Thanks!
Comment #32
Gábor HojtsyAmazing, thanks!