pkp#10292 Don't remove unset settings during Model update

taslangraham · Nov 12, 2024 · 88ab86b · 88ab86b
1 parent 40e8dd1
commit 88ab86b
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/classes/announcement/Announcement.php b/classes/announcement/Announcement.php
@@ -111,20 +111,28 @@ public function save(array $options = [])
         $newlyCreated = !$this->exists;
         $saved = parent::save($options);
 
-        // If it's a new model with an image attribute, upload an image
-        if ($saved && $newlyCreated && $this->hasAttribute('image')) {
-            $this->handleImageUpload();
+        if (!$saved) {
+            return $saved;
         }
 
-        // If it's updated model and a new image is uploaded, first, delete an old one
+        Hook::call('Announcement::add', [$this]);
+
         $hasNewImage = $this?->image?->temporaryFileId;
-        if ($saved && !$newlyCreated && $hasNewImage) {
+
+        // if announcement is being inserted and includes new image, upload it
+        if ($newlyCreated) {
+            if ($hasNewImage) {
+                $this->handleImageUpload();
+            }
+            return $saved;
+        }
+
+        // The announcement is being updated, check if it contains new image
+        if ($hasNewImage) {
             $this->deleteImage();
             $this->handleImageUpload();
         }
 
-        Hook::call('Announcement::add', [$this]);
-
         return $saved;
     }
 

diff --git a/classes/core/SettingsBuilder.php b/classes/core/SettingsBuilder.php
@@ -98,7 +98,7 @@ public function update(array $values)
 
     /**
      * Insert the given attributes and set the ID on the model.
-     * Overrides Builder's method to insert setting values for a models with
+     * Overrides Builder's method to insert setting values for a models with settings
      *
      * @param  string|null  $sequence
      *
@@ -111,14 +111,14 @@ public function insertGetId(array $values, $sequence = null)
             fn (mixed $value, string $key) => in_array(Str::camel($key), $this->model->getSettings())
         );
 
+
         $id = parent::insertGetId($primaryValues->toArray(), $sequence);
 
         if ($settingValues->isEmpty()) {
             return $id;
         }
 
         $rows = $this->getSettingRows($settingValues, $id);
-
         DB::table($this->getSettingsTable())->insert($rows);
 
         return $id;

diff --git a/classes/core/traits/EntityUpdate.php b/classes/core/traits/EntityUpdate.php
@@ -56,8 +56,14 @@ abstract public function isSetting(string $settingName): bool;
     public function updateSettings(array $props, int $modelId, $schema = null): void
     {
         $schemaService = app()->get('schema'); /** @var PKPSchemaService $schemaService */
+        $schemaName = $this->getSchemaName();
+
         if (is_null($schema)) {
-            $schema = $schemaService->get($this->getSchemaName());
+            $schema = $schemaService->get($schemaName);
+        }
+
+        if ($schemaName) {
+            $props = $schemaService->sanitize($this->getSchemaName(), $props);
         }
 
         $deleteSettings = [];
@@ -110,7 +116,8 @@ public function updateSettings(array $props, int $modelId, $schema = null): void
             }
         }
 
-        if (count($deleteSettings)) {
+        // Entity DAO passes all properties for the update and removes all that aren't set
+        if (count($deleteSettings) && is_a($this, DataObject::class)) {
             DB::table($this->getSettingsTable())
                 ->where($this->getPrimaryKeyName(), '=', $modelId)
                 ->whereIn('setting_name', $deleteSettings)