From c4eac5310e1f4070ad05acf6dc0200e9fc025636 Mon Sep 17 00:00:00 2001
From: Jeremy Shipman <jeremy@burnbright.net>
Date: Fri, 19 Apr 2013 15:45:43 +1200
Subject: [PATCH] FIX: Instead of CsvBulkLoader->findExistingRecord out right
 failing (i.e. no duplicate found) when the duplicate check field is empty, it
 will now continue on to check other duplicateCheck fields. Added extra
 testing data to CSVBulkLoaderTest so that it fails.

---
 dev/BulkLoader.php                            |  2 +-
 dev/CsvBulkLoader.php                         | 14 +++++---------
 tests/dev/CsvBulkLoaderTest.php               |  5 ++++-
 tests/dev/CsvBulkLoaderTest_PlayersWithId.csv |  9 +++++----
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/dev/BulkLoader.php b/dev/BulkLoader.php
index dbdc92c5f..269b7d140 100644
--- a/dev/BulkLoader.php
+++ b/dev/BulkLoader.php
@@ -101,7 +101,7 @@ abstract class BulkLoader extends ViewableData {
 	 * 
 	 * {@see Member::$unique_identifier_field}.
 	 * 
-	 * If multiple checks are specified, the first one "wins".
+	 * If multiple checks are specified, the first non-empty field "wins".
 	 * 
 	 *  <code>
 	 * <?php
diff --git a/dev/CsvBulkLoader.php b/dev/CsvBulkLoader.php
index eebc97eeb..0fb246be9 100644
--- a/dev/CsvBulkLoader.php
+++ b/dev/CsvBulkLoader.php
@@ -162,15 +162,12 @@ class CsvBulkLoader extends BulkLoader {
 	 */
 	public function findExistingObject($record) {
 		$SNG_objectClass = singleton($this->objectClass);
-		
 		// checking for existing records (only if not already found)
 		foreach($this->duplicateChecks as $fieldName => $duplicateCheck) {
 			if(is_string($duplicateCheck)) {
 				$SQL_fieldName = Convert::raw2sql($duplicateCheck); 
-				if(!isset($record[$fieldName])) {
-					return false;
-					//user_error("CsvBulkLoader:processRecord: Couldn't find duplicate identifier '{$fieldName}'
-					//in columns", E_USER_ERROR);
+				if(!isset($record[$fieldName]) || empty($record[$fieldName])) { //skip current duplicate check if field value is empty
+					continue;
 				}
 				$SQL_fieldValue = Convert::raw2sql($record[$fieldName]);
 				$existingRecord = DataObject::get_one($this->objectClass, "\"$SQL_fieldName\" = '{$SQL_fieldValue}'");
@@ -184,17 +181,16 @@ class CsvBulkLoader extends BulkLoader {
 					user_error("CsvBulkLoader::processRecord():"
 						. " {$duplicateCheck['callback']} not found on importer or object class.", E_USER_ERROR);
 				}
-				
-				if($existingRecord) return $existingRecord;
+				if($existingRecord) {
+					return $existingRecord;
+				}
 			} else {
 				user_error('CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks', E_USER_ERROR);
 			}
 		}
-		
 		return false;
 	}
 	
-	
 	/**
 	 * Determine wether any loaded files should be parsed
 	 * with a header-row (otherwise we rely on {@link self::$columnMap}.
diff --git a/tests/dev/CsvBulkLoaderTest.php b/tests/dev/CsvBulkLoaderTest.php
index 6ed542165..d1df15000 100644
--- a/tests/dev/CsvBulkLoaderTest.php
+++ b/tests/dev/CsvBulkLoaderTest.php
@@ -151,7 +151,10 @@ class CsvBulkLoaderTest extends SapphireTest {
 		$loader = new CsvBulkLoader('CsvBulkLoaderTest_Player');
 		$filepath = $this->getCurrentAbsolutePath() . '/CsvBulkLoaderTest_PlayersWithId.csv';
 		$loader->duplicateChecks = array(
-			'ExternalIdentifier' => 'ExternalIdentifier' 
+			'ExternalIdentifier' => 'ExternalIdentifier',
+			'NonExistantIdentifier' => 'ExternalIdentifier',
+			'ExternalIdentifier' => 'ExternalIdentifier',
+			'AdditionalIdentifier' => 'ExternalIdentifier'
 		);
 		$results = $loader->load($filepath);
 		$createdPlayers = $results->Created();
diff --git a/tests/dev/CsvBulkLoaderTest_PlayersWithId.csv b/tests/dev/CsvBulkLoaderTest_PlayersWithId.csv
index dc6e37ebb..b1460f41c 100644
--- a/tests/dev/CsvBulkLoaderTest_PlayersWithId.csv
+++ b/tests/dev/CsvBulkLoaderTest_PlayersWithId.csv
@@ -1,4 +1,5 @@
-"ExternalIdentifier","FirstName","Biography","Birthday"
-222b,"John","","31/01/1988"
-222b,"John","He's a good guy",""
-9000a,"Jamie","Pretty old\, with an escaped comma","31/01/1882"
\ No newline at end of file
+"ExternalIdentifier","FirstName","Biography","Birthday","  AdditionalIdentifier " 
+222b,"John","","31/01/1988",""
+222b,"John","He's a good guy","",""
+9000a,"Jamie","Pretty old\, with an escaped comma","31/01/1882",""
+,"Fiona","A wise woman","","sql'unsafeid"
\ No newline at end of file