From 2afe018dc7e380ac84f8e1f7986ce0247e9a254b Mon Sep 17 00:00:00 2001
From: Loz Calver <lozcalver@bigfork.co.uk>
Date: Wed, 14 Jun 2017 17:11:10 +0100
Subject: [PATCH] FIX: Ensure HasManyList foreign ID filter includes table name
 (fixes #7023)

---
 model/HasManyList.php           |  8 +++-
 tests/model/HasManyListTest.php | 71 ++++++++++++++++++++++++++++++++-
 tests/model/HasManyListTest.yml | 26 ++++++++++++
 3 files changed, 102 insertions(+), 3 deletions(-)
 create mode 100644 tests/model/HasManyListTest.yml

diff --git a/model/HasManyList.php b/model/HasManyList.php
index afc4979f3..d1a500f11 100644
--- a/model/HasManyList.php
+++ b/model/HasManyList.php
@@ -44,8 +44,14 @@ class HasManyList extends RelationList {
 	protected function foreignIDFilter($id = null) {
 		if ($id === null) $id = $this->getForeignID();
 
+		// Try to include the table name for the given foreign key
+		if ($table = ClassInfo::table_for_object_field($this->dataClass, $this->foreignKey)) {
+			$key = "\"$table\".\"$this->foreignKey\"";
+		} else {
+			$key = "\"$this->foreignKey\"";
+		}
+
 		// Apply relation filter
-		$key = "\"$this->foreignKey\"";
 		if(is_array($id)) {
 			return array("$key IN (".DB::placeholders($id).")"  => $id);
 		} else if($id !== null){
diff --git a/tests/model/HasManyListTest.php b/tests/model/HasManyListTest.php
index a9a72403f..e6a0b6760 100644
--- a/tests/model/HasManyListTest.php
+++ b/tests/model/HasManyListTest.php
@@ -2,16 +2,26 @@
 
 class HasManyListTest extends SapphireTest {
 
-	// Borrow the model from DataObjectTest
-	protected static $fixture_file = 'DataObjectTest.yml';
+	protected static $fixture_file = array(
+		'DataObjectTest.yml', // Borrow the model from DataObjectTest
+		'HasManyListTest.yml'
+	);
 
 	protected $extraDataObjects = array(
 		'DataObjectTest_Team',
 		'DataObjectTest_SubTeam',
 		'DataObjectTest_Player',
 		'DataObjectTest_TeamComment',
+		'DataObjectTest_Sortable',
+		'DataObjectTest_Company',
+		'DataObjectTest_EquipmentCompany',
+		'DataObjectTest_SubEquipmentCompany',
+		'DataObjectTest_Fan',
 		'ManyManyListTest_Product',
 		'ManyManyListTest_Category',
+		'HasManyListTest_Company',
+		'HasManyListTest_Employee',
+		'HasManyListTest_CompanyCar',
 	);
 
 	public function testRelationshipEmptyOnNewRecords() {
@@ -59,4 +69,61 @@ class HasManyListTest extends SapphireTest {
 		$this->assertEmpty($team2comment->TeamID);
 	}
 
+	/**
+	 * Test that multiple models with the same "has_one" relation name (and therefore the same "<hasone>ID"
+	 * column name) do not trigger a "Column '<hasone>ID' in where clause is ambiguous" error
+	 */
+	public function testAmbiguousRelationshipNames() {
+		$company = $this->objFromFixture('HasManyListTest_Company', 'silverstripe');
+
+		$johnsCars = $company->CompanyCars()->filter(array('User.Name' => 'John Smith'));
+		$this->assertCount(1, $johnsCars, 'John Smith has one company car');
+
+		$jennysCars = $company->CompanyCars()->filter(array('User.Name' => 'Jenny Smith'));
+		$this->assertCount(2, $jennysCars, 'Jenny Smith has two company cars');
+	}
+
+}
+
+class HasManyListTest_Company extends DataObject implements TestOnly {
+
+	private static $db = array(
+		'Name' => 'Varchar(100)'
+	);
+
+	private static $has_many = array(
+		'Employees' => 'HasManyListTest_Employee',
+		'CompanyCars' => 'HasManyListTest_CompanyCar'
+	);
+
+}
+
+class HasManyListTest_Employee extends DataObject implements TestOnly {
+
+	private static $db = array(
+		'Name' => 'Varchar(100)'
+	);
+
+	private static $has_one = array(
+		'Company' => 'HasManyListTest_Company'
+	);
+
+	private static $has_many = array(
+		'CompanyCars' => 'HasManyListTest_CompanyCar'
+	);
+
+}
+
+class HasManyListTest_CompanyCar extends DataObject implements TestOnly {
+
+	private static $db = array(
+		'Make' => 'Varchar(100)',
+		'Model' => 'Varchar(100)'
+	);
+
+	private static $has_one = array(
+		'User' => 'HasManyListTest_Employee',
+		'Company' => 'HasManyListTest_Company'
+	);
+
 }
diff --git a/tests/model/HasManyListTest.yml b/tests/model/HasManyListTest.yml
new file mode 100644
index 000000000..34468ac93
--- /dev/null
+++ b/tests/model/HasManyListTest.yml
@@ -0,0 +1,26 @@
+HasManyListTest_Company:
+  silverstripe:
+    Name: 'SilverStripe Ltd'
+HasManyListTest_Employee:
+  john:
+    Name: 'John Smith'
+    Company: =>HasManyListTest_Company.silverstripe
+  jenny:
+    Name: 'Jenny Smith'
+    Company: =>HasManyListTest_Company.silverstripe
+HasManyListTest_CompanyCar:
+  jaguar:
+    Make: 'Jaguar'
+    Model: 'E Type'
+    User: =>HasManyListTest_Employee.john
+    Company: =>HasManyListTest_Company.silverstripe
+  ferrari:
+    Make: 'Ferrari'
+    Model: 'F40'
+    User: =>HasManyListTest_Employee.jenny
+    Company: =>HasManyListTest_Company.silverstripe
+  lamborghini:
+    Make: 'Lamborghini'
+    Model: 'Countach'
+    User: =>HasManyListTest_Employee.jenny
+    Company: =>HasManyListTest_Company.silverstripe