Merge pull request #334 from creative-commoners/pulls/2.0/fix-file-su…

…bsites API Move subsite dropdown logic for folders into FolderFormFactoryExtension
silverstripe · Jan 22, 2018 · 17e0db4 · 17e0db4
2 parents a201e7e + a88ac5d
commit 17e0db4
Show file tree

Hide file tree

Showing 16 changed files with 103 additions and 80 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -3,8 +3,7 @@ language: php
 addons:
   firefox: "31.0"
 
-# Required for Behat, currently
-dist: precise
+dist: trusty
 
 env:
   global:
@@ -16,9 +15,9 @@ matrix:
   include:
     - php: 5.6
       env: DB=MYSQL PHPCS_TEST=1 PHPUNIT_TEST=1
-    - php: 7.0
-      env: DB=PGSQL PHPUNIT_TEST=1
     - php: 7.1
+      env: DB=PGSQL PHPUNIT_TEST=1
+    - php: 7.2
       env: DB=MYSQL PHPUNIT_COVERAGE_TEST=1
     - php: 7.0
       env: DB=MYSQL BEHAT_TEST=1

diff --git a/_config/extensions.yml b/_config/extensions.yml
@@ -28,6 +28,10 @@ SilverStripe\Assets\File:
   extensions:
     - SilverStripe\Subsites\Extensions\FileSubsites
 
+SilverStripe\AssetAdmin\Forms\FolderFormFactory:
+  extensions:
+    - SilverStripe\Subsites\Extensions\FolderFormFactoryExtension
+
 SilverStripe\ErrorPage\ErrorPage:
   extensions:
     - SilverStripe\Subsites\Extensions\ErrorPageSubsite

diff --git a/css/LeftAndMain_Subsites.css b/css/LeftAndMain_Subsites.css
@@ -76,7 +76,7 @@ body.SubsiteAdmin .right form #URL .fieldgroup * {
     display: none;
 }
 
-#Root_DetailsView .subsites-move-dropdown,
-#Form_ItemEditForm .subsites-move-dropdown {
+.editor__details .subsites-move-dropdown,
+#Form_fileEditorForm.subsites-move-dropdown {
     display: block;
 }
diff --git a/...tension/CMSPageAddControllerExtension.php → ...ensions/CMSPageAddControllerExtension.php b/...tension/CMSPageAddControllerExtension.php → ...ensions/CMSPageAddControllerExtension.php
diff --git a/src/Extension/ControllerSubsites.php → src/Extensions/ControllerSubsites.php b/src/Extension/ControllerSubsites.php → src/Extensions/ControllerSubsites.php
diff --git a/src/Extension/ErrorPageSubsite.php → src/Extensions/ErrorPageSubsite.php b/src/Extension/ErrorPageSubsite.php → src/Extensions/ErrorPageSubsite.php
diff --git a/src/Extension/FileSubsites.php → src/Extensions/FileSubsites.php b/src/Extension/FileSubsites.php → src/Extensions/FileSubsites.php
@@ -2,10 +2,6 @@
 
 namespace SilverStripe\Subsites\Extensions;
 
-use SilverStripe\Assets\Folder;
-use SilverStripe\Forms\DropdownField;
-use SilverStripe\Forms\FieldList;
-use SilverStripe\Forms\LiteralField;
 use SilverStripe\ORM\DataExtension;
 use SilverStripe\ORM\DataQuery;
 use SilverStripe\ORM\Queries\SQLSelect;
@@ -41,43 +37,6 @@ public function alternateTreeTitle()
         return $this->owner->Title;
     }
 
