From 39d1ef7a4b2b83337b1ac4bd7d3c03a3cf33f1d6 Mon Sep 17 00:00:00 2001
From: Antony Thorpe <Antony.Thorpe@budget12.com>
Date: Wed, 1 Jun 2016 09:55:06 +1200
Subject: [PATCH] Bugfix: BulkLoader_Result class - Deleted Items (#5598)

* Bugfix: BulkLoader_Result class - Deleted Items

The Deleted function, within BulkLoader_Result class, calls mapToArrayList, which tries to find the recently deleted dataobjects.
To correct:
- when calling addDeleted, store a map of each dataobject within the $this->deleted array; and,
- have the Deleted function return an ArrayList based upon the $this->deleted array.

* Added tests

- created a new test file for the BulkLoader_Result class
- included tests for other methods of this class
- slightly altered the addDeleted function to be consistent other methods of this class
---
 dev/BulkLoader.php                 | 14 +++--
 tests/dev/BulkLoaderResultTest.php | 94 ++++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+), 6 deletions(-)
 create mode 100644 tests/dev/BulkLoaderResultTest.php

diff --git a/dev/BulkLoader.php b/dev/BulkLoader.php
index 98765e813..e90670619 100644
--- a/dev/BulkLoader.php
+++ b/dev/BulkLoader.php
@@ -343,7 +343,11 @@ class BulkLoader_Result extends Object {
 	 * @return ArrayList
 	 */
 	public function Deleted() {
-		return $this->mapToArrayList($this->deleted);
+		$set = new ArrayList();
+		foreach ($this->deleted as $arrItem) {
+			$set->push(ArrayData::create($arrItem));
+		}
+		return $set;
 	}
 
 	/**
@@ -386,11 +390,9 @@ class BulkLoader_Result extends Object {
 	 * @param $message string
 	 */
 	public function addDeleted($obj, $message = null) {
-		$this->deleted[] = $this->lastChange = array(
-			'ID' => $obj->ID,
-			'ClassName' => $obj->class,
-			'Message' => $message
-		);
+		$data = $obj->toMap();
+		$data['_BulkLoaderMessage'] = $message;
+		$this->deleted[] = $this->lastChange = $data;
 		$this->lastChange['ChangeType'] = 'deleted';
 	}
 
diff --git a/tests/dev/BulkLoaderResultTest.php b/tests/dev/BulkLoaderResultTest.php
new file mode 100644
index 000000000..15b2dbdc1
--- /dev/null
+++ b/tests/dev/BulkLoaderResultTest.php
@@ -0,0 +1,94 @@
+<?php
+/**
+ * @package framework
+ * @subpackage tests
+ */
+class BulkLoaderResultTest extends SapphireTest
+{
+    protected $extraDataObjects = array('BulkLoaderTestPlayer');
+
+    public function setUp()
+    {
+        parent::setUp();
+        BulkLoaderTestPlayer::create(array('Name' => 'Vincent', 'Status' => 'Available'))->write();
+    }
+
+    public function testBulkLoaderResultCreated()
+    {
+        $results = BulkLoader_Result::create();
+        $player = BulkLoaderTestPlayer::create(array('Name' => 'Rangi', 'Status' => 'Possible'));
+        $player->write();
+        $results->addCreated($player, 'Speedster');
+
+        $this->assertEquals($results->CreatedCount(), 1);
+        $this->assertSame(
+            'Rangi',
+            $results->Created()->find('Name', 'Rangi')->Name,
+            'The player Rangi should be recorded as created in $results'
+        );
+        $this->assertSame(
+            'Possible',
+            $results->Created()->find('Name', 'Rangi')->Status,
+            'The player Rangi should have Status of "Possible" in $results'
+        );
+        $this->assertSame(
+            'Speedster',
+            $results->Created()->find('Name', 'Rangi')->_BulkLoaderMessage,
+            'Rangi should have _BulkLoaderMessage of Speedster'
+        );
+    }
+
+    public function testBulkLoaderResultDeleted()
+    {
+        $results = BulkLoader_Result::create();
+        $player = BulkLoaderTestPlayer::get()->find('Name', 'Vincent');
+        $results->addDeleted($player, 'Retired');
+        $player->delete();
+
+        $this->assertEquals($results->DeletedCount(), 1);
+        $this->assertSame(
+            'Vincent',
+            $results->Deleted()->find('Name', 'Vincent')->Name,
+            'The player Vincent should be recorded as deleted'
+        );
+        $this->assertSame(
+            'Retired',
+            $results->Deleted()->find('Name', 'Vincent')->_BulkLoaderMessage,
+            'Vincent should have a _BulkLoaderMessage of Retired'
+        );
+    }
+
+    public function testBulkLoaderResultUpdated()
+    {
+        $results = BulkLoader_Result::create();
+        $player = BulkLoaderTestPlayer::get()->find('Name', 'Vincent');
+        $player->Status = 'Unavailable';
+        $player->write();
+        $results->addUpdated($player, 'Injured');
+
+        $this->assertEquals($results->UpdatedCount(), 1);
+        $this->assertSame(
+            'Vincent',
+            $results->Updated()->find('Name', 'Vincent')->Name,
+            'The player Vincent should be recorded as updated'
+        );
+        $this->assertSame(
+            'Unavailable',
+            $results->Updated()->find('Name', 'Vincent')->Status,
+            'The player Vincent should have a Status of Unavailable'
+        );
+        $this->assertSame(
+            'Injured',
+            $results->Updated()->find('Name', 'Vincent')->_BulkLoaderMessage,
+            'Vincent is injured'
+        );
+    }
+}
+
+class BulkLoaderTestPlayer extends DataObject implements TestOnly
+{
+    private static $db = array(
+        'Name' => 'Varchar',
+        'Status' => 'Varchar',
+    );
+}