Don’t write an “updated” change when only published state changed

BKWLD · Dec 19, 2017 · 175b286 · 175b286
1 parent 2ed56b4
commit 175b286
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 16 deletions.
diff --git a/classes/Models/Change.php b/classes/Models/Change.php
@@ -95,14 +95,11 @@ public function shouldLogChange($action)
      */
     public static function log(Model $model, $action, Admin $admin = null)
     {
-        // Get the changed attributes
-        $changed = $model->getDirty();
-        if ($action == 'deleted' || empty($changed)) {
-            $changed = null;
-        }
-
         // Create a new change instance
-        $change = static::createLog($model, $action, $admin, $changed);
+        if (static::shouldWriteChange($model)) {
+            $changed = static::getChanged($model, $action);
+            $change = static::createLog($model, $action, $admin, $changed);
+        }
 
         // Log published / unpblished changes
         static::logPublishingChange($model, $action, $admin);
@@ -113,20 +110,51 @@ public static function log(Model $model, $action, Admin $admin = null)
             DB::table('changes')
                 ->where('model', get_class($model))
                 ->where('key', $model->getKey())
-                ->update(['deleted' => 1])
-            ;
+                ->update(['deleted' => 1]);
         }
 
         // Return the changed instance
-        return $change;
+        if (isset($change)) {
+            return $change;
+        }
     }
 
     /**
-     * Create a change entry
+     * Don't log changes when the only thing that changed was the published
+     * state
      *
      * @param  Model  $model  The model being touched
-     * @param  string $action Generally a CRUD verb: "created", "updated", "deleted"
-     * @param  Admin  $admin  The admin acting on the record
+     * @return boolean
+     */
+    static private function shouldWriteChange(Model $model)
+    {
+        $only_published_changed = count($model->getDirty()) == 1
+            && $model->isDirty('public');
+        return !$only_published_changed;
+    }
+
+    /**
+     * Get the changes attributes
+     *
+     * @param  Model  $model  The model being touched
+     * @param  string $action
+     * @return array|null
+     */
+    static private function getChanged(Model $model, $action)
+    {
+        $changed = $model->getDirty();
+        if ($action == 'deleted' || empty($changed)) {
+            $changed = null;
+        }
+        return $changed;
+    }
+
+    /**
+     * Create a change entry
+     *
+     * @param  Model  $model  Th
+     * @param  string $action
+     * @param  Admin  $admin
      */
     static protected function createLog(
         Model $model,
@@ -147,7 +175,7 @@ static protected function createLog(
     /**
      * Get the title of the model
      *
-     * @param  Model  $model  The model being touched
+     * @param  Model  $model
      * @return string
      */
     static protected function getModelTitle(Model $model)
@@ -174,7 +202,7 @@ static protected function getAdminId(Admin $admin = null)
      * Log changes to publishing state.  The initial publish should be logged
      * but not an initil unpublished state.
      *
-     * @param  Model  $model  The model being touched
+     * @param  Model  $model
      * @param  string $action
      * @param  Admin $admin
      * @return void

diff --git a/tests/Integration/ChangesTest.php b/tests/Integration/ChangesTest.php
@@ -143,7 +143,7 @@ public function testPublishChanges()
             'model' => 'App\Article',
         ]);
 
-        // Make an article that isn't publisehd
+        // Make an article that isn't published
         $article = factory(Article::class)->create([
             'public' => 0,
         ]);
@@ -168,6 +168,13 @@ public function testPublishChanges()
             'action' => 'unpublished',
         ]);
 
+        // Confirm that only changing the published state does not create a
+        // change
+        $this->assertDatabaseMissing('changes', [
+            'model' => 'App\Article',
+            'action' => 'updated',
+        ]);
+
         // Unpublish it
         $article->public = 0;
         $article->save();
@@ -179,5 +186,15 @@ public function testPublishChanges()
             'model' => 'App\Article',
             'action' => 'unpublished',
         ]);
+        $this->assertDatabaseMissing('changes', [
+            'model' => 'App\Article',
+            'action' => 'updated',
+        ]);
+
+        // No update should be written
+        $this->assertDatabaseMissing('changes', [
+            'model' => 'App\Article',
+            'action' => 'updated',
+        ]);
     }
 }