Skip to content

Commit c76f76e

Browse files
sethettersethetter
committed
User can receive daily notification digest email
Summary: - User can choose frequency option for notifications - If "immediately", send single email right away - If "daily", send a digest of all daily notifications once daily - Daily notification email should group like notifications together. Resolves T283 Test Plan: - Log in and go to notification preferences - Change some to "Daily" - Do things that trigger those notifications - Test grouped notifications: - Support vote changes - Comments on a document - Likes / flags on a comment - Replies to a comment - Do things that trigger immediate notifications too - ..to ensure they don't end up in the digest - Check the `notifications` table to be sure it's populated - Run `php artisan send-daily-notifications` - Check the email that is sent - Make sure notifications are grouped properly - Check the `notifications` table, it should be empty Reviewers: doshitan Reviewed By: doshitan Maniphest Tasks: T283 Differential Revision: https://phabricator.opengovfoundation.org/D184
1 parent 8aa5213 commit c76f76e

28 files changed

+853
-250
lines changed

‎app/Console/Commands/SendDailyNotifications.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
namespace App\Console\Commands;
3+
4+
use Illuminate\Console\Command;
5+
use App\Models\User;
6+
use App\Mail\DailyNotifications;
7+
use Mail;
8+
9+
class SendDailyNotifications extends Command
10+
{
11+
/**
12+
* The console command name.
13+
*
14+
* @var string
15+
*/
16+
protected $signature = 'send-daily-notifications';
17+
18+
/**
19+
* The console command description.
20+
*
21+
* @var string
22+
*/
23+
protected $description = 'Processes notifications that are sitting in the database to be sent as a daily digest.';
24+
25+
/**
26+
* Create a new command instance.
27+
*/
28+
public function __construct()
29+
{
30+
parent::__construct();
31+
}
32+
33+
/**
34+
* Execute the console command.
35+
*
36+
* @return mixed
37+
*/
38+
public function fire()
39+
{
40+
User::all()->each(function ($user) {
41+
if ($user->notifications()->count() > 0) {
42+
// Make sure email exists and is verified
43+
if ($user->email && empty($user->token)) {
44+
Mail::to($user)->send(new DailyNotifications($user));
45+
$user->notifications()->delete();
46+
}
47+
}
48+
});
49+
}
50+
}

‎app/Console/Kernel.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ class Kernel extends ConsoleKernel
2626
\App\Console\Commands\DatabaseClear::class,
2727
\App\Console\Commands\DatabaseRebuild::class,
2828
\App\Console\Commands\DatabaseRestore::class,
29+
\App\Console\Commands\SendDailyNotifications::class,
2930
];
3031

