Update UserGroupEvent details format and log interface

app/Models/UserGroupEvent.php

@@ -41,58 +41,75 @@ class UserGroupEvent extends Model
         'hidden' => 'boolean',
     ];
 
-    public static function logGroupRename(?User $actor, Group $group, string $previousName, string $name): self
+    public static function logGroupAdd(?User $actor, Group $group): void
     {
-        return static::log($actor, static::GROUP_RENAME, null, $group, [
+        static::log($actor, static::GROUP_ADD, null, $group);
+    }
+
+    public static function logGroupRemove(?User $actor, Group $group): void
+    {
+        static::log($actor, static::GROUP_REMOVE, null, $group);
+    }
+
+    public static function logGroupRename(?User $actor, Group $group, string $previousName, string $name): void
+    {
+        static::log($actor, static::GROUP_RENAME, null, $group, [
             'details' => [
                 'group_name' => $name,
                 'previous_group_name' => $previousName,
             ],
         ]);
     }
 
-    public static function logUserAdd(?User $actor, User $user, Group $group, ?array $playmodes = null): self
+    public static function logUserAdd(?User $actor, User $user, Group $group, ?array $playmodes = null): void
     {
-        return static::log($actor, static::USER_ADD, $user, $group, [
+        // Never log additions to the default group
+        if ($group->identifier === 'default') {
+            return;
+        }
+
+        if (empty($playmodes)) {
+            $playmodes = null;
+        }
+
+        static::log($actor, static::USER_ADD, $user, $group, [
             'details' => compact('playmodes'),
         ]);
     }
 
-    public static function logUserAddPlaymodes(?User $actor, User $user, Group $group, array $playmodes): self
+    public static function logUserAddPlaymodes(?User $actor, User $user, Group $group, array $playmodes): void
     {
         if (empty($playmodes)) {
             throw new InvalidArgumentException('playmodes must not be empty');
         }
 
-        return static::log($actor, static::USER_ADD_PLAYMODES, $user, $group, [
+        static::log($actor, static::USER_ADD_PLAYMODES, $user, $group, [
             'details' => compact('playmodes'),
         ]);
     }
 
-    public static function logUserRemove(?User $actor, User $user, Group $group): self
+    public static function logUserRemove(?User $actor, User $user, Group $group): void
     {
-        return static::log($actor, static::USER_REMOVE, $user, $group);
+        static::log($actor, static::USER_REMOVE, $user, $group);
     }
 
-    public static function logUserRemovePlaymodes(?User $actor, User $user, Group $group, array $playmodes): self
+    public static function logUserRemovePlaymodes(?User $actor, User $user, Group $group, array $playmodes): void
     {
         if (empty($playmodes)) {
             throw new InvalidArgumentException('playmodes must not be empty');
         }
 
-        return static::log($actor, static::USER_REMOVE_PLAYMODES, $user, $group, [
+        static::log($actor, static::USER_REMOVE_PLAYMODES, $user, $group, [
             'details' => compact('playmodes'),
         ]);
     }
 
-    public static function logUserSetDefault(?User $actor, User $user, Group $group): self
+    public static function logUserSetDefault(?User $actor, User $user, Group $group): void
     {
-        return static::log($actor, static::USER_SET_DEFAULT, $user, $group, [
-            'hidden' => true,
-        ]);
+        static::log($actor, static::USER_SET_DEFAULT, $user, $group);
     }
 
-    private static function log(?User $actor, string $type, ?User $user, Group $group, array $attributes = []): self
+    private static function log(?User $actor, string $type, ?User $user, Group $group, array $attributes = []): void
     {
         $attributes['details'] = array_merge(
             [
@@ -103,7 +120,7 @@ private static function log(?User $actor, string $type, ?User $user, Group $grou
             $attributes['details'] ?? [],
         );
 
-        return static::create(array_merge(
+        (new static(array_merge(
             [
                 'actor_id' => $actor?->getKey(),
                 'group_id' => $group->getKey(),
@@ -112,7 +129,7 @@ private static function log(?User $actor, string $type, ?User $user, Group $grou
                 'user_id' => $user?->getKey(),
             ],
             $attributes,
-        ));
+        )))->saveOrExplode();
     }
 
     public function actor(): BelongsTo
@@ -139,7 +156,7 @@ public function getAttribute($key)
             'type',
             'user_id' => $this->getRawAttribute($key),
 
-            'details' => json_decode($this->getRawAttribute($key) ?? '[]', true),
+            'details' => json_decode($this->getRawAttribute($key), true),
 
             'created_at' => $this->getTimeFast($key),
 

database/migrations/2023_01_05_022852_require_user_group_events_details_and_type.php

@@ -0,0 +1,61 @@
+<?php
+
+// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the GNU Affero General Public License v3.0.
+// See the LICENCE file in the repository root for full licence text.
+
+use App\Models\UserGroupEvent;
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+class RequireUserGroupEventsDetailsAndType extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        // The format of `details` before this was not finalised, and on
+        // production nothing important was logged anyway. No going back!
+        UserGroupEvent::truncate();
+
+        Schema::table('user_group_events', function (Blueprint $table) {
+            $table->json('details')->nullable(false)->change();
+        });
+        DB::statement("ALTER TABLE user_group_events CHANGE type type ENUM(
+            'group_add',
+            'group_remove',
+            'group_rename',
+            'user_add',
+            'user_add_playmodes',
+            'user_remove',
+            'user_remove_playmodes',
+            'user_set_default'
+        ) NOT NULL");
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('user_group_events', function (Blueprint $table) {
+            $table->json('details')->nullable()->change();
+        });
+        DB::statement("ALTER TABLE user_group_events CHANGE type type ENUM(
+            'group_add',
+            'group_remove',
+            'group_rename',
+            'user_add',
+            'user_add_playmodes',
+            'user_remove',
+            'user_remove_playmodes',
+            'user_set_default'
+        )");
+    }
+}