From e99c22b0076069ebbdc48799bbcc7daed34ba36e Mon Sep 17 00:00:00 2001 From: Nabeel S Date: Thu, 3 Sep 2020 12:50:42 -0400 Subject: [PATCH] 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 | 12 ++++++ .../Notification.php} | 6 +-- app/Database/factories/UserFactory.php | 4 +- app/Events/AcarsUpdate.php | 3 +- app/Events/BaseEvent.php | 12 +++--- app/Events/CronHourly.php | 4 +- app/Events/CronMonthly.php | 4 +- app/Events/CronNightly.php | 4 +- app/Events/CronWeekly.php | 4 +- app/Events/Expenses.php | 3 +- app/Events/NewsAdded.php | 3 +- app/Events/PirepAccepted.php | 3 +- app/Events/PirepCancelled.php | 3 +- app/Events/PirepFiled.php | 3 +- app/Events/PirepPrefiled.php | 3 +- app/Events/PirepRejected.php | 3 +- app/Events/PirepUpdated.php | 3 +- app/Events/TestEvent.php | 3 +- app/Events/UserAccepted.php | 3 +- app/Events/UserRegistered.php | 3 +- app/Events/UserStateChanged.php | 3 +- app/Events/UserStatsChanged.php | 3 +- app/Notifications/EventHandler.php | 30 +++++++++---- .../Messages/AdminUserRegistered.php | 4 +- app/Notifications/Messages/NewsAdded.php | 5 ++- app/Notifications/Messages/PirepAccepted.php | 4 +- app/Notifications/Messages/PirepRejected.php | 4 +- app/Notifications/Messages/PirepSubmitted.php | 4 +- app/Notifications/Messages/UserPending.php | 4 +- app/Notifications/Messages/UserRegistered.php | 4 +- app/Notifications/Messages/UserRejected.php | 4 +- app/Providers/EventServiceProvider.php | 1 - tests/NewsTest.php | 43 +++++++++++++++++++ tests/PIREPTest.php | 34 +++++++++++++++ 34 files changed, 179 insertions(+), 54 deletions(-) create mode 100644 app/Contracts/Event.php rename app/{Notifications/BaseNotification.php => Contracts/Notification.php} (85%) create mode 100644 tests/NewsTest.php diff --git a/app/Contracts/Event.php b/app/Contracts/Event.php new file mode 100644 index 00000000..a844d449 --- /dev/null +++ b/app/Contracts/Event.php @@ -0,0 +1,12 @@ +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), diff --git a/app/Events/AcarsUpdate.php b/app/Events/AcarsUpdate.php index 4b4d7711..f0b1445d 100644 --- a/app/Events/AcarsUpdate.php +++ b/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; diff --git a/app/Events/BaseEvent.php b/app/Events/BaseEvent.php index 71bcddc2..90f7bb7a 100644 --- a/app/Events/BaseEvent.php +++ b/app/Events/BaseEvent.php @@ -2,13 +2,11 @@ namespace App\Events; -use Illuminate\Broadcasting\InteractsWithSockets; -use Illuminate\Foundation\Events\Dispatchable; -use Illuminate\Queue\SerializesModels; +use App\Contracts\Event; -class BaseEvent +/** + * @deprecated Extend App\Contracts\Event directly + */ +class BaseEvent extends Event { - use Dispatchable; - use InteractsWithSockets; - use SerializesModels; } diff --git a/app/Events/CronHourly.php b/app/Events/CronHourly.php index 38207f49..15138756 100644 --- a/app/Events/CronHourly.php +++ b/app/Events/CronHourly.php @@ -2,6 +2,8 @@ namespace App\Events; -class CronHourly extends BaseEvent +use App\Contracts\Event; + +class CronHourly extends Event { } diff --git a/app/Events/CronMonthly.php b/app/Events/CronMonthly.php index 04bcb435..b05aeb0c 100644 --- a/app/Events/CronMonthly.php +++ b/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 { } diff --git a/app/Events/CronNightly.php b/app/Events/CronNightly.php index 2d47a75b..49e8f8d6 100644 --- a/app/Events/CronNightly.php +++ b/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 { } diff --git a/app/Events/CronWeekly.php b/app/Events/CronWeekly.php index ba985c9c..dcceacbe 100644 --- a/app/Events/CronWeekly.php +++ b/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() { diff --git a/app/Events/Expenses.php b/app/Events/Expenses.php index 2ba8acaa..6b70d891 100644 --- a/app/Events/Expenses.php +++ b/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; diff --git a/app/Events/NewsAdded.php b/app/Events/NewsAdded.php index 20496407..8bb8d1f9 100644 --- a/app/Events/NewsAdded.php +++ b/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; diff --git a/app/Events/PirepAccepted.php b/app/Events/PirepAccepted.php index 7619d721..05b95b7c 100644 --- a/app/Events/PirepAccepted.php +++ b/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; diff --git a/app/Events/PirepCancelled.php b/app/Events/PirepCancelled.php index 38b72479..74c30faf 100644 --- a/app/Events/PirepCancelled.php +++ b/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; diff --git a/app/Events/PirepFiled.php b/app/Events/PirepFiled.php index b9f6a3e6..e4a21bd4 100644 --- a/app/Events/PirepFiled.php +++ b/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; diff --git a/app/Events/PirepPrefiled.php b/app/Events/PirepPrefiled.php index 830d6366..786204a8 100644 --- a/app/Events/PirepPrefiled.php +++ b/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; diff --git a/app/Events/PirepRejected.php b/app/Events/PirepRejected.php index 1d78e1eb..a9bfde59 100644 --- a/app/Events/PirepRejected.php +++ b/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; diff --git a/app/Events/PirepUpdated.php b/app/Events/PirepUpdated.php index 78ff0e11..032cf8ff 100644 --- a/app/Events/PirepUpdated.php +++ b/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; diff --git a/app/Events/TestEvent.php b/app/Events/TestEvent.php index 27cb623e..6b8bdd95 100644 --- a/app/Events/TestEvent.php +++ b/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; diff --git a/app/Events/UserAccepted.php b/app/Events/UserAccepted.php index 738188d8..85f149de 100644 --- a/app/Events/UserAccepted.php +++ b/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; diff --git a/app/Events/UserRegistered.php b/app/Events/UserRegistered.php index 7bf622f7..3493cad2 100644 --- a/app/Events/UserRegistered.php +++ b/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; diff --git a/app/Events/UserStateChanged.php b/app/Events/UserStateChanged.php index 34291ae2..5dfec530 100644 --- a/app/Events/UserStateChanged.php +++ b/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; diff --git a/app/Events/UserStatsChanged.php b/app/Events/UserStatsChanged.php index 211efcd5..8e6ca973 100644 --- a/app/Events/UserStatsChanged.php +++ b/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; diff --git a/app/Notifications/EventHandler.php b/app/Notifications/EventHandler.php index f1fa94a0..48a9753d 100644 --- a/app/Notifications/EventHandler.php +++ b/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 \Illuminate\Notifications\Notification $notification + * @param User $user + * @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 */ diff --git a/app/Notifications/Messages/AdminUserRegistered.php b/app/Notifications/Messages/AdminUserRegistered.php index 9ddbcd2b..1e72a1b9 100644 --- a/app/Notifications/Messages/AdminUserRegistered.php +++ b/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; diff --git a/app/Notifications/Messages/NewsAdded.php b/app/Notifications/Messages/NewsAdded.php index 3032cab2..660d366d 100644 --- a/app/Notifications/Messages/NewsAdded.php +++ b/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; diff --git a/app/Notifications/Messages/PirepAccepted.php b/app/Notifications/Messages/PirepAccepted.php index a4b702ec..599b9f0b 100644 --- a/app/Notifications/Messages/PirepAccepted.php +++ b/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; diff --git a/app/Notifications/Messages/PirepRejected.php b/app/Notifications/Messages/PirepRejected.php index c28f63da..7b1629ce 100644 --- a/app/Notifications/Messages/PirepRejected.php +++ b/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; diff --git a/app/Notifications/Messages/PirepSubmitted.php b/app/Notifications/Messages/PirepSubmitted.php index 40500250..829f2599 100644 --- a/app/Notifications/Messages/PirepSubmitted.php +++ b/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; diff --git a/app/Notifications/Messages/UserPending.php b/app/Notifications/Messages/UserPending.php index f737ea47..fee14cb0 100644 --- a/app/Notifications/Messages/UserPending.php +++ b/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; diff --git a/app/Notifications/Messages/UserRegistered.php b/app/Notifications/Messages/UserRegistered.php index 894a03da..605898fd 100644 --- a/app/Notifications/Messages/UserRegistered.php +++ b/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; diff --git a/app/Notifications/Messages/UserRejected.php b/app/Notifications/Messages/UserRejected.php index 03d64efd..df2ed2d7 100644 --- a/app/Notifications/Messages/UserRejected.php +++ b/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; diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 4f3b0994..86c86441 100755 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -38,7 +38,6 @@ class EventServiceProvider extends ServiceProvider UpdateAvailable::class => [], UpdateSucceeded::class => [], - ]; protected $subscribe = [ diff --git a/tests/NewsTest.php b/tests/NewsTest.php new file mode 100644 index 00000000..0bae0b53 --- /dev/null +++ b/tests/NewsTest.php @@ -0,0 +1,43 @@ +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); + } +} diff --git a/tests/PIREPTest.php b/tests/PIREPTest.php index ef2c34ab..f04df710 100644 --- a/tests/PIREPTest.php +++ b/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 */