3132
/**
@@ -39,6 +40,9 @@ protected function schedule(Schedule $schedule)
3940
$schedule->call(function () {
4041
LoginToken::where('expires_at', '<', Carbon::now())->delete();
4142
})->daily();
43+
44+
// Runs at midnight
45+
$schedule->call('send-daily-notifications')->daily();
4246
}
4347

4448
protected function bootstrappers()

‎app/Http/Controllers/UserController.php‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,22 @@ public function editSettingsNotifications(Requests\Edit $request, User $user)
4646
$currentNotifications = $user
4747
->notificationPreferences()
4848
->whereIn('event', array_keys($validNotifications))
49-
->pluck('event')
50-
->flip();
49+
->pluck('frequency', 'event')
50+
;
5151

5252
// Build array of notifications and their selected status
5353
$notificationPreferenceGroups = [];
5454
foreach ($validNotifications as $notificationName => $className) {
5555
if (!isset($notificationPreferenceGroups[$className::getType()])) {
5656
$notificationPreferenceGroups[$className::getType()] = [];
5757
}
58-
$notificationPreferenceGroups[$className::getType()][$className] = isset($currentNotifications[$notificationName]);
58+
$value = isset($currentNotifications[$notificationName]) ? $currentNotifications[$notificationName] : null;
59+
$notificationPreferenceGroups[$className::getType()][$className] = $value;
5960
}
6061

61-
return view('users.settings.notifications', compact('user', 'notificationPreferenceGroups'));
62+
$frequencyOptions = NotificationPreference::getValidFrequencies();
63+
64+
return view('users.settings.notifications', compact('user', 'notificationPreferenceGroups', 'frequencyOptions'));
6265
}
6366

6467
/**
@@ -104,26 +107,25 @@ public function updateSettingsNotifications(Requests\Settings\UpdateNotification
104107

105108
foreach ($validNotifications as $notificationName) {
106109
$notificationParamName = str_replace('.', '_', $notificationName);
107-
$newValue = !empty($request->input($notificationParamName));
110+
111+
$newValue = $request->input($notificationParamName);
112+
if ($newValue === '') { $newValue = null; } // Turn empty strings to their proper null value
108113

109114
// Grab this notification from the database
110115
$pref = $user
111116
->notificationPreferences()
112117
->where('event', $notificationName)
113-
->first();
114-
115-
// If we don't want that notification (and it exists), delete it
116-
if (!$newValue && !empty($pref)) {
117-
$pref->delete();
118-
} else {
119-
// If the entry doesn't already exist, create it.
120-
if (!isset($pref)) {
121-
$user->notificationPreferences()->create([
122-
'event' => $notificationName,
123-
'type' => NotificationPreference::TYPE_EMAIL,
124-
]);
125-
}
126-
// Otherwise, ignore (there was no change)
118+
->first()
119+
;
120+
121+
if (isset($pref)) {
122+
$newValue ? $pref->update([ 'frequency' => $newValue ]) : $pref->delete();
123+
} else if ($newValue !== null) {
124+
$user->notificationPreferences()->create([
125+
'event' => $notificationName,
126+
'type' => NotificationPreference::TYPE_EMAIL,
127+
'frequency' => $newValue,
128+
]);
127129
}
128130
}
129131

‎app/Listeners/ShouldSendNotification.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,11 @@ public function handle(NotificationSending $event)
6262
->where('type', NotificationPreference::TYPE_EMAIL);
6363
break;
6464
case 'database':
65-
// unsupported at the moment
66-
// $recipientNotificationPreferenceQuery
67-
// ->where('type', NotificationPreference::TYPE_IN_APP);
68-
return false;
65+
// Only notifications that are not sent immediately should go to the database
66+
$recipientNotificationPreferenceQuery
67+
->where('type', NotificationPreference::TYPE_EMAIL)
68+
->where('frequency', '!=', NotificationPreference::FREQUENCY_IMMEDIATELY)
69+
;
6970
break;
7071
case 'nexmo':
7172
// unsupported at the moment

‎app/Mail/DailyNotifications.php‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
namespace App\Mail;
4+
5+
use App\Models\User;
6+
use App\Models\NotificationPreference;
7+
use App\Notifications\Notification;
8+
use Illuminate\Bus\Queueable;
9+
use Illuminate\Mail\Mailable;
10+
use Illuminate\Queue\SerializesModels;
11+
use Illuminate\Contracts\Queue\ShouldQueue;
12+
use Carbon\Carbon;
13+
14+
class DailyNotifications extends Mailable
15+
{
16+
use Queueable, SerializesModels;
17+
18+
public $user;
19+
20+
/**
21+
* Create a new message instance.
22+
*
23+
* @return void
24+
*/
25+
public function __construct(User $user)
26+
{
27+
$this->user = $user;
28+
$this->unsubscribeMarkdown = NotificationPreference::getUnsubscribeMarkdown(null, $user);
29+
}
30+
31+
/**
32+
* Build the message.
33+
*
34+
* @return $this
35+
*/
36+
public function build()
37+
{
38+
$groupedAndFormattedNotifications = Notification::groupAndFormatNotifications($this->user->notifications);
39+
40+
return $this->subject(
41+
trans(
42+
'messages.notifications.frequencies.' . NotificationPreference::FREQUENCY_DAILY . '.subject',
43+
['dateStr' => Carbon::now()->toFormattedDateString()]
44+
)
45+
)->markdown('emails.daily_notifications', [
46+
'groupedAndFormattedNotifications' => $groupedAndFormattedNotifications,
47+
'unsubscribeMarkdown' => $this->unsubscribeMarkdown,
48+
]);
49+
}
50+
}

‎app/Models/NotificationPreference.php‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,13 @@
1010
class NotificationPreference extends Model
1111
{
1212
const TYPE_EMAIL = "email";
13-
const TYPE_TEXT = "text";
13+
14+
const FREQUENCY_IMMEDIATELY = 'immediately';
15+
const FREQUENCY_DAILY = 'daily';
16+
const FREQUENCY_NEVER = null;
1417

1518
protected $table = 'notification_preferences';
16-
protected $fillable = ['event', 'type', 'user_id', 'sponsor_id'];
19+
protected $fillable = ['event', 'type', 'frequency', 'user_id', 'sponsor_id'];
1720
public $timestamps = false;
1821

1922
public function sponsor()
@@ -87,6 +90,20 @@ public static function getUserNotifications()
8790
return static::buildNotificationsFromEventNames($validNotifications);
8891
}
8992

93+
/**
94+
* Return an array of valid notification preference frequencies.
95+
*
96+
* @return array
97+
*/
98+
public static function getValidFrequencies()
99+
{
100+
return [
101+
static::FREQUENCY_IMMEDIATELY,
102+
static::FREQUENCY_DAILY,
103+
static::FREQUENCY_NEVER,
104+
];
105+
}
106+
90107
protected static function buildNotificationsFromEventNames($names)
91108
{
92109
$ret = [];
@@ -205,19 +222,26 @@ public static function setDefaultPreferences(User $user)
205222

206223
public static function getUnsubscribeMarkdown($notification, $notifiable)
207224
{
225+
$markdown = '';
226+
208227
$token = $notifiable->loginTokens()->create([]);
209228
$params = [
210229
'user' => $notifiable,
211230
'login_token' => $token->token,
212231
'login_email' => $notifiable->email,
213232
];
214233

215-
$specificLink = route('users.settings.notifications.edit', $params + [
216-
'notification' => $notification::getName(),
217-
]);
234+
if ($notification) {
235+
$specificLink = route('users.settings.notifications.edit', $params + [
236+
'notification' => $notification::getName(),
237+
]);
238+
239+
$markdown = trans('messages.notifications.unsubscribe_specific', compact('specificLink')) . ' ';
240+
}
218241

219242
$allLink = route('users.settings.notifications.edit', $params);
243+
$markdown .= trans('messages.notifications.unsubscribe_all', compact('allLink'));
220244

221-
return trans('messages.notifications.unsubscribe', compact('specificLink', 'allLink'));
245+
return $markdown;
222246
}
223247
}

