From 53b8a4e6b87b4753c7e0c95092cda2c30181755b Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Wed, 15 Jun 2022 18:22:18 +0100 Subject: [PATCH 1/8] fix: Switch Enum data from INT to STRING --- .../2022_02_12_195950_create_approvals_table.php | 4 ++-- src/Enums/ApprovalStatus.php | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/database/migrations/2022_02_12_195950_create_approvals_table.php b/database/migrations/2022_02_12_195950_create_approvals_table.php index 9888fc1..03231f4 100644 --- a/database/migrations/2022_02_12_195950_create_approvals_table.php +++ b/database/migrations/2022_02_12_195950_create_approvals_table.php @@ -8,10 +8,10 @@ { public function up() { - Schema::create('approvals', function (Blueprint $table) { + Schema::create(table: 'approvals', callback: function (Blueprint $table) { $table->id(); $table->morphs(config(key: 'approval.approval.approval_pivot')); - $table->enum('state', [0, 1, 2])->default(0)->comment('0:PENDING, 1:APPROVED, 2:REJECTED'); + $table->enum(column: 'state', allowed: ['pending', 'approved', 'rejected'])->default('pending'); $table->json(config(key: 'approval.approval.new_data'))->nullable(); $table->json(config(key: 'approval.approval.original_data'))->nullable(); $table->timestamps(); diff --git a/src/Enums/ApprovalStatus.php b/src/Enums/ApprovalStatus.php index cee587b..1ee303f 100644 --- a/src/Enums/ApprovalStatus.php +++ b/src/Enums/ApprovalStatus.php @@ -2,9 +2,9 @@ namespace Cjmellor\Approval\Enums; -enum ApprovalStatus: int +enum ApprovalStatus: string { - case Pending = 0; - case Approved = 1; - case Rejected = 2; + case Pending = 'pending'; + case Approved = 'approved'; + case Rejected = 'rejected'; } From 3b8979cd0cbf77c3208c4e531737ea70422800c2 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 5 Jul 2022 23:36:51 +0100 Subject: [PATCH 2/8] fix: Revert config --- config/approval.php | 16 ---------------- .../2022_02_12_195950_create_approvals_table.php | 13 ++++++------- src/Concerns/MustBeApproved.php | 6 +++--- src/Models/Approval.php | 4 ++-- tests/Feature/MustBeApprovedTraitTest.php | 4 ++-- 5 files changed, 13 insertions(+), 30 deletions(-) diff --git a/config/approval.php b/config/approval.php index 044fa58..9b16085 100644 --- a/config/approval.php +++ b/config/approval.php @@ -2,22 +2,6 @@ return [ 'approval' => [ - - /** - * The column name for new data. - * - * Default: 'new_data' - */ - 'new_data' => 'new_data', - - - /** - * The column name for original data - * - * Default: 'original_data - */ - 'original_data' => 'original_data', - /** * The approval polymorphic pivot name * diff --git a/database/migrations/2022_02_12_195950_create_approvals_table.php b/database/migrations/2022_02_12_195950_create_approvals_table.php index 9888fc1..c0c96c5 100644 --- a/database/migrations/2022_02_12_195950_create_approvals_table.php +++ b/database/migrations/2022_02_12_195950_create_approvals_table.php @@ -4,22 +4,21 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; -return new class extends Migration -{ +return new class() extends Migration { public function up() { Schema::create('approvals', function (Blueprint $table) { $table->id(); - $table->morphs(config(key: 'approval.approval.approval_pivot')); - $table->enum('state', [0, 1, 2])->default(0)->comment('0:PENDING, 1:APPROVED, 2:REJECTED'); - $table->json(config(key: 'approval.approval.new_data'))->nullable(); - $table->json(config(key: 'approval.approval.original_data'))->nullable(); + $table->nullableMorphs(config(key: 'approval.approval.approval_pivot')); + $table->enum('state', ['pending', 'approved', 'rejected'])->default('pending'); + $table->json('new_data')->nullable(); + $table->json('original_data')->nullable(); $table->timestamps(); }); } public function down() { - Schema::dropIfExists('approvals'); + Schema::dropIfExists(table: 'approvals'); } }; diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index 5c68deb..5af1e67 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -22,8 +22,8 @@ public static function bootMustBeApproved(): void } $model->approvals()->create([ - (string)config(key: 'approval.approval.new_data') => $model->when($model->wasChanged(), fn () => $model->getChanges()), - (string)config(key: 'approval.approval.original_data') => $model->when($model->wasChanged(), fn () => $model->getOriginalMatchingChanges()), + 'new_data' => $model->getDirty(), + 'original_data' => $model->getOriginalMatchingChanges(), ]); }); } @@ -35,7 +35,7 @@ public function isApprovalBypassed(): bool public function approvals(): MorphMany { - return $this->morphMany(related: Approval::class, name: config(key: 'approval.approval.approval_pivot')); + return $this->morphMany(related: Approval::class, name: 'approvalable'); } protected function getOriginalMatchingChanges(): Collection diff --git a/src/Models/Approval.php b/src/Models/Approval.php index 8c71c9d..1bb66cb 100644 --- a/src/Models/Approval.php +++ b/src/Models/Approval.php @@ -12,8 +12,8 @@ class Approval extends Model protected $guarded = []; protected $casts = [ - "{config('approval.approval.new_data')}" => AsArrayObject::class, - "{config('approval.approval.original_data')}" => AsArrayObject::class, + 'new_data' => AsArrayObject::class, + 'original_data' => AsArrayObject::class, 'state' => ApprovalStatus::class, ]; diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index 08d7b89..ea49a52 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -8,8 +8,8 @@ 'approvalable_type' => 'App\Models\User', 'approvalable_id' => 1, 'state' => ApprovalStatus::Pending, - 'new_data' => '{"name":"Chris"}', - 'original_data' => '{"name":"Bob"}', + 'new_data' => json_encode(['name' => 'Chris']), + 'original_data' => json_encode(['name' => 'Bob']), ]; }); From 978cd8cfeb24689c655962bbe57215a2e8b800b1 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 5 Jul 2022 23:38:33 +0100 Subject: [PATCH 3/8] chore: Refactor trait --- src/Concerns/MustBeApproved.php | 59 ++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/src/Concerns/MustBeApproved.php b/src/Concerns/MustBeApproved.php index 5af1e67..ed5aca8 100644 --- a/src/Concerns/MustBeApproved.php +++ b/src/Concerns/MustBeApproved.php @@ -2,9 +2,9 @@ namespace Cjmellor\Approval\Concerns; +use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Models\Approval; use Illuminate\Database\Eloquent\Relations\MorphMany; -use Illuminate\Support\Collection; trait MustBeApproved { @@ -12,38 +12,73 @@ trait MustBeApproved public static function bootMustBeApproved(): void { - static::saved(callback: function ($model) { - if (! $model->wasChanged()) { - return; - } + static::creating(callback: fn ($model) => static::insertApprovalRequest($model)); + static::updating(callback: fn ($model) => static::insertApprovalRequest($model)); + } - if ($model->isApprovalBypassed()) { - return; + /** + * Create an Approval request before committing to the database. + */ + protected static function insertApprovalRequest($model) + { + if (! $model->isApprovalBypassed()) { + /** + * Create the new Approval model + */ + if (self::approvalModelExists($model)) { + return false; } $model->approvals()->create([ 'new_data' => $model->getDirty(), 'original_data' => $model->getOriginalMatchingChanges(), ]); - }); + + return false; + } } - public function isApprovalBypassed(): bool + /** + * Check if the Approval model been created already exists with a 'pending' state + */ + protected static function approvalModelExists($model): bool { - return $this->bypassApproval; + return Approval::where([ + ['state', '=', ApprovalStatus::Pending], + ['new_data', '=', json_encode($model->getDirty())], + ['original_data', '=', json_encode($model->getOriginalMatchingChanges())], + ])->exists(); } + /** + * The polymorphic relationship for the Approval model. + */ public function approvals(): MorphMany { return $this->morphMany(related: Approval::class, name: 'approvalable'); } - protected function getOriginalMatchingChanges(): Collection + /** + * Gets the original model data and only gets the keys that match the dirty attributes. + */ + protected function getOriginalMatchingChanges(): array { return collect($this->getOriginal()) - ->only(collect($this->getChanges())->keys()); + ->only(collect($this->getDirty())->keys()) + ->toArray(); + } + + /** + * Check is the approval can be bypassed. + */ + public function isApprovalBypassed(): bool + { + return $this->bypassApproval; } + /** + * Approval is ignored and persisted to the database. + */ public function withoutApproval(): static { $this->bypassApproval = true; From 5d1210a27682115861173307c86c0113817438db Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 5 Jul 2022 23:39:15 +0100 Subject: [PATCH 4/8] fix: Change Enum type --- src/Enums/ApprovalStatus.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Enums/ApprovalStatus.php b/src/Enums/ApprovalStatus.php index cee587b..1ee303f 100644 --- a/src/Enums/ApprovalStatus.php +++ b/src/Enums/ApprovalStatus.php @@ -2,9 +2,9 @@ namespace Cjmellor\Approval\Enums; -enum ApprovalStatus: int +enum ApprovalStatus: string { - case Pending = 0; - case Approved = 1; - case Rejected = 2; + case Pending = 'pending'; + case Approved = 'approved'; + case Rejected = 'rejected'; } From 666478e7138e41132cd4c0d0b02f59e427b200e4 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 5 Jul 2022 23:39:51 +0100 Subject: [PATCH 5/8] feat: Add scopes --- src/Models/Approval.php | 6 ++ src/Scopes/ApprovalStateScope.php | 117 ++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 src/Scopes/ApprovalStateScope.php diff --git a/src/Models/Approval.php b/src/Models/Approval.php index 1bb66cb..4fddfea 100644 --- a/src/Models/Approval.php +++ b/src/Models/Approval.php @@ -3,6 +3,7 @@ namespace Cjmellor\Approval\Models; use Cjmellor\Approval\Enums\ApprovalStatus; +use Cjmellor\Approval\Scopes\ApprovalStateScope; use Illuminate\Database\Eloquent\Casts\AsArrayObject; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\MorphTo; @@ -17,6 +18,11 @@ class Approval extends Model 'state' => ApprovalStatus::class, ]; + public static function booted() + { + static::addGlobalScope(new ApprovalStateScope()); + } + public function approvalable(): MorphTo { return $this->morphTo(); diff --git a/src/Scopes/ApprovalStateScope.php b/src/Scopes/ApprovalStateScope.php new file mode 100644 index 0000000..51c2710 --- /dev/null +++ b/src/Scopes/ApprovalStateScope.php @@ -0,0 +1,117 @@ +withAnyState(); + } + + /** + * Extend the query builder with the needed functions. + */ + public function extend(Builder $builder) + { + foreach ($this->extensions as $extension) { + $this->{"add{$extension}"}($builder); + } + } + + /** + * Return all query results with no state. + */ + protected function addWithAnyState(Builder $builder) + { + $builder->macro('withAnyState', fn (Builder $builder) => $builder->withoutGlobalScope($this)); + } + + /** + * Return only Approval states that are set to 'approved'. + */ + protected function addApproved(Builder $builder) + { + $builder->macro('approved', fn (Builder $builder) => $builder + ->withAnyState() + ->where(column: 'state', operator: ApprovalStatus::Approved)); + } + + /** + * Return only Approval states that are set to 'pending'. + */ + protected function addPending(Builder $builder) + { + $builder->macro('pending', fn (Builder $builder) => $builder + ->withAnyState() + ->where(column: 'state', operator: ApprovalStatus::Pending)); + } + + /** + * Return only Approval states that are set to 'rejected'. + */ + protected function addRejected(Builder $builder) + { + $builder->macro('rejected', fn (Builder $builder) => $builder + ->withAnyState() + ->where(column: 'state', operator: ApprovalStatus::Rejected)); + } + + /** + * Set state as 'approved'. + */ + protected function addApprove(Builder $builder) + { + $builder->macro('approve', fn (Builder $builder) => $this->updateApprovalState($builder, state: ApprovalStatus::Approved)); + } + + /** + * Set state as 'pending' (default). + */ + protected function addPostpone(Builder $builder) + { + $builder->macro('postpone', fn (Builder $builder) => $this->updateApprovalState($builder, state: ApprovalStatus::Pending)); + } + + /** + * Set state as 'rejected' + */ + protected function addReject(Builder $builder) + { + $builder->macro('reject', fn (Builder $builder) => $this->updateApprovalState($builder, state: ApprovalStatus::Rejected)); + } + + /** + * A helper method for updating the approvals state. + */ + protected function updateApprovalState(Builder $builder, $state): int + { + return $builder->update([ + 'state' => $state, + ]); + } +} From 92ddb651813a5abdd82ae6fd7544734c774966fc Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 5 Jul 2022 23:40:13 +0100 Subject: [PATCH 6/8] tests: Add more tests --- tests/Feature/MustBeApprovedTraitTest.php | 71 +++++++++++++++++-- tests/Models/FakeModel.php | 21 ++++++ tests/Pest.php | 2 + tests/TestCase.php | 3 + ...2_06_20_231650_create_fake_model_table.php | 16 +++++ 5 files changed, 109 insertions(+), 4 deletions(-) create mode 100644 tests/Models/FakeModel.php create mode 100644 tests/database/migrations/2022_06_20_231650_create_fake_model_table.php diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index ea49a52..59e95fe 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -2,22 +2,85 @@ use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Models\Approval; +use Cjmellor\Approval\Tests\Models\FakeModel; -beforeEach(function () { +beforeEach(closure: function (): void { $this->approvalData = [ - 'approvalable_type' => 'App\Models\User', + 'approvalable_type' => 'App\Models\FakeModel', 'approvalable_id' => 1, 'state' => ApprovalStatus::Pending, 'new_data' => json_encode(['name' => 'Chris']), 'original_data' => json_encode(['name' => 'Bob']), ]; + + $this->fakeModelData = [ + 'name' => 'Chris', + 'meta' => 'red', + ]; }); it(description: 'stores the data correctly in the database') ->tap( - fn () => Approval::create($this->approvalData) + fn (): Approval => Approval::create($this->approvalData) )->assertDatabaseHas('approvals', [ - 'approvalable_type' => 'App\Models\User', + 'approvalable_type' => 'App\Models\FakeModel', 'approvalable_id' => 1, 'state' => ApprovalStatus::Pending, ]); + +test(description: 'an approvals model is created when a model is created with MustBeApproved trait set') + // create a fake model + ->tap(callable: fn () => FakeModel::create($this->fakeModelData)) + // check it has been put in the approvals' table before the fake_models table + ->assertDatabaseHas('approvals', [ + 'new_data' => json_encode([ + 'name' => 'Chris', 'meta' => 'red', + ]), + 'original_data' => json_encode([]), + ]) + // this should be blank as the trait has intervened + ->assertDatabaseMissing('fake_models', [ + 'name' => 'Chris', + 'meta' => 'red', + ]); + +test(description: 'a model is added when the "withoutApproval()" method is used', closure: function () { + // build a query + $fakeModel = new FakeModel(); + + $fakeModel->name = 'Bob'; + $fakeModel->meta = 'green'; + + // save the model, bypassing approval + $fakeModel->withoutApproval()->save(); + + $this->assertDatabaseHas('fake_models', [ + 'name' => 'Bob', + 'meta' => 'green', + ]); + + $this->assertDatabaseMissing('approvals', [ + 'new_data' => json_encode([ + 'name' => 'Bob', + ]), + ]); +}); + +test(description: 'an approval model cannot be duplicated', closure: function () { + // create a fake model with data + FakeModel::create($this->fakeModelData); + + // check it was added to the db + $this->assertDatabaseHas('approvals', [ + 'new_data' => json_encode([ + 'name' => 'Chris', + 'meta' => 'red', + ]), + ]); + + // add another model with the same data... + FakeModel::create($this->fakeModelData); + + // as it is a duplicate, it should not be added to the DB + $this->assertDatabaseCount('approvals', 1); +}); diff --git a/tests/Models/FakeModel.php b/tests/Models/FakeModel.php new file mode 100644 index 0000000..036f92b --- /dev/null +++ b/tests/Models/FakeModel.php @@ -0,0 +1,21 @@ +in(__DIR__); +uses(RefreshDatabase::class); diff --git a/tests/TestCase.php b/tests/TestCase.php index 5aedbda..93e35a6 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -17,6 +17,9 @@ protected function setUp(): void $this->loadLaravelMigrations(); $this->loadMigrationsFrom(__DIR__.'/../database/migrations'); + + // Use this for the FakeModel in tests. + $this->loadMigrationsFrom(__DIR__.'/database/migrations'); } protected function getPackageProviders($app): array diff --git a/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php b/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php new file mode 100644 index 0000000..87d0ad5 --- /dev/null +++ b/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php @@ -0,0 +1,16 @@ +id(); + $table->string('name'); + $table->string('meta'); + }); + } +}; From 6b1df4b6c3e5eace4039b2b249ee1445f288826d Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 5 Jul 2022 23:42:34 +0100 Subject: [PATCH 7/8] chore: Update README --- README.md | 52 ++++++++++++++++++++++++++++++++++----------------- composer.json | 1 + 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index e0a37b3..8d72664 100644 --- a/README.md +++ b/README.md @@ -36,22 +36,6 @@ This is the contents of the published config file: ```php return [ 'approval' => [ - - /** - * The column name for new data. - * - * Default: 'new_data' - */ - 'new_data' => 'new_data', - - - /** - * The column name for original data - * - * Default: 'original_data - */ - 'original_data' => 'original_data', - /** * The approval polymorphic pivot name * @@ -62,7 +46,7 @@ return [ ]; ``` -The config allows you to change the column names as well as the polymorphic pivot name. The latter should always end with `able` though. +The config allows you to change the polymorphic pivot name. It should end with `able` though. ## Usage @@ -107,6 +91,40 @@ If you want to check if the Model data will be bypassed, use the `isApprovalBypa return $model->isApprovalBypassed(); ``` +## Scopes + +The package comes with some helper methods for the Builder, utilising a custom scope - `ApprovalStateScope` + +By default, all queries to the `approvals` table will return all the Models' no matter the state. + +There are three methods to help you retrieve the state of an Approval. + +```php +get(); +Approval::rejected()->get(); +Approval::pending()->count(); +``` + +You can also set a state for an approval: + +```php +approve(); +Approval::where('id', 2)->reject(); +Approval::where('id', 3)->postpone(); +``` + +In the event you need to reset a state, you can use the `withAnyState` helper. + +## Disable Approvals + If you don't want Model data to be approved, you can bypass it with the `withoutApproval` method. ```php diff --git a/composer.json b/composer.json index 58522a8..1ec8dd3 100644 --- a/composer.json +++ b/composer.json @@ -21,6 +21,7 @@ "illuminate/contracts": "^9.0" }, "require-dev": { + "laravel/pint": "^0.1.7", "nunomaduro/collision": "^6.0", "orchestra/testbench": "^7.0", "pestphp/pest": "^1.21", From b14dfbce6c172d548367b79937f85b6c84b3ffb4 Mon Sep 17 00:00:00 2001 From: cjmellor Date: Tue, 5 Jul 2022 22:43:01 +0000 Subject: [PATCH 8/8] style: fix --- .../migrations/2022_06_20_231650_create_fake_model_table.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php b/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php index 87d0ad5..e1f3582 100644 --- a/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php +++ b/tests/database/migrations/2022_06_20_231650_create_fake_model_table.php @@ -4,7 +4,7 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; -return new class() extends Migration { +return new class () extends Migration { public function up() { Schema::create('fake_models', function (Blueprint $table) {