From f45882de5fef63d309e0fbc8bf329c2748f814f6 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 21 Nov 2022 15:18:29 -0600 Subject: [PATCH 1/6] Clone the form model before passing it to the RelationController This prevents issues caused by extensions to the RelationController making changes to the relation's parent model for use cases needed in relations. These changes were previously also affecting the model instance used by the FormController behaviour. If it is desired to interact with the exactly model instance as used by the FormController behaviour inside of your extensions to the RelationController you can use $controller->formGetModel() instead. --- modules/backend/behaviors/FormController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/backend/behaviors/FormController.php b/modules/backend/behaviors/FormController.php index 25660de317..ae58bb475f 100644 --- a/modules/backend/behaviors/FormController.php +++ b/modules/backend/behaviors/FormController.php @@ -177,7 +177,7 @@ public function initForm($model, $context = null) * Detected Relation controller behavior */ if ($this->controller->isClassExtendedWith(\Backend\Behaviors\RelationController::class)) { - $this->controller->initRelation($model); + $this->controller->initRelation(clone $model); } $this->prepareVars($model); From 94f6c9caa79a0ce445799bf0fb213acfba5ad497 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 21 Nov 2022 15:19:37 -0600 Subject: [PATCH 2/6] Use the model instance returned by getRelationModel directly rather than as a static class reference --- modules/backend/formwidgets/FileUpload.php | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/backend/formwidgets/FileUpload.php b/modules/backend/formwidgets/FileUpload.php index ae401ad6f4..f19df77977 100644 --- a/modules/backend/formwidgets/FileUpload.php +++ b/modules/backend/formwidgets/FileUpload.php @@ -173,7 +173,7 @@ protected function getFileRecord() $record = false; if (!empty(post('file_id'))) { - $record = $this->getRelationModel()::find(post('file_id')) ?: false; + $record = $this->getRelationModel()->find(post('file_id')) ?: false; } return $record; @@ -333,18 +333,19 @@ public function getAcceptedFileTypes($includeDot = false) /** * Removes a file attachment. */ - public function onRemoveAttachment() + public function onRemoveAttachment(): void { - $fileModel = $this->getRelationModel(); - if (($fileId = post('file_id')) && ($file = $fileModel::find($fileId))) { + if ($file = $this->getFileRecord()) { $this->getRelationObject()->remove($file, $this->sessionKey); } } /** * Sorts file attachments. + * + * Expects (array) sortOrder [$fileId => $fileOrder] in the POST data. */ - public function onSortAttachments() + public function onSortAttachments(): void { if ($sortData = post('sortOrder')) { $ids = array_keys($sortData); From 981bb1c343c7afb95f11fd57b7cda19ab49fecf3 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 21 Nov 2022 15:20:03 -0600 Subject: [PATCH 3/6] Type hinting and style tweaks --- modules/backend/formwidgets/FileUpload.php | 34 ++++++++------------ modules/backend/formwidgets/RecordFinder.php | 5 ++- modules/backend/formwidgets/TagList.php | 3 +- modules/backend/traits/FormModelWidget.php | 32 ++++++++---------- modules/backend/widgets/Lists.php | 3 +- 5 files changed, 31 insertions(+), 46 deletions(-) diff --git a/modules/backend/formwidgets/FileUpload.php b/modules/backend/formwidgets/FileUpload.php index f19df77977..dc92668304 100644 --- a/modules/backend/formwidgets/FileUpload.php +++ b/modules/backend/formwidgets/FileUpload.php @@ -181,10 +181,8 @@ protected function getFileRecord() /** * Get the instantiated config Form widget - * - * @return void */ - public function getConfigFormWidget() + public function getConfigFormWidget(): Form { if ($this->configFormWidget) { return $this->configFormWidget; @@ -222,9 +220,8 @@ protected function getFileList() /** * Returns the display mode for the file upload. Eg: file-multi, image-single, etc. - * @return string */ - protected function getDisplayMode() + protected function getDisplayMode(): string { $mode = $this->getConfig('mode', 'image'); @@ -233,16 +230,15 @@ protected function getDisplayMode() } $relationType = $this->getRelationType(); - $mode .= ($relationType == 'attachMany' || $relationType == 'morphMany') ? '-multi' : '-single'; + $mode .= ($relationType === 'attachMany' || $relationType === 'morphMany') ? '-multi' : '-single'; return $mode; } /** * Returns the escaped and translated prompt text to display according to the type. - * @return string */ - protected function getPromptText() + protected function getPromptText(): string { if ($this->prompt === null) { $isMulti = ends_with($this->getDisplayMode(), 'multi'); @@ -257,10 +253,8 @@ protected function getPromptText() /** * Returns the CSS dimensions for the uploaded image, * uses auto where no dimension is provided. - * @param string $mode - * @return string */ - protected function getCssDimensions($mode = null) + protected function getCssDimensions(?string $mode = null): string { if (!$this->imageWidth && !$this->imageHeight) { return ''; @@ -270,20 +264,19 @@ protected function getCssDimensions($mode = null) if ($mode == 'block') { $cssDimensions .= $this->imageWidth - ? 'width: '.$this->imageWidth.'px;' - : 'width: '.$this->imageHeight.'px;'; + ? 'width: ' . $this->imageWidth . 'px;' + : 'width: ' . $this->imageHeight . 'px;'; $cssDimensions .= ($this->imageHeight) - ? 'max-height: '.$this->imageHeight.'px;' + ? 'max-height: ' . $this->imageHeight . 'px;' : 'height: auto;'; - } - else { + } else { $cssDimensions .= $this->imageWidth - ? 'width: '.$this->imageWidth.'px;' + ? 'width: ' . $this->imageWidth . 'px;' : 'width: auto;'; $cssDimensions .= ($this->imageHeight) - ? 'max-height: '.$this->imageHeight.'px;' + ? 'max-height: ' . $this->imageHeight . 'px;' : 'height: auto;'; } @@ -358,10 +351,11 @@ public function onSortAttachments(): void /** * Loads the configuration form for an attachment, allowing title and description to be set. + * + * @throws ApplicationException if unable to find the file record */ - public function onLoadAttachmentConfig() + public function onLoadAttachmentConfig(): string { - $fileModel = $this->getRelationModel(); if ($file = $this->getFileRecord()) { $file = $this->decorateFileAttributes($file); diff --git a/modules/backend/formwidgets/RecordFinder.php b/modules/backend/formwidgets/RecordFinder.php index 3ea6522c68..982e12dc01 100644 --- a/modules/backend/formwidgets/RecordFinder.php +++ b/modules/backend/formwidgets/RecordFinder.php @@ -3,6 +3,7 @@ use Lang; use ApplicationException; use Backend\Classes\FormWidgetBase; +use Winter\Storm\Database\Model; /** * Record Finder @@ -312,10 +313,8 @@ public function onFindRecord() /** * Gets the base model instance used by this field - * - * @return \Winter\Storm\Database\Model */ - protected function getRecordModel() + protected function getRecordModel(): Model { $model = null; if ($this->useRelation) { diff --git a/modules/backend/formwidgets/TagList.php b/modules/backend/formwidgets/TagList.php index 0bdc0c13c4..8389654108 100644 --- a/modules/backend/formwidgets/TagList.php +++ b/modules/backend/formwidgets/TagList.php @@ -120,9 +120,8 @@ public function getSaveValue($value) /** * Returns an array suitable for saving against a relation (array of keys). * This method also creates non-existent tags. - * @return array */ - protected function hydrateRelationSaveValue($names) + protected function hydrateRelationSaveValue($names): array { if (!$names) { return $names; diff --git a/modules/backend/traits/FormModelWidget.php b/modules/backend/traits/FormModelWidget.php index 5da7cc1d90..23215c4bef 100644 --- a/modules/backend/traits/FormModelWidget.php +++ b/modules/backend/traits/FormModelWidget.php @@ -3,6 +3,8 @@ use Lang; use ApplicationException; use Exception; +use Winter\Storm\Database\Model; +use Winter\Storm\Database\Relations\Relation; /** * Form Model Widget Trait @@ -12,18 +14,14 @@ * @package winter\wn-backend-module * @author Alexey Bobkov, Samuel Georges */ - trait FormModelWidget { - /** - * Returns the final model and attribute name of - * a nested HTML array attribute. + * Returns the final model and attribute name of a nested HTML array attribute. * Eg: list($model, $attribute) = $this->resolveModelAttribute($this->valueFrom); - * @param string $attribute. - * @return array + * @throws ApplicationException if something goes wrong when attempting to resolve the model attribute */ - public function resolveModelAttribute($attribute) + public function resolveModelAttribute(string $attribute): array { try { return $this->formField->resolveModelAttribute($this->model, $attribute); @@ -37,11 +35,10 @@ public function resolveModelAttribute($attribute) } /** - * Returns the model of a relation type, - * supports nesting via HTML array. - * @return Relation + * Returns the model of a relation type, supports nesting via HTML array. + * @throws ApplicationException if the related model cannot be resolved */ - public function getRelationModel() + public function getRelationModel(): Model { list($model, $attribute) = $this->resolveModelAttribute($this->valueFrom); @@ -63,11 +60,10 @@ public function getRelationModel() } /** - * Returns the value as a relation object from the model, - * supports nesting via HTML array. - * @return Relation + * Returns the value as a relation object from the model, supports nesting via HTML array. + * @throws ApplicationException if the relationship cannot be resolved */ - protected function getRelationObject() + protected function getRelationObject(): Relation { list($model, $attribute) = $this->resolveModelAttribute($this->valueFrom); @@ -89,11 +85,9 @@ protected function getRelationObject() } /** - * Returns the value as a relation type from the model, - * supports nesting via HTML array. - * @return Relation + * Returns the value as a relation type from the model, supports nesting via HTML array. */ - protected function getRelationType() + protected function getRelationType(): ?string { list($model, $attribute) = $this->resolveModelAttribute($this->valueFrom); return $model->getRelationType($attribute); diff --git a/modules/backend/widgets/Lists.php b/modules/backend/widgets/Lists.php index dc9e181254..c0837e2885 100644 --- a/modules/backend/widgets/Lists.php +++ b/modules/backend/widgets/Lists.php @@ -1802,11 +1802,10 @@ public function onToggleTreeNode() /** * Check if column refers to a relation of the model - * @param ListColumn $column List column object * @param boolean $multi If set, returns true only if the relation is a "multiple relation type" * @return boolean */ - protected function isColumnRelated($column, $multi = false) + protected function isColumnRelated(ListColumn $column, bool $multi = false): bool { if (!isset($column->relation) || $this->isColumnPivot($column)) { return false; From 5be4454c447a7cb236bb1aaa6b1462987c656dc9 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 21 Nov 2022 15:34:18 -0600 Subject: [PATCH 4/6] Don't register backend permissions in the testing suite --- modules/backend/ServiceProvider.php | 8 +++++--- modules/cms/ServiceProvider.php | 7 +++++-- modules/system/ServiceProvider.php | 8 +++++--- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/modules/backend/ServiceProvider.php b/modules/backend/ServiceProvider.php index 839f81d808..bc0708e6d5 100644 --- a/modules/backend/ServiceProvider.php +++ b/modules/backend/ServiceProvider.php @@ -1,6 +1,5 @@ registerConsole(); $this->registerMailer(); $this->registerAssetBundles(); - $this->registerBackendPermissions(); + + if (!$this->app->runningUnitTests()) { + $this->registerBackendPermissions(); + } /* * Backend specific */ - if (App::runningInBackend()) { + if ($this->app->runningInBackend()) { $this->registerBackendNavigation(); $this->registerBackendReportWidgets(); $this->registerBackendWidgets(); diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index ae901c4a94..575bf76496 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -43,12 +43,15 @@ public function register() $this->registerThemeLogging(); $this->registerCombinerEvents(); $this->registerHalcyonModels(); - $this->registerBackendPermissions(); + + if (!$this->app->runningUnitTests()) { + $this->registerBackendPermissions(); + } /* * Backend specific */ - if (App::runningInBackend()) { + if ($this->app->runningInBackend()) { $this->registerBackendNavigation(); $this->registerBackendReportWidgets(); $this->registerBackendWidgets(); diff --git a/modules/system/ServiceProvider.php b/modules/system/ServiceProvider.php index 0d2a4118f1..57befa751f 100644 --- a/modules/system/ServiceProvider.php +++ b/modules/system/ServiceProvider.php @@ -61,16 +61,18 @@ public function register() */ foreach (Config::get('cms.loadModules', []) as $module) { if (strtolower(trim($module)) != 'system') { - App::register('\\' . $module . '\ServiceProvider'); + $this->app->register('\\' . $module . '\ServiceProvider'); } } - $this->registerBackendPermissions(); + if (!$this->app->runningUnitTests()) { + $this->registerBackendPermissions(); + } /* * Backend specific */ - if (App::runningInBackend()) { + if ($this->app->runningInBackend()) { $this->registerBackendNavigation(); $this->registerBackendReportWidgets(); $this->registerBackendSettings(); From f2447bb136da2d7f15ae5305ded719f86e26e2f3 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 21 Nov 2022 15:40:39 -0600 Subject: [PATCH 5/6] Use local app instance rather than facade --- modules/cms/ServiceProvider.php | 38 ++++++++++----------- modules/system/ServiceProvider.php | 53 ++++++++++++++---------------- 2 files changed, 43 insertions(+), 48 deletions(-) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index 575bf76496..2f318b302a 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -1,30 +1,28 @@ app->bind('twig.environment.cms', function ($app) { // Load Twig options $useCache = !Config::get('cms.twigNoCache'); $isDebugMode = Config::get('app.debug', false); @@ -166,7 +164,7 @@ protected function registerThemeLogging() */ protected function registerCombinerEvents() { - if (App::runningInBackend() || App::runningInConsole()) { + if ($this->app->runningInBackend() || $this->app->runningInConsole()) { return; } @@ -389,7 +387,7 @@ protected function registerBackendSettings() */ protected function bootBackendLocalization() { - $theme = CmsTheme::getActiveTheme(); + $theme = Theme::getActiveTheme(); if (is_null($theme)) { return; diff --git a/modules/system/ServiceProvider.php b/modules/system/ServiceProvider.php index 57befa751f..6027d8fd31 100644 --- a/modules/system/ServiceProvider.php +++ b/modules/system/ServiceProvider.php @@ -1,33 +1,30 @@ app->singleton('cms.helper', function () { return new \Cms\Helpers\Cms; }); - App::singleton('backend.helper', function () { + $this->app->singleton('backend.helper', function () { return new \Backend\Helpers\Backend; }); - App::singleton('backend.menu', function () { + $this->app->singleton('backend.menu', function () { return \Backend\Classes\NavigationManager::instance(); }); - App::singleton('backend.auth', function () { + $this->app->singleton('backend.auth', function () { return \Backend\Classes\AuthManager::instance(); }); } @@ -159,7 +156,7 @@ protected function registerPrivilegedActions() /* * CLI */ - if (App::runningInConsole() && count(array_intersect($commands, Request::server('argv', []))) > 0) { + if ($this->app->runningInConsole() && count(array_intersect($commands, Request::server('argv', []))) > 0) { PluginManager::$noInit = true; } } @@ -223,7 +220,7 @@ protected function registerConsole() */ Event::listen('console.schedule', function ($schedule) { // Fix initial system migration with plugins that use settings for scheduling - see #3208 - if (App::hasDatabase() && !Schema::hasTable(UpdateManager::instance()->getMigrationTableName())) { + if ($this->app->hasDatabase() && !Schema::hasTable(UpdateManager::instance()->getMigrationTableName())) { return; } @@ -309,20 +306,20 @@ protected function registerLogging() protected function registerTwigParser() { // Register System Twig environment - App::singleton('twig.environment', function ($app) { + $this->app->singleton('twig.environment', function ($app) { return MarkupManager::makeBaseTwigEnvironment(); }); // Register Mailer Twig environment - App::singleton('twig.environment.mailer', function ($app) { + $this->app->singleton('twig.environment.mailer', function ($app) { $twig = MarkupManager::makeBaseTwigEnvironment(); $twig->addTokenParser(new \System\Twig\MailPartialTokenParser); return $twig; }); // Register .htm extension for Twig views - App::make('view')->addExtension('htm', 'twig', function () { - return new TwigEngine(App::make('twig.environment')); + $this->app->make('view')->addExtension('htm', 'twig', function () { + return new TwigEngine($this->app->make('twig.environment')); }); } From 1d8af5964c65df5e7c29eb50efb59cd60d96f9d6 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 21 Nov 2022 15:44:45 -0600 Subject: [PATCH 6/6] Fix overlooked reference to App facade instead of local instance --- modules/cms/ServiceProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cms/ServiceProvider.php b/modules/cms/ServiceProvider.php index 2f318b302a..2398e6f476 100644 --- a/modules/cms/ServiceProvider.php +++ b/modules/cms/ServiceProvider.php @@ -69,7 +69,7 @@ public function boot() $this->bootMenuItemEvents(); $this->bootRichEditorEvents(); - if (App::runningInBackend()) { + if ($this->app->runningInBackend()) { $this->bootBackendLocalization(); } }