‎app/Notifications/AddedToSponsor.php‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,12 @@ public function __construct(SponsorMember $sponsorMember, User $instigator)
2121
{
2222
parent::__construct($instigator);
2323
$this->sponsorMember = $sponsorMember;
24-
}
25-
26-
/**
27-
* Get the notification's delivery channels.
28-
*
29-
* @param mixed $notifiable
30-
* @return array
31-
*/
32-
public function via($notifiable)
33-
{
34-
return ['mail'];
24+
$this->actionUrl = route('sponsors.documents.index', $sponsorMember->sponsor);
25+
$this->subjectText = trans(static::baseMessageLocation().'.added_to_sponsor', [
26+
'name' => $this->instigator->getDisplayName(),
27+
'sponsor' => $this->sponsorMember->sponsor->display_name,
28+
'role' => $this->sponsorMember->role,
29+
]);
3530
}
3631

3732
/**
@@ -42,15 +37,10 @@ public function via($notifiable)
4237
*/
4338
public function toMail($notifiable)
4439
{
45-
$url = route('sponsors.documents.index', $this->sponsorMember->sponsor);
4640

4741
return (new MailMessage($this, $notifiable))
48-
->subject(trans(static::baseMessageLocation().'.added_to_sponsor', [
49-
'name' => $this->instigator->getDisplayName(),
50-
'sponsor' => $this->sponsorMember->sponsor->display_name,
51-
'role' => $this->sponsorMember->role,
52-
]))
53-
->action(trans('messages.notifications.see_sponsor'), $url)
42+
->subject($this->subjectText)
43+
->action(trans('messages.notifications.see_sponsor'), $this->actionUrl)
5444
;
5545
}
5646

@@ -63,6 +53,7 @@ public function toMail($notifiable)
6353
public function toArray($notifiable)
6454
{
6555
return [
56+
'line' => $this->toLine(),
6657
'name' => static::getName(),
6758
'sponsor_member_id' => $this->sponsorMember->id,
6859
'instigator_id' => $this->instigator->id,

‎app/Notifications/CommentCreatedOnSponsoredDocument.php‎

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ class CommentCreatedOnSponsoredDocument extends Notification implements ShouldQu
1212
use Queueable;
1313

1414
public $comment;
15+
public $commentType;
1516

1617
/**
1718
* Create a new notification instance.
@@ -21,17 +22,19 @@ class CommentCreatedOnSponsoredDocument extends Notification implements ShouldQu
2122
public function __construct(Annotation $comment)
2223
{
2324
$this->comment = $comment;
24-
}
25+
$this->actionUrl = $comment->getLink();
2526

26-
/**
27-
* Get the notification's delivery channels.
28-
*
29-
* @param mixed $notifiable
30-
* @return array
31-
*/
32-
public function via($notifiable)
33-
{
34-
return ['mail'];
27+
if ($this->comment->isNote()) {
28+
$this->commentType = trans('messages.notifications.comment_type_note');
29+
} else {
30+
$this->commentType = trans('messages.notifications.comment_type_comment');
31+
}
32+
33+
$this->subjectText = trans(static::baseMessageLocation().'.subject', [
34+
'name' => $this->comment->user->getDisplayName(),
35+
'comment_type' => $this->commentType,
36+
'document' => $this->comment->rootAnnotatable->title,
37+
]);
3538
}
3639

3740
/**
@@ -42,21 +45,9 @@ public function via($notifiable)
4245
*/
4346
public function toMail($notifiable)
4447
{
45-
if ($this->comment->isNote()) {
46-
$commentType = trans('messages.notifications.comment_type_note');
47-
} else {
48-
$commentType = trans('messages.notifications.comment_type_comment');
49-
}
50-
51-
$url = $this->comment->getLink();
52-
5348
return (new MailMessage($this, $notifiable))
54-
->subject(trans(static::baseMessageLocation().'.subject', [
55-
'name' => $this->comment->user->getDisplayName(),
56-
'comment_type' => $commentType,
57-
'document' => $this->comment->rootAnnotatable->title,
58-
]))
59-
->action(trans('messages.notifications.see_comment', ['comment_type' => $commentType]), $url)
49+
->subject($this->subjectText)
50+
->action(trans('messages.notifications.see_comment', ['comment_type' => $this->commentType]), $this->actionUrl)
6051
;
6152
}
6253

@@ -69,6 +60,7 @@ public function toMail($notifiable)
6960
public function toArray($notifiable)
7061
{
7162
return [
63+
'line' => $this->toLine(),
7264
'name' => static::getName(),
7365
'comment_id' => $this->comment->id,
7466
];

0 commit comments

Comments
 (0)