-    /**
-     * Add subsites-specific fields to the folder editor.
-     * @param FieldList $fields
-     */
-    public function updateCMSFields(FieldList $fields)
-    {
-        if ($this->owner instanceof Folder) {
-            $sites = Subsite::accessible_sites('CMS_ACCESS_AssetAdmin');
-            $values = [];
-            $values[0] = _t(__CLASS__ . '.AllSitesDropdownOpt', 'All sites');
-            foreach ($sites as $site) {
-                $values[$site->ID] = $site->Title;
-            }
-            ksort($values);
-            if ($sites) {
-                //Dropdown needed to move folders between subsites
-                /** @var @skipUpgrade */
-                $dropdown = new DropdownField(
-                    'SubsiteID',
-                    _t(__CLASS__ . '.SubsiteFieldLabel', 'Subsite'),
-                    $values
-                );
-                $dropdown->addExtraClass('subsites-move-dropdown');
-                $fields->push($dropdown);
-                $fields->push(new LiteralField(
-                    'Message',
-                    '<p class="message notice">' .
-                    _t(
-                        'ASSETADMIN.SUBSITENOTICE',
-                        'Folders and files created in the main site are accessible by all subsites.'
-                    )
-                    . '</p>'
-                ));
-            }
-        }
-    }
-
     /**
      * Update any requests to limit the results to the current site
      * @param SQLSelect $query
@@ -90,7 +49,8 @@ public function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null)
         }
 
         // If you're querying by ID, ignore the sub-site - this is a bit ugly... (but it was WAYYYYYYYYY worse)
-        //@TODO I don't think excluding if SiteTree_ImageTracking is a good idea however because of the SS 3.0 api and ManyManyList::removeAll() changing the from table after this function is called there isn't much of a choice
+        // @TODO I don't think excluding if SiteTree_ImageTracking is a good idea however because of the SS 3.0 api and
+        // ManyManyList::removeAll() changing the from table after this function is called there isn't much of a choice
 
         $from = $query->getFrom();
         if (isset($from['SiteTree_ImageTracking']) || $query->filtersOnID()) {

diff --git a/src/Extensions/FolderFormFactoryExtension.php b/src/Extensions/FolderFormFactoryExtension.php
@@ -0,0 +1,47 @@
+<?php
+
+namespace SilverStripe\Subsites\Extensions;
+
+use SilverStripe\Core\Extension;
+use SilverStripe\Forms\DropdownField;
+use SilverStripe\Forms\FieldList;
+use SilverStripe\Forms\LiteralField;
+use SilverStripe\Subsites\Model\Subsite;
+
+class FolderFormFactoryExtension extends Extension
+{
+    /**
+     * Add subsites-specific fields to the folder editor.
+     * @param FieldList $fields
+     */
+    public function updateFormFields(FieldList $fields)
+    {
+        $sites = Subsite::accessible_sites('CMS_ACCESS_AssetAdmin');
+        $values = [];
+        $values[0] = _t(__CLASS__ . '.AllSitesDropdownOpt', 'All sites');
+        foreach ($sites as $site) {
+            $values[$site->ID] = $site->Title;
+        }
+        ksort($values);
+        if ($sites) {
+            // Dropdown needed to move folders between subsites
+            /** @var @skipUpgrade */
+            $dropdown = DropdownField::create(
+                'SubsiteID',
+                _t(__CLASS__ . '.SubsiteFieldLabel', 'Subsite'),
+                $values
+            );
+            $dropdown->addExtraClass('subsites-move-dropdown');
+            $fields->push($dropdown);
+            $fields->push(LiteralField::create(
+                'Message',
+                '<p class="message notice">' .
+                _t(
+                    __CLASS__ . '.SUBSITENOTICE',
+                    'Folders and files created in the main site are accessible by all subsites.'
+                )
+                . '</p>'
+            ));
+        }
+    }
+}
diff --git a/src/Extension/GroupSubsites.php → src/Extensions/GroupSubsites.php b/src/Extension/GroupSubsites.php → src/Extensions/GroupSubsites.php
diff --git a/src/Extension/LeftAndMainSubsites.php → src/Extensions/LeftAndMainSubsites.php b/src/Extension/LeftAndMainSubsites.php → src/Extensions/LeftAndMainSubsites.php
diff --git a/src/Extension/SiteConfigSubsites.php → src/Extensions/SiteConfigSubsites.php b/src/Extension/SiteConfigSubsites.php → src/Extensions/SiteConfigSubsites.php
diff --git a/src/Extension/SiteTreeSubsites.php → src/Extensions/SiteTreeSubsites.php b/src/Extension/SiteTreeSubsites.php → src/Extensions/SiteTreeSubsites.php
diff --git a/src/Extension/SubsiteMenuExtension.php → src/Extensions/SubsiteMenuExtension.php b/src/Extension/SubsiteMenuExtension.php → src/Extensions/SubsiteMenuExtension.php
diff --git a/tests/php/Extensions/FolderFormFactoryExtensionTest.php b/tests/php/Extensions/FolderFormFactoryExtensionTest.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace SilverStripe\Subsites\Tests\Extensions;
+
+use SilverStripe\AssetAdmin\Forms\FolderFormFactory;
+use SilverStripe\Assets\Folder;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Forms\Form;
+use SilverStripe\Forms\FormFactory;
+
+class FolderFormFactoryExtensionTest extends SapphireTest
+{
+    protected static $fixture_file = 'FolderFormFactoryExtensionTest.yml';
+
+    public function testSubsitesFolderDropdown()
+    {
+        $this->logInWithPermission('ADMIN');
+
+        /** @var Folder $folder */
+        $folder = $this->objFromFixture(Folder::class, 'folder_a');
+
+        /** @var Form $folderForm */
+        $folderForm = FolderFormFactory::create()->getForm(null, FormFactory::DEFAULT_NAME, [
+            'Record' => $folder
+        ]);
+
+        $source = array_values($folderForm->Fields()->fieldByName('SubsiteID')->getSource());
+        $result = array_values($source);
+
+        $this->assertContains('Main site', $result);
+        $this->assertContains('Subsite A', $result);
+        $this->assertContains('Subsite B', $result);
+    }
+}
diff --git a/tests/php/Extensions/FolderFormFactoryExtensionTest.yml b/tests/php/Extensions/FolderFormFactoryExtensionTest.yml
@@ -0,0 +1,11 @@
+SilverStripe\Subsites\Model\Subsite:
+  main:
+    Title: Template
+  subsite_a:
+    Title: Subsite A
+  subsite_b:
+    Title: Subsite B
+
+SilverStripe\Assets\Folder:
+  folder_a:
+    Title: Folder A
diff --git a/tests/php/FileSubsitesTest.php b/tests/php/FileSubsitesTest.php
@@ -12,15 +12,6 @@ class FileSubsitesTest extends BaseSubsiteTest
 {
     protected static $fixture_file = 'SubsiteTest.yml';
 
-    /**
-     * Disable other file extensions
-     *
-     * @var array
-     */
-    protected static $illegal_extensions = [
-        SiteTree::class => ['Translatable'], // @todo implement namespace for Translatable
-    ];
-
     public function testTrivialFeatures()
     {
         $this->assertTrue(is_array(singleton(FileSubsites::class)->extraStatics()));
@@ -73,27 +64,4 @@ public function testWritingSubsiteID()
         $file->onAfterUpload();
         $this->assertEquals($folder->SubsiteID, $file->SubsiteID);
     }
-
-    public function testSubsitesFolderDropdown()
-    {
-        $this->logInAs('admin');
-
-        $file = new Folder();
-
-        $source = array_values($file->getCMSFields()->dataFieldByName('SubsiteID')->getSource());
-        asort($source);
-
-        $this->assertEquals([
-            'Main site',
-            'Subsite1 Template',
-            'Subsite2 Template',
-            'Template',
-            'Test 1',
-            'Test 2',
-            'Test 3',
-            'Test Non-SSL',
-            'Test SSL',
-            'Test Vagrant VM on port 8080'
-        ], array_values($source));
-    }
 }