From 7adc29584b52d2422c2f90813451bc7a318356eb Mon Sep 17 00:00:00 2001
From: erikn69 <erikn_69@hotmail.com>
Date: Tue, 10 Oct 2023 15:15:36 -0500
Subject: [PATCH] Refactor auditSync, auditDetach, auditAttach methods (#866)

---
 src/Auditable.php                 | 80 ++++++++++++++++---------------
 tests/Functional/AuditingTest.php | 34 +++++++++++--
 2 files changed, 72 insertions(+), 42 deletions(-)

diff --git a/src/Auditable.php b/src/Auditable.php
index ce6bb889..77fddf30 100644
--- a/src/Auditable.php
+++ b/src/Auditable.php
@@ -647,6 +647,7 @@ public function transitionTo(Contracts\Audit $audit, bool $old = false): Contrac
      * @param mixed $id
      * @param array $attributes
      * @param bool $touch
+     * @param array $columns
      * @return void
      * @throws AuditingException
      */
@@ -655,23 +656,18 @@ public function auditAttach(string $relationName, $id, array $attributes = [], $
         if (!method_exists($this, $relationName) || !method_exists($this->{$relationName}(), 'attach')) {
             throw new AuditingException('Relationship ' . $relationName . ' was not found or does not support method attach');
         }
-        $this->auditEvent = 'attach';
-        $this->isCustomEvent = true;
-        $this->auditCustomOld = [
-            $relationName => $this->{$relationName}()->get($columns)->toArray()
-        ];
+
+        $old = $this->{$relationName}()->get($columns);
         $this->{$relationName}()->attach($id, $attributes, $touch);
-        $this->auditCustomNew = [
-            $relationName => $this->{$relationName}()->get($columns)->toArray()
-        ];
-        Event::dispatch(AuditCustom::class, [$this]);
-        $this->isCustomEvent = false;
+        $new = $this->{$relationName}()->get($columns);
+        $this->dispatchRelationAuditEvent($relationName, 'attach', $old, $new);
     }
 
     /**
      * @param string $relationName
      * @param mixed $ids
      * @param bool $touch
+     * @param array $columns
      * @return int
      * @throws AuditingException
      */
@@ -681,17 +677,11 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c
             throw new AuditingException('Relationship ' . $relationName . ' was not found or does not support method detach');
         }
 
-        $this->auditEvent = 'detach';
-        $this->isCustomEvent = true;
-        $this->auditCustomOld = [
-            $relationName => $this->{$relationName}()->get($columns)->toArray()
-        ];
+        $old = $this->{$relationName}()->get($columns);
         $results = $this->{$relationName}()->detach($ids, $touch);
-        $this->auditCustomNew = [
-            $relationName => $this->{$relationName}()->get($columns)->toArray()
-        ];
-        Event::dispatch(AuditCustom::class, [$this]);
-        $this->isCustomEvent = false;
+        $new = $this->{$relationName}()->get($columns);
+        $this->dispatchRelationAuditEvent($relationName, 'detach', $old, $new);
+
         return empty($results) ? 0 : $results;
     }
 
