From 8.x-3.x.

CommentFileSizeAuthor
#25 drupal-1828528-25.patch15.45 KBdawehner
#15 interdiff.txt1.03 KBtim.plunkett
#15 vdc-1828528-15.patch20.63 KBtim.plunkett
#9 interdiff.txt645 bytesdawehner
#9 drupal-1828528-9.patch20.62 KBdawehner
#7 interdiff.txt3.12 KBdawehner
#7 drupal-1828528-7.patch20.21 KBdawehner
#3 language-1828528-3.patch13.48 KBxjm
#3 interdiff.txt11.04 KBxjm
language.patch12.53 KBxjm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Needs work 
+++ b/core/modules/language/lib/Drupal/language/Plugin/views/argument/Language.phpundefined
@@ -0,0 +1,46 @@
+ * Definition of Drupal\language\Plugin\views\argument\Language.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/field/Language.phpundefined
@@ -0,0 +1,50 @@
+ * Definition of Drupal\language\Plugin\views\field\Language.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/filter/Language.phpundefined
@@ -0,0 +1,38 @@
+ * Definition of Drupal\language\Plugin\views\filter\Language.

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/ArgumentLanguage.phpundefined
@@ -0,0 +1,47 @@
+ * Definition of Drupal\views\Tests\Language\ArgumentLanguage.

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/FieldLanguage.phpundefined
@@ -0,0 +1,42 @@
+ * Definition of Drupal\views\Tests\Language\FieldLanguage.

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/FilterLanguage.phpundefined
@@ -0,0 +1,48 @@
+ * Definition of Drupal\views\Tests\Language\FilterLanguage.

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/LanguageTestBase.phpundefined
@@ -0,0 +1,78 @@
+ * Definition of Drupal\views\Tests\Language\LanguageTestBase.

Switch to "contains".

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/argument/Language.phpundefined
@@ -0,0 +1,46 @@
+ * Argument handler to accept a language.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/field/Language.phpundefined
@@ -0,0 +1,50 @@
+ * Field handler to translate a language into its readable form.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/filter/Language.phpundefined
@@ -0,0 +1,38 @@
+ * Filter by language.

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/LanguageTestBase.phpundefined
@@ -0,0 +1,78 @@
+ * Base class for all Language handler tests.

