Files: 
CommentFileSizeAuthor
#11 interdiff-2002482-10.txt840 byteschrisjlee
#11 2002482-ensure_path-11.patch2.74 KBchrisjlee
PASSED: [[SimpleTest]]: [MySQL] 54,973 pass(es).
[ View ]
#9 2002482-ensure_path-9.patch2.22 KBchrisjlee
FAILED: [[SimpleTest]]: [MySQL] 54,599 pass(es), 592 fail(s), and 285 exception(s).
[ View ]
#9 rerolled-9--2002482-ensure_path-6.patch1.55 KBchrisjlee
FAILED: [[SimpleTest]]: [MySQL] 54,761 pass(es), 591 fail(s), and 285 exception(s).
[ View ]
#9 interdiff-2002482-9.txt1.33 KBchrisjlee
#6 2002482-ensure_path-6.patch2.07 KBshixish
Test request sent.
[ View ]
#6 interdiff.txt849 bytesshixish
#1 ensure_path-2002482.patch2.06 KBbrennanmh
FAILED: [[SimpleTest]]: [MySQL] 55,754 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

brennanmh’s picture

Status:Active» Needs review
StatusFileSize
new2.06 KB
FAILED: [[SimpleTest]]: [MySQL] 55,754 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Changed ensure_path to ensurePath in Sql.php

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

The last submitted patch, ensure_path-2002482.patch, failed testing.

connorwk’s picture

Status:Needs work» Needs review

#1: ensure_path-2002482.patch queued for re-testing.

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

The last submitted patch, ensure_path-2002482.patch, failed testing.

oenie’s picture

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.phpundefined
@@ -587,7 +587,7 @@ function ensure_table($table, $relationship = NULL, JoinPluginBase $join = NULL)
+  function ensurePath($table, $relationship = NULL, $join = NULL, $traced = array(), $add = array()) {

Add protected access modifier in front of the function to adher to the new OOP standards.

shixish’s picture

Status:Needs work» Needs review
StatusFileSize
new849 bytes
new2.07 KB
Test request sent.
[ View ]

I added the protected keyword.

Updated patch, and interdiff attached.

oenie’s picture

Looks good to me, apart from a minor comment issue:

core/modules/views/lib/Drupal/views/ManyToOneHelper.php, Line 89:
// ensure_path logic. Perhaps it should be.

So i suggest RTBC when the test returns ok.

aspilicious’s picture

Status:Needs review» Needs work

Lets fix the comment.

chrisjlee’s picture

StatusFileSize
new1.33 KB
new1.55 KB
FAILED: [[SimpleTest]]: [MySQL] 54,761 pass(es), 591 fail(s), and 285 exception(s).
[ View ]
new2.22 KB
FAILED: [[SimpleTest]]: [MySQL] 54,599 pass(es), 592 fail(s), and 285 exception(s).
[ View ]

Attempted to reroll and caused a nasty merge conflict that undid lots other api changes (add_table() -> addTable()). Instead i just manually recreated the patch since it's a couple changes. Interdiff will show changes i made from the manually rerolled.

Also, added that comment fix as per request by aspilicious in #8.

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.phpundefined
@@ -632,7 +632,7 @@ function ensure_path($table, $relationship = NULL, $join = NULL, $traced = array
     $traced[$join->leftTable] = TRUE;
-    return $this->ensure_path($join->leftTable, $relationship, $left_join, $traced, $add);
+    return $this->ensurePpath($join->leftTable, $relationship, $left_join, $traced, $add);

Finally, also fixed the typo in the method name. Also, there's a typo here. "ensurePpath" -> "ensurePath".

chrisjlee’s picture

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

Whoops forgot to update tags / status

chrisjlee’s picture

StatusFileSize
new2.74 KB
PASSED: [[SimpleTest]]: [MySQL] 54,973 pass(es).
[ View ]
new840 bytes

Missed one more ensure_path. This should turn green. the other should turn red.

oenie’s picture

Status:Needs review» Reviewed & tested by the community

Looks good to me now !

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 17c06e0 and pushed to 8.x. Thanks!

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