diff --git a/app/Commands/SaveAccountSettingsCommand.php b/app/Commands/SaveAccountSettingsCommand.php index eb2d8afa..68e1b545 100644 --- a/app/Commands/SaveAccountSettingsCommand.php +++ b/app/Commands/SaveAccountSettingsCommand.php @@ -20,6 +20,7 @@ namespace Poniverse\Ponyfm\Commands; +use DB; use Poniverse\Ponyfm\Models\Image; use Poniverse\Ponyfm\Models\User; use Gate; @@ -53,6 +54,8 @@ class SaveAccountSettingsCommand extends CommandBase */ public function execute() { + $this->_input['notifications'] = json_decode($this->_input['notifications'], true); + $rules = [ 'display_name' => 'required|min:3|max:26', 'bio' => 'textarea_length:250', @@ -62,7 +65,9 @@ class SaveAccountSettingsCommand extends CommandBase 'min:'.config('ponyfm.user_slug_minimum_length'), 'regex:/^[a-z\d-]+$/', 'is_not_reserved_slug' - ] + ], + 'notifications.*.activity_type' => 'required|exists:activity_types,activity_type', + 'notifications.*.receive_emails' => 'present|boolean', ]; if ($this->_input['uses_gravatar'] == 'true') { @@ -80,6 +85,7 @@ class SaveAccountSettingsCommand extends CommandBase return CommandResponse::fail($validator); } + $this->_user->bio = $this->_input['bio']; $this->_user->display_name = $this->_input['display_name']; $this->_user->slug = $this->_input['slug']; @@ -101,7 +107,27 @@ class SaveAccountSettingsCommand extends CommandBase } } - $this->_user->save(); + DB::transaction(function() { + $this->_user->save(); + + // Sync email subscriptions + $emailSubscriptions = $this->_user->emailSubscriptions->keyBy('activity_type'); + foreach ($this->_input['notifications'] as $notificationSetting) { + + if ( + $notificationSetting['receive_emails'] && + !$emailSubscriptions->offsetExists($notificationSetting['activity_type']) + ) { + $this->_user->emailSubscriptions()->create(['activity_type' => $notificationSetting['activity_type']]); + + } elseif ( + !$notificationSetting['receive_emails'] && + $emailSubscriptions->offsetExists($notificationSetting['activity_type']) + ) { + $emailSubscriptions->get($notificationSetting['activity_type'])->delete(); + } + } + }); return CommandResponse::succeed(); } diff --git a/app/Http/Controllers/Api/Web/AccountController.php b/app/Http/Controllers/Api/Web/AccountController.php index bfb77005..b86a85ba 100644 --- a/app/Http/Controllers/Api/Web/AccountController.php +++ b/app/Http/Controllers/Api/Web/AccountController.php @@ -20,13 +20,12 @@ namespace Poniverse\Ponyfm\Http\Controllers\Api\Web; -use Carbon\Carbon; use Poniverse\Ponyfm\Http\Controllers\ApiControllerBase; use Poniverse\Ponyfm\Commands\SaveAccountSettingsCommand; use Poniverse\Ponyfm\Models\User; use Gate; use Auth; -use Illuminate\Support\Facades\Request; +use Request; use Response; class AccountController extends ApiControllerBase @@ -71,7 +70,8 @@ class AccountController extends ApiControllerBase 'username' => $user->username, 'gravatar' => $user->gravatar ? $user->gravatar : $user->email, 'avatar_url' => !$user->uses_gravatar ? $user->getAvatarUrl() : null, - 'uses_gravatar' => $user->uses_gravatar == 1 + 'uses_gravatar' => $user->uses_gravatar == 1, + 'notifications' => $user->getNotificationSettings() ], 200); } diff --git a/app/Http/Controllers/NotificationsController.php b/app/Http/Controllers/NotificationsController.php index f93dba87..633ad653 100644 --- a/app/Http/Controllers/NotificationsController.php +++ b/app/Http/Controllers/NotificationsController.php @@ -25,13 +25,13 @@ use DB; use Poniverse\Ponyfm\Models\Email; use Poniverse\Ponyfm\Models\EmailSubscription; -// TODO: #25 - finish these endpoints and secure them properly class NotificationsController extends Controller { + /** + * @param $emailKey + * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector + */ public function getEmailClick($emailKey) { - App::abort(403, "This isn't implemented yet!"); - - $emailKey = decrypt($emailKey); /** @var Email $email */ $email = Email::findOrFail($emailKey); @@ -45,11 +45,9 @@ class NotificationsController extends Controller { } public function getEmailUnsubscribe($subscriptionKey) { - App::abort(403, "This isn't implemented yet!"); + $subscription = EmailSubscription::findOrFail($subscriptionKey); + $subscription->delete(); - $subscriptionId = decrypt($subscriptionKey); - $subscription = EmailSubscription::findOrFail($subscriptionId); - - return var_export($subscription); + return 'Unsubscribed!'; } } diff --git a/app/Mail/BaseNotification.php b/app/Mail/BaseNotification.php index c5a6cd16..ff50a252 100644 --- a/app/Mail/BaseNotification.php +++ b/app/Mail/BaseNotification.php @@ -96,8 +96,7 @@ abstract class BaseNotification extends Mailable { * @return string */ protected function generateUnsubscribeUrl() { - $subscriptionKey = encrypt($this->emailRecord->getSubscription()->id); - return route('email:unsubscribe', ['subscriptionKey' => $subscriptionKey]); + return route('email:unsubscribe', ['subscriptionKey' => $this->emailRecord->getSubscription()->id]); } /** @@ -106,8 +105,7 @@ abstract class BaseNotification extends Mailable { * @return string */ protected function generateNotificationUrl() { - $emailKey = encrypt($this->emailRecord->id); - return route('email:click', ['emailKey' => $emailKey]); + return route('email:click', ['emailKey' => $this->emailRecord->id]); } /** diff --git a/app/Models/Email.php b/app/Models/Email.php index f4b0991f..6b3c837f 100644 --- a/app/Models/Email.php +++ b/app/Models/Email.php @@ -42,6 +42,8 @@ use Poniverse\Ponyfm\Models\Notification; class Email extends Model { use UuidModelTrait; + // Non-sequential UUID's are desirable for this model. + protected $uuidVersion = 4; public function notification() { return $this->belongsTo(Notification::class, 'notification_id', 'id', 'notifications'); diff --git a/app/Models/EmailSubscription.php b/app/Models/EmailSubscription.php index 97479bee..6aff6bb3 100644 --- a/app/Models/EmailSubscription.php +++ b/app/Models/EmailSubscription.php @@ -46,6 +46,10 @@ use Poniverse\Ponyfm\Models\User; class EmailSubscription extends Model { use UuidModelTrait, SoftDeletes; + // Non-sequential UUID's are desirable for this model. + protected $uuidVersion = 4; + + protected $fillable = ['activity_type']; public function user() { return $this->belongsTo(User::class, 'user_id', 'id', 'users'); diff --git a/app/Models/User.php b/app/Models/User.php index 53e50265..7be462c2 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -420,6 +420,38 @@ class User extends Model implements AuthenticatableContract, CanResetPasswordCon ]; } + /** + * Helper method that returns a row for every type of notifiable activity. + * It's meant to be used for the notification settings screen. + * + * @return array + */ + private function emailSubscriptionsJoined() { + return DB::select(' + SELECT "subscriptions".*, "activity_types".* FROM + (SELECT * FROM "email_subscriptions" + WHERE "email_subscriptions"."deleted_at" IS NULL + AND "email_subscriptions"."user_id" = ?) as "subscriptions" + RIGHT JOIN "activity_types" + ON "subscriptions"."activity_type" = "activity_types"."activity_type" + ', [$this->id]); + } + + public function getNotificationSettings() { + $settings = []; + $emailSubscriptions = $this->emailSubscriptionsJoined(); + + foreach($emailSubscriptions as $subscription) { + $settings[] = [ + 'description' => $subscription->description, + 'activity_type' => $subscription->activity_type, + 'receive_emails' => $subscription->id !== NULL + ]; + } + + return $settings; + } + /** * Returns this model in Elasticsearch-friendly form. The array returned by * this method should match the current mapping for this model's ES type. diff --git a/database/migrations/2016_12_24_133558_UpdateActivityDescriptions.php b/database/migrations/2016_12_24_133558_UpdateActivityDescriptions.php new file mode 100644 index 00000000..2958c86a --- /dev/null +++ b/database/migrations/2016_12_24_133558_UpdateActivityDescriptions.php @@ -0,0 +1,58 @@ +. + */ + +use Illuminate\Support\Facades\Schema; +use Illuminate\Database\Schema\Blueprint; +use Illuminate\Database\Migrations\Migration; + +class UpdateActivityDescriptions extends Migration +{ + /** + * Run the migrations. + * + * @return void + */ + public function up() + { + DB::table('activity_types')->where('activity_type', 1)->update(['description' => 'Updates from the Pony.fm team']); + DB::table('activity_types')->where('activity_type', 2)->update(['description' => 'Someone you follow publishes a track']); + DB::table('activity_types')->where('activity_type', 3)->update(['description' => 'Someone you follow publishes an album']); + DB::table('activity_types')->where('activity_type', 4)->update(['description' => 'Someone you follow creates a playlist']); + DB::table('activity_types')->where('activity_type', 5)->update(['description' => 'You get a new follower']); + DB::table('activity_types')->where('activity_type', 6)->update(['description' => 'Someone leaves you a comment']); + DB::table('activity_types')->where('activity_type', 7)->update(['description' => 'Something of yours is favourited']); + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + DB::table('activity_types')->where('activity_type', 1)->update(['description' => 'news']); + DB::table('activity_types')->where('activity_type', 2)->update(['description' => 'followee published a track']); + DB::table('activity_types')->where('activity_type', 3)->update(['description' => 'followee published an album']); + DB::table('activity_types')->where('activity_type', 4)->update(['description' => 'followee published a playlist']); + DB::table('activity_types')->where('activity_type', 5)->update(['description' => 'new follower']); + DB::table('activity_types')->where('activity_type', 6)->update(['description' => 'new comment']); + DB::table('activity_types')->where('activity_type', 7)->update(['description' => 'new favourite']); + } +} diff --git a/documentation/notifications.md b/documentation/notifications.md index 9c988634..d188fca5 100644 --- a/documentation/notifications.md +++ b/documentation/notifications.md @@ -1,9 +1,8 @@ Developing notifications for Pony.fm ==================================== -Pony.fm's notification system is designed around "drivers" for various -notification delivery methods. The types of notification one can receive -are defined in the +Pony.fm's notification system is designed to support various notification +delivery methods. The types of notification one can receive are defined in the [`NotificationHandler`](app/Contracts/NotificationHandler.php) interface, which is implemented by every class that needs to know about the various notification types. @@ -60,20 +59,15 @@ Adding new notification types [`Activity`](../app/Models/Activity.php) model. -Adding new notification drivers -------------------------------- +Adding new notification delivery methods +---------------------------------------- -1. Create a new class for the driver that implements the - [`NotificationHandler`](../app/Contracts/NotificationHandler.php) - interface. +1. Implement a method for sending notifications via the new delivery method in + the [`PonyfmDriver`](../app/Library/Notifications/PonyfmDriver.php) class. + Use how email delivery is implemented as a guide. -2. Make each method from the above interface send the corresponding type - of notification to everyone who is to receive it via that driver. - Implement UI and API integrations as needed. - -3. Modify the - [`RecipientFinder`](../app/Library/Notifications/RecipientFinder.php) - class to build recipient lists for the new driver. +2. Add UI for subscribing and unsubscribing to the delivery method to the + [`account settings area`](../public/templates/account/settings.html). Architectural notes @@ -86,14 +80,19 @@ notifications asynchronously. To that end, the [`NotificationManager`](../app/Library/Notifications/NotificationManager.php) class is a thin wrapper around the `SendNotifications` job. The job -calls the notification drivers asynchronously to actually send the -notifications. This job should run on a dedicated queue in production. +calls the notification logic asynchronously to actually send notifications. This +job should run on a dedicated queue in production. The [`NotificationHandler`](../app/Contracts/NotificationHandler.php) -interface is key to maintaining type safety - it ensures that drivers -and `NotificationManager` all support every type of notification. All -classes that have logic specific to a notification type implement this -interface to ensure that all notification types are handled. +interface is key to maintaining type safety - it ensures that many classes +associated with notifications all support every type of notification. Classes +that have logic specific to a notification type implement this interface to +ensure that all notification types are handled. + +Furthermore, the `activity_types` table is used to provide referential data +integrity in the database - all notifications are linked to an activity record, +and each activity record must correspond to a valid activity type. This table is +also used for validation of users' subscription preferences. There's one exception to the use of `NotificationHandler` - the [`Activity`](../app/Models/Activity.php) model. The logic for mapping the @@ -112,7 +111,7 @@ interface here would have made this logic a lot more obtuse. 3. An `Activity` record is created for the action. 4. A `Notification` record is created for every user who is to receive a - notification about that activity. These records act as Pony.fm's on-site + notification about that activity. These records double as Pony.fm's on-site notifications and cannot be disabled. 5. Depending on subscription preferences, push and email notifications will be diff --git a/public/templates/account/settings.html b/public/templates/account/settings.html index e345027e..fb73c5a9 100644 --- a/public/templates/account/settings.html +++ b/public/templates/account/settings.html @@ -7,38 +7,59 @@ -
-
- - -
{{errors.display_name}}
-
- -
- - -
{{errors.slug}}
-
- -
- -
- -
- - -
{{errors.description}}
-
- -
- -
- +
+
+
+ + +
{{errors.display_name}}
+
+ +
+ + +
{{errors.slug}}
+
+ +
+ +
+ +
+ + +
{{errors.description}}
+
+ +
+ +
+ +
+ +
{{errors.avatar}}
+
{{errors.gravatar}}
- -
{{errors.avatar}}
-
{{errors.gravatar}}
+
+

Notification settings

+

On-site notifications are always on. That way, you can always see + what you've missed whenever you log on to Pony.fm!

+ + + + + + + + + + + + +
Email me!Give me a push notification!
Coming soon!
+
+
diff --git a/resources/assets/scripts/app/controllers/account-settings.coffee b/resources/assets/scripts/app/controllers/account-settings.coffee index 24718fba..7049c59f 100644 --- a/resources/assets/scripts/app/controllers/account-settings.coffee +++ b/resources/assets/scripts/app/controllers/account-settings.coffee @@ -81,6 +81,8 @@ module.exports = angular.module('ponyfm').controller "account-settings", [ return if value == null if typeof(value) == 'object' formData.append name, value, value.name + else if name == 'notifications' + formData.append name, JSON.stringify(value) else formData.append name, value diff --git a/routes/web.php b/routes/web.php index 5dd1b672..5b3fe5ed 100644 --- a/routes/web.php +++ b/routes/web.php @@ -78,8 +78,10 @@ Route::get('p{id}/dl.{extension}', 'PlaylistsController@getDownload'); Route::get('notifications', 'AccountController@getNotifications'); -Route::get('notifications/email/unsubscribe/{subscriptionKey}', 'NotificationsController@getEmailUnsubscribe')->name('email:unsubscribe'); -Route::get('notifications/email/click/{emailKey}', 'NotificationsController@getEmailClick')->name('email:click'); +Route::group(['prefix' => 'notifications/email'], function() { + Route::get('/unsubscribe/{subscriptionKey}', 'NotificationsController@getEmailUnsubscribe')->name('email:unsubscribe'); + Route::get('/click/{emailKey}', 'NotificationsController@getEmailClick')->name('email:click'); +}); Route::get('oembed', 'TracksController@getOembed');