From 71e6fb84c354e94f7811cca6cea46728dc2b530d Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Tue, 9 Jan 2024 17:51:27 +0000 Subject: [PATCH 01/10] assets field - aalidate volume is not tempUploadsLocation --- src/fields/Assets.php | 29 +++++++++++++++++++ .../fieldtypes/Assets/settings.twig | 4 +-- src/translations/en/app.php | 1 + 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index d73ac33b740..2741b1dbb7b 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -238,11 +238,40 @@ protected function defineRules(): array }, ]; + $rules[] = [ + ['source', 'sources', 'defaultUploadLocationSource', 'restrictedLocationSource'], 'validateNotTempVolume', + ]; + $rules[] = [['previewMode'], 'in', 'range' => [self::PREVIEW_MODE_FULL, self::PREVIEW_MODE_THUMBS], 'skipOnEmpty' => false]; return $rules; } + /** + * Ensure that you can't select tempUploadsLocation volume as a source or default uploads location or restricted location for an Assets field. + * + * @param string $attribute + * @return void + * @throws InvalidConfigException + * @since 4.6.0 + */ + public function validateNotTempVolume(string $attribute, ?array $params): void + { + [$tempVolume, $tempSubpath] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); + if ($tempVolume !== null) { + $tempVolumeUid = 'volume:' . $tempVolume->uid; + $inputSources = $this->getInputSources(); + + if (in_array($tempVolumeUid, $inputSources)) { + $this->addError($attribute, Craft::t( + 'app', + 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.', + ['volumeName' => $tempVolume->name]) + ); + } + } + } + /** * @inheritdoc */ diff --git a/src/templates/_components/fieldtypes/Assets/settings.twig b/src/templates/_components/fieldtypes/Assets/settings.twig index 23ab3d42297..726db327cdf 100644 --- a/src/templates/_components/fieldtypes/Assets/settings.twig +++ b/src/templates/_components/fieldtypes/Assets/settings.twig @@ -62,7 +62,7 @@ sourceOptions: sourceOptions, sourceValue: field.restrictedLocationSource, subpathValue: field.restrictedLocationSubpath, - errors: field.getErrors('restrictedLocationSubpath') + errors: field.getErrors('restrictedLocationSource') + field.getErrors('restrictedLocationSubpath') }) }} {{ forms.checkboxField({ @@ -103,7 +103,7 @@ sourceOptions: sourceOptions, sourceValue: field.defaultUploadLocationSource, subpathValue: field.defaultUploadLocationSubpath, - errors: field.getErrors('defaultUploadLocationSubpath') + errors: field.getErrors('defaultUploadLocationSource') + field.getErrors('defaultUploadLocationSubpath') }) }} {% endtag %} diff --git a/src/translations/en/app.php b/src/translations/en/app.php index c1dc70fb912..6ba25580347 100644 --- a/src/translations/en/app.php +++ b/src/translations/en/app.php @@ -1744,6 +1744,7 @@ 'Village/Township' => 'Village/Township', 'Visit webpage' => 'Visit webpage', 'Volume - {volume}' => 'Volume - {volume}', + 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.' => 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.', 'Volume path' => 'Volume path', 'Volume saved.' => 'Volume saved.', 'Volume' => 'Volume', From 6157d0e3e36dd6caf64a4bbd8e1db5aca834e893 Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Tue, 9 Jan 2024 18:22:36 +0000 Subject: [PATCH 02/10] don't show volume selected as tempUploadLocation --- src/fields/Assets.php | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index 2741b1dbb7b..c71f1e8ee3d 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -278,17 +278,31 @@ public function validateNotTempVolume(string $attribute, ?array $params): void public function getSourceOptions(): array { $sourceOptions = []; + [$tempVolume, $tempSubpath] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); + $tempVolumeKey = null; + $isTempVolumeSelected = false; + + if ($tempVolume !== null) { + $tempVolumeKey = 'volume:' . $tempVolume->uid; + $isTempVolumeSelected = $this->_isVolumeSelected($tempVolumeKey); + } foreach (Asset::sources('settings') as $volume) { if (!isset($volume['heading'])) { - $sourceOptions[] = [ + $option = [ 'label' => $volume['label'], 'value' => $volume['key'], ]; + + if ($tempVolumeKey && !$isTempVolumeSelected && $volume['key'] === $tempVolumeKey) { + $option = null; + } + + $sourceOptions[] = $option; } } - return $sourceOptions; + return array_filter($sourceOptions); } /** @@ -1095,4 +1109,21 @@ private function _folderWithAncestors(VolumeFolder $folder, ?VolumeFolder $until return $folders; } + + /** + * Returns if given volume is already selected for this field + * + * @param string $volumeKey + * @return bool + */ + private function _isVolumeSelected(string $volumeKey): bool + { + // we consider volume as already selected if: + // id is set and sources is set to 'all' + // or if volumeKey is on the list of sources + // or if volumeKey is the source + return ($this->id !== null && $this->sources == '*') || + (is_array($this->sources) && in_array($volumeKey, $this->sources)) || + $volumeKey == $this->source; + } } From a4dc6aebf0f0cd30f59dd0b494843d0960b84610 Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Tue, 9 Jan 2024 18:22:51 +0000 Subject: [PATCH 03/10] cleanup --- src/fields/Assets.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index c71f1e8ee3d..b090accae17 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -259,10 +259,10 @@ public function validateNotTempVolume(string $attribute, ?array $params): void { [$tempVolume, $tempSubpath] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); if ($tempVolume !== null) { - $tempVolumeUid = 'volume:' . $tempVolume->uid; + $tempVolumeKey = 'volume:' . $tempVolume->uid; $inputSources = $this->getInputSources(); - if (in_array($tempVolumeUid, $inputSources)) { + if (in_array($tempVolumeKey, $inputSources)) { $this->addError($attribute, Craft::t( 'app', 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.', From c9ea0ae480aff4f078022da2f1b122cd56581ec0 Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Wed, 10 Jan 2024 12:11:02 +0000 Subject: [PATCH 04/10] added volume warning to asset settings --- src/templates/settings/assets/settings.twig | 2 +- src/translations/en/app.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/templates/settings/assets/settings.twig b/src/templates/settings/assets/settings.twig index 17f6b2a94e6..12440bef330 100644 --- a/src/templates/settings/assets/settings.twig +++ b/src/templates/settings/assets/settings.twig @@ -54,7 +54,7 @@ id: 'tempUploadLocation', label: 'Temp Uploads Location'|t('app'), instructions: 'Where do you want to store temporary asset uploads?'|t('app'), - warning: allVolumes is empty ? 'No volumes exist yet.'|t('app') + warning: allVolumes is empty ? 'No volumes exist yet.'|t('app') : 'Don’t select a volume that’s used by any Assets fields.'|t('app') }, tempVolumeInput) }}
diff --git a/src/translations/en/app.php b/src/translations/en/app.php index 43011faab0c..40da0e9a69e 100644 --- a/src/translations/en/app.php +++ b/src/translations/en/app.php @@ -533,6 +533,7 @@ 'District' => 'District', 'Do Si' => 'Do Si', 'Documentation' => 'Documentation', + 'Don’t select a volume that’s used by any Assets fields.' => 'Don’t select a volume that’s used by any Assets fields.', 'Done' => 'Done', 'Download backup' => 'Download backup', 'Download' => 'Download', From 2e8f9e691462bb396fbf97d5e48a539ba5aaf22a Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Wed, 10 Jan 2024 12:11:49 +0000 Subject: [PATCH 05/10] keep showing volume selected as tempUploadsLocation --- src/fields/Assets.php | 35 ++--------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index 1698e93cb17..b6fe3cf405a 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -278,31 +278,17 @@ public function validateNotTempVolume(string $attribute, ?array $params): void public function getSourceOptions(): array { $sourceOptions = []; - [$tempVolume, $tempSubpath] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); - $tempVolumeKey = null; - $isTempVolumeSelected = false; - - if ($tempVolume !== null) { - $tempVolumeKey = 'volume:' . $tempVolume->uid; - $isTempVolumeSelected = $this->_isVolumeSelected($tempVolumeKey); - } foreach (Asset::sources('settings') as $volume) { if (!isset($volume['heading'])) { - $option = [ + $sourceOptions[] = [ 'label' => $volume['label'], 'value' => $volume['key'], ]; - - if ($tempVolumeKey && !$isTempVolumeSelected && $volume['key'] === $tempVolumeKey) { - $option = null; - } - - $sourceOptions[] = $option; } } - return array_filter($sourceOptions); + return $sourceOptions; } /** @@ -1109,21 +1095,4 @@ private function _folderWithAncestors(VolumeFolder $folder, ?VolumeFolder $until return $folders; } - - /** - * Returns if given volume is already selected for this field - * - * @param string $volumeKey - * @return bool - */ - private function _isVolumeSelected(string $volumeKey): bool - { - // we consider volume as already selected if: - // id is set and sources is set to 'all' - // or if volumeKey is on the list of sources - // or if volumeKey is the source - return ($this->id !== null && $this->sources == '*') || - (is_array($this->sources) && in_array($volumeKey, $this->sources)) || - $volumeKey == $this->source; - } } From ac71477f03834c13155ff80f54c8e11bf8bd392b Mon Sep 17 00:00:00 2001 From: Iwona Just Date: Wed, 10 Jan 2024 12:16:24 +0000 Subject: [PATCH 06/10] better validation --- src/fields/Assets.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index b6fe3cf405a..657897e7a0f 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -262,7 +262,11 @@ public function validateNotTempVolume(string $attribute, ?array $params): void $tempVolumeKey = 'volume:' . $tempVolume->uid; $inputSources = $this->getInputSources(); - if (in_array($tempVolumeKey, $inputSources)) { + if ( + (in_array($attribute, ['source', 'sources']) && in_array($tempVolumeKey, $inputSources)) || + ($attribute == 'defaultUploadLocationSource' && $this->defaultUploadLocationSource === $tempVolumeKey) || + ($attribute == 'restrictedLocationSource' && $this->restrictedLocationSource === $tempVolumeKey) + ) { $this->addError($attribute, Craft::t( 'app', 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.', From 3247f6aa97bc01ab6e114e307db67a12dbcf595e Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Mon, 22 Jan 2024 15:44:47 -0800 Subject: [PATCH 07/10] Cleanup --- src/fields/Assets.php | 17 ++++++----------- src/templates/settings/assets/settings.twig | 2 +- src/translations/en/app.php | 2 -- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index 657897e7a0f..6abacc1fa4e 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -251,15 +251,13 @@ protected function defineRules(): array * Ensure that you can't select tempUploadsLocation volume as a source or default uploads location or restricted location for an Assets field. * * @param string $attribute - * @return void - * @throws InvalidConfigException - * @since 4.6.0 + * @since 4.7.0 */ - public function validateNotTempVolume(string $attribute, ?array $params): void + public function validateNotTempVolume(string $attribute): void { - [$tempVolume, $tempSubpath] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); + [$tempVolume] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); if ($tempVolume !== null) { - $tempVolumeKey = 'volume:' . $tempVolume->uid; + $tempVolumeKey = "volume:$tempVolume->uid"; $inputSources = $this->getInputSources(); if ( @@ -267,11 +265,8 @@ public function validateNotTempVolume(string $attribute, ?array $params): void ($attribute == 'defaultUploadLocationSource' && $this->defaultUploadLocationSource === $tempVolumeKey) || ($attribute == 'restrictedLocationSource' && $this->restrictedLocationSource === $tempVolumeKey) ) { - $this->addError($attribute, Craft::t( - 'app', - 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.', - ['volumeName' => $tempVolume->name]) - ); + // intentionally not translating this since it's short-lived (>= 4.7, < 5.0) and dev-facing only. + $this->addError($attribute, "Volume “{$tempVolume->name}” is used to store temporary asset uploads, so it cannot be used in an Assets field."); } } } diff --git a/src/templates/settings/assets/settings.twig b/src/templates/settings/assets/settings.twig index 12440bef330..c98c5a064d8 100644 --- a/src/templates/settings/assets/settings.twig +++ b/src/templates/settings/assets/settings.twig @@ -54,7 +54,7 @@ id: 'tempUploadLocation', label: 'Temp Uploads Location'|t('app'), instructions: 'Where do you want to store temporary asset uploads?'|t('app'), - warning: allVolumes is empty ? 'No volumes exist yet.'|t('app') : 'Don’t select a volume that’s used by any Assets fields.'|t('app') + warning: allVolumes is empty ? 'No volumes exist yet.'|t('app') : 'Don’t select a volume that’s used by any Assets fields.', }, tempVolumeInput) }}
diff --git a/src/translations/en/app.php b/src/translations/en/app.php index 40da0e9a69e..376822af7fb 100644 --- a/src/translations/en/app.php +++ b/src/translations/en/app.php @@ -533,7 +533,6 @@ 'District' => 'District', 'Do Si' => 'Do Si', 'Documentation' => 'Documentation', - 'Don’t select a volume that’s used by any Assets fields.' => 'Don’t select a volume that’s used by any Assets fields.', 'Done' => 'Done', 'Download backup' => 'Download backup', 'Download' => 'Download', @@ -1746,7 +1745,6 @@ 'Village/Township' => 'Village/Township', 'Visit webpage' => 'Visit webpage', 'Volume - {volume}' => 'Volume - {volume}', - 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.' => 'Volume “{volumeName}” is used to store temporary asset uploads, so it cannot be used in a field.', 'Volume path' => 'Volume path', 'Volume saved.' => 'Volume saved.', 'Volume' => 'Volume', From 4df393b3cbbf3f5ab9807f0038646792c289f80b Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Mon, 22 Jan 2024 16:28:00 -0800 Subject: [PATCH 08/10] Don't even allow selecting the temp volume anymore --- src/fields/Assets.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index 6abacc1fa4e..bf843b38acb 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -277,8 +277,26 @@ public function validateNotTempVolume(string $attribute): void public function getSourceOptions(): array { $sourceOptions = []; + /** @var Volume|null $tempVolume */ + [$tempVolume] = Craft::$app->getAssets()->getTempVolumeAndSubpath(); + if ($tempVolume) { + $tempVolumeKey = 'volume:' . $tempVolume->uid; + } else { + $tempVolumeKey = null; + } foreach (Asset::sources('settings') as $volume) { + if ($tempVolumeKey !== null && $volume['key'] === $tempVolumeKey) { + // only allow it if already selected + if ( + (!is_array($this->sources) || !in_array($tempVolumeKey, $this->sources)) && + $this->defaultUploadLocationSource !== $tempVolumeKey && + $this->restrictedLocationSource !== $tempVolumeKey + ) { + continue; + } + } + if (!isset($volume['heading'])) { $sourceOptions[] = [ 'label' => $volume['label'], From f5b8b99b5d92c951c5cbc71a3b9e3db369632738 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Mon, 22 Jan 2024 16:24:04 -0800 Subject: [PATCH 09/10] Assets::$source should never be set --- src/fields/Assets.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fields/Assets.php b/src/fields/Assets.php index bf843b38acb..ece9d049756 100644 --- a/src/fields/Assets.php +++ b/src/fields/Assets.php @@ -239,7 +239,7 @@ protected function defineRules(): array ]; $rules[] = [ - ['source', 'sources', 'defaultUploadLocationSource', 'restrictedLocationSource'], 'validateNotTempVolume', + ['sources', 'defaultUploadLocationSource', 'restrictedLocationSource'], 'validateNotTempVolume', ]; $rules[] = [['previewMode'], 'in', 'range' => [self::PREVIEW_MODE_FULL, self::PREVIEW_MODE_THUMBS], 'skipOnEmpty' => false]; From d2292c41ec10ae695bfbdd70cec17d55b8ebcbae Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Mon, 22 Jan 2024 16:30:01 -0800 Subject: [PATCH 10/10] Release note --- CHANGELOG-WIP.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG-WIP.md b/CHANGELOG-WIP.md index 1fa8d95e4fc..bfe094b5a40 100644 --- a/CHANGELOG-WIP.md +++ b/CHANGELOG-WIP.md @@ -5,6 +5,7 @@ ### Administration - Added “Save and continue editing” actions to all core settings pages with full-page forms. ([#14168](https://github.com/craftcms/cms/discussions/14168)) +- It’s no longer possible to select the temp asset volume within Assets fields. ([#11405](https://github.com/craftcms/cms/issues/11405), [#14141](https://github.com/craftcms/cms/pull/14141)) - Added the `utils/prune-orphaned-matrix-blocks` command. ([#14154](https://github.com/craftcms/cms/pull/14154)) ### Extensibility