Verbs.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/argument/Language.phpundefined
@@ -0,0 +1,46 @@
+class Language extends ArgumentPluginBase {

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/field/Language.phpundefined
@@ -0,0 +1,50 @@
+class Language extends FieldPluginBase {

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/filter/Language.phpundefined
@@ -0,0 +1,38 @@
+class Language extends InOperator {

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/FieldLanguage.phpundefined
@@ -0,0 +1,42 @@
+class FieldLanguage extends LanguageTestBase {

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/FilterLanguage.phpundefined
@@ -0,0 +1,48 @@
+class FilterLanguage extends LanguageTestBase {

These class names could use some work. See #1809930: [META] Many core class names violate naming standards. I'd prefer to just name them correctly here. They'd be different from the handlers already in core, but IMO that's fine.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/argument/Language.phpundefined
@@ -0,0 +1,46 @@
+   * Override the behavior of summary_name(). Get the user friendly version
+   * of the language.
...
+   * Override the behavior of title(). Get the user friendly version
+   * of the language.

These docblocks are nonstandard.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/argument/Language.phpundefined
@@ -0,0 +1,46 @@
+  function language($langcode) {

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/field/Language.phpundefined
@@ -0,0 +1,50 @@
+  protected function defineOptions() {
...
+  public function buildOptionsForm(&$form, &$form_state) {
...
+  function render($values) {

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/filter/Language.phpundefined
@@ -0,0 +1,38 @@
+  function get_value_options() {

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/ArgumentLanguage.phpundefined
@@ -0,0 +1,47 @@
+  public function testFilter() {

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/FieldLanguage.phpundefined
@@ -0,0 +1,42 @@
+  public function testField() {

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/FilterLanguage.phpundefined
@@ -0,0 +1,48 @@
+  public function testFilter() {

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/LanguageTestBase.phpundefined
@@ -0,0 +1,78 @@
+  protected function schemaDefinition() {
...
+  protected function viewsData() {

Need docblocks.

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/field/Language.phpundefined
@@ -0,0 +1,50 @@
+    // @todo: Drupal Core dropped native language until config translation is
+    // ready, see http://drupal.org/node/1616594.

Check on the status of this.

+++ b/core/modules/views/lib/Drupal/views/Tests/Language/LanguageTestBase.phpundefined
@@ -0,0 +1,78 @@
+    // Create another language beside english.

Capitalize "English".

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Title: Add language integration for views » Add language module integration for views
xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review
FileSize
interdiff.txt11.04 KB
language-1828528-3.patch13.48 KB
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

The changes are looking perfect and i think for various reasons it is important (as there are thrown debug messages on a lot of tests).

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

Let's make sure this does actually pass the bot before we commit it since I renamed a bunch of classes. Also I'd like Gábor to take a glance at it to make sure this still makes sense in D8.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Needs work

The patch generally looks good. I caught this thing though which should be solved:

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/filter/LanguageFilter.phpundefined
@@ -0,0 +1,38 @@
+      $languages = array(
+        '***CURRENT_LANGUAGE***' => t("Current user's language"),
+        '***DEFAULT_LANGUAGE***' => t("Default site language"),
+        LANGUAGE_NOT_SPECIFIED => t('No language')
+      );
+      $languages = array_merge($languages, views_language_list());
+      $this->value_options = $languages;

I'm not sure about this. Drupal 8 certainly has 3 special languages, not 1 only. Also language_list() (as seem to be used in views_language_list()) will not return the locked languages, so you might want to be add it back again here.

But the reason of them being returned by language_list() is that users can reorder them (and/or modules can add more). So relying on that would be Drupal 8 compatible. As-is this right now it does not use the two new special languages and mischaracterizes the not specified language.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1828528-7.patch20.21 KB
interdiff.txt3.12 KB

Well we do rely on that already with some really funny code :)

  if ($all) {
    $languages = language_list();
  }
  else {
    $languages = language_list();
  }

This patches fixes that function and sets LANGUAGE_ALL as default behavior, because views is using that most of the time.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

Looks like a good improvement to me. Speaking of which, seems like the view data does not inclue the locked column for languages, so you cannot filter for languages like that, etc. I think it would be pretty useful, think listing content in known languages vs. special/unknown languages separately on an admin overview, building a view with content only in known languages, etc.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
drupal-1828528-9.patch20.62 KB
interdiff.txt645 bytes

Thanks for you feedback!

Would this helper method be useful for language.module itself?

Added the locked column.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs work » Needs review

Well, there used to be a helper like that in D7 I think and it was removed because it was not used much and was confusing to have multiple language list functions.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Okay then it is probably fine to have it just in views. Would you think, with this locking enabled that everything is setup probably?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented

Looks good to me, found no other issues.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, drupal-1828528-9.patch, failed testing.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
vdc-1828528-15.patch20.63 KB
interdiff.txt1.03 KB

Rerolled for the protected properties change.

Status: Needs review » Needs work
Issue tags: -VDC

The last submitted patch, vdc-1828528-15.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

#15: vdc-1828528-15.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, vdc-1828528-15.patch, failed testing.

webflo’s picture

webflo CreditAttribution: webflo commented
Status: Needs work » Needs review

#15: vdc-1828528-15.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, vdc-1828528-15.patch, failed testing.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs work » Needs review

#15: vdc-1828528-15.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, vdc-1828528-15.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

#15: vdc-1828528-15.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +VDC

The last submitted patch, vdc-1828528-15.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1828528-25.patch15.45 KB

Let's

+++ b/core/modules/language/lib/Drupal/language/Plugin/views/filter/LanguageFilter.phpundefined
index cf68a49..e733572 100644
--- a/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php

--- a/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php
+++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.phpundefined

This is actually part of another issue, so let's skip those files.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Reviewed & tested by the community

Looks good to me now :)

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

I've been assuming that languages will at some point become configuration entities but can't find the issue. If we made that change though then most of the code added here would be removed again. I don't mind committing it then removing it later but it'd be good to know what the eventual plan is.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

#1754246: Languages should be configuration entities is that issue.

I'd prefer to put the integration in now, since apparently various other things are blocked on it, and it adds test coverage which will be useful in the other issue.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

There are basically two parts of the patch.

  • Integrate the language table to views.
  • Provides generic handlers to filter/show any kind of langcode in various tables.

Removing the first part later is easy but the second part is actually helpful, because for example node.views.inc assumes it is there
already.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

OK fair enough. Committed/pushed to 8.x.

Status: Fixed » Closed (fixed)

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

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Assigned: xjm » Unassigned