From 2a4bff21139d7e18d2683c3da5260be0a622a66b Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Thu, 22 Feb 2024 19:45:35 +0000 Subject: [PATCH 1/2] Add support for JSON fields in MustBeApproved trait This update enhances support for JSON fields in the MustBeApproved trait. Changes include modifications to getDirtyAttributes and callCastAttribute methods to handle JSON casting. Extended tests are also added to validate the correct functionality with Models containing JSON fields. --- src/Concerns/MustBeApproved.php | 12 ++++ tests/Feature/MustBeApprovedTraitTest.php | 67 +++++++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index 6c6571a..dc7ddf1 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -23,6 +23,12 @@ protected static function insertApprovalRequest($model) { $filteredDirty = $model->getDirtyAttributes(); + foreach ($filteredDirty as $key => $value) { + if (isset($model->casts[$key]) && $model->casts[$key] === 'json') { + $filteredDirty[$key] = json_decode(json: $value, associative: true); + } + } + if (auth()->check()) { $filteredDirty['user_id'] = auth()->id(); } @@ -141,6 +147,12 @@ public function withoutApproval(): static public function callCastAttribute($key, $value): mixed { if (array_key_exists($key, $this->casts)) { + // If the value is already an array, return it as is + if (is_array($value)) { + return $value; + } + + // Otherwise, cast the attribute to its defined type return $this->castAttribute($key, $value); } diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index cffc86a..6baa288 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -139,8 +139,7 @@ }); test(description: 'an approvals model is created when a model is created with MustBeApproved trait set and has the approvalInclude array set', closure: function () { - $model = new class extends Model - { + $model = new class extends Model { use MustBeApproved; protected $table = 'fake_models'; @@ -179,8 +178,7 @@ $table->json('data')->nullable(); }); - $model = new class extends Model - { + $model = new class extends Model { use MustBeApproved; protected $table = 'fake_models_with_array'; @@ -219,9 +217,68 @@ 'data' => json_encode(['foo', 'bar']), ]); - // double check the model + // double-check the model $modelFromDatabase = $model->firstWhere('name', 'Neo'); expect($modelFromDatabase->data) ->toBe(['foo', 'bar']); }); + +test(description: 'a Model can be rolled back when the data contains JSON fields', closure: function () { + Schema::create('posts', function (Blueprint $table) { + $table->id(); + $table->string('title'); + $table->string('content'); + $table->json('config'); + $table->timestamps(); + }); + + $model = new class extends Model { + use MustBeApproved; + + protected $table = 'posts'; + + protected $guarded = []; + + protected $casts = ['config' => 'json']; + }; + + // create a model + $model->create([ + 'title' => 'My First Post', + 'content' => 'This is my first post', + 'config' => ['checked' => true], + ]); + + // check if the data is stored correctly in the approval table + $this->assertDatabaseHas(table: Approval::class, data: [ + 'new_data' => json_encode([ + 'title' => 'My First Post', + 'content' => 'This is my first post', + 'config' => ['checked' => true], + ]), + 'original_data' => json_encode([]), + ]); + + // nothing should be in the 'posts' table + $this->assertDatabaseCount('posts', 0); + + // approve the model + Approval::first()->approve(); + + // after approval, there should be in an entry in the 'posts' table + $this->assertDatabaseCount('posts', 1); + + // After Approval, the contents of the database should look like this + $this->assertDatabaseHas(table: 'posts', data: [ + 'title' => 'My First Post', + 'content' => 'This is my first post', + 'config' => json_encode(['checked' => true]), + ]); + + // double-check the model + $modelFromDatabase = $model->firstWhere('title', 'My First Post'); + + expect($modelFromDatabase->config) + ->toBe(['checked' => true]); +}); From 7d5824709ad6c7b4227b4a2e6cf557f361985982 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Mon, 26 Feb 2024 22:26:18 +0000 Subject: [PATCH 2/2] Remove user authentication from MustBeApproved trait This commit removes the user authentication check from the MustBeApproved trait. This change ensures that the trait is solely focused on approval processes and not concerned with user authentication, making for cleaner code and separation of concerns. --- src/Concerns/MustBeApproved.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index dc7ddf1..89b4244 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -29,10 +29,6 @@ protected static function insertApprovalRequest($model) } } - if (auth()->check()) { - $filteredDirty['user_id'] = auth()->id(); - } - if ($model->isApprovalBypassed() || empty($filteredDirty)) { return; }