From 1dd67f508dddfabe8dcafcbcd019b7e0ca708798 Mon Sep 17 00:00:00 2001 From: txdFrancesco <118364688+txdFrancesco@users.noreply.github.com> Date: Wed, 3 Jul 2024 10:35:21 +0000 Subject: [PATCH 1/7] support for auditSync and auditDetach on Auditable Custom Pivot Class --- src/Auditable.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Auditable.php b/src/Auditable.php index 3696afb2..016f1268 100644 --- a/src/Auditable.php +++ b/src/Auditable.php @@ -5,6 +5,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\MorphMany; +use Illuminate\Database\Eloquent\Relations\Pivot; use Illuminate\Database\Eloquent\SoftDeletes; use Illuminate\Support\Arr; use Illuminate\Support\Collection; @@ -746,7 +747,14 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c } $old = $relationCall->get($columns); - $results = $relationCall->detach($ids, $touch); + if($relationCall->getPivotClass() !== Pivot::class){ + $pivotClass = $relationCall->getPivotClass(); + $results = $pivotClass::withoutAuditing(function () use ($relationCall, $ids, $touch) { + return $relationCall->detach($ids, $touch); + }); + }else{ + $results = $relationCall->detach($ids, $touch); + } $new = $relationCall->get($columns); $this->dispatchRelationAuditEvent($relationName, 'detach', $old, $new); @@ -772,9 +780,16 @@ public function auditSync(string $relationName, $ids, $detaching = true, $column if ($callback instanceof \Closure) { $this->applyClosureToRelationship($relationCall, $callback); } - + $old = $relationCall->get($columns); - $changes = $relationCall->sync($ids, $detaching); + if($relationCall->getPivotClass() !== Pivot::class){ + $pivotClass = $relationCall->getPivotClass(); + $changes =$pivotClass::withoutAuditing(function () use ($relationCall, $ids, $detaching) { + return $relationCall->sync($ids, $detaching); + }); + }else{ + $changes = $relationCall->sync($ids, $detaching); + } if (collect($changes)->flatten()->isEmpty()) { $old = $new = collect([]); From 3819df837fc7a4393fd054d3b02ac42e79a5bf98 Mon Sep 17 00:00:00 2001 From: txdFrancesco <118364688+txdFrancesco@users.noreply.github.com> Date: Wed, 3 Jul 2024 10:50:59 +0000 Subject: [PATCH 2/7] keep default behavior if pivot class is not auditable --- src/Auditable.php | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Auditable.php b/src/Auditable.php index 016f1268..bd1714e7 100644 --- a/src/Auditable.php +++ b/src/Auditable.php @@ -14,6 +14,7 @@ use Illuminate\Support\Facades\Event; use OwenIt\Auditing\Contracts\AttributeEncoder; use OwenIt\Auditing\Contracts\AttributeRedactor; +use OwenIt\Auditing\Contracts\Auditable as ContractsAuditable; use OwenIt\Auditing\Contracts\Resolver; use OwenIt\Auditing\Events\AuditCustom; use OwenIt\Auditing\Exceptions\AuditableTransitionException; @@ -747,8 +748,8 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c } $old = $relationCall->get($columns); - if($relationCall->getPivotClass() !== Pivot::class){ - $pivotClass = $relationCall->getPivotClass(); + $pivotClass = $relationCall->getPivotClass(); + if($pivotClass !== Pivot::class && is_a($pivotClass,ContractsAuditable::class,true)){ $results = $pivotClass::withoutAuditing(function () use ($relationCall, $ids, $touch) { return $relationCall->detach($ids, $touch); }); @@ -782,8 +783,8 @@ public function auditSync(string $relationName, $ids, $detaching = true, $column } $old = $relationCall->get($columns); - if($relationCall->getPivotClass() !== Pivot::class){ - $pivotClass = $relationCall->getPivotClass(); + $pivotClass = $relationCall->getPivotClass(); + if($pivotClass !== Pivot::class && is_a($pivotClass,ContractsAuditable::class,true)){ $changes =$pivotClass::withoutAuditing(function () use ($relationCall, $ids, $detaching) { return $relationCall->sync($ids, $detaching); }); From f520a240869311421d6b4ef73275baf9c84112ca Mon Sep 17 00:00:00 2001 From: Francesco Date: Wed, 3 Jul 2024 18:07:46 +0200 Subject: [PATCH 3/7] formatting --- src/Auditable.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Auditable.php b/src/Auditable.php index bd1714e7..c2e5ebe3 100644 --- a/src/Auditable.php +++ b/src/Auditable.php @@ -748,12 +748,14 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c } $old = $relationCall->get($columns); + $pivotClass = $relationCall->getPivotClass(); - if($pivotClass !== Pivot::class && is_a($pivotClass,ContractsAuditable::class,true)){ + + if($pivotClass !== Pivot::class && is_a($pivotClass, ContractsAuditable::class, true)) { $results = $pivotClass::withoutAuditing(function () use ($relationCall, $ids, $touch) { return $relationCall->detach($ids, $touch); }); - }else{ + } else { $results = $relationCall->detach($ids, $touch); } $new = $relationCall->get($columns); @@ -783,12 +785,14 @@ public function auditSync(string $relationName, $ids, $detaching = true, $column } $old = $relationCall->get($columns); + $pivotClass = $relationCall->getPivotClass(); - if($pivotClass !== Pivot::class && is_a($pivotClass,ContractsAuditable::class,true)){ - $changes =$pivotClass::withoutAuditing(function () use ($relationCall, $ids, $detaching) { + + if($pivotClass !== Pivot::class && is_a($pivotClass, ContractsAuditable::class, true)) { + $changes = $pivotClass::withoutAuditing(function () use ($relationCall, $ids, $detaching) { return $relationCall->sync($ids, $detaching); }); - }else{ + } else { $changes = $relationCall->sync($ids, $detaching); } From 5c6b9ded51051dde100c82063651db10d0298568 Mon Sep 17 00:00:00 2001 From: Francesco Date: Wed, 3 Jul 2024 18:39:25 +0200 Subject: [PATCH 4/7] tests for auditSync and auditDetach with auditable custom pivot class --- tests/Functional/AuditingTest.php | 69 +++++++++++++++++++ tests/Models/Group.php | 7 ++ tests/Models/GroupMember.php | 17 +++++ tests/Models/User.php | 5 ++ tests/database/factories/GroupFactory.php | 16 +++++ ..._00_00_000004_create_groups_test_table.php | 41 +++++++++++ 6 files changed, 155 insertions(+) create mode 100644 tests/Models/Group.php create mode 100644 tests/Models/GroupMember.php create mode 100644 tests/database/factories/GroupFactory.php create mode 100644 tests/database/migrations/0000_00_00_000004_create_groups_test_table.php diff --git a/tests/Functional/AuditingTest.php b/tests/Functional/AuditingTest.php index 6bc063d6..e79ce79e 100644 --- a/tests/Functional/AuditingTest.php +++ b/tests/Functional/AuditingTest.php @@ -20,6 +20,7 @@ use OwenIt\Auditing\Tests\Models\ArticleCustomAuditMorph; use OwenIt\Auditing\Tests\Models\ArticleExcludes; use OwenIt\Auditing\Tests\Models\Category; +use OwenIt\Auditing\Tests\Models\Group; use OwenIt\Auditing\Tests\Models\User; class AuditingTest extends AuditingTestCase @@ -1092,4 +1093,72 @@ public function canAuditCustomAuditModelImplementation() $this->assertNotEmpty($audit); $this->assertSame(get_class($audit), \OwenIt\Auditing\Tests\Models\CustomAudit::class); } + + /** + * @test + * @return void + */ + public function itWillAuditSyncWithAuditablePivotClass() + { + $group = factory(Group::class)->create(); + $user = factory(User::class)->create(); + + $no_of_audits_before = Audit::where('auditable_type', User::class)->count(); + + $user->auditSync('groups', [$group->getKey() => ["role" => "admin"]]); + + $no_of_audits_mid = Audit::where('auditable_type', User::class)->count(); + $memberRole = $user->groups()->first()->pivot->role; + + $user->auditSync('groups', []); + + $no_of_audits_after = Audit::where('auditable_type', User::class)->count(); + + $this->assertSame("admin", $memberRole); + $this->assertGreaterThan($no_of_audits_before, $no_of_audits_mid); + $this->assertGreaterThan($no_of_audits_mid, $no_of_audits_after); + } + + /** + * @test + * @return void + */ + public function itWillAuditAttachWithAuditablePivotClass() + { + $group = factory(Group::class)->create(); + $user = factory(User::class)->create(); + + $no_of_audits_before = Audit::where('auditable_type', User::class)->count(); + + $user->auditAttach('groups', $group); + + $attachedGroup = $user->groups()->first()->getKey(); + $no_of_audits_after = Audit::where('auditable_type', User::class)->count(); + + $this->assertSame($group->getKey(), $attachedGroup); + $this->assertGreaterThan($no_of_audits_before, $no_of_audits_after); + } + + /** + * @test + * @return void + */ + public function itWillAuditDetachWithAuditablePivotClass() + { + $group = factory(Group::class)->create(); + $user = factory(User::class)->create(); + + $user->groups()->attach($group); + + $attachedGroup = $user->groups()->first()->getKey(); + $no_of_audits_before = Audit::where('auditable_type', User::class)->count(); + + $detachedGroups = $user->auditDetach('groups', $group); + + $no_of_audits_after = Audit::where('auditable_type', User::class)->count(); + + $this->assertSame($group->getKey(), $attachedGroup); + $this->assertSame(1, $detachedGroups); + $this->assertGreaterThan($no_of_audits_before, $no_of_audits_after); + } } diff --git a/tests/Models/Group.php b/tests/Models/Group.php new file mode 100644 index 00000000..5e13d42d --- /dev/null +++ b/tests/Models/Group.php @@ -0,0 +1,7 @@ +belongsToMany(Group::class, 'group_members', 'user_id', 'group_id')->using(GroupMember::class)->withPivot('id','role'); + } } diff --git a/tests/database/factories/GroupFactory.php b/tests/database/factories/GroupFactory.php new file mode 100644 index 00000000..f9789ef1 --- /dev/null +++ b/tests/database/factories/GroupFactory.php @@ -0,0 +1,16 @@ +define(\OwenIt\Auditing\Tests\Models\Group::class, function (Faker $faker) { + return [ + 'name' => $faker->unique()->colorName(), + ]; +}); diff --git a/tests/database/migrations/0000_00_00_000004_create_groups_test_table.php b/tests/database/migrations/0000_00_00_000004_create_groups_test_table.php new file mode 100644 index 00000000..b5d985d0 --- /dev/null +++ b/tests/database/migrations/0000_00_00_000004_create_groups_test_table.php @@ -0,0 +1,41 @@ +increments('id'); + $table->string('name'); + $table->timestamps(); + }); + + Schema::create('group_members', function (Blueprint $table) { + $table->increments('id'); + $table->unsignedBigInteger('user_id'); + $table->unsignedBigInteger('group_id'); + $table->enum('role',['member','admin'])->default('member'); + $table->timestamps(); + }); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + Schema::drop('group_members'); + Schema::drop('groups'); + } +} From fc3b22fc5c194c324274bb6837502e2ff976a64a Mon Sep 17 00:00:00 2001 From: Will Power <1619102+willpower232@users.noreply.github.com> Date: Fri, 13 Sep 2024 08:27:54 +0100 Subject: [PATCH 5/7] Apply suggestions from code review --- src/Auditable.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Auditable.php b/src/Auditable.php index c2e5ebe3..72949412 100644 --- a/src/Auditable.php +++ b/src/Auditable.php @@ -751,7 +751,7 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c $pivotClass = $relationCall->getPivotClass(); - if($pivotClass !== Pivot::class && is_a($pivotClass, ContractsAuditable::class, true)) { + if ($pivotClass !== Pivot::class && is_a($pivotClass, ContractsAuditable::class, true)) { $results = $pivotClass::withoutAuditing(function () use ($relationCall, $ids, $touch) { return $relationCall->detach($ids, $touch); }); @@ -783,12 +783,12 @@ public function auditSync(string $relationName, $ids, $detaching = true, $column if ($callback instanceof \Closure) { $this->applyClosureToRelationship($relationCall, $callback); } - + $old = $relationCall->get($columns); $pivotClass = $relationCall->getPivotClass(); - if($pivotClass !== Pivot::class && is_a($pivotClass, ContractsAuditable::class, true)) { + if ($pivotClass !== Pivot::class && is_a($pivotClass, ContractsAuditable::class, true)) { $changes = $pivotClass::withoutAuditing(function () use ($relationCall, $ids, $detaching) { return $relationCall->sync($ids, $detaching); }); From 3e593519154603fd20fa064394f25901c4856c14 Mon Sep 17 00:00:00 2001 From: Francesco Date: Thu, 24 Oct 2024 10:46:59 +0200 Subject: [PATCH 6/7] spacing --- src/Auditable.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Auditable.php b/src/Auditable.php index 72949412..950a6bc6 100644 --- a/src/Auditable.php +++ b/src/Auditable.php @@ -758,6 +758,7 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c } else { $results = $relationCall->detach($ids, $touch); } + $new = $relationCall->get($columns); $this->dispatchRelationAuditEvent($relationName, 'detach', $old, $new); From a888b0cf67c9656851a4c7e243fc1751ec3051cc Mon Sep 17 00:00:00 2001 From: Francesco Date: Wed, 26 Feb 2025 15:00:48 +0100 Subject: [PATCH 7/7] test model factory update --- tests/Functional/AuditingTest.php | 12 +++++------ tests/Models/Group.php | 5 +++++ tests/database/factories/GroupFactory.php | 26 ++++++++++++----------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/tests/Functional/AuditingTest.php b/tests/Functional/AuditingTest.php index 2c69afc1..6e4bedfe 100644 --- a/tests/Functional/AuditingTest.php +++ b/tests/Functional/AuditingTest.php @@ -971,8 +971,8 @@ public function test_can_audit_custom_audit_model_implementation(): void */ public function itWillAuditSyncWithAuditablePivotClass() { - $group = factory(Group::class)->create(); - $user = factory(User::class)->create(); + $group = Group::factory()->create(); + $user = User::factory()->create(); $no_of_audits_before = Audit::where('auditable_type', User::class)->count(); @@ -996,8 +996,8 @@ public function itWillAuditSyncWithAuditablePivotClass() */ public function itWillAuditAttachWithAuditablePivotClass() { - $group = factory(Group::class)->create(); - $user = factory(User::class)->create(); + $group = Group::factory()->create(); + $user = User::factory()->create(); $no_of_audits_before = Audit::where('auditable_type', User::class)->count(); @@ -1016,8 +1016,8 @@ public function itWillAuditAttachWithAuditablePivotClass() */ public function itWillAuditDetachWithAuditablePivotClass() { - $group = factory(Group::class)->create(); - $user = factory(User::class)->create(); + $group = Group::factory()->create(); + $user = User::factory()->create(); $user->groups()->attach($group); diff --git a/tests/Models/Group.php b/tests/Models/Group.php index 5e13d42d..783e4daf 100644 --- a/tests/Models/Group.php +++ b/tests/Models/Group.php @@ -2,6 +2,11 @@ namespace OwenIt\Auditing\Tests\Models; +use Illuminate\Database\Eloquent\Factories\HasFactory; +use OwenIt\Auditing\Tests\database\factories\GroupFactory; + class Group extends \Illuminate\Database\Eloquent\Model { + use HasFactory; + protected static string $factory = GroupFactory::class; } diff --git a/tests/database/factories/GroupFactory.php b/tests/database/factories/GroupFactory.php index f9789ef1..1bd0a270 100644 --- a/tests/database/factories/GroupFactory.php +++ b/tests/database/factories/GroupFactory.php @@ -1,16 +1,18 @@ define(\OwenIt\Auditing\Tests\Models\Group::class, function (Faker $faker) { - return [ - 'name' => $faker->unique()->colorName(), - ]; -}); +class GroupFactory extends Factory +{ + protected $model = Group::class; + + public function definition() + { + return [ + 'name' => fake()->unique()->colorName(), + ]; + } +} \ No newline at end of file