From 7fac20fc8c3296b13362f29f4c314fec7738f98f Mon Sep 17 00:00:00 2001
From: Jan Jakes <jan.jakes@automattic.com>
Date: Fri, 22 Sep 2023 14:51:03 +0200
Subject: [PATCH] Make default error value NULL, make data non-nullable

[MAILPOET-5599]
---
 .../lib/Automation/Engine/Data/AutomationRunLog.php    | 10 +++++-----
 .../lib/Migrations/Db/Migration_20230831_143755_Db.php |  7 ++++---
 .../Automation/Engine/Control/StepRunLoggerTest.php    |  4 ++--
 .../Engine/Storage/AutomationRunLogStorageTest.php     |  2 +-
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/mailpoet/lib/Automation/Engine/Data/AutomationRunLog.php b/mailpoet/lib/Automation/Engine/Data/AutomationRunLog.php
index b082b2b1e5..bade4e0291 100644
--- a/mailpoet/lib/Automation/Engine/Data/AutomationRunLog.php
+++ b/mailpoet/lib/Automation/Engine/Data/AutomationRunLog.php
@@ -53,8 +53,8 @@ class AutomationRunLog {
   /** @var array */
   private $data = [];
 
-  /** @var array */
-  private $error = [];
+  /** @var array|null */
+  private $error;
 
   public function __construct(
     int $automationRunId,
@@ -147,7 +147,7 @@ class AutomationRunLog {
     $this->updatedAt = new DateTimeImmutable();
   }
 
-  public function getError(): array {
+  public function getError(): ?array {
     return $this->error;
   }
 
@@ -163,7 +163,7 @@ class AutomationRunLog {
       'updated_at' => $this->updatedAt->format(DateTimeImmutable::W3C),
       'run_number' => $this->runNumber,
       'data' => Json::encode($this->data),
-      'error' => Json::encode($this->error),
+      'error' => $this->error ? Json::encode($this->error) : null,
     ];
   }
 
@@ -189,7 +189,7 @@ class AutomationRunLog {
     $log->updatedAt = new DateTimeImmutable($data['updated_at']);
     $log->runNumber = (int)$data['run_number'];
     $log->data = Json::decode($data['data']);
-    $log->error = Json::decode($data['error']);
+    $log->error = isset($data['error']) ? Json::decode($data['error']) : null;
     return $log;
   }
 
diff --git a/mailpoet/lib/Migrations/Db/Migration_20230831_143755_Db.php b/mailpoet/lib/Migrations/Db/Migration_20230831_143755_Db.php
index 54a029a95c..177f7eac46 100644
--- a/mailpoet/lib/Migrations/Db/Migration_20230831_143755_Db.php
+++ b/mailpoet/lib/Migrations/Db/Migration_20230831_143755_Db.php
@@ -40,8 +40,9 @@ class Migration_20230831_143755_Db extends DbMigration {
     $this->connection->executeStatement("UPDATE $logsTable SET status = 'complete' WHERE status = 'completed'");
 
     // fix empty values for errors and data
+    $this->connection->executeStatement("ALTER TABLE $logsTable CHANGE `data` `data` longtext NOT NULL AFTER run_number");
     $this->connection->executeStatement("UPDATE $logsTable SET data = '{}' WHERE data = '[]' OR data IS NULL");
-    $this->connection->executeStatement("UPDATE $logsTable SET error = '{}' WHERE error = '[]' OR error IS NULL");
+    $this->connection->executeStatement("UPDATE $logsTable SET error = NULL WHERE error = '[]' OR error IS NULL");
 
     // remove "completed_at" column (with "updated_at" it's no longer needed), backfill "updated_at"
     if ($this->columnExists($logsTable, 'completed_at')) {
@@ -98,8 +99,8 @@ class Migration_20230831_143755_Db extends DbMigration {
           $startedAt = strval($item['started_at']);
           $date = "DATE_SUB('$startedAt', INTERVAL 1 SECOND)";
           $queries[] = "
-            INSERT INTO {$logsTable} (automation_run_id, step_id, step_type, step_key, status, started_at, updated_at, run_number)
-            VALUES ($runId, '$triggerId', 'trigger', '$triggerKey', 'complete', $date, $date, 1)
+            INSERT INTO {$logsTable} (automation_run_id, step_id, step_type, step_key, status, started_at, updated_at, run_number, data)
+            VALUES ($runId, '$triggerId', 'trigger', '$triggerKey', 'complete', $date, $date, 1, '{}')
           ";
           $triggerAddedMap[$runId] = true;
         }
diff --git a/mailpoet/tests/integration/Automation/Engine/Control/StepRunLoggerTest.php b/mailpoet/tests/integration/Automation/Engine/Control/StepRunLoggerTest.php
index b7cf6e85d2..c08ab5431b 100644
--- a/mailpoet/tests/integration/Automation/Engine/Control/StepRunLoggerTest.php
+++ b/mailpoet/tests/integration/Automation/Engine/Control/StepRunLoggerTest.php
@@ -195,7 +195,7 @@ class StepRunLoggerTest extends MailPoetTest {
       'errorClass' => get_class($data['error']),
       'code' => $data['error']->getCode(),
       'trace' => Json::decode(Json::encode($data['error']->getTrace())), // normalize objects to arrays
-    ] : [];
+    ] : null;
 
     $expected = [
       'id' => $data['id'] ?? $log->getId(),
@@ -208,7 +208,7 @@ class StepRunLoggerTest extends MailPoetTest {
       'updated_at' => $data['updated_at'] ?? $log->getUpdatedAt()->format(DateTimeImmutable::W3C),
       'run_number' => $data['row_number'] ?? 1,
       'data' => $data['data'] ?? '{}',
-      'error' => Json::encode($error),
+      'error' => $error ? Json::encode($error) : null,
     ];
     $this->assertSame($expected, $log->toArray());
   }
diff --git a/mailpoet/tests/integration/Automation/Engine/Storage/AutomationRunLogStorageTest.php b/mailpoet/tests/integration/Automation/Engine/Storage/AutomationRunLogStorageTest.php
index f9d096b571..4f6efc6867 100644
--- a/mailpoet/tests/integration/Automation/Engine/Storage/AutomationRunLogStorageTest.php
+++ b/mailpoet/tests/integration/Automation/Engine/Storage/AutomationRunLogStorageTest.php
@@ -37,7 +37,7 @@ class AutomationRunLogStorageTest extends \MailPoetTest {
     $log = $this->storage->getAutomationRunLog($id);
     $this->assertInstanceOf(AutomationRunLog::class, $log);
     $errors = $log->getError();
-    expect($errors)->array();
+    $this->assertIsArray($errors);
     expect(array_keys($errors))->equals([
       'message',
       'errorClass',