From 84aa9588432865b3dc339e45ecd62db51361fa1d Mon Sep 17 00:00:00 2001 From: Gerbrand Sterrenburg Date: Wed, 1 Nov 2023 17:02:25 +0100 Subject: [PATCH 1/3] added test that shows the issue --- tests/Feature/MustBeApprovedTraitTest.php | 32 +++++++++++++++++++ tests/Models/FakeModelWithArray.php | 21 ++++++++++++ ...500_create_fake_model_with_array_table.php | 19 +++++++++++ 3 files changed, 72 insertions(+) create mode 100644 tests/Models/FakeModelWithArray.php create mode 100644 tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index 7d0ca6d..52a33d6 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -4,6 +4,7 @@ use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Models\Approval; use Cjmellor\Approval\Tests\Models\FakeModel; +use Cjmellor\Approval\Tests\Models\FakeModelWithArray; use Illuminate\Database\Eloquent\Model; it(description: 'stores the data correctly in the database') @@ -166,3 +167,34 @@ 'meta' => 'blue', ]); }); + +test(description: 'approve a attribute of the type Array', closure: function () { + $model = new FakeModelWithArray(); + + // create a model + $model->create([ + 'name' => 'Neo', + 'data' => ['foo', 'bar'] + ]); + + // check if the data is stored correctly in the approval table + $this->assertDatabaseHas(table: Approval::class, data: [ + 'new_data' => json_encode(['name' => 'Neo', 'data' => json_encode(['foo', 'bar'])]), + 'original_data' => json_encode([]), + ]); + + // nothing should be in the 'fake_models_with_array' table + $this->assertDatabaseCount('fake_models_with_array', 0); + + // approve the model + Approval::first()->approve(); + + // after approval, there should be in a entry in the 'fake_models_with_array' table + $this->assertDatabaseCount('fake_models_with_array', 1); + + // After Approval, the contents of the database should look like this + $this->assertDatabaseHas(table: FakeModelWithArray::class, data: [ + 'name' => 'Neo', + 'data' => ['foo', 'bar'] + ]); +}); diff --git a/tests/Models/FakeModelWithArray.php b/tests/Models/FakeModelWithArray.php new file mode 100644 index 0000000..7dc21f1 --- /dev/null +++ b/tests/Models/FakeModelWithArray.php @@ -0,0 +1,21 @@ + 'array' + ]; +} diff --git a/tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php b/tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php new file mode 100644 index 0000000..16936b6 --- /dev/null +++ b/tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php @@ -0,0 +1,19 @@ +id(); + $table->string('name')->nullable(); + $table->string('meta')->nullable(); + + $table->json('data')->nullable(); + }); + } +}; From 48e1c51202486a4464b4cb94f0206bd29173850c Mon Sep 17 00:00:00 2001 From: Gerbrand Sterrenburg Date: Wed, 8 Nov 2023 15:27:43 +0100 Subject: [PATCH 2/3] possible fix for issue #45 --- src/Concerns/MustBeApproved.php | 16 ++++++++++++++++ src/Scopes/ApprovalStateScope.php | 10 +++++++++- tests/Feature/MustBeApprovedTraitTest.php | 7 ++++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index 21a52d0..ee79375 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -127,4 +127,20 @@ public function withoutApproval(): static return $this; } + + /** + * Wrapper to access the castAttribute function + * + * @param $key + * @param $value + * @return mixed + */ + public function callCastAttribute($key, $value): mixed + { + if (array_key_exists($key, $this->casts)) { + return $this->castAttribute($key, $value); + } + + return $value; + } } diff --git a/src/Scopes/ApprovalStateScope.php b/src/Scopes/ApprovalStateScope.php index abb3e7e..15faf35 100644 --- a/src/Scopes/ApprovalStateScope.php +++ b/src/Scopes/ApprovalStateScope.php @@ -104,7 +104,15 @@ protected function addApprove(Builder $builder): void $model = $model->find($modelId); } - $model->forceFill($builder->getModel()->new_data->toArray()); + $newData = $builder->getModel()->new_data->toArray(); + + // make sure we cast all attributes + foreach ($newData as $key => $value) { + $newData[$key] = $model->callCastAttribute($key, $value); + } + + $model->forceFill($newData); + $model->withoutApproval()->save(); } diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index 52a33d6..17c105e 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -195,6 +195,11 @@ // After Approval, the contents of the database should look like this $this->assertDatabaseHas(table: FakeModelWithArray::class, data: [ 'name' => 'Neo', - 'data' => ['foo', 'bar'] + 'data' => json_encode(['foo', 'bar']) ]); + + // doublecheck the model + $modelFromDatabase = FakeModelWithArray::firstWhere('name', 'Neo'); + expect($modelFromDatabase->data) + ->toBe(['foo', 'bar']); }); From d623d43b355658181c6e86f5607992cd07cef3ab Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Thu, 9 Nov 2023 22:55:25 +0000 Subject: [PATCH 3/3] Inline test schema and model --- src/Concerns/MustBeApproved.php | 2 +- tests/Feature/MustBeApprovedTraitTest.php | 37 +++++++++++++++---- tests/Models/FakeModelWithArray.php | 21 ----------- ...500_create_fake_model_with_array_table.php | 19 ---------- 4 files changed, 30 insertions(+), 49 deletions(-) delete mode 100644 tests/Models/FakeModelWithArray.php delete mode 100644 tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index ee79375..183353a 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -58,7 +58,7 @@ protected static function insertApprovalRequest($model) if ($noNeedToProceed) { return false; } - return; + } /** diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index 17c105e..ab71acd 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -4,8 +4,9 @@ use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Models\Approval; use Cjmellor\Approval\Tests\Models\FakeModel; -use Cjmellor\Approval\Tests\Models\FakeModelWithArray; use Illuminate\Database\Eloquent\Model; +use Illuminate\Database\Schema\Blueprint; +use Illuminate\Support\Facades\Schema; it(description: 'stores the data correctly in the database') ->defer( @@ -169,12 +170,31 @@ }); test(description: 'approve a attribute of the type Array', closure: function () { - $model = new FakeModelWithArray(); + Schema::create('fake_models_with_array', function (Blueprint $table) { + $table->id(); + $table->string('name')->nullable(); + $table->string('meta')->nullable(); + + $table->json('data')->nullable(); + }); + + $model = new class extends Model + { + use MustBeApproved; + + protected $table = 'fake_models_with_array'; + + protected $guarded = []; + + public $timestamps = false; + + protected $casts = ['data' => 'array']; + }; // create a model $model->create([ 'name' => 'Neo', - 'data' => ['foo', 'bar'] + 'data' => ['foo', 'bar'], ]); // check if the data is stored correctly in the approval table @@ -189,17 +209,18 @@ // approve the model Approval::first()->approve(); - // after approval, there should be in a entry in the 'fake_models_with_array' table + // after approval, there should be in an entry in the 'fake_models_with_array' table $this->assertDatabaseCount('fake_models_with_array', 1); // After Approval, the contents of the database should look like this - $this->assertDatabaseHas(table: FakeModelWithArray::class, data: [ + $this->assertDatabaseHas(table: 'fake_models_with_array', data: [ 'name' => 'Neo', - 'data' => json_encode(['foo', 'bar']) + 'data' => json_encode(['foo', 'bar']), ]); - // doublecheck the model - $modelFromDatabase = FakeModelWithArray::firstWhere('name', 'Neo'); + // double check the model + $modelFromDatabase = $model->firstWhere('name', 'Neo'); + expect($modelFromDatabase->data) ->toBe(['foo', 'bar']); }); diff --git a/tests/Models/FakeModelWithArray.php b/tests/Models/FakeModelWithArray.php deleted file mode 100644 index 7dc21f1..0000000 --- a/tests/Models/FakeModelWithArray.php +++ /dev/null @@ -1,21 +0,0 @@ - 'array' - ]; -} diff --git a/tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php b/tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php deleted file mode 100644 index 16936b6..0000000 --- a/tests/database/migrations/2023_11_01_165500_create_fake_model_with_array_table.php +++ /dev/null @@ -1,19 +0,0 @@ -id(); - $table->string('name')->nullable(); - $table->string('meta')->nullable(); - - $table->json('data')->nullable(); - }); - } -};