JoinPluginBase doesn't allow extra conditions on left table

Sorry but this is not a bug, but rather a feature/task.

Awesome!

This issue is a normal task so we need to outline how it fits within the allowable Drupal 8 beta criteria. Can someone add Drupal 8 beta phase evaluation template to the issue summary.

@alexpott
Automated answer?

@dawehner unfortunately

Unfrozen because it only changes JoinPluginBase ...

Is not true. There is no agreement that internal refactors are unfrozen.

+++ b/core/modules/views/src/Plugin/views/join/JoinPluginBase.php
+++ b/core/modules/views/src/Plugin/views/join/JoinPluginBase.php
@@ -257,18 +257,27 @@ public function buildJoin($select_query, $table, $view_query) {

@@ -257,18 +257,27 @@ public function buildJoin($select_query, $table, $view_query) {
           else {
             // With a single value, the '=' operator is implicit.
             $operator = !empty($info['operator']) ? $info['operator'] : '=';
+            $placeholder = ':views_join_condition_' . $select_query->nextPlaceholder();
+          }
+          // Set 'field' as join table field if available or set 'left field' as
+          // join table field is not set.
+          if (isset($info['field'])) {
+            $join_table_field = "$join_table$info[field]";
             // Allow the value to be set either with the 'value' element or
             // with 'left_field'.
             if (isset($info['left_field'])) {
               $placeholder = "$left[alias].$info[left_field]";
             }
             else {
-              $placeholder = ':views_join_condition_' . $select_query->nextPlaceholder();
               $arguments[$placeholder] = $info['value'];
             }
           }
-
-          $extras[] = "$join_table$info[field] $operator $placeholder";
+          // Set 'left field' as join table field is not set.
+          else {
+            $join_table_field = "$left[alias].$info[left_field]";
+            $arguments[$placeholder] = $info['value'];
+          }
+          $extras[] = "$join_table_field $operator $placeholder";
         }

... It would be so great if you could add an example at the top of the class, which would explain how to join on a field with a given value.

You added the appropriate test coverage, so things are nice, and nearly RTBC.

We have some good docs, we have some tests

+++ b/core/modules/views/src/Tests/Plugin/JoinTest.php
@@ -174,7 +175,36 @@ public function testBasePlugin() {
+    $configuration['extra'] = array(
+      array(
+        'field' => 'langcode',
+        'value' => 'en'
+      ),
+      array(
+        'left_field' => 'status',
+        'value' => 0,
+        'numeric' => TRUE,
+      ),
+      array(
+        'field' => 'name',
+        'left_field' => 'name'
+      ),
+    );
+    $join = $this->manager->createInstance('standard', $configuration);
+    $table = array('alias' => 'users5');
+    $join->buildJoin($query, $table, $view->query);
+
+    $tables = $query->getTables();
+    $join_info = $tables['users5'];
+    $this->assertTrue(strpos($join_info['condition'], "views_test_data.uid = users5.uid") !== FALSE, 'Make sure the join condition appears in the query.');
+    $this->assertTrue(strpos($join_info['condition'], "users5.langcode = :views_join_condition_6") !== FALSE, 'Make sure the first extra join condition appears in the query.');
+    $this->assertTrue(strpos($join_info['condition'], "views_test_data.status = :views_join_condition_7") !== FALSE, 'Make sure the second extra join condition appears in the query.');
+    $this->assertTrue(strpos($join_info['condition'], "users5.name = views_test_data.name") !== FALSE, 'Make sure the third extra join condition appears in the query.');

It feels like we should also be testing the $join_info['arguments'] contains the expected values.

The feedback from alex got adressed

I'm committing this because it helps contrib modules do more with core and is not disruptive. Committed 155a314 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

This issue shoud be backported to D7. There's a separate issue with a patch tests in green: #2642100: views_join doesn't allow extra conditions on left table.