add test for class migration

Author: lekoala

src/ExcelBulkLoader.php

@@ -109,43 +109,34 @@ public function load($filepath)
         Environment::increaseTimeLimitTo(3600);
         Environment::increaseMemoryLimitTo('512M');
 
-        if ($this->useTransaction) {
-            DB::get_conn()->transactionStart();
-        }
-
-        try {
-            //get all instances of the to be imported data object
-            if ($this->deleteExistingRecords) {
-                if ($this->getCheckPermissions()) {
-                    // We need to check each record, in case there's some fancy conditional logic in the canDelete method.
-                    // If we can't delete even a single record, we should bail because otherwise the result would not be
-                    // what the user expects.
-                    /** @var DataObject $record */
-                    foreach (DataObject::get($this->objectClass) as $record) {
-                        if (!$record->canDelete()) {
-                            $type = $record->i18n_singular_name();
-                            throw new HTTPResponse_Exception(
-                                _t(__CLASS__ . '.CANNOT_DELETE', "Not allowed to delete '{type}' records", ["type" => $type]),
-                                403
-                            );
-                        }
+        //get all instances of the to be imported data object
+        if ($this->deleteExistingRecords) {
+            if ($this->getCheckPermissions()) {
+                // We need to check each record, in case there's some fancy conditional logic in the canDelete method.
+                // If we can't delete even a single record, we should bail because otherwise the result would not be
+                // what the user expects.
+                /** @var DataObject $record */
+                foreach (DataObject::get($this->objectClass) as $record) {
+                    if (!$record->canDelete()) {
+                        $type = $record->i18n_singular_name();
+                        throw new HTTPResponse_Exception(
+                            _t(__CLASS__ . '.CANNOT_DELETE', "Not allowed to delete '{type}' records", ["type" => $type]),
+                            403
+                        );
                     }
                 }
-                DataObject::get($this->objectClass)->removeAll();
             }
-
-            $result = $this->processAll($filepath);
-
             if ($this->useTransaction) {
-                DB::get_conn()->transactionEnd();
+                DB::get_conn()->transactionStart();
             }
-        } catch (Exception $e) {
+            DataObject::get($this->objectClass)->removeAll();
             if ($this->useTransaction) {
-                DB::get_conn()->transactionRollback();
+                DB::get_conn()->transactionEnd();
             }
-            $code = $e->getCode() ?: 500;
-            throw new HTTPResponse_Exception($e->getMessage(), $code);
         }
+
+        $result = $this->processAll($filepath);
+
         return $result;
     }
 
@@ -186,13 +177,11 @@ protected function processAll($filepath, $preview = false)
     {
         $this->extend('onBeforeProcessAll', $filepath, $preview);
 
-        $results = new BulkLoader_Result();
-        $ext = $this->getUploadFileExtension();
-
         if (!is_readable($filepath)) {
             throw new Exception("Cannot read $filepath");
         }
 
+        $ext = $this->getUploadFileExtension();
         $opts = [
             'separator' => $this->delimiter,
             'enclosure' => $this->enclosure,
@@ -204,22 +193,50 @@ protected function processAll($filepath, $preview = false)
 
         $data = SpreadCompat::read($filepath, ...$opts);
 
+        $results = $this->processData($data, $preview);
+
+        $this->extend('onAfterProcessAll', $result, $preview);
+
+        return $results;
+    }
+
+    /**
+     * @param iterable $data
+     * @param bool $preview
+     * @return BulkLoader_Result
+     */
+    public function processData($data, $preview = false)
+    {
+        $results = new BulkLoader_Result();
+
         $objectClass = $this->objectClass;
         $objectConfig = $objectClass::config();
         $this->db = $objectConfig->db;
         $this->singleton = singleton($objectClass);
 
-        foreach ($data as $row) {
-            $this->processRecord(
-                $row,
-                $this->columnMap,
-                $results,
-                $preview
-            );
+        try {
+            if ($this->useTransaction) {
+                DB::get_conn()->transactionStart();
+            }
+            foreach ($data as $row) {
+                $this->processRecord(
+                    $row,
+                    $this->columnMap,
+                    $results,
+                    $preview
+                );
+            }
+            if ($this->useTransaction) {
+                DB::get_conn()->transactionEnd();
+            }
+        } catch (Exception $e) {
+            if ($this->useTransaction) {
+                DB::get_conn()->transactionRollback();
+            }
+            $code = $e->getCode() ?: 500;
+            throw new HTTPResponse_Exception($e->getMessage(), $code);
         }
 
-        $this->extend('onAfterProcessAll', $result, $preview);
-
         return $results;
     }
 

src/ExcelGroupBulkLoader.php

@@ -16,9 +16,9 @@ class ExcelGroupBulkLoader extends ExcelBulkLoader
     /**
      * @var array<string,string>
      */
-    public $duplicateChecks = array(
+    public $duplicateChecks = [
         'Code' => 'Code',
-    );
+    ];
 
     /**
      * @param class-string $objectClass

src/ExcelMemberBulkLoader.php

@@ -24,9 +24,9 @@ class ExcelMemberBulkLoader extends ExcelBulkLoader
     /**
      * @var array<string,string>
      */
-    public $duplicateChecks = array(
+    public $duplicateChecks = [
         'Email' => 'Email',
-    );
+    ];
 
     /**
      * @param class-string $objectClass

tests/ExcelImportExportTest.php

@@ -126,6 +126,33 @@ public function testImportMultipleClasses(): void
         $this->assertEquals($newTotalCount, $totalCount);
     }
 
+    public function testImportMigrateClasses(): void
+    {
+        // For members, duplicate is done on emails
+        $duplicateEmail = '[email protected]';
+
+        $baseMember = new Member();
+        $baseMember->Email = $duplicateEmail;
+        $baseMember->write();
+
+        $loader = new ExcelMemberBulkLoader();
+        $result = $loader->processData([
+            [
+                'ID' => $baseMember->ID,
+                'ClassName' => TestExcelMember::class,
+                'Email' => $duplicateEmail,
+            ]
+        ]);
+
+        $this->assertEquals(0, $result->CreatedCount());
+        $this->assertEquals(1, $result->UpdatedCount());
+
+        // Check that our existing member has the new class
+        $member = TestExcelMember::get()->filter('Email', $duplicateEmail)->first();
+        $this->assertNotEmpty($member);
+        $this->assertEquals($baseMember->ID, $member->ID);
+    }
+
     public function testTransaction()
     {
         $beforeCount = Member::get()->count();