From 3d3cb0600a34ebcc919586dec1e4a5d3d242ef13 Mon Sep 17 00:00:00 2001 From: Yaohan Chen Date: Fri, 16 Nov 2018 14:23:06 -0500 Subject: [PATCH 1/3] Add failing testcase for model with a JSON field --- tests/VersionableTest.php | 16 ++++++++++++++++ tests/VersionableTestCase.php | 6 ++++++ 2 files changed, 22 insertions(+) diff --git a/tests/VersionableTest.php b/tests/VersionableTest.php index 6160b57..073a0a1 100644 --- a/tests/VersionableTest.php +++ b/tests/VersionableTest.php @@ -289,6 +289,15 @@ public function testGetVersionModel() } + public function testGetVersionModelWithJsonField() + { + $model = new ModelWithJsonField(); + $model->json_field = ["foo" => "bar"]; + $model->save(); + + $this->assertEquals(["foo" => "bar"], $model->getVersionModel(1)->json_field); + } + public function testUseReasonAttribute() { // Create 3 versions @@ -547,4 +556,11 @@ class ModelWithDynamicVersion extends Model use VersionableTrait ; protected $versionClass = DynamicVersionModel::class ; } +class ModelWithJsonField extends Model +{ + const TABLENAME = 'table_with_json_field'; + public $table = self::TABLENAME ; + use VersionableTrait ; + protected $casts = ['json_field' => 'array']; +} diff --git a/tests/VersionableTestCase.php b/tests/VersionableTestCase.php index c09e36b..52c4cbd 100644 --- a/tests/VersionableTestCase.php +++ b/tests/VersionableTestCase.php @@ -65,5 +65,11 @@ public function migrateUsersTable() $table->index('versionable_id'); $table->timestamps(); }); + + $this->app['db']->connection()->getSchemaBuilder()->create(ModelWithJsonField::TABLENAME, function ($table) { + $table->increments('id'); + $table->json('json_field'); + $table->timestamps(); + }); } } \ No newline at end of file From 5d0aa7179c6f171292a75b09760ab26e7eb2fbf5 Mon Sep 17 00:00:00 2001 From: Yaohan Chen Date: Fri, 16 Nov 2018 14:38:52 -0500 Subject: [PATCH 2/3] Fix saving version of model with JSON field When a model uses `$casts = ['field' => 'array']` with a JSON field, getAttributes() will return the JSON-encoded string, while $model->fill() expects the input to contain the un-encoded JSON value. So instead, use attributesToArray() to get the model data, which will work properly with fill(). Fixes #43 --- src/Mpociot/Versionable/VersionableTrait.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mpociot/Versionable/VersionableTrait.php b/src/Mpociot/Versionable/VersionableTrait.php index cb377ed..ffca6c6 100644 --- a/src/Mpociot/Versionable/VersionableTrait.php +++ b/src/Mpociot/Versionable/VersionableTrait.php @@ -175,7 +175,7 @@ protected function versionablePostSave() $version->versionable_id = $this->getKey(); $version->versionable_type = get_class($this); $version->user_id = $this->getAuthUserId(); - $version->model_data = serialize($this->getAttributes()); + $version->model_data = serialize($this->attributesToArray()); if (!empty( $this->reason )) { $version->reason = $this->reason; From e35bca6aee8fe8b5ccc9c16180b72874bfa67ba8 Mon Sep 17 00:00:00 2001 From: Yaohan Chen Date: Fri, 16 Nov 2018 14:04:01 -0500 Subject: [PATCH 3/3] Move model data serialization and deserialization to separate methods This allows extending the Version class and changing the migration to store model data differently Implements #44 --- src/Mpociot/Versionable/Version.php | 26 ++++++++++++++++---- src/Mpociot/Versionable/VersionableTrait.php | 2 +- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/Mpociot/Versionable/Version.php b/src/Mpociot/Versionable/Version.php index bf18534..36cb40d 100644 --- a/src/Mpociot/Versionable/Version.php +++ b/src/Mpociot/Versionable/Version.php @@ -47,13 +47,9 @@ public function getResponsibleUserAttribute() */ public function getModel() { - $modelData = is_resource($this->model_data) - ? stream_get_contents($this->model_data) - : $this->model_data; - $model = new $this->versionable_type(); $model->unguard(); - $model->fill(unserialize($modelData)); + $model->fill($this->modelData()); $model->exists = true; $model->reguard(); return $model; @@ -104,4 +100,24 @@ public function diff(Version $againstVersion = null) return $diffArray; } + /** + * Copy attributes of a model to be saved with the version + * + * @param Model $model + */ + public function copyModelData(Model $model) + { + $this->model_data = serialize($model->attributesToArray()); + } + + /** + * Model attributes + */ + protected function modelData() + { + $serialized = is_resource($this->model_data) + ? stream_get_contents($this->model_data) + : $this->model_data; + return unserialize($serialized); + } } diff --git a/src/Mpociot/Versionable/VersionableTrait.php b/src/Mpociot/Versionable/VersionableTrait.php index ffca6c6..8ff0acd 100644 --- a/src/Mpociot/Versionable/VersionableTrait.php +++ b/src/Mpociot/Versionable/VersionableTrait.php @@ -175,7 +175,7 @@ protected function versionablePostSave() $version->versionable_id = $this->getKey(); $version->versionable_type = get_class($this); $version->user_id = $this->getAuthUserId(); - $version->model_data = serialize($this->attributesToArray()); + $version->copyModelData($this); if (!empty( $this->reason )) { $version->reason = $this->reason;