fix: fixed #80

overtrue · Jul 9, 2024 · 059d042 · 059d042
1 parent bcff615
commit 059d042
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 26 deletions.
diff --git a/src/Version.php b/src/Version.php
@@ -95,7 +95,7 @@ public static function createForModel(Model $model, array $replacements = [], $t
         $version->versionable_id = $model->getKey();
         $version->versionable_type = $model->getMorphClass();
         $version->{$userForeignKeyName} = $model->getVersionUserId();
-        $version->contents = $model->getVersionableAttributes($replacements);
+        $version->contents = $model->getVersionableAttributes($model->getVersionStrategy(), $replacements);
 
         if ($time) {
             $version->created_at = Carbon::parse($time);

diff --git a/src/Versionable.php b/src/Versionable.php
@@ -86,8 +86,9 @@ public function createInitialVersion(Model $model): Version
         /**
          * As initial version should include all $versionable fields,
          * we need to get the latest version from database.
+         * so we force to create a snapshot version.
          */
-        $attributes = $refreshedModel->getSnapshotAttributes();
+        $attributes = $refreshedModel->getVersionableAttributes(VersionStrategy::SNAPSHOT);
 
         return Version::createForModel($refreshedModel, $attributes, $refreshedModel->updated_at);
     }
@@ -181,7 +182,7 @@ public function removeVersions(array $ids)
             return $this->forceRemoveVersions($ids);
         }
 
-        return $this->versions()->find($ids)->each->delete();
+        return $this->versions()->findMany($ids)->each->delete();
     }
 
     public function removeVersion(int $id)
@@ -224,32 +225,26 @@ public function shouldBeVersioning(): bool
             return call_user_func([$this, 'shouldVersioning']);
         }
 
-        if ($this->versions()->count() === 0 || Arr::hasAny($this->getDirty(), array_keys($this->getVersionableAttributes()))) {
-            return true;
-        }
+        $versionableAttributes = $this->getVersionableAttributes($this->getVersionStrategy());
 
-        return false;
+        return $this->versions()->count() === 0 || Arr::hasAny($this->getDirty(), array_keys($versionableAttributes));
     }
 
-    public function getVersionableAttributes(array $replacements = []): array
-    {
-        return match ($this->getVersionStrategy()) {
-            VersionStrategy::DIFF => $this->getDiffAttributes($replacements),
-            VersionStrategy::SNAPSHOT => $this->getSnapshotAttributes($replacements),
-        };
-    }
-
-    protected function getDiffAttributes(array $replacements = []): array
-    {
-        return array_merge($this->getDirty(), $replacements);
-    }
-
-    protected function getSnapshotAttributes(array $replacements = []): array
+    public function getVersionableAttributes(VersionStrategy $strategy, array $replacements = []): array
     {
         $versionable = $this->getVersionable();
         $dontVersionable = $this->getDontVersionable();
 
-        $attributes = count($versionable) > 0 ? $this->only($versionable) : $this->attributesToArray();
+        $attributes = match ($strategy) {
+            VersionStrategy::DIFF => $this->getDirty(),
+            // To avoid some attributes are empty (not sync to database)
+            // we should get the latest version from database.
+            VersionStrategy::SNAPSHOT => $this->newQueryWithoutScopes()->find($this->getKey())?->attributesToArray() ?? [],
+        };
+
+        if (count($versionable) > 0) {
+            $attributes = Arr::only($attributes, $versionable);
+        }
 
         return Arr::except(array_merge($attributes, $replacements), $dontVersionable);
     }
@@ -294,7 +289,12 @@ public function getDontVersionable(): array
 
     public function getVersionStrategy(): VersionStrategy
     {
-        return \property_exists($this, 'versionStrategy') ? $this->versionStrategy : VersionStrategy::DIFF;
+        if (\property_exists($this, 'versionStrategy')) {
+            return $this->versionStrategy instanceof VersionStrategy ? $this->versionStrategy : VersionStrategy::from($this->versionStrategy);
+        }
+
+        // TODO: set default strategy to SNAPSHOT at 6.x
+        return VersionStrategy::DIFF;
     }
 
     /**

diff --git a/tests/FeatureTest.php b/tests/FeatureTest.php
@@ -90,11 +90,12 @@ public function it_can_create_version_with_snapshot_strategy()
         $post = new Post(['title' => 'version1', 'content' => 'version1 content', 'user_id' => 1234]);
 
         // change strategy to snapshot
-        $post->setVersionStrategy(VersionStrategy::SNAPSHOT);
+        $post->setVersionStrategy(VersionStrategy::SNAPSHOT); // snapshot
 
-        $post->save();
+        $post->save(); // version 1
+
+        $post->update(['title' => 'version2']); // version 2
 
-        $post->update(['title' => 'version2']);
         $post->refresh();
 
         $this->assertCount(2, $post->versions);