From d862fda7b7264880186aca0bf08ce595364ed285 Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 11:43:58 +0200 Subject: [PATCH 1/7] Fix backwards compatibility elements mapper --- src/Mappers/ElementIndexMapper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mappers/ElementIndexMapper.php b/src/Mappers/ElementIndexMapper.php index f2c8111b..fabc1e9e 100644 --- a/src/Mappers/ElementIndexMapper.php +++ b/src/Mappers/ElementIndexMapper.php @@ -59,7 +59,7 @@ public function import(array $settingDefinitions, array $elementTypes): array { foreach ($settingDefinitions as $elementType => $settings) { // Backwards compatibility - if (class_exists('craft\\elements\\'.$elementType, false)) { + if (class_exists('craft\\elements\\'.$elementType)) { $elementType = 'craft\\elements\\'.$elementType; } $mappedSettings = $this->getMappedSettings($settings, 'handle', 'id'); From 565ef8444b455df865a8e9da9e13429735aca37c Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 12:18:14 +0200 Subject: [PATCH 2/7] Fix for broken mapping between asset sources and folders --- src/Behaviors/SourcesBehavior.php | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/Behaviors/SourcesBehavior.php b/src/Behaviors/SourcesBehavior.php index 1282e035..2d675a48 100644 --- a/src/Behaviors/SourcesBehavior.php +++ b/src/Behaviors/SourcesBehavior.php @@ -6,6 +6,7 @@ use TypeError; use yii\base\Behavior; use craft\base\Model; +use craft\records\VolumeFolder; use NerdsAndCompany\Schematic\Schematic; use NerdsAndCompany\Schematic\Events\SourceMappingEvent; @@ -123,6 +124,9 @@ public function getSource(string $fieldType, string $source = null, string $inde $method = 'getGroupBy'; break; case 'folder': + $service = $this; + $method = 'getFolderBy'; + break; case 'createFoldersInVolume': case 'deleteFilesAndFoldersInVolume': case 'saveAssetInVolume': @@ -178,4 +182,36 @@ public function getSource(string $fieldType, string $source = null, string $inde return null; } + + /** + * Get a folder by id + * + * @param int $folderId + * @return object + */ + private function getFolderById(int $folderId): object + { + $folder = Craft::$app->assets->getFolderById($folderId); + $volume = $folder->getVolume(); + return (object) [ + 'id' => $folderId, + 'handle' => $volume->handle + ]; + } + + /** + * Get a folder by handle + * + * @param string $folderHandle + * @return object + */ + private function getFolderByHandle(string $folderHandle): object + { + $volume = Craft::$app->volumes->getVolumeByHandle($folderHandle); + $folder = VolumeFolder::findOne(['volumeId' => $volume->id]); + return (object) [ + 'id' => $folder->id, + 'handle' => $folderHandle + ]; + } } From f2985730589bf1283bbd63ad143f3442bf2a1bb5 Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 12:20:23 +0200 Subject: [PATCH 3/7] updated changelog --- CHANGELOG.md | 5 +++++ composer.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4e6777b..0257e032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +### 4.0.17 - 2018-09-24 +### Fixed +- Fixed backwards compatibility of element index mapper +- Fixed issue with volume and volume folder ids going out of sink + ### 4.0.16 - 2018-09-11 ### Added - Added map_source event for mapping custom sources. See the Events section in the README for more. diff --git a/composer.json b/composer.json index 84920d02..efffb876 100644 --- a/composer.json +++ b/composer.json @@ -1,5 +1,5 @@ { - "version": "4.0.16", + "version": "4.0.17", "name": "nerds-and-company/schematic", "description": "Craft setup and sync tool", "type": "craft-plugin", From c27bbec3b2d5910b00629efcc45dd4110aab2872 Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 13:43:13 +0200 Subject: [PATCH 4/7] fix shameful spelling error and asset field specs --- CHANGELOG.md | 2 +- src/Behaviors/SourcesBehavior.php | 49 ++++++++++++---- tests/_support/Helper/Unit.php | 5 ++ tests/unit/Converters/Fields/AssetsTest.php | 64 +++++++++++++++++---- 4 files changed, 97 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0257e032..a60a8399 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ### 4.0.17 - 2018-09-24 ### Fixed - Fixed backwards compatibility of element index mapper -- Fixed issue with volume and volume folder ids going out of sink +- Fixed issue with volume and volume folder ids going out of sync ### 4.0.16 - 2018-09-11 ### Added diff --git a/src/Behaviors/SourcesBehavior.php b/src/Behaviors/SourcesBehavior.php index 2d675a48..0bca8896 100644 --- a/src/Behaviors/SourcesBehavior.php +++ b/src/Behaviors/SourcesBehavior.php @@ -23,6 +23,9 @@ */ class SourcesBehavior extends Behavior { + /** Hack to be able to avoid the active record call in VolumeFolder::findOne() */ + public $mockFolder = null; + /** * Recursively find sources in definition attributes. * @@ -192,15 +195,34 @@ public function getSource(string $fieldType, string $source = null, string $inde private function getFolderById(int $folderId): object { $folder = Craft::$app->assets->getFolderById($folderId); - $volume = $folder->getVolume(); - return (object) [ - 'id' => $folderId, - 'handle' => $volume->handle - ]; + if ($folder) { + $volume = $folder->getVolume(); + return (object) [ + 'id' => $folderId, + 'handle' => $volume->handle + ]; + } + return null; } /** - * Get a folder by handle + * Get folder by volume id + * + * @param int $volumeId + * @return VolumeFolder + */ + private function getFolderByVolumeId(int $volumeId): VolumeFolder + { + return $this->mockFolder ? $this->mockFolder : VolumeFolder::findOne(['volumeId' => $volumeId]); + } + + public function setMockFolder(VolumeFolder $mockFolder): void + { + $this->mockFolder = $mockFolder; + } + + /** + * Get a folder by volume handle * * @param string $folderHandle * @return object @@ -208,10 +230,15 @@ private function getFolderById(int $folderId): object private function getFolderByHandle(string $folderHandle): object { $volume = Craft::$app->volumes->getVolumeByHandle($folderHandle); - $folder = VolumeFolder::findOne(['volumeId' => $volume->id]); - return (object) [ - 'id' => $folder->id, - 'handle' => $folderHandle - ]; + if ($volume) { + $folder = $this->getFolderByVolumeId($volume->id); + if ($folder) { + return (object) [ + 'id' => $folder->id, + 'handle' => $folderHandle + ]; + } + } + return null; } } diff --git a/tests/_support/Helper/Unit.php b/tests/_support/Helper/Unit.php index a42f34ff..183e28e9 100644 --- a/tests/_support/Helper/Unit.php +++ b/tests/_support/Helper/Unit.php @@ -3,9 +3,11 @@ namespace Helper; use Craft; +use Yii; use craft\console\Application; use yii\console\Controller; use craft\i18n\I18n; +use craft\services\Assets; use craft\services\AssetTransforms; use craft\services\Categories; use craft\services\Content; @@ -50,6 +52,7 @@ public function _before(TestCase $test) $mockApp->controller->module = $this->getMockModule($test); Craft::$app = $mockApp; + Yii::$app = $mockApp; Schematic::$force = false; } @@ -83,6 +86,7 @@ private function getMockModule(TestCase $test) private function getMockApp(TestCase $test) { $mockApp = $this->getMock($test, Application::class); + $mockAssets = $this->getMock($test, Assets::class); $mockAssetTransforms = $this->getMock($test, AssetTransforms::class); $mockCategoryGroups = $this->getMock($test, Categories::class); $mockContent = $this->getMock($test, Content::class); @@ -105,6 +109,7 @@ private function getMockApp(TestCase $test) $mockApp->expects($test->any()) ->method('__get') ->willReturnMap([ + ['assets', $mockAssets], ['assetTransforms', $mockAssetTransforms], ['categories', $mockCategoryGroups], ['content', $mockContent], diff --git a/tests/unit/Converters/Fields/AssetsTest.php b/tests/unit/Converters/Fields/AssetsTest.php index e2b9e209..04a59689 100644 --- a/tests/unit/Converters/Fields/AssetsTest.php +++ b/tests/unit/Converters/Fields/AssetsTest.php @@ -5,6 +5,8 @@ use Craft; use craft\fields\Assets as AssetsField; use craft\base\Volume; +use craft\models\VolumeFolder; +use craft\records\VolumeFolder as VolumeFolderRecord; use Codeception\Test\Unit; /** @@ -43,14 +45,14 @@ protected function _before() * * @param AssetsField $assets * @param array $definition - * @param Mock|Volume|null $mockVolume + * @param Mock|Volume|null $mockFolder */ - public function testGetRecordDefinition(AssetsField $assets, array $definition, $mockVolume) + public function testGetRecordDefinition(AssetsField $assets, array $definition, $mockFolder) { - Craft::$app->volumes->expects($this->any()) - ->method('getVolumeById') - ->with($mockVolume->id) - ->willReturn($mockVolume); + Craft::$app->assets->expects($this->any()) + ->method('getFolderById') + ->with($mockFolder->id) + ->willReturn($mockFolder); $result = $this->converter->getRecordDefinition($assets); @@ -62,18 +64,20 @@ public function testGetRecordDefinition(AssetsField $assets, array $definition, * * @param AssetsField $assets * @param array $definition - * @param Mock|Volume|null $mockVolume + * @param Mock|Volume|null $mockFolder */ - public function testSetRecordAttributes(AssetsField $assets, array $definition, $mockVolume) + public function testSetRecordAttributes(AssetsField $assets, array $definition, $mockFolder) { + $this->setMockFolderRecord($mockFolder); + $mockVolume = $mockFolder->getVolume(); Craft::$app->volumes->expects($this->any()) ->method('getVolumeByHandle') + ->with($mockVolume->handle) ->willReturn($mockVolume); $newAssets = new AssetsField(); $this->converter->setRecordAttributes($newAssets, $definition, []); - $this->assertSame($assets->name, $newAssets->name); $this->assertSame($assets->handle, $newAssets->handle); $this->assertSame($assets->defaultUploadLocationSource, $newAssets->defaultUploadLocationSource); @@ -91,13 +95,13 @@ public function testSetRecordAttributes(AssetsField $assets, array $definition, public function provideAssets() { $mockAssets1 = $this->getMockAssets(1, 1); - $mockVolume1 = $this->getMockVolume(1); + $mockFolder1 = $this->getMockFolder(1); return [ 'assets' => [ 'Assets' => $mockAssets1, 'definition' => $this->getMockAssetsDefinition($mockAssets1), - 'volume' => $mockVolume1, + 'volume' => $mockFolder1, ], ]; } @@ -194,6 +198,30 @@ private function getMockFieldGroup(int $groupId) return $mockGroup; } + /** + * Get a mock folder. + * + * @param int $folderId + * + * @return Mock|VolumeFolder + */ + private function getMockFolder(int $folderId) + { + $mockVolume = $this->getMockVolume($folderId); + $mockFolder = $this->getMockBuilder(VolumeFolder::class) + ->disableOriginalConstructor() + ->getmock(); + + $mockFolder->id = $folderId; + $mockFolder->handle = 'folderHandle'.$folderId; + + $mockFolder->expects($this->any()) + ->method('getVolume') + ->willReturn($mockVolume); + + return $mockFolder; + } + /** * Get a mock volume. * @@ -212,4 +240,18 @@ private function getMockVolume(int $volumeId) return $mockVolume; } + + /** + * Set a mock folder record on converter based on mock folder + * + * @param VolumeFolder $mockFolder + */ + private function setMockFolderRecord(VolumeFolder $mockFolder) + { + $mockFolderRecord = $this->getMockBuilder(VolumeFolderRecord::class)->getMock(); + $mockFolderRecord->expects($this->any()) + ->method('__get') + ->willReturnMap([['id', $mockFolder->id]]); + $this->converter->setMockFolder($mockFolderRecord); + } } From cd42e0b02785d98d5f5a66ce1bfd181df10223d3 Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 14:49:27 +0200 Subject: [PATCH 5/7] Better typehints --- src/Behaviors/SourcesBehavior.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Behaviors/SourcesBehavior.php b/src/Behaviors/SourcesBehavior.php index 0bca8896..1938f564 100644 --- a/src/Behaviors/SourcesBehavior.php +++ b/src/Behaviors/SourcesBehavior.php @@ -216,7 +216,12 @@ private function getFolderByVolumeId(int $volumeId): VolumeFolder return $this->mockFolder ? $this->mockFolder : VolumeFolder::findOne(['volumeId' => $volumeId]); } - public function setMockFolder(VolumeFolder $mockFolder): void + /** + * Set a mock folder for the tests + * + * @param VolumeFolder $mockFolder + */ + public function setMockFolder(VolumeFolder $mockFolder) { $this->mockFolder = $mockFolder; } From e01e1a5a4d362b5aa85eaaa26c4cf253f45403da Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 14:59:43 +0200 Subject: [PATCH 6/7] ignore false positive unused private method --- src/Behaviors/SourcesBehavior.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Behaviors/SourcesBehavior.php b/src/Behaviors/SourcesBehavior.php index 1938f564..cb72b4db 100644 --- a/src/Behaviors/SourcesBehavior.php +++ b/src/Behaviors/SourcesBehavior.php @@ -188,6 +188,7 @@ public function getSource(string $fieldType, string $source = null, string $inde /** * Get a folder by id + * @SuppressWarnings(PHPMD.UnusedPrivateMethod) * * @param int $folderId * @return object @@ -228,6 +229,7 @@ public function setMockFolder(VolumeFolder $mockFolder) /** * Get a folder by volume handle + * @SuppressWarnings(PHPMD.UnusedPrivateMethod) * * @param string $folderHandle * @return object From 72c4842124b0ad4900cd0f9a21fa18d88f1bfc06 Mon Sep 17 00:00:00 2001 From: Bart van Gennep Date: Mon, 24 Sep 2018 15:33:02 +0200 Subject: [PATCH 7/7] Even better typehints --- src/Behaviors/SourcesBehavior.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Behaviors/SourcesBehavior.php b/src/Behaviors/SourcesBehavior.php index cb72b4db..3f969fdd 100644 --- a/src/Behaviors/SourcesBehavior.php +++ b/src/Behaviors/SourcesBehavior.php @@ -193,7 +193,7 @@ public function getSource(string $fieldType, string $source = null, string $inde * @param int $folderId * @return object */ - private function getFolderById(int $folderId): object + private function getFolderById(int $folderId): \stdClass { $folder = Craft::$app->assets->getFolderById($folderId); if ($folder) { @@ -234,7 +234,7 @@ public function setMockFolder(VolumeFolder $mockFolder) * @param string $folderHandle * @return object */ - private function getFolderByHandle(string $folderHandle): object + private function getFolderByHandle(string $folderHandle): \stdClass { $volume = Craft::$app->volumes->getVolumeByHandle($folderHandle); if ($volume) {