From c1b02a88ab373b22d37ceb9ed080a282ba15636c Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 10 Oct 2023 00:08:28 +0100 Subject: [PATCH 1/5] feat: Add rollback feature + tests refactor: some minor refactoring done also --- ...lled_back_at_column_to_approvals_table.php | 21 ++++++++++ src/ApprovalServiceProvider.php | 5 ++- src/Events/ModelRolledBackEvent.php | 15 +++++++ src/Models/Approval.php | 25 ++++++++++++ tests/Feature/Models/ApprovalTest.php | 40 +++++++++++++++++++ tests/Feature/MustBeApprovedTraitTest.php | 23 ++--------- .../Feature/Scopes/ApprovalStateScopeTest.php | 7 ---- tests/Pest.php | 20 +++++++++- 8 files changed, 127 insertions(+), 29 deletions(-) create mode 100644 database/migrations/2023_10_09_204810_add_rolled_back_at_column_to_approvals_table.php create mode 100644 src/Events/ModelRolledBackEvent.php create mode 100644 tests/Feature/Models/ApprovalTest.php diff --git a/database/migrations/2023_10_09_204810_add_rolled_back_at_column_to_approvals_table.php b/database/migrations/2023_10_09_204810_add_rolled_back_at_column_to_approvals_table.php new file mode 100644 index 0000000..a2e1142 --- /dev/null +++ b/database/migrations/2023_10_09_204810_add_rolled_back_at_column_to_approvals_table.php @@ -0,0 +1,21 @@ +timestamp(column: 'rolled_back_at')->nullable()->after('original_data'); + }); + } + + public function down(): void + { + Schema::table('approvals', function (Blueprint $table) { + $table->dropColumn(columns: 'rolled_back_at'); + }); + } +}; diff --git a/src/ApprovalServiceProvider.php b/src/ApprovalServiceProvider.php index c734717..93aa7f4 100644 --- a/src/ApprovalServiceProvider.php +++ b/src/ApprovalServiceProvider.php @@ -12,6 +12,9 @@ public function configurePackage(Package $package): void $package ->name(name: 'approval') ->hasConfigFile() - ->hasMigration(migrationFileName: '2022_02_12_195950_create_approvals_table'); + ->hasMigrations([ + '2022_02_12_195950_create_approvals_table', + '2023_10_09_204810_add_rolled_back_at_column_to_approvals_table', + ]); } } diff --git a/src/Events/ModelRolledBackEvent.php b/src/Events/ModelRolledBackEvent.php new file mode 100644 index 0000000..74c855a --- /dev/null +++ b/src/Events/ModelRolledBackEvent.php @@ -0,0 +1,15 @@ + AsArrayObject::class, 'original_data' => AsArrayObject::class, 'state' => ApprovalStatus::class, + 'rolled_back_at' => 'datetime', ]; public static function booted(): void @@ -69,4 +73,25 @@ public function rejectUnless(bool $boolean): void $this->reject(); } } + + public function rollback(): void + { + throw_if( + condition: $this->state !== ApprovalStatus::Approved, + exception: Exception::class, + message: 'Cannot rollback an Approval that has not been approved.' + ); + + $model = $this->approvalable; + $model->update($this->original_data->getArrayCopy()); + + $this->update([ + 'state' => ApprovalStatus::Pending, + 'new_data' => $this->original_data, + 'original_data' => $this->new_data, + 'rolled_back_at' => now(), + ]); + + Event::dispatch(new ModelRolledBackEvent($this)); + } } diff --git a/tests/Feature/Models/ApprovalTest.php b/tests/Feature/Models/ApprovalTest.php new file mode 100644 index 0000000..4046842 --- /dev/null +++ b/tests/Feature/Models/ApprovalTest.php @@ -0,0 +1,40 @@ +name = 'Bob'; + $fakeModel->meta = 'green'; + + // save the model, bypassing approval + $fakeModel->withoutApproval()->save(); + + // Update a fresh instance of the model + $fakeModel->fresh()->update(['name' => 'Chris']); + + // Approve the new changes + $fakeModel->fresh()->approvals()->first()->approve(); + + // Test for Events + Event::fake(); + + // Rollback the data + $fakeModel->fresh()->approvals()->first()->rollback(); + + // Check the model has been rolled back + expect($fakeModel->fresh()->approvals()->first()) + ->state->toBe(expected: ApprovalStatus::Pending) + ->new_data->toMatchArray(['name' => 'Bob']) + ->original_data->toMatchArray(['name' => 'Chris']) + ->rolled_back_at->not->toBeNull(); + + // Asser the Events were fired + Event::assertDispatched(ModelRolledBackEvent::class); +}); + diff --git a/tests/Feature/MustBeApprovedTraitTest.php b/tests/Feature/MustBeApprovedTraitTest.php index 0228cbf..e84f5b1 100644 --- a/tests/Feature/MustBeApprovedTraitTest.php +++ b/tests/Feature/MustBeApprovedTraitTest.php @@ -4,33 +4,18 @@ use Cjmellor\Approval\Models\Approval; use Cjmellor\Approval\Tests\Models\FakeModel; -beforeEach(closure: function (): void { - $this->approvalData = [ - '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( + ->defer( fn (): Approval => Approval::create($this->approvalData) )->assertDatabaseHas('approvals', [ - 'approvalable_type' => 'App\Models\FakeModel', + 'approvalable_type' => FakeModel::class, '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)) + ->defer(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([ @@ -92,7 +77,7 @@ 'new_data' => json_encode($this->fakeModelData), ]); - // sanity check that is hasn't been added to the fake_models table + // check that it hasn't been added to the fake_models table $this->assertDatabaseMissing('fake_models', $this->fakeModelData); // approve the model diff --git a/tests/Feature/Scopes/ApprovalStateScopeTest.php b/tests/Feature/Scopes/ApprovalStateScopeTest.php index 56a4ed8..a1f1f82 100644 --- a/tests/Feature/Scopes/ApprovalStateScopeTest.php +++ b/tests/Feature/Scopes/ApprovalStateScopeTest.php @@ -8,13 +8,6 @@ use Cjmellor\Approval\Tests\Models\FakeModel; use Illuminate\Support\Facades\Event; -beforeEach(closure: function (): void { - $this->fakeModelData = [ - 'name' => 'Chris', - 'meta' => 'red', - ]; -}); - test('Check if an Approval Model is approved', closure: function (): void { $this->approvalData = [ 'approvalable_type' => 'App\Models\FakeModel', diff --git a/tests/Pest.php b/tests/Pest.php index 1240de9..b45abd6 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -1,7 +1,23 @@ in(__DIR__); -uses(RefreshDatabase::class); +uses(TestCase::class, RefreshDatabase::class) + ->beforeEach(hook: function (): void { + $this->approvalData = [ + 'approvalable_type' => FakeModel::class, + 'approvalable_id' => 1, + 'state' => ApprovalStatus::Pending, + 'new_data' => json_encode(['name' => 'Chris']), + 'original_data' => json_encode(['name' => 'Bob']), + ]; + + $this->fakeModelData = [ + 'name' => 'Chris', + 'meta' => 'red', + ]; + }) + ->in(__DIR__); From f62f664ff582328ee780dceac2d0b41c3e357a0b Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 10 Oct 2023 00:17:25 +0100 Subject: [PATCH 2/5] docs: Add an UPGRADE doc --- UPGRADE.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 UPGRADE.md diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 0000000..415b39d --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,19 @@ +# Upgrade Guide + +## v1.3.1 -> v1.4.0 + +To support the new `rollback` functionality, a new migration file is needed + +```bash +2023_10_09_204810_add_rolled_back_at_column_to_approvals_table +``` + +Be sure to migrate your database if you plan on using the `rollback` feature. + +If you'd prefer to do it manually, you can add the following column to your `approvals` table: + +```php +Schema::table('approvals', function (Blueprint $table) { + $table->timestamp(column: 'rolled_back_at')->nullable()->after('original_data'); +}); +``` From b7d7c588d23c5a7038e5ae1886959a79c26e9c7d Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 10 Oct 2023 00:20:53 +0100 Subject: [PATCH 3/5] docs: Document feature --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 1288b60..ab3f650 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,16 @@ If you don't want to persist to the database on approval, set a `false` flag on Approval::find(1)->approve(persist: false); ``` +## Rollbacks + +If you need to rollback an approval, you can use the `rollback` method. + +```php +Approval::first()->rollback(); +``` + +This will revert the data and set the state to `pending` and touch the `rolled_back_at` timestamp so you have a record of when it was rolled back. + ## Disable Approvals If you don't want Model data to be approved, you can bypass it with the `withoutApproval` method. From 6b1d9288714da45439e84fb903ec5d37c33d90d5 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 10 Oct 2023 21:08:38 +0100 Subject: [PATCH 4/5] feat: Roll back Approvals based on a conditional docs: Update docs --- README.md | 12 ++++++++-- src/Models/Approval.php | 7 +++++- tests/Feature/Models/ApprovalTest.php | 32 ++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index ab3f650..098bc8e 100644 --- a/README.md +++ b/README.md @@ -157,13 +157,21 @@ Approval::find(1)->approve(persist: false); ## Rollbacks -If you need to rollback an approval, you can use the `rollback` method. +If you need to roll back an approval, you can use the `rollback` method. ```php Approval::first()->rollback(); ``` -This will revert the data and set the state to `pending` and touch the `rolled_back_at` timestamp so you have a record of when it was rolled back. +This will revert the data and set the state to `pending` and touch the `rolled_back_at` timestamp, so you have a record of when it was rolled back. + +### Conditional Rollbacks + +A roll-back can be conditional, so you can roll back an approval if a condition is met. + +```php +Approval::first()->rollback(fn () => true); +``` ## Disable Approvals diff --git a/src/Models/Approval.php b/src/Models/Approval.php index 0287803..386d683 100644 --- a/src/Models/Approval.php +++ b/src/Models/Approval.php @@ -5,6 +5,7 @@ use Cjmellor\Approval\Enums\ApprovalStatus; use Cjmellor\Approval\Events\ModelRolledBackEvent; use Cjmellor\Approval\Scopes\ApprovalStateScope; +use Closure; use Exception; use Illuminate\Database\Eloquent\Casts\AsArrayObject; use Illuminate\Database\Eloquent\Model; @@ -74,8 +75,12 @@ public function rejectUnless(bool $boolean): void } } - public function rollback(): void + public function rollback(Closure $condition = null): void { + if ($condition && ! $condition($this)) { + return; + } + throw_if( condition: $this->state !== ApprovalStatus::Approved, exception: Exception::class, diff --git a/tests/Feature/Models/ApprovalTest.php b/tests/Feature/Models/ApprovalTest.php index 4046842..bc198a4 100644 --- a/tests/Feature/Models/ApprovalTest.php +++ b/tests/Feature/Models/ApprovalTest.php @@ -6,13 +6,13 @@ use Illuminate\Support\Facades\Event; test(description: 'an Approved Model can be rolled back', closure: function (): void { - // build a query + // Build a query $fakeModel = new FakeModel(); $fakeModel->name = 'Bob'; $fakeModel->meta = 'green'; - // save the model, bypassing approval + // Save the model, bypassing approval $fakeModel->withoutApproval()->save(); // Update a fresh instance of the model @@ -34,7 +34,33 @@ ->original_data->toMatchArray(['name' => 'Chris']) ->rolled_back_at->not->toBeNull(); - // Asser the Events were fired + // Assert the Events were fired Event::assertDispatched(ModelRolledBackEvent::class); }); +test('a rolled back Approval can be conditionally set', function () { + // Build a query + $fakeModel = new FakeModel(); + + $fakeModel->name = 'Bob'; + $fakeModel->meta = 'green'; + + // Save the model, bypassing approval + $fakeModel->withoutApproval()->save(); + + // Update a fresh instance of the model + $fakeModel->fresh()->update(['name' => 'Chris']); + + // Approve the new changes + $fakeModel->fresh()->approvals()->first()->approve(); + + // Conditionally rollback the data + $fakeModel->fresh()->approvals()->first()->rollback(fn () => true); + + // Check the model has been rolled back + expect($fakeModel->fresh()->approvals()->first()) + ->state->toBe(expected: ApprovalStatus::Pending) + ->new_data->toMatchArray(['name' => 'Bob']) + ->original_data->toMatchArray(['name' => 'Chris']) + ->rolled_back_at->not->toBeNull(); +}); From 423f66a7c04b961174ab91d9c3f90f0555680734 Mon Sep 17 00:00:00 2001 From: Chris Mellor Date: Tue, 10 Oct 2023 21:37:54 +0100 Subject: [PATCH 5/5] feat: Pass data to roll-back event docs: Update docs --- README.md | 10 ++++++++++ src/Events/ModelRolledBackEvent.php | 9 ++++++--- src/Models/Approval.php | 2 +- tests/Feature/Models/ApprovalTest.php | 7 +++++-- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 098bc8e..4c1df5f 100644 --- a/README.md +++ b/README.md @@ -173,6 +173,16 @@ A roll-back can be conditional, so you can roll back an approval if a condition Approval::first()->rollback(fn () => true); ``` +### Events + +When a Model has been rolled back, a `ModelRolledBack` event will be fired with the Approval Model that was rolled back, as well as the User that rolled it back. + +```php +// ModelRolledBackEvent::class + +public Model $approval, +public Authenticatable|null $user, +```` ## Disable Approvals If you don't want Model data to be approved, you can bypass it with the `withoutApproval` method. diff --git a/src/Events/ModelRolledBackEvent.php b/src/Events/ModelRolledBackEvent.php index 74c855a..c7fe57b 100644 --- a/src/Events/ModelRolledBackEvent.php +++ b/src/Events/ModelRolledBackEvent.php @@ -2,14 +2,17 @@ namespace Cjmellor\Approval\Events; +use Illuminate\Contracts\Auth\Authenticatable; +use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Events\Dispatchable; class ModelRolledBackEvent { use Dispatchable; - public function __construct() - { - // + public function __construct( + public Model $approval, + public Authenticatable|null $user, + ) { } } diff --git a/src/Models/Approval.php b/src/Models/Approval.php index 386d683..be76ada 100644 --- a/src/Models/Approval.php +++ b/src/Models/Approval.php @@ -97,6 +97,6 @@ public function rollback(Closure $condition = null): void 'rolled_back_at' => now(), ]); - Event::dispatch(new ModelRolledBackEvent($this)); + Event::dispatch(new ModelRolledBackEvent(approval: $this, user: auth()->user())); } } diff --git a/tests/Feature/Models/ApprovalTest.php b/tests/Feature/Models/ApprovalTest.php index bc198a4..22d354e 100644 --- a/tests/Feature/Models/ApprovalTest.php +++ b/tests/Feature/Models/ApprovalTest.php @@ -35,10 +35,13 @@ ->rolled_back_at->not->toBeNull(); // Assert the Events were fired - Event::assertDispatched(ModelRolledBackEvent::class); + Event::assertDispatched(function (ModelRolledBackEvent $event) use ($fakeModel): bool { + return $event->approval->is($fakeModel->fresh()->approvals()->first()) + && $event->user === null; + }); }); -test('a rolled back Approval can be conditionally set', function () { +test(description: 'a rolled back Approval can be conditionally set', closure: function () { // Build a query $fakeModel = new FakeModel();