Fixed #41

overtrue · Oct 6, 2022 · 41547ec · 41547ec
1 parent 366a193
commit 41547ec
Show file tree

Hide file tree

Showing 10 changed files with 88 additions and 114 deletions.
diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php
diff --git a/composer.json b/composer.json
@@ -10,8 +10,8 @@
     "require-dev": {
         "phpunit/phpunit": "^9.0",
         "orchestra/testbench": "^7.0",
-        "friendsofphp/php-cs-fixer": "^3.6",
-        "mockery/mockery": "^1.4"
+        "mockery/mockery": "^1.4",
+        "laravel/pint": "^1.2"
     },
     "autoload": {
         "psr-4": {
@@ -42,8 +42,8 @@
     "scripts": {
         "post-merge": "composer install",
         "cghooks": "vendor/bin/cghooks",
-        "check-style": "php-cs-fixer fix --using-cache=no --diff --dry-run --ansi",
-        "fix-style": "php-cs-fixer fix --using-cache=no --ansi",
+        "check-style": "vendor/bin/pint --test",
+        "fix-style": "vendor/bin/pint",
         "test": "phpunit --colors"
     },
     "scripts-descriptions": {

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -1,21 +1,13 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<phpunit backupGlobals="false"
-         backupStaticAttributes="false"
-         bootstrap="vendor/autoload.php"
-         colors="true"
-         convertErrorsToExceptions="true"
-         convertNoticesToExceptions="true"
-         convertWarningsToExceptions="true"
-         processIsolation="false"
-         stopOnFailure="false">
-    <testsuites>
-        <testsuite name="Application Test Suite">
-            <directory>./tests/</directory>
-        </testsuite>
-    </testsuites>
-    <filter>
-        <whitelist>
-            <directory suffix=".php">src/</directory>
-        </whitelist>
-    </filter>
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" backupGlobals="false" backupStaticAttributes="false" bootstrap="vendor/autoload.php" colors="true" convertErrorsToExceptions="true" convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="false" xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
+  <coverage>
+    <include>
+      <directory suffix=".php">src/</directory>
+    </include>
+  </coverage>
+  <testsuites>
+    <testsuite name="Application Test Suite">
+      <directory>./tests/</directory>
+    </testsuite>
+  </testsuites>
 </phpunit>
diff --git a/src/Version.php b/src/Version.php
@@ -8,7 +8,7 @@
 /**
  * @property Model|\Overtrue\LaravelVersionable\Versionable $versionable
  * @property array $contents
- * @property integer $id
+ * @property int $id
  */
 class Version extends Model
 {
@@ -49,8 +49,7 @@ public function versionable(): \Illuminate\Database\Eloquent\Relations\MorphTo
 
     /**
      * @param  \Illuminate\Database\Eloquent\Model  $model
-     * @param  array                                $attributes
-     *
+     * @param  array  $attributes
      * @return \Overtrue\LaravelVersionable\Version
      */
     public static function createForModel(Model $model, array $attributes = []): Version
@@ -74,7 +73,7 @@ public static function createForModel(Model $model, array $attributes = []): Ver
 
     public function revert(): bool
     {
-        return $this->versionable->forceFill($this->contents)->save();
+        return $this->revertWithoutSaving()->save();
     }
 
     public function revertWithoutSaving(): ?Model
@@ -94,7 +93,7 @@ public function nextVersion(): ?static
 
     public function diff(Version $toVersion = null, array $differOptions = [], array $renderOptions = []): Diff
     {
-        if (!$toVersion) {
+        if (! $toVersion) {
             $toVersion = $this->previousVersion() ?? new static();
         }
 

diff --git a/src/VersionStrategy.php b/src/VersionStrategy.php
@@ -5,5 +5,6 @@
 class VersionStrategy
 {
     public const DIFF = 'diff';
+
     public const SNAPSHOT = 'snapshot';
 }
diff --git a/src/Versionable.php b/src/Versionable.php
@@ -10,6 +10,7 @@
 trait Versionable
 {
     protected static bool $versioning = true;
+
     protected bool $forceDeleteVersion = false;
 
     // You can add these properties to you versionable model
@@ -68,12 +69,11 @@ public function firstVersion(): MorphOne
     /**
      * Get the version for a specific time.
      *
-     * @param string|DateTimeInterface|null $time
-     * @param DateTimeZone|string|null      $tz
+     * @param  string|DateTimeInterface|null  $time
+     * @param  DateTimeZone|string|null  $tz
+     * @return static
      *
      * @throws \Carbon\Exceptions\InvalidFormatException
-     *
-     * @return static
      */
     public function versionAt($time = null, $tz = null): ?Version
     {
@@ -157,7 +157,7 @@ public function forceRemoveAllVersions(): void
 
     public function shouldVersioning(): bool
     {
-        return !empty($this->getVersionableAttributes());
+        return ! empty($this->getVersionableAttributes());
     }
 
     public function getVersionableAttributes(): array
@@ -168,21 +168,23 @@ public function getVersionableAttributes(): array
             return [];
         }
 
-        $contents = $this->attributesToArray();
+        $changes = $this->versionableFromArray($changes);
+        $changedKeys = array_keys($changes);
 
-        if ($this->getVersionStrategy() == VersionStrategy::DIFF) {
-            $contents = $this->only(\array_keys($changes));
+        if ($this->getVersionStrategy() === VersionStrategy::SNAPSHOT && ! empty($changes)) {
+            $changedKeys = array_keys($this->getAttributes());
         }
 
-        return $this->versionableFromArray($contents);
+        // to keep casts and mutators works, we need to get the updated attributes from the model
+        return $this->only($changedKeys);
     }
 
     /**
      * @throws \Exception
      */
     public function setVersionable(array $attributes): static
     {
-        if (!\property_exists($this, 'versionable')) {
+        if (! \property_exists($this, 'versionable')) {
             throw new \Exception('Property $versionable not exist.');
         }
 
@@ -196,7 +198,7 @@ public function setVersionable(array $attributes): static
      */
     public function setDontVersionable(array $attributes): static
     {
-        if (!\property_exists($this, 'dontVersionable')) {
+        if (! \property_exists($this, 'dontVersionable')) {
             throw new \Exception('Property $dontVersionable not exist.');
         }
 
@@ -225,7 +227,7 @@ public function getVersionStrategy(): string
      */
     public function setVersionStrategy(string $strategy): static
     {
-        if (!\property_exists($this, 'versionStrategy')) {
+        if (! \property_exists($this, 'versionStrategy')) {
             throw new \Exception('Property $versionStrategy not exist.');
         }
 

diff --git a/tests/DiffTest.php b/tests/DiffTest.php
@@ -40,8 +40,8 @@ public function test_diff_to_context_text()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "Context"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "Context"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'Context'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'Context'),
             ],
             (new Diff($new, $old))->toContextText()
         );
@@ -54,8 +54,8 @@ public function test_diff_to_text()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "Unified"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "Unified"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'Unified'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'Unified'),
             ],
             (new Diff($new, $old))->toText()
         );
@@ -68,8 +68,8 @@ public function test_diff_to_json_text()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "JsonText"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "JsonText"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'JsonText'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'JsonText'),
             ],
             (new Diff($new, $old))->toJsonText()
         );
@@ -82,8 +82,8 @@ public function test_diff_to_html()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "Combined"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "Combined"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'Combined'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'Combined'),
             ],
             (new Diff($new, $old))->toHtml()
         );
@@ -96,8 +96,8 @@ public function test_diff_to_inline_html()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "Inline"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "Inline"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'Inline'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'Inline'),
             ],
             (new Diff($new, $old))->toInlineHtml()
         );
@@ -110,8 +110,8 @@ public function test_diff_to_json_html()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "JsonHtml"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "JsonHtml"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'JsonHtml'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'JsonHtml'),
             ],
             (new Diff($new, $old))->toJsonHtml()
         );
@@ -124,8 +124,8 @@ public function test_diff_to_side_by_side_html()
 
         $this->assertSame(
             [
-                'content' => DiffHelper::calculate("version1 content", "version2 content", "SideBySide"),
-                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), "SideBySide"),
+                'content' => DiffHelper::calculate('version1 content', 'version2 content', 'SideBySide'),
+                'user_id' => DiffHelper::calculate(json_encode(null), json_encode(123), 'SideBySide'),
             ],
             (new Diff($new, $old))->toSideBySideHtml()
         );