Notifications fixes (#804)

* Emails not sending #795 #722

* Fix for news items not being sent; refactor some events; check for opt-in #722

* Formatting

* Remove extra parameter

app/Contracts/Event.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace App\Contracts;
+
+use Illuminate\Foundation\Events\Dispatchable;
+use Illuminate\Queue\SerializesModels;
+
+class Event
+{
+    use Dispatchable;
+    use SerializesModels;
+}

app/Contracts/Notification.php

@@ -1,17 +1,17 @@
 <?php
 
-namespace App\Notifications;
+namespace App\Contracts;
 
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
-use Illuminate\Notifications\Notification;
 use Illuminate\Support\Facades\Log;
 
-class BaseNotification extends Notification implements ShouldQueue
+class Notification extends \Illuminate\Notifications\Notification implements ShouldQueue
 {
     use Queueable;
 
     public $channels = [];
+    public $requires_opt_in = false;
 
     public function __construct()
     {

app/Database/factories/UserFactory.php

@@ -1,7 +1,9 @@
 <?php
 
+use App\Models\Airline;
 use App\Models\Enums\UserState;
 use Faker\Generator as Faker;
+use Illuminate\Support\Facades\Hash;
 
 $factory->define(App\Models\User::class, function (Faker $faker) {
     static $password;
@@ -14,7 +16,7 @@ $factory->define(App\Models\User::class, function (Faker $faker) {
         'password'   => $password ?: $password = Hash::make('secret'),
         'api_key'    => $faker->sha1,
         'airline_id' => function () {
-            return factory(\App\Models\Airline::class)->create()->id;
+            return factory(Airline::class)->create()->id;
         },
         'rank_id'        => 1,
         'flights'        => $faker->numberBetween(0, 1000),

app/Events/AcarsUpdate.php

@@ -2,10 +2,11 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Acars;
 use App\Models\Pirep;
 
-class AcarsUpdate extends BaseEvent
+class AcarsUpdate extends Event
 {
     /** @var Pirep */
     public $pirep;

app/Events/BaseEvent.php

@@ -2,13 +2,11 @@
 
 namespace App\Events;
 
-use Illuminate\Broadcasting\InteractsWithSockets;
+use App\Contracts\Event;
-use Illuminate\Foundation\Events\Dispatchable;
-use Illuminate\Queue\SerializesModels;
 
-class BaseEvent
+/**
+ * @deprecated Extend App\Contracts\Event directly
+ */
+class BaseEvent extends Event
 {
-    use Dispatchable;
-    use InteractsWithSockets;
-    use SerializesModels;
 }

app/Events/CronHourly.php

@@ -2,6 +2,8 @@
 
 namespace App\Events;
 
-class CronHourly extends BaseEvent
+use App\Contracts\Event;
+
+class CronHourly extends Event
 {
 }

app/Events/CronMonthly.php

@@ -2,10 +2,12 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
+
 /**
  * This event is dispatched when the monthly cron is run
  * It happens after all of the default nightly tasks
  */
-class CronMonthly extends BaseEvent
+class CronMonthly extends Event
 {
 }

app/Events/CronNightly.php

@@ -2,10 +2,12 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
+
 /**
  * This event is dispatched when the daily cron is run
  * It happens after all of the default nightly tasks
  */
-class CronNightly extends BaseEvent
+class CronNightly extends Event
 {
 }

app/Events/CronWeekly.php

@@ -2,11 +2,13 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
+
 /**
  * This event is dispatched when the weekly cron is run
  * It happens after all of the default nightly tasks
  */
-class CronWeekly extends BaseEvent
+class CronWeekly extends Event
 {
     public function __construct()
     {

app/Events/Expenses.php

@@ -2,6 +2,7 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
 /**
@@ -25,7 +26,7 @@ use App\Models\Pirep;
  *
  * The event will have a copy of the PIREP model, if it's applicable
  */
-class Expenses extends BaseEvent
+class Expenses extends Event
 {
     public $pirep;
 

app/Events/NewsAdded.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\News;
 
-class NewsAdded extends BaseEvent
+class NewsAdded extends Event
 {
     public $news;
 

app/Events/PirepAccepted.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
-class PirepAccepted extends BaseEvent
+class PirepAccepted extends Event
 {
     public $pirep;
 

app/Events/PirepCancelled.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
-class PirepCancelled extends BaseEvent
+class PirepCancelled extends Event
 {
     public $pirep;
 

app/Events/PirepFiled.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
-class PirepFiled extends BaseEvent
+class PirepFiled extends Event
 {
     public $pirep;
 

app/Events/PirepPrefiled.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
-class PirepPrefiled extends BaseEvent
+class PirepPrefiled extends Event
 {
     public $pirep;
 

app/Events/PirepRejected.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
-class PirepRejected extends BaseEvent
+class PirepRejected extends Event
 {
     public $pirep;
 

app/Events/PirepUpdated.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\Pirep;
 
-class PirepUpdated extends BaseEvent
+class PirepUpdated extends Event
 {
     public $pirep;
 

app/Events/TestEvent.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\User;
 
-class TestEvent extends BaseEvent
+class TestEvent extends Event
 {
     public $user;
 

app/Events/UserAccepted.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\User;
 
-class UserAccepted extends BaseEvent
+class UserAccepted extends Event
 {
     public $user;
 

app/Events/UserRegistered.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\User;
 
-class UserRegistered extends BaseEvent
+class UserRegistered extends Event
 {
     public $user;
 

app/Events/UserStateChanged.php

@@ -2,12 +2,13 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\User;
 
 /**
  * Event triggered when a user's state changes
  */
-class UserStateChanged extends BaseEvent
+class UserStateChanged extends Event
 {
     public $old_state;
     public $user;

app/Events/UserStatsChanged.php

@@ -2,9 +2,10 @@
 
 namespace App\Events;
 
+use App\Contracts\Event;
 use App\Models\User;
 
-class UserStatsChanged extends BaseEvent
+class UserStatsChanged extends Event
 {
     public $stat_name;
     public $old_value;

app/Notifications/EventHandler.php

@@ -16,6 +16,7 @@ use App\Notifications\Messages\UserPending;
 use App\Notifications\Messages\UserRejected;
 use App\Notifications\Notifiables\Broadcast;
 use Exception;
+use Illuminate\Support\Collection;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Notification;
 
@@ -27,6 +28,7 @@ class EventHandler extends Listener
     private static $broadcastNotifyable;
 
     public static $callbacks = [
+        NewsAdded::class        => 'onNewsAdded',
         PirepAccepted::class    => 'onPirepAccepted',
         PirepFiled::class       => 'onPirepFile',
         PirepRejected::class    => 'onPirepRejected',
@@ -42,7 +44,7 @@ class EventHandler extends Listener
     /**
      * Send a notification to all of the admins
      *
-     * @param \Illuminate\Notifications\Notification $notification
+     * @param \App\Contracts\Notification $notification
      */
     protected function notifyAdmins($notification)
     {
@@ -56,8 +58,8 @@ class EventHandler extends Listener
     }
 
     /**
-     * @param User                                   $user
+     * @param User                        $user
-     * @param \Illuminate\Notifications\Notification $notification
+     * @param \App\Contracts\Notification $notification
      */
     protected function notifyUser($user, $notification)
     {
@@ -69,13 +71,25 @@ class EventHandler extends Listener
     }
 
     /**
-     * Send a notification to all users
+     * Send a notification to all users. Also can specify if a particular notification
+     * requires an opt-in
      *
-     * @param $notification
+     * @param \App\Contracts\Notification $notification
      */
-    protected function notifyAllUsers($notification)
+    protected function notifyAllUsers(\App\Contracts\Notification $notification)
     {
-        $users = User::all()->get();
+        $where = [];
+        if ($notification->requires_opt_in === true) {  // If the opt-in is required
+            $where['opt_in'] = true;
+        }
+
+        /** @var Collection $users */
+        $users = User::where($where)->get();
+        if (empty($users) || $users->count() === 0) {
+            return;
+        }
+
+        Log::info('Sending notification to '.$users->count().' users');
 
         try {
             Notification::send($users, $notification);
@@ -164,7 +178,7 @@ class EventHandler extends Listener
     }
 
     /**
-     * Notify all users of a news event
+     * Notify all users of a news event, but only the users which have opted in
      *
      * @param \App\Events\NewsAdded $event
      */

app/Notifications/Messages/AdminUserRegistered.php

@@ -2,11 +2,11 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\User;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class AdminUserRegistered extends BaseNotification
+class AdminUserRegistered extends Notification
 {
     use MailChannel;
 

app/Notifications/Messages/NewsAdded.php

@@ -2,15 +2,16 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\News;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class NewsAdded extends BaseNotification
+class NewsAdded extends Notification
 {
     use MailChannel;
 
     public $channels = ['mail'];
+    public $requires_opt_in = true;
 
     private $news;
 

app/Notifications/Messages/PirepAccepted.php

@@ -2,14 +2,14 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\Pirep;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
 /**
  * Send the PIREP accepted message to a particular user
  */
-class PirepAccepted extends BaseNotification
+class PirepAccepted extends Notification
 {
     use MailChannel;
 

app/Notifications/Messages/PirepRejected.php

@@ -2,11 +2,11 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\Pirep;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class PirepRejected extends BaseNotification
+class PirepRejected extends Notification
 {
     use MailChannel;
 

app/Notifications/Messages/PirepSubmitted.php

@@ -2,11 +2,11 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\Pirep;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class PirepSubmitted extends BaseNotification
+class PirepSubmitted extends Notification
 {
     use MailChannel;
 

app/Notifications/Messages/UserPending.php

@@ -2,11 +2,11 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\User;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class UserPending extends BaseNotification
+class UserPending extends Notification
 {
     use MailChannel;
 

app/Notifications/Messages/UserRegistered.php

@@ -2,11 +2,11 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\User;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class UserRegistered extends BaseNotification
+class UserRegistered extends Notification
 {
     use MailChannel;
 

app/Notifications/Messages/UserRejected.php

@@ -2,11 +2,11 @@
 
 namespace App\Notifications\Messages;
 
+use App\Contracts\Notification;
 use App\Models\User;
-use App\Notifications\BaseNotification;
 use App\Notifications\Channels\MailChannel;
 
-class UserRejected extends BaseNotification
+class UserRejected extends Notification
 {
     use MailChannel;
 

app/Providers/EventServiceProvider.php

@@ -38,7 +38,6 @@ class EventServiceProvider extends ServiceProvider
 
         UpdateAvailable::class => [],
         UpdateSucceeded::class => [],
-
     ];
 
     protected $subscribe = [

tests/NewsTest.php

@@ -0,0 +1,43 @@
+<?php
+
+namespace Tests;
+
+use App\Models\User;
+use App\Notifications\Messages\NewsAdded;
+use App\Services\NewsService;
+use Illuminate\Support\Facades\Notification;
+
+class NewsTest extends TestCase
+{
+    /** @var NewsService */
+    private $newsSvc;
+
+    public function setUp(): void
+    {
+        parent::setUp();
+
+        $this->newsSvc = app(NewsService::class);
+    }
+
+    /**
+     * Tests that news is added but mainly that notifications are sent out to all users
+     *
+     * @throws \Prettus\Validator\Exceptions\ValidatorException
+     */
+    public function testNewsNotifications()
+    {
+        /** @var User[] $users */
+        $users_opt_in = factory(User::class, 5)->create(['opt_in' => true]);
+
+        /** @var User[] $users */
+        $users_opt_out = factory(User::class, 5)->create(['opt_in' => false]);
+
+        $this->newsSvc->addNews([
+            'user_id' => $users_opt_out[0]->id,
+            'subject' => 'News Item',
+            'body'    => 'News!',
+        ]);
+
+        Notification::assertSentTo($users_opt_in, NewsAdded::class);
+    }
+}

tests/PIREPTest.php

@@ -13,11 +13,13 @@ use App\Models\Navdata;
 use App\Models\Pirep;
 use App\Models\User;
 use App\Notifications\Messages\PirepAccepted;
+use App\Notifications\Messages\PirepSubmitted;
 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 Carbon\Carbon;
 use Illuminate\Support\Facades\Notification;
 
@@ -212,6 +214,38 @@ class PIREPTest extends TestCase
         $this->assertFalse($pirep_ids->contains($pirep_cancelled->id));
     }
 
+    /**
+     * Make sure that a notification has been sent out to admins when a PIREP is submitted
+     *
+     * @throws \Exception
+     */
+    public function testPirepNotifications()
+    {
+        /** @var User $user */
+        $user = factory(User::class)->create([
+            'flights'     => 0,
+            'flight_time' => 0,
+            'rank_id'     => 1,
+        ]);
+
+        $admin = factory(User::class)->create();
+
+        /** @var UserService $userSvc */
+        $userSvc = app(UserService::class);
+        $userSvc->addUserToRole($admin, 'admin');
+
+        $pirep = factory(Pirep::class)->create([
+            'airline_id' => 1,
+            'user_id'    => $user->id,
+        ]);
+
+        $this->pirepSvc->create($pirep);
+        $this->pirepSvc->submit($pirep);
+
+        // Make sure a notification was sent out to the admin
+        Notification::assertSentTo([$admin], PirepSubmitted::class);
+    }
+
     /**
      * check the stats/ranks, etc have incremented properly
      */