@@ -699,7 +689,7 @@ public function auditDetach(string $relationName, $ids = null, $touch = true, $c
      * @param $relationName
      * @param \Illuminate\Support\Collection|\Illuminate\Database\Eloquent\Model|array $ids
      * @param bool $detaching
-     * @param bool $skipUnchanged
+     * @param array $columns
      * @return array
      * @throws AuditingException
      */
@@ -709,26 +699,14 @@ public function auditSync($relationName, $ids, $detaching = true, $columns = ['*
             throw new AuditingException('Relationship ' . $relationName . ' was not found or does not support method sync');
         }
 
-        $this->auditEvent = 'sync';
-
-        $this->auditCustomOld = [
-            $relationName => $this->{$relationName}()->get($columns)->toArray()
-        ];
-
+        $old = $this->{$relationName}()->get($columns);
         $changes = $this->{$relationName}()->sync($ids, $detaching);
-
         if (collect($changes)->flatten()->isEmpty()) {
-            $this->auditCustomOld = [];
-            $this->auditCustomNew = [];
+            $old = $new = collect([]);
         } else {
-            $this->auditCustomNew = [
-                $relationName => $this->{$relationName}()->get($columns)->toArray()
-            ];
+            $new = $this->{$relationName}()->get($columns);
         }
-
-        $this->isCustomEvent = true;
-        Event::dispatch(AuditCustom::class, [$this]);
-        $this->isCustomEvent = false;
+        $this->dispatchRelationAuditEvent($relationName, 'sync', $old, $new);
 
         return $changes;
     }
@@ -736,7 +714,7 @@ public function auditSync($relationName, $ids, $detaching = true, $columns = ['*
     /**
      * @param string $relationName
      * @param \Illuminate\Support\Collection|\Illuminate\Database\Eloquent\Model|array $ids
-     * @param bool $skipUnchanged
+     * @param array $columns
      * @return array
      * @throws AuditingException
      */
@@ -745,6 +723,32 @@ public function auditSyncWithoutDetaching(string $relationName, $ids, $columns =
         if (!method_exists($this, $relationName) || !method_exists($this->{$relationName}(), 'syncWithoutDetaching')) {
             throw new AuditingException('Relationship ' . $relationName . ' was not found or does not support method syncWithoutDetaching');
         }
+
         return $this->auditSync($relationName, $ids, false, $columns);
     }
+
+    /**
+     * @param string $relationName
+     * @param string $event
+     * @param \Illuminate\Support\Collection $old
+     * @param \Illuminate\Support\Collection $new
+     * @return void
+     */
+    private function dispatchRelationAuditEvent($relationName, $event, $old, $new)
+    {
+        $this->auditCustomOld[$relationName] = $old->toArray();
+        $this->auditCustomNew[$relationName] = $new->toArray();
+
+        if (
+            empty($this->auditCustomOld[$relationName]) &&
+            empty($this->auditCustomNew[$relationName])
+        ) {
+            $this->auditCustomOld = $this->auditCustomNew = [];
+        }
+
+        $this->auditEvent = $event;
+        $this->isCustomEvent = true;
+        Event::dispatch(AuditCustom::class, [$this]);
+        $this->isCustomEvent = false;
+    }
 }
diff --git a/tests/Functional/AuditingTest.php b/tests/Functional/AuditingTest.php
index a4ee2c8d..1f787c33 100644
--- a/tests/Functional/AuditingTest.php
+++ b/tests/Functional/AuditingTest.php
@@ -594,11 +594,10 @@ public function itWillAuditCustomEventData()
 
         $article->auditAttach('categories', $firstCategory);
         $article->auditAttach('categories', $secondCategory);
+        $lastAttachedArticle = \end($article->audits->last()->getModified()['categories']['new']);
+        
         $this->assertSame($firstCategory->name, $article->categories->first()->name);
-        $this->assertSame(
-            $secondCategory->name,
-            $article->audits->last()->getModified()['categories']['new'][1]['name']
-        );
+        $this->assertSame($secondCategory->name, $lastAttachedArticle['name']);
     }
 
     /**
@@ -627,6 +626,33 @@ public function itWillAuditSync()
         $this->assertGreaterThan($no_of_audits_before, $no_of_audits_after);
     }
 
+    /**
+     * @test
+     * @return void
+     */
+    public function itWillAuditDetach()
+    {
+        $firstCategory = factory(Category::class)->create();
+        $secondCategory = factory(Category::class)->create();
+        $article = factory(Article::class)->create();
+
+        $article->categories()->attach($firstCategory);
+        $article->categories()->attach($secondCategory);
+
+        $no_of_audits_before = Audit::where('auditable_type', Article::class)->count();
+        $categoryBefore = $article->categories()->first()->getKey();
+
+        $article->auditDetach('categories', [$firstCategory->getKey()]);
+
+        $no_of_audits_after = Audit::where('auditable_type', Article::class)->count();
+        $categoryAfter = $article->categories()->first()->getKey();
+
+        $this->assertSame($firstCategory->getKey(), $categoryBefore);
+        $this->assertSame($secondCategory->getKey(), $categoryAfter);
+        $this->assertNotSame($categoryBefore, $categoryAfter);
+        $this->assertGreaterThan($no_of_audits_before, $no_of_audits_after);
+    }
+
     /**
      * @test
      * @return void