wintercms / winter

Free, open-source, self-hosted CMS platform based on the Laravel PHP Framework.
https://wintercms.com
MIT License
1.31k stars 188 forks source link

Adding event context to EventLog service provider #1092

Closed interworks-morr closed 3 months ago

interworks-morr commented 3 months ago

Package targeted

Winter CMS

Description

The MessageLogged event listener in the ServiceProvider class excludes the event context instead of placing it in the details column (by passing it in as the details argument to EventLog::add). I have dynamically overridden this event locally for a while now with no ill effects and the stock output renders quite nicely as-is. I would like to submit a pull request to make this change upstream.

Additionally, a recent change added a type declaration to the EventLog::add function for the details parameter. The type declaration was set to ?string when the function assumes and even explicitly casts it to an array. The model has the details attribute set to JSONable, so I believe this type declaration is incorrect and causes needless double-encoding to JSON when using this function. I believe the type declaration should have been ?array instead.

Will this change be backwards-compatible?

I have had this event dynamically overridden in a custom plugin since version 1.0.x with no negative consequences, so it should be backward compatible.

interworks-morr commented 3 months ago

Here's a preview of the intended change

diff --git a/modules/system/ServiceProvider.php b/modules/system/ServiceProvider.php
index 3dfd76c7a..771971f18 100644
--- a/modules/system/ServiceProvider.php
+++ b/modules/system/ServiceProvider.php
@@ -315,7 +315,8 @@ protected function registerLogging()
     {
         Event::listen(\Illuminate\Log\Events\MessageLogged::class, function ($event) {
             if (EventLog::useLogging()) {
-                EventLog::add($event->message, $event->level);
+                $details = $event->context ?? null;
+                EventLog::add($event->message, $event->level, $details);
             }
         });
     }
diff --git a/modules/system/models/EventLog.php b/modules/system/models/EventLog.php
index f72c4f459..2d2efc358 100644
--- a/modules/system/models/EventLog.php
+++ b/modules/system/models/EventLog.php
@@ -40,7 +40,7 @@ class_exists('Model') &&
     /**
      * Creates a log record
      */
-    public static function add(string $message, string $level = 'info', ?string $details = null): self
+    public static function add(string $message, string $level = 'info', ?array $details = null): self
     {
         $record = new static;
         $record->message = $message;
LukeTowers commented 3 months ago

@interworks-morr sounds good, submit the PR please!