Fix broken suspension cleaner

This commit rewrites the previously broken auto ban cleaner. It's now much more efficient and compact, and it removes bans correctly and on the right date.

Fixes #1

app/Console/Kernel.php

@@ -53,6 +53,7 @@ class Kernel extends ConsoleKernel
         $schedule->job(new ProcessDueSuspensions)
             ->daily();
         // Production value: Every day
+        // Development value: Every minute
     }
 
     /**

app/Http/Controllers/Auth/RegisterController.php

@@ -125,7 +125,7 @@ class RegisterController extends Controller
             'name' => $data['name'],
             'email' => $data['email'],
             'password' => Hash::make($data['password']),
-            'originalIP' => IP::shouldCollect() ? '0.0.0.0' : request()->ip(),
+            'originalIP' => IP::shouldCollect() ? request()->ip() : '0.0.0.0',
         ]);
 
         $user->assignRole('user');

app/Jobs/ProcessDueSuspensions.php

@@ -22,6 +22,7 @@
 namespace App\Jobs;
 
 use App\Ban;
+use App\Services\AccountSuspensionService;
 use Carbon\Carbon;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
@@ -34,8 +35,6 @@ class ProcessDueSuspensions implements ShouldQueue
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
-    public $bans;
-
     /**
      * Create a new job instance.
      *
@@ -50,20 +49,10 @@ class ProcessDueSuspensions implements ShouldQueue
      *
      * @return void
      */
-    public function handle()
+    public function handle(AccountSuspensionService $service)
     {
-        Log::debug('Running automatic suspension cleaner...');
-        $bans = Ban::all();
+        Log::info('(suspension cleaner) Purging all expired suspension records.');
 
-        if (! is_null($bans)) {
-            foreach ($this->bans as $ban) {
-                $bannedUntil = Carbon::parse($ban->bannedUntil);
-
-                if ($bannedUntil->isToday()) {
-                    Log::debug('Lifted expired suspension ID '.$ban->id.' for '.$ban->user->name);
-                    $ban->delete();
-                }
-            }
-        }
+        $service->purgeExpired();
     }
 }

app/Services/AccountSuspensionService.php

@@ -5,16 +5,32 @@ namespace App\Services;
 
 use App\Ban;
 use App\User;
+use Carbon\Carbon;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Log;
 
 class AccountSuspensionService
 {
 
+    /**
+     * Suspends a user account, with given $reason.
+     *
+     * This method will take the target user and add a suspension to the database,
+     * effectively banning the user from the app. Suspensions may be temporary or permanent.
+     * Suspensions also block registration attempts.
+     *
+     * @param string $reason Suspension reason.
+     * @param string $duration Duration. This is a timestamp.
+     * @param User $target Who to suspend.
+     * @param string $type Permanent or temporary?
+     * @return Ban The ban itself
+     */
     public function suspend($reason, $duration, User $target, $type = "on"): Ban {
 
-        Log::debug("AccountSuspensionService: Suspending user account", [
-            'userID' => $target->id
+        Log::alert("An user account has just been suspended.", [
+            'taget_email' => $target->email,
+            'suspended_by' => Auth::user()->email,
+            'reason' => $reason
         ]);
 
         if ($type == "on") {
@@ -32,16 +48,42 @@ class AccountSuspensionService
         return $ban;
     }
 
+    /**
+     * Lifts someone's suspension
+     *
+     * @param User $user The user to unsuspend
+     */
     public function unsuspend(User $user): void {
+
+        Log::alert("A suspension has just been lifted.", [
+            'target_email' => $user->email,
+        ]);
+
         $user->bans->delete();
     }
 
+    /**
+     * Checks whether a user is suspended
+     *
+     * @param User $user The user to check
+     * @return bool Whether the mentioned user is suspended
+     */
     public function isSuspended(User $user): bool {
         return !is_null($user->bans);
     }
 
+
+    /**
+     * Takes a suspension directly and makes it permanent.
+     *
+     * @param Ban $ban The suspension to make permanent
+     */
     public function makePermanent(Ban $ban): void {
 
+        Log::alert('A suspension has just been made permanent.', [
+            'target_email' => $ban->user->email
+        ]);
+
         $ban->bannedUntil = null;
         $ban->isPermanent = true;
 
@@ -49,5 +91,16 @@ class AccountSuspensionService
 
     }
 
+    /**
+     * Purges old, expired suspensions from the database
+     *
+     * @return bool Whether any suspensions were lifted
+     */
+    public function purgeExpired()
+    {
+        // Unban on the last day, not on the exact time (with Carbon::now()).
+        return (bool) Ban::whereDate('bannedUntil', '=', Carbon::today())->delete();
+    }
+
 
 }