From 625e285b147d10ceee5578ea7dbf1851488c8c1f Mon Sep 17 00:00:00 2001 From: Nuno Donato Date: Tue, 14 Nov 2023 11:04:46 +0000 Subject: [PATCH 1/5] added the model and the user to all event constructors --- src/Events/ModelApproved.php | 8 ++++++-- src/Events/ModelRejected.php | 8 ++++++-- src/Events/ModelSetPending.php | 8 ++++++-- src/Scopes/ApprovalStateScope.php | 12 +++++++----- tests/Feature/Scopes/ApprovalStateScopeTest.php | 3 ++- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Events/ModelApproved.php b/src/Events/ModelApproved.php index 9200565..46af1d9 100644 --- a/src/Events/ModelApproved.php +++ b/src/Events/ModelApproved.php @@ -2,13 +2,17 @@ namespace Cjmellor\Approval\Events; +use Illuminate\Contracts\Auth\Authenticatable; +use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Events\Dispatchable; class ModelApproved { use Dispatchable; - public function __construct() - { + public function __construct( + public Model $approval, + public Authenticatable|null $user, + ) { } } diff --git a/src/Events/ModelRejected.php b/src/Events/ModelRejected.php index 9d76bc7..d80c54f 100644 --- a/src/Events/ModelRejected.php +++ b/src/Events/ModelRejected.php @@ -2,13 +2,17 @@ namespace Cjmellor\Approval\Events; +use Illuminate\Contracts\Auth\Authenticatable; +use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Events\Dispatchable; class ModelRejected { use Dispatchable; - public function __construct() - { + public function __construct( + public Model $approval, + public Authenticatable|null $user, + ) { } } diff --git a/src/Events/ModelSetPending.php b/src/Events/ModelSetPending.php index b137580..fb1bb11 100644 --- a/src/Events/ModelSetPending.php +++ b/src/Events/ModelSetPending.php @@ -2,13 +2,17 @@ namespace Cjmellor\Approval\Events; +use Illuminate\Contracts\Auth\Authenticatable; +use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Events\Dispatchable; class ModelSetPending { use Dispatchable; - public function __construct() - { + public function __construct( + public Model $approval, + public Authenticatable|null $user, + ) { } } diff --git a/src/Scopes/ApprovalStateScope.php b/src/Scopes/ApprovalStateScope.php index 15faf35..7ac1adc 100644 --- a/src/Scopes/ApprovalStateScope.php +++ b/src/Scopes/ApprovalStateScope.php @@ -125,14 +125,16 @@ protected function addApprove(Builder $builder): void */ protected function updateApprovalState(Builder $builder, ApprovalStatus $state): int { + $model = $builder + ->find(id: $builder->getModel()->id); + match ($state) { - ApprovalStatus::Approved => Event::dispatch(new ModelApproved()), - ApprovalStatus::Pending => Event::dispatch(new ModelSetPending()), - ApprovalStatus::Rejected => Event::dispatch(new ModelRejected()), + ApprovalStatus::Approved => Event::dispatch(new ModelApproved($model, auth()->user()) ), + ApprovalStatus::Pending => Event::dispatch(new ModelSetPending($model, auth()->user())), + ApprovalStatus::Rejected => Event::dispatch(new ModelRejected($model, auth()->user())), }; - return $builder - ->find(id: $builder->getModel()->id) + return $model ->update([ 'state' => $state, ]); diff --git a/tests/Feature/Scopes/ApprovalStateScopeTest.php b/tests/Feature/Scopes/ApprovalStateScopeTest.php index a1f1f82..aad6ba7 100644 --- a/tests/Feature/Scopes/ApprovalStateScopeTest.php +++ b/tests/Feature/Scopes/ApprovalStateScopeTest.php @@ -3,6 +3,7 @@ use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Events\ModelApproved; use Cjmellor\Approval\Events\ModelRejected; +use Cjmellor\Approval\Events\ModelRolledBackEvent; use Cjmellor\Approval\Events\ModelSetPending; use Cjmellor\Approval\Models\Approval; use Cjmellor\Approval\Tests\Models\FakeModel; @@ -96,7 +97,7 @@ }); test(description: 'An event is fired when a Model\'s state is changed', closure: function (string $state): void { - FakeModel::create($this->fakeModelData); + $fakeModel = FakeModel::create($this->fakeModelData); Event::fake(); From 4d44878faa6c301ef7c1a6cf005877fd8987aa2c Mon Sep 17 00:00:00 2001 From: Nuno Donato Date: Tue, 14 Nov 2023 11:06:15 +0000 Subject: [PATCH 2/5] added the model and the user to all event constructors --- tests/Feature/Scopes/ApprovalStateScopeTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/Feature/Scopes/ApprovalStateScopeTest.php b/tests/Feature/Scopes/ApprovalStateScopeTest.php index aad6ba7..a1f1f82 100644 --- a/tests/Feature/Scopes/ApprovalStateScopeTest.php +++ b/tests/Feature/Scopes/ApprovalStateScopeTest.php @@ -3,7 +3,6 @@ use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Events\ModelApproved; use Cjmellor\Approval\Events\ModelRejected; -use Cjmellor\Approval\Events\ModelRolledBackEvent; use Cjmellor\Approval\Events\ModelSetPending; use Cjmellor\Approval\Models\Approval; use Cjmellor\Approval\Tests\Models\FakeModel; @@ -97,7 +96,7 @@ }); test(description: 'An event is fired when a Model\'s state is changed', closure: function (string $state): void { - $fakeModel = FakeModel::create($this->fakeModelData); + FakeModel::create($this->fakeModelData); Event::fake(); From 326087e1338ed00606225cefbccdf7c4f6507dec Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Sat, 17 Feb 2024 15:23:12 +0000 Subject: [PATCH 3/5] Refactor code for readability and improve tests In this commit, an unnecessary space was removed in MustBeApproved.php file to make the code cleaner. In addition, the migration file's structure was slightly adjusted for better readability. Tests were also modified to increase test coverage and improve its quality, especially in the handling of 'fake_users'. --- ..._11_17_002135_add_audited_by_column_to_approvals_table.php | 3 ++- src/Concerns/MustBeApproved.php | 1 - tests/Feature/Scopes/ApprovalStateScopeTest.php | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/database/migrations/2023_11_17_002135_add_audited_by_column_to_approvals_table.php b/database/migrations/2023_11_17_002135_add_audited_by_column_to_approvals_table.php index 90e890c..b350fbe 100644 --- a/database/migrations/2023_11_17_002135_add_audited_by_column_to_approvals_table.php +++ b/database/migrations/2023_11_17_002135_add_audited_by_column_to_approvals_table.php @@ -4,7 +4,8 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; -return new class extends Migration { +return new class extends Migration +{ /** * Run the migrations. */ diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index 183353a..e5d9aff 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -58,7 +58,6 @@ protected static function insertApprovalRequest($model) if ($noNeedToProceed) { return false; } - } /** diff --git a/tests/Feature/Scopes/ApprovalStateScopeTest.php b/tests/Feature/Scopes/ApprovalStateScopeTest.php index 6fba2d3..286c018 100644 --- a/tests/Feature/Scopes/ApprovalStateScopeTest.php +++ b/tests/Feature/Scopes/ApprovalStateScopeTest.php @@ -201,7 +201,7 @@ }); test(description: 'The model approver is listed correctly', closure: function () { - Schema::create('fake_users', callback: function (\Illuminate\Database\Schema\Blueprint $table) { + Schema::create('fake_users', callback: function (Illuminate\Database\Schema\Blueprint $table) { $table->id(); $table->string(column: 'name'); $table->string(column: 'email')->unique(); @@ -211,7 +211,9 @@ class FakeUser extends \Illuminate\Foundation\Auth\User { protected $guarded = []; + protected $table = 'fake_users'; + public $timestamps = false; } From 98f33857d241b4a0e75de7103fddca958dc8abd8 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Sat, 17 Feb 2024 15:23:31 +0000 Subject: [PATCH 4/5] Update Dispatch Events in ApprovalStateScope In this commit, we've updated the matched cases of ApprovalStatus in ApprovalStateScope.php. Rather than using a $model, we've changed to using $builder->getModel() in Dispatch Events to improve consistency and clarity of the code. --- src/Scopes/ApprovalStateScope.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Scopes/ApprovalStateScope.php b/src/Scopes/ApprovalStateScope.php index cf0d4ed..46b35e0 100644 --- a/src/Scopes/ApprovalStateScope.php +++ b/src/Scopes/ApprovalStateScope.php @@ -127,9 +127,9 @@ protected function addApprove(Builder $builder): void protected function updateApprovalState(Builder $builder, ApprovalStatus $state): int { match ($state) { - ApprovalStatus::Approved => Event::dispatch(new ModelApproved($model, auth()->user()) ), - ApprovalStatus::Pending => Event::dispatch(new ModelSetPending($model, auth()->user())), - ApprovalStatus::Rejected => Event::dispatch(new ModelRejected($model, auth()->user())), + ApprovalStatus::Approved => Event::dispatch(new ModelApproved($builder->getModel(), auth()->user())), + ApprovalStatus::Pending => Event::dispatch(new ModelSetPending($builder->getModel(), auth()->user())), + ApprovalStatus::Rejected => Event::dispatch(new ModelRejected($builder->getModel(), auth()->user())), }; $auditedData = ['state' => $state]; From 2b0982dd654723598b4ed8af9da1b4365d6fe43d Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Sat, 17 Feb 2024 15:24:40 +0000 Subject: [PATCH 5/5] Add Schema import to ApprovalStateScopeTest In this commit, Schema import has been added to the ApprovalStateScopeTest. This extra import is necessary for the functioning of our DB tests and ensures that the test for Approval Model states runs smoothly. --- tests/Feature/Scopes/ApprovalStateScopeTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Feature/Scopes/ApprovalStateScopeTest.php b/tests/Feature/Scopes/ApprovalStateScopeTest.php index 286c018..119c4fa 100644 --- a/tests/Feature/Scopes/ApprovalStateScopeTest.php +++ b/tests/Feature/Scopes/ApprovalStateScopeTest.php @@ -7,6 +7,7 @@ use Cjmellor\Approval\Models\Approval; use Cjmellor\Approval\Tests\Models\FakeModel; use Illuminate\Support\Facades\Event; +use Illuminate\Support\Facades\Schema; test('Check if an Approval Model is approved', closure: function (): void { $this->approvalData = [