FIX ModelAdmin::$model_importers alias support

Hasn't been considered when introducing aliases rather than class names in $managed_models.
silverstripe · May 13, 2021 · ce9c294 · ce9c294
1 parent 15258b2
commit ce9c294
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 36 deletions.
diff --git a/code/ModelAdmin.php b/code/ModelAdmin.php
@@ -2,33 +2,34 @@
 
 namespace SilverStripe\Admin;
 
-use SilverStripe\Control\Controller;
-use SilverStripe\Control\HTTPRequest;
-use SilverStripe\Control\HTTPResponse;
+use SilverStripe\Forms\Form;
 use SilverStripe\Core\Convert;
+use SilverStripe\ORM\ArrayList;
 use SilverStripe\Dev\BulkLoader;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\View\ArrayData;
 use SilverStripe\Dev\Deprecation;
-use SilverStripe\Forms\CheckboxField;
 use SilverStripe\Forms\FieldList;
 use SilverStripe\Forms\FileField;
-use SilverStripe\Forms\Form;
 use SilverStripe\Forms\FormAction;
+use SilverStripe\Dev\CsvBulkLoader;
+use SilverStripe\Forms\HiddenField;
+use SilverStripe\Security\Security;
+use SilverStripe\Control\Controller;
+use SilverStripe\Forms\LiteralField;
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Forms\CheckboxField;
+use SilverStripe\Control\HTTPResponse;
+use SilverStripe\ORM\ValidationResult;
 use SilverStripe\Forms\GridField\GridField;
 use SilverStripe\Forms\GridField\GridFieldConfig;
-use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor;
+use SilverStripe\Forms\GridField\GridFieldPaginator;
 use SilverStripe\Forms\GridField\GridFieldDetailForm;
+use SilverStripe\Forms\GridField\GridFieldPrintButton;
 use SilverStripe\Forms\GridField\GridFieldExportButton;
 use SilverStripe\Forms\GridField\GridFieldFilterHeader;
 use SilverStripe\Forms\GridField\GridFieldImportButton;
