Refactor broadcast notifications (#1402)

* Refactor broadcast notifications

* StyleCI fixes

* Fix the planned_distance accidental changes

app/Contracts/Notification.php

@@ -2,7 +2,6 @@
 
 namespace App\Contracts;
 
-use App\Notifications\Channels\Discord\DiscordMessage;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
 
@@ -26,24 +25,4 @@ class Notification extends \Illuminate\Notifications\Notification implements Sho
 
         $this->channels = $notif_config[$klass];*/
     }
-
-    /**
-     * Get the notification's delivery channels.
-     *
-     * @param mixed $notifiable
-     *
-     * @return array
-     */
-    /*public function via($notifiable)
-    {
-        return $this->channels;
-    }*/
-
-    /**
-     * @return DiscordMessage|null
-     */
-    public function toDiscordChannel($notifiable): ?DiscordMessage
-    {
-        return null;
-    }
 }

app/Notifications/Channels/Discord/DiscordWebhook.php

@@ -20,14 +20,23 @@ class DiscordWebhook
     public function send($notifiable, Notification $notification)
     {
         $message = $notification->toDiscordChannel($notifiable);
-        if ($message === null || empty($message->webhook_url)) {
+        if ($message === null) {
-            Log::debug('Discord notifications not configured, skipping');
+            //Log::debug('Discord notifications not configured, skipping');
             return;
         }
 
+        $webhook_url = $message->webhook_url;
+        if (empty($webhook_url)) {
+            $webhook_url = setting('notifications.discord_private_webhook_url');
+            if (empty($webhook_url)) {
+                //Log::debug('Discord notifications not configured, skipping');
+                return;
+            }
+        }
+
         try {
             $data = $message->toArray();
-            $this->httpClient->post($message->webhook_url, $data);
+            $this->httpClient->post($webhook_url, $data);
         } catch (RequestException $e) {
             $request = Psr7\Message::toString($e->getRequest());
             $response = Psr7\Message::toString($e->getResponse());

app/Notifications/Messages/AdminUserRegistered.php

@@ -4,8 +4,6 @@ namespace App\Notifications\Messages;
 
 use App\Contracts\Notification;
 use App\Models\User;
-use App\Notifications\Channels\Discord\DiscordMessage;
-use App\Notifications\Channels\Discord\DiscordWebhook;
 use App\Notifications\Channels\MailChannel;
 use Illuminate\Contracts\Queue\ShouldQueue;
 
@@ -32,32 +30,21 @@ class AdminUserRegistered extends Notification implements ShouldQueue
         );
     }
 
+    /**
+     * @param $notifiable
+     *
+     * @return string[]
+     */
     public function via($notifiable)
     {
-        return ['mail', DiscordWebhook::class];
+        return ['mail'];
     }
 
     /**
-     * Send a Discord notification
+     * @param $notifiable
      *
-     * @param User  $pirep
+     * @return array
-     * @param mixed $user
-     *
-     * @return DiscordMessage|null
      */
-    public function toDiscordChannel($user): ?DiscordMessage
-    {
-        if (empty(setting('notifications.discord_private_webhook_url'))) {
-            return null;
-        }
-
-        $dm = new DiscordMessage();
-        return $dm->webhook(setting('notifications.discord_private_webhook_url'))
-            ->success()
-            ->title('New User Registered: '.$user->ident)
-            ->fields([]);
-    }
-
     public function toArray($notifiable)
     {
         return [

app/Notifications/Messages/Broadcast/NewsAdded.php

@@ -0,0 +1,57 @@
+<?php
+
+namespace App\Notifications\Messages\Broadcast;
+
+use App\Contracts\Notification;
+use App\Models\News;
+use App\Notifications\Channels\Discord\DiscordMessage;
+use Illuminate\Contracts\Queue\ShouldQueue;
+
+class NewsAdded extends Notification implements ShouldQueue
+{
+    private $news;
+
+    public function __construct(News $news)
+    {
+        parent::__construct();
+
+        $this->news = $news;
+    }
+
+    public function via($notifiable)
+    {
+        return ['discord_webhook'];
+    }
+
+    /**
+     * @param News $news
+     *
+     * @return DiscordMessage|null
+     */
+    public function toDiscordChannel($news): ?DiscordMessage
+    {
+        $dm = new DiscordMessage();
+        return $dm->success()
+            ->title('News: '.$news->subject)
+            ->author([
+                'name'     => $news->user->ident.' - '.$news->user->name_private,
+                'url'      => '',
+                'icon_url' => $news->user->resolveAvatarUrl(),
+            ])
+            ->description($news->body);
+    }
+
+    /**
+     * Get the array representation of the notification.
+     *
+     * @param mixed $notifiable
+     *
+     * @return array
+     */
+    public function toArray($notifiable)
+    {
+        return [
+            'news_id' => $this->news->id,
+        ];
+    }
+}

app/Notifications/Messages/Broadcast/PirepFiled.php

@@ -1,22 +1,18 @@
 <?php
 
-namespace App\Notifications\Messages;
+namespace App\Notifications\Messages\Broadcast;
 
 use App\Contracts\Notification;
 use App\Models\Pirep;
 use App\Notifications\Channels\Discord\DiscordMessage;
-use App\Notifications\Channels\Discord\DiscordWebhook;
-use App\Notifications\Channels\MailChannel;
 use App\Support\Units\Distance;
 use App\Support\Units\Time;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use PhpUnitsOfMeasure\Exception\NonNumericValue;
 use PhpUnitsOfMeasure\Exception\NonStringUnitName;
 
-class PirepSubmitted extends Notification implements ShouldQueue
+class PirepFiled extends Notification implements ShouldQueue
 {
-    use MailChannel;
-
     private $pirep;
 
     /**
@@ -29,17 +25,11 @@ class PirepSubmitted extends Notification implements ShouldQueue
         parent::__construct();
 
         $this->pirep = $pirep;
-
-        $this->setMailable(
-            'New PIREP Submitted',
-            'notifications.mail.admin.pirep.submitted',
-            ['pirep' => $this->pirep]
-        );
     }
 
     public function via($notifiable)
     {
-        return ['mail', DiscordWebhook::class];
+        return ['discord_webhook'];
     }
 
     /**
@@ -51,35 +41,30 @@ class PirepSubmitted extends Notification implements ShouldQueue
      */
     public function toDiscordChannel($pirep): ?DiscordMessage
     {
-        if (empty(setting('notifications.discord_public_webhook_url'))) {
-            return null;
-        }
-
         $title = 'Flight '.$pirep->ident.' Filed';
         $fields = [
-            'Flight'                => $pirep->ident,
+            'Flight'            => $pirep->ident,
-            'Departure Airport'     => $pirep->dpt_airport_id,
+            'Departure Airport' => $pirep->dpt_airport_id,
-            'Arrival Airport'       => $pirep->arr_airport_id,
+            'Arrival Airport'   => $pirep->arr_airport_id,
-            'Equipment'             => $pirep->aircraft->ident,
+            'Equipment'         => $pirep->aircraft->ident,
-            'Flight Time (Planned)' => Time::minutesToTimeString($pirep->planned_flight_time),
+            'Flight Time'       => Time::minutesToTimeString($pirep->flight_time),
         ];
 
-        if ($pirep->planned_distance) {
+        if ($pirep->distance) {
             try {
-                $planned_distance = new Distance(
+                $distance = new Distance(
-                    $pirep->planned_distance,
+                    $pirep->distance,
                     config('phpvms.internal_units.distance')
                 );
 
-                $pd = $planned_distance[$planned_distance->unit].' '.$planned_distance->unit;
+                $pd = $distance[$distance->unit].' '.$distance->unit;
-                $fields['Distance (Planned)'] = $pd;
+                $fields['Distance'] = $pd;
             } catch (NonNumericValue|NonStringUnitName $e) {
             }
         }
 
         $dm = new DiscordMessage();
-        return $dm->webhook(setting('notifications.discord_public_webhook_url'))
+        return $dm->success()
-            ->success()
             ->title($title)
             ->description($pirep->user->discord_id ? 'Flight by <@'.$pirep->user->discord_id.'>' : '')
             ->url(route('frontend.pireps.show', [$pirep->id]))

app/Notifications/Messages/Broadcast/PirepPrefiled.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace App\Notifications\Messages;
+namespace App\Notifications\Messages\Broadcast;
 
 use App\Contracts\Notification;
 use App\Models\Pirep;

app/Notifications/Messages/Broadcast/PirepStatusChanged.php

@@ -1,6 +1,6 @@
 <?php
 
-namespace App\Notifications\Messages;
+namespace App\Notifications\Messages\Broadcast;
 
 use App\Contracts\Notification;
 use App\Models\Enums\PirepStatus;
@@ -9,9 +9,11 @@ use App\Notifications\Channels\Discord\DiscordMessage;
 use App\Notifications\Channels\Discord\DiscordWebhook;
 use App\Support\Units\Distance;
 use App\Support\Units\Time;
+use function config;
 use Illuminate\Contracts\Queue\ShouldQueue;
 use PhpUnitsOfMeasure\Exception\NonNumericValue;
 use PhpUnitsOfMeasure\Exception\NonStringUnitName;
+use function route;
 
 /**
  * Send the PIREP accepted message to a particular user, can also be sent to Discord
@@ -73,10 +75,6 @@ class PirepStatusChanged extends Notification implements ShouldQueue
      */
     public function toDiscordChannel($pirep): ?DiscordMessage
     {
-        if (empty(setting('notifications.discord_public_webhook_url'))) {
-            return null;
-        }
-
         $title = 'Flight '.$pirep->ident.' '.self::$verbs[$pirep->status];
 
         $fields = [
@@ -112,8 +110,7 @@ class PirepStatusChanged extends Notification implements ShouldQueue
         }
 
         $dm = new DiscordMessage();
-        return $dm->webhook(setting('notifications.discord_public_webhook_url'))
+        return $dm->success()
-            ->success()
             ->title($title)
             ->description($pirep->user->discord_id ? 'Flight by <@'.$pirep->user->discord_id.'>' : '')
             ->url(route('frontend.pireps.show', [$pirep->id]))

app/Notifications/Messages/Broadcast/UserRegistered.php

@@ -0,0 +1,73 @@
+<?php
+
+namespace App\Notifications\Messages\Broadcast;
+
+use App\Contracts\Notification;
+use App\Models\User;
+use App\Notifications\Channels\Discord\DiscordMessage;
+use App\Notifications\Channels\MailChannel;
+use Illuminate\Contracts\Queue\ShouldQueue;
+
+/**
+ * Send a message to a Discord channel that a user was registered
+ */
+class UserRegistered extends Notification implements ShouldQueue
+{
+    use MailChannel;
+
+    private $user;
+
+    /**
+     * Create a new notification instance.
+     *
+     * @param \App\Models\User $user
+     */
+    public function __construct(User $user)
+    {
+        parent::__construct();
+
+        $this->user = $user;
+    }
+
+    /**
+     * @param $notifiable
+     *
+     * @return string[]
+     */
+    public function via($notifiable)
+    {
+        return ['discord_webhook'];
+    }
+
+    /**
+     * Send a Discord notification
+     *
+     * @param $notifiable
+     *
+     * @return DiscordMessage|null
+     */
+    public function toDiscordChannel($notifiable): ?DiscordMessage
+    {
+        $dm = new DiscordMessage();
+        return $dm->success()
+            ->title('New User Registered: '.$this->user->ident)
+            ->author([
+                'name'     => $this->user->ident.' - '.$this->user->name_private,
+                'url'      => '',
+                'icon_url' => $this->user->resolveAvatarUrl(),
+            ])
+            ->fields([]);
+    }
+
+    /**
+     * @param $notifiable
+     *
+     * @return array
+     */
+    public function toArray($notifiable)
+    {
+        return [
+            'user_id' => $this->user->id,
+        ];
+    }
+}

app/Notifications/Messages/PirepFiled.php

@@ -0,0 +1,53 @@
+<?php
+
+namespace App\Notifications\Messages;
+
+use App\Contracts\Notification;
+use App\Models\Pirep;
+use App\Notifications\Channels\MailChannel;
+use Illuminate\Contracts\Queue\ShouldQueue;
+
+class PirepFiled extends Notification implements ShouldQueue
+{
+    use MailChannel;
+
+    private $pirep;
+
+    /**
+     * Create a new notification instance.
+     *
+     * @param \App\Models\Pirep $pirep
+     */
+    public function __construct(Pirep $pirep)
+    {
+        parent::__construct();
+
+        $this->pirep = $pirep;
+
+        $this->setMailable(
+            'New PIREP Submitted',
+            'notifications.mail.admin.pirep.submitted',
+            ['pirep' => $this->pirep]
+        );
+    }
+
+    public function via($notifiable)
+    {
+        return ['mail'];
+    }
+
+    /**
+     * Get the array representation of the notification.
+     *
+     * @param mixed $notifiable
+     *
+     * @return array
+     */
+    public function toArray($notifiable)
+    {
+        return [
+            'pirep_id' => $this->pirep->id,
+            'user_id'  => $this->pirep->user_id,
+        ];
+    }
+}

app/Notifications/NotificationEventsHandler.php

@@ -134,10 +134,10 @@ class NotificationEventsHandler extends Listener
          */
         $this->notifyAdmins(new Messages\AdminUserRegistered($event->user));
 
-        /**
+        /*
-         * Discord and other notifications
+         * Broadcast notifications
          */
-        Notification::send([$event->user], new Messages\AdminUserRegistered($event->user));
+        Notification::send([$event->user], new Messages\Broadcast\UserRegistered($event->user));
     }
 
     /**
@@ -166,16 +166,20 @@ class NotificationEventsHandler extends Listener
     public function onPirepPrefile(PirepPrefiled $event): void
     {
         Log::info('NotificationEvents::onPirepPrefile: '.$event->pirep->id.' prefiled');
-        Notification::send([$event->pirep], new Messages\PirepPrefiled($event->pirep));
+
+        /*
+         * Broadcast notifications
+         */
+        Notification::send([$event->pirep], new Messages\Broadcast\PirepPrefiled($event->pirep));
     }
 
     /**
-     * Status Change notification
+     * Status Change notification. Disabled for now, too noisy
      */
     public function onPirepStatusChange(PirepStatusChange $event): void
     {
-        Log::info('NotificationEvents::onPirepStatusChange: '.$event->pirep->id.' prefiled');
+        // Log::info('NotificationEvents::onPirepStatusChange: '.$event->pirep->id.' prefiled');
-        Notification::send([$event->pirep], new Messages\PirepStatusChanged($event->pirep));
+        // Notification::send([$event->pirep], new Messages\Discord\PirepStatusChanged($event->pirep));
     }
 
     /**
@@ -186,8 +190,12 @@ class NotificationEventsHandler extends Listener
     public function onPirepFile(PirepFiled $event): void
     {
         Log::info('NotificationEvents::onPirepFile: '.$event->pirep->id.' filed');
-        $this->notifyAdmins(new Messages\PirepSubmitted($event->pirep));
+        $this->notifyAdmins(new Messages\PirepFiled($event->pirep));
-        Notification::send([$event->pirep], new Messages\PirepSubmitted($event->pirep));
+
+        /*
+         * Broadcast notifications
+         */
+        Notification::send([$event->pirep], new Messages\Broadcast\PirepFiled($event->pirep));
     }
 
     /**
@@ -221,6 +229,10 @@ class NotificationEventsHandler extends Listener
     {
         Log::info('NotificationEvents::onNewsAdded');
         $this->notifyAllUsers(new Messages\NewsAdded($event->news));
-        Notification::send([$event->news], new Messages\NewsAdded($event->news));
+
+        /*
+         * Broadcast notifications
+         */
+        Notification::send([$event->news], new Messages\Broadcast\NewsAdded($event->news));
     }
 }

app/Providers/AppServiceProvider.php

@@ -2,10 +2,12 @@
 
 namespace App\Providers;
 
+use App\Notifications\Channels\Discord\DiscordWebhook;
 use App\Services\ModuleService;
 use App\Support\ThemeViewFinder;
 use App\Support\Utils;
 use Illuminate\Pagination\Paginator;
+use Illuminate\Support\Facades\Notification;
 use Illuminate\Support\Facades\Schema;
 use Illuminate\Support\Facades\View;
 use Illuminate\Support\ServiceProvider;
@@ -15,8 +17,13 @@ class AppServiceProvider extends ServiceProvider
     public function boot(): void
     {
         Schema::defaultStringLength(191);
+
         Paginator::useBootstrap();
         View::share('moduleSvc', app(ModuleService::class));
+
+        Notification::extend('discord_webhook', function ($app) {
+            return app(DiscordWebhook::class);
+        });
     }
 
     /**

app/Services/PirepService.php

@@ -243,7 +243,7 @@ class PirepService extends Service
 
         // Copy some fields over from Flight if we have it
         if ($pirep->flight) {
-            $pirep->planned_distance = $pirep->flight->distance;
+            $pirep->planned_distance = $pirep->flight->planned_distance;
             $pirep->planned_flight_time = $pirep->flight->flight_time;
         }
 
@@ -336,7 +336,7 @@ class PirepService extends Service
 
         // Copy some fields over from Flight if we have it
         if ($pirep->flight) {
-            $pirep->planned_distance = $pirep->flight->distance;
+            $pirep->distance = $pirep->flight->distance;
             $pirep->planned_flight_time = $pirep->flight->flight_time;
         }
 

config/notifications.php

@@ -3,8 +3,8 @@
 use App\Notifications\Messages\AdminUserRegistered;
 use App\Notifications\Messages\NewsAdded;
 use App\Notifications\Messages\PirepAccepted;
+use App\Notifications\Messages\PirepFiled;
 use App\Notifications\Messages\PirepRejected;
-use App\Notifications\Messages\PirepSubmitted;
 use App\Notifications\Messages\UserPending;
 use App\Notifications\Messages\UserRegistered;
 use App\Notifications\Messages\UserRejected;
@@ -18,7 +18,7 @@ return [
         NewsAdded::class           => ['mail'],
         PirepAccepted::class       => ['mail'],
         PirepRejected::class       => ['mail'],
-        PirepSubmitted::class      => ['mail'],
+        PirepFiled::class          => ['mail'],
         UserPending::class         => ['mail'],
         UserRegistered::class      => ['mail'],
         UserRejected::class        => ['mail'],

tests/PIREPTest.php

@@ -14,13 +14,12 @@ use App\Models\Pirep;
 use App\Models\Rank;
 use App\Models\User;
 use App\Notifications\Messages\PirepAccepted;
-use App\Notifications\Messages\PirepSubmitted;
+use App\Notifications\Messages\PirepFiled;
 use App\Repositories\SettingRepository;
 use App\Services\AircraftService;
 use App\Services\BidService;
 use App\Services\FlightService;
 use App\Services\PirepService;
-use App\Services\UserService;
 use App\Support\Units\Fuel;
 use Carbon\Carbon;
 use Illuminate\Support\Facades\Notification;
@@ -156,6 +155,7 @@ class PIREPTest extends TestCase
      */
     public function testUnitFields()
     {
+        /** @var Pirep $pirep */
         $pirep = $this->createPirep();
         $pirep->save();
 
@@ -251,16 +251,13 @@ class PIREPTest extends TestCase
     {
         /** @var User $user */
         $user = factory(User::class)->create([
+            'name'        => 'testPirepNotifications user',
             'flights'     => 0,
             'flight_time' => 0,
             'rank_id'     => 1,
         ]);
 
-        $admin = factory(User::class)->create();
+        $admin = $this->createAdminUser(['name' => 'testPirepNotifications Admin']);
-
-        /** @var UserService $userSvc */
-        $userSvc = app(UserService::class);
-        $userSvc->addUserToRole($admin, 'admin');
 
         $pirep = factory(Pirep::class)->create([
             'airline_id' => 1,
@@ -271,7 +268,8 @@ class PIREPTest extends TestCase
         $this->pirepSvc->submit($pirep);
 
         // Make sure a notification was sent out to the admin
-        Notification::assertSentTo([$admin], PirepSubmitted::class);
+        Notification::assertSentTo([$admin], PirepFiled::class);
+        Notification::assertNotSentTo([$user], PirepFiled::class);
     }
 
     /**
@@ -281,6 +279,7 @@ class PIREPTest extends TestCase
     {
         $this->updateSetting('pilots.count_transfer_hours', false);
 
+        /** @var User $user */
         $user = factory(User::class)->create([
             'flights'     => 0,
             'flight_time' => 0,
@@ -304,6 +303,7 @@ class PIREPTest extends TestCase
             $this->pirepSvc->accept($pirep);
         }
 
+        /** @var User $pilot */
         $pilot = User::find($user->id);
         $last_pirep = Pirep::where('id', $pilot->last_pirep_id)->first();
 

tests/RegistrationTest.php

@@ -2,11 +2,10 @@
 
 namespace Tests;
 
-use App\Events\UserRegistered;
 use App\Models\Enums\UserState;
 use App\Models\User;
+use App\Notifications\Messages\AdminUserRegistered;
 use App\Services\UserService;
-use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Notification;
 
@@ -21,13 +20,12 @@ class RegistrationTest extends TestCase
      */
     public function testRegistration()
     {
-        Event::fake();
+        $admin = $this->createAdminUser(['name' => 'testRegistration Admin']);
-        Notification::fake();
 
         /** @var UserService $userSvc */
         $userSvc = app(UserService::class);
 
-        setting('pilots.auto_accept', true);
+        $this->updateSetting('pilots.auto_accept', true);
 
         $attrs = factory(User::class)->make()->makeVisible(['api_key', 'name', 'email'])->toArray();
         $attrs['password'] = Hash::make('secret');
@@ -35,14 +33,7 @@ class RegistrationTest extends TestCase
 
         $this->assertEquals(UserState::ACTIVE, $user->state);
 
-        Event::assertDispatched(UserRegistered::class, function ($e) use ($user) {
+        Notification::assertSentTo([$admin], AdminUserRegistered::class);
-            return $e->user->id === $user->id
+        Notification::assertNotSentTo([$user], AdminUserRegistered::class);
-                && $e->user->state === $user->state;
-        });
-
-        /*Notification::assertSentTo(
-            [$user],
-            \App\Notifications\Messages\UserRegistered::class
-        );*/
     }
 }

tests/TestCase.php

@@ -58,7 +58,7 @@ abstract class TestCase extends \Illuminate\Foundation\Testing\TestCase
         );
 
         Artisan::call('database:create', ['--reset' => true]);
-        Artisan::call('migrate:refresh', ['--env' => 'testing', '--force' => true]);
+        Artisan::call('migrate', ['--env' => 'testing', '--force' => true]);
 
         Notification::fake();
         // $this->disableExceptionHandling();
@@ -71,6 +71,7 @@ abstract class TestCase extends \Illuminate\Foundation\Testing\TestCase
     protected function disableExceptionHandling()
     {
         $this->app->instance(ExceptionHandler::class, new class() extends Handler {
+            /** @noinspection PhpMissingParentConstructorInspection */
             public function __construct()
             {
             }

tests/TestData.php

@@ -9,6 +9,7 @@ use App\Models\Pirep;
 use App\Models\Role;
 use App\Models\Subfleet;
 use App\Models\User;
+use App\Services\UserService;
 use Exception;
 
 trait TestData
@@ -32,6 +33,25 @@ trait TestData
         ], $attrs));
     }
 
+    /**
+     * Create an admin user
+     *
+     * @param array $attrs
+     *
+     * @return User
+     */
+    public function createAdminUser(array $attrs = []): User
+    {
+        /** @var User $admin */
+        $admin = factory(User::class)->create($attrs);
+
+        /** @var UserService $userSvc */
+        $userSvc = app(UserService::class);
+        $userSvc->addUserToRole($admin, 'admin');
+
+        return $admin;
+    }
+
     /**
      * Create a new PIREP with a proper subfleet/rank/user and an
      * aircraft that the user is allowed to fly