-use SilverStripe\Forms\GridField\GridFieldPaginator;
-use SilverStripe\Forms\GridField\GridFieldPrintButton;
-use SilverStripe\Forms\HiddenField;
-use SilverStripe\Forms\LiteralField;
-use SilverStripe\ORM\ArrayList;
-use SilverStripe\ORM\DataObject;
-use SilverStripe\ORM\ValidationResult;
-use SilverStripe\Security\Security;
-use SilverStripe\View\ArrayData;
+use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor;
 
 /**
  * Generates a three-pane UI for editing model classes, tabular results and edit forms.
@@ -485,23 +486,25 @@ public function getManagedModels()
      * with a default {@link CsvBulkLoader} class. In this case the column names of the first row
      * in the CSV file are assumed to have direct mappings to properties on the object.
      *
-     * @return array Map of model class names to importer instances
+     * @return array Map of model keys to importer instances (same keys as $managed_models)
      */
     public function getModelImporters()
     {
-        $importerClasses = $this->config()->get('model_importers');
+        $importers = [];
+        $importerSpec = $this->config()->get('model_importers');
+        $models = $this->getManagedModels();
 
-        // fallback to all defined models if not explicitly defined
-        if (is_null($importerClasses)) {
-            $models = $this->getManagedModels();
-            foreach ($models as $modelName => $options) {
-                $importerClasses[$modelName] = 'SilverStripe\\Dev\\CsvBulkLoader';
+        foreach ($models as $modelName => $options) {
+            $modelClass = $options['dataClass'];
+            if (isset($importerSpec[$modelName])) {
+                $importerClass = $importerSpec[$modelName];
+            } elseif (isset($importerSpec[$modelClass])) {
+                $importerClass = $importerSpec[$modelClass];
+            } else {
+                $importerClass = CsvBulkLoader::class;
             }
-        }
-
-        $importers = array();
-        foreach ($importerClasses as $modelClass => $importerClass) {
-            $importers[$modelClass] = new $importerClass($modelClass);
+            // Needs to be indexed by name to avoid collisions
+            $importers[$modelName] = new $importerClass($modelClass);
         }
 
         return $importers;
@@ -608,7 +611,7 @@ public function import($data, $form, $request)
 
         $importers = $this->getModelImporters();
         /** @var BulkLoader $loader */
-        $loader = $importers[$this->modelClass];
+        $loader = $importers[$this->modelTab];
 
         // File wasn't properly uploaded, show a reminder to the user
         if (empty($_FILES['_CsvFile']['tmp_name']) ||

diff --git a/tests/php/ModelAdminTest.php b/tests/php/ModelAdminTest.php
@@ -2,17 +2,19 @@
 
 namespace SilverStripe\Admin\Tests;
 
-use SilverStripe\Admin\Tests\ModelAdminTest\Contact;
-use SilverStripe\Admin\Tests\ModelAdminTest\Player;
-use SilverStripe\Control\HTTPRequest;
+use SilverStripe\View\ArrayData;
 use SilverStripe\Control\Session;
+use SilverStripe\Dev\CsvBulkLoader;
+use SilverStripe\Dev\FunctionalTest;
+use SilverStripe\Control\HTTPRequest;
+use SilverStripe\Security\Permission;
 use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Admin\Tests\ModelAdminTest\Player;
+use SilverStripe\Admin\Tests\ModelAdminTest\Contact;
+use SilverStripe\Forms\GridField\GridFieldPrintButton;
 use SilverStripe\Forms\GridField\GridFieldExportButton;
 use SilverStripe\Forms\GridField\GridFieldImportButton;
-use SilverStripe\Forms\GridField\GridFieldPrintButton;
-use SilverStripe\Security\Permission;
-use SilverStripe\Dev\FunctionalTest;
-use SilverStripe\View\ArrayData;
+use SilverStripe\Admin\Tests\ModelAdminTest\ModelAdminTestBulkLoader;
 
 class ModelAdminTest extends FunctionalTest
 {
@@ -164,6 +166,45 @@ public function testGetManagedModels()
         );
     }
 
+    public function testGetModelImporters()
+    {
+        $admin = new ModelAdminTest\MultiModelAdmin();
+        $importers = $admin->getModelImporters();
+
+        $this->assertArrayHasKey(
+            Contact::class,
+            $importers,
+            'Implicit models'
+        );
+        $this->assertInstanceOf(
+            CsvBulkLoader::class,
+            $importers[Contact::class],
+            'Implicit models create default bulk loaders'
+        );
+
+        $this->assertArrayHasKey(
+            'Player',
+            $importers,
+            'Explicit models by alias'
+        );
+        $this->assertInstanceOf(
+            ModelAdminTestBulkLoader::class,
+            $importers['Player'],
+            'Explicit models by alias allow custom bulk loaders'
+        );
+
+        $this->assertArrayHasKey(
+            Player::class,
+            $importers,
+            'Explicit models by class'
+        );
+        $this->assertInstanceOf(
+            ModelAdminTestBulkLoader::class,
+            $importers[Player::class],
+            'Explicit models by alias allow custom bulk loaders'
+        );
+    }
+
     public function testGetManagedModelTabs()
     {
         $mock = $this->getMockBuilder(ModelAdminTest\MultiModelAdmin::class)

diff --git a/tests/php/ModelAdminTest/ModelAdminTestBulkLoader.php b/tests/php/ModelAdminTest/ModelAdminTestBulkLoader.php
@@ -0,0 +1,10 @@
+<?php
+
+namespace SilverStripe\Admin\Tests\ModelAdminTest;
+
+use SilverStripe\Dev\CsvBulkLoader;
+
+class ModelAdminTestBulkLoader extends CsvBulkLoader
+{
+
+}
diff --git a/tests/php/ModelAdminTest/MultiModelAdmin.php b/tests/php/ModelAdminTest/MultiModelAdmin.php
@@ -2,9 +2,10 @@
 
 namespace SilverStripe\Admin\Tests\ModelAdminTest;
 
+use SilverStripe\Dev\TestOnly;
 use SilverStripe\Admin\ModelAdmin;
 use SilverStripe\Control\Controller;
-use SilverStripe\Dev\TestOnly;
+use SilverStripe\Admin\Tests\ModelAdminTest\ModelAdminTestBulkLoader;
 
 class MultiModelAdmin extends ModelAdmin implements TestOnly
 {
@@ -21,6 +22,15 @@ class MultiModelAdmin extends ModelAdmin implements TestOnly
         ]
     ];
 
+    private static $model_importers = [
+        // Implicit for class
+        // Contact::class,
+        // Explicit for alias
+        'Player' => ModelAdminTestBulkLoader::class,
+        // Explicit for class
+        Player::class => ModelAdminTestBulkLoader::class
+    ];
+
     public function Link($action = null)
     {
         if (!$action) {