feat(subscriptions): add option to send notifications when not caught up (#3503)

extensions/subscriptions/extend.php

@@ -61,4 +61,7 @@ return [
 
     (new Extend\SimpleFlarumSearch(DiscussionSearcher::class))
         ->addGambit(SubscriptionFilterGambit::class),
+
+    (new Extend\User())
+        ->registerPreference('flarum-subscriptions.notify_for_all_posts', 'boolval', false),
 ];

extensions/subscriptions/js/package.json

@@ -4,11 +4,12 @@
     "version": "0.0.0",
     "prettier": "@flarum/prettier-config",
     "devDependencies": {
-        "prettier": "^2.5.1",
+        "@flarum/prettier-config": "^1.0.0",
         "flarum-webpack-config": "^2.0.0",
-        "webpack": "^5.65.0",
+        "prettier": "^2.7.1",
-        "webpack-cli": "^4.9.1",
+        "webpack": "^5.73.0",
-        "@flarum/prettier-config": "^1.0.0"
+        "webpack-cli": "^4.10.0",
+        "flarum-tsconfig": "^1.0.2"
     },
     "scripts": {
         "dev": "webpack --mode development --watch",

extensions/subscriptions/js/src/forum/addSubscriptionSettings.tsx

@@ -4,7 +4,7 @@ import SettingsPage from 'flarum/forum/components/SettingsPage';
 import Switch from 'flarum/common/components/Switch';
 
 export default function () {
-  extend(SettingsPage.prototype, 'notificationsItems', function (items) {
+  extend(SettingsPage.prototype, 'notificationsItems', function (this: SettingsPage, items) {
     items.add(
       'followAfterReply',
       Switch.component(
@@ -23,5 +23,18 @@ export default function () {
         app.translator.trans('flarum-subscriptions.forum.settings.follow_after_reply_label')
       )
     );
+
+    items.add(
+      'notifyForAllPosts',
+      <Switch
+        id="flarum_subscriptions__notify_for_all_posts"
+        state={!!this.user!.preferences()?.['flarum-subscriptions.notify_for_all_posts']}
+        onchange={(val: boolean) => {
+          this.user!.savePreferences({ 'flarum-subscriptions.notify_for_all_posts': val });
+        }}
+      >
+        {app.translator.trans('flarum-subscriptions.forum.settings.notify_for_all_posts_label')}
+      </Switch>
+    );
   });
 }

extensions/subscriptions/js/tsconfig.json

@@ -0,0 +1,17 @@
+{
+  // Use Flarum's tsconfig as a starting point
+  "extends": "flarum-tsconfig",
+  // This will match all .ts, .tsx, .d.ts, .js, .jsx files in your `src` folder
+  // and also tells your Typescript server to read core's global typings for
+  // access to `dayjs` and `$` in the global namespace.
+  "include": ["src/**/*", "../vendor/*/*/js/dist-typings/@types/**/*", "@types/**/*"],
+  "compilerOptions": {
+    // This will output typings to `dist-typings`
+    "declarationDir": "./dist-typings",
+    "baseUrl": ".",
+    "paths": {
+      "flarum/*": ["../vendor/flarum/core/js/dist-typings/*"],
+      "@flarum/core/*": ["../vendor/flarum/core/js/dist-typings/*"]
+    }
+  }
+}

extensions/subscriptions/locale/en.yml

@@ -1,12 +1,10 @@
 flarum-subscriptions:
-
   ##
   # UNIQUE KEYS - The following keys are used in only one location each.
   ##
 
   # Translations in this namespace are used by the forum user interface.
   forum:
-
     # These translations are displayed as tooltips for discussion badges.
     badge:
       following_tooltip: => flarum-subscriptions.ref.following
@@ -33,6 +31,7 @@ flarum-subscriptions:
     # These translations are used in the Settings page.
     settings:
       follow_after_reply_label: Automatically follow discussions that I reply to
+      notify_for_all_posts_label: Notify about every new post instead of only the last in a discussion
       notify_new_post_label: Someone posts in a discussion I'm following
 
     # These translations are used in the subscription menu displayed to the right of the post stream.
@@ -49,7 +48,6 @@ flarum-subscriptions:
 
   # Translations in this namespace are used in emails sent by the forum.
   email:
-
     # These translations are used in emails sent when a post is made in a subscribed discussion
     new_post:
       subject: "[New Post] {title}"

extensions/subscriptions/src/Job/SendReplyNotification.php

@@ -11,10 +11,14 @@ namespace Flarum\Subscriptions\Job;
 
 use Flarum\Notification\NotificationSyncer;
 use Flarum\Post\Post;
+use Flarum\Settings\SettingsRepositoryInterface;
 use Flarum\Subscriptions\Notification\NewPostBlueprint;
+use Flarum\User\User;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Queue\SerializesModels;
+use Illuminate\Support\Arr;
 
 class SendReplyNotification implements ShouldQueue
 {
@@ -41,20 +45,45 @@ class SendReplyNotification implements ShouldQueue
         $this->lastPostNumber = $lastPostNumber;
     }
 
-    public function handle(NotificationSyncer $notifications)
+    public function handle(NotificationSyncer $notifications, SettingsRepositoryInterface $settings)
     {
         $post = $this->post;
         $discussion = $post->discussion;
 
-        $notify = $discussion->readers()
+        $usersToNotify = [];
+
+        $followers = $discussion->readers()
+            ->select('users.id', 'users.preferences', 'discussion_user.last_read_post_number')
             ->where('users.id', '!=', $post->user_id)
-            ->where('discussion_user.subscription', 'follow')
+            ->where('discussion_user.subscription', 'follow');
-            ->where('discussion_user.last_read_post_number', $this->lastPostNumber - 1)
+
-            ->get();
+        $followers->chunk(150, function (Collection $followers) use (&$usersToNotify) {
+            $allUnreadUsers = [];
+            $firstUnreadUsers = [];
+
+            /**
+             * @var \Flarum\User\User $user
+             */
+            foreach ($followers as $user) {
+                $notifyForAll = $user->getPreference('flarum-subscriptions.notify_for_all_posts', false);
+
+                if ($notifyForAll) {
+                    $allUnreadUsers[] = $user;
+                }
+                // Only notify if this is the next post after the user's last read post
+                // i.e., their next new post to read
+                elseif ($user->last_read_post_number === $this->lastPostNumber - 1) {
+                    $firstUnreadUsers[] = $user;
+                }
+            }
+
+            $userIdsToNotify = Arr::pluck(array_merge($allUnreadUsers, $firstUnreadUsers), 'id');
+            $usersToNotify = array_merge($usersToNotify, User::query()->whereIn('id', $userIdsToNotify)->get()->all());
+        });
 
         $notifications->sync(
             new NewPostBlueprint($post),
-            $notify->all()
+            $usersToNotify
         );
     }
 }

extensions/subscriptions/tests/integration/api/discussions/ReplyNotificationTest.php

@@ -27,49 +27,79 @@ class ReplyNotificationTest extends TestCase
         $this->prepareDatabase([
             'users' => [
                 $this->normalUser(),
+                ['id' => 3, 'username' => 'acme', 'email' => 'acme@machine.local', 'is_email_confirmed' => 1, 'preferences' => json_encode(['flarum-subscriptions.notify_for_all_posts' => true])],
+                ['id' => 4, 'username' => 'acme2', 'email' => 'acme2@machine.local', 'is_email_confirmed' => 1],
             ],
             'discussions' => [
                 ['id' => 1, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 1, 'first_post_id' => 1, 'comment_count' => 1, 'last_post_number' => 1, 'last_post_id' => 1],
                 ['id' => 2, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 1, 'first_post_id' => 2, 'comment_count' => 1, 'last_post_number' => 1, 'last_post_id' => 2],
+
+                ['id' => 33, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 1, 'first_post_id' => 33, 'comment_count' => 6, 'last_post_number' => 6, 'last_post_id' => 38],
             ],
             'posts' => [
                 ['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 1],
                 ['id' => 2, 'discussion_id' => 2, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 1],
+
+                ['id' => 33, 'discussion_id' => 33, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 1],
+                ['id' => 34, 'discussion_id' => 33, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 2],
+                ['id' => 35, 'discussion_id' => 33, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 3],
+                ['id' => 36, 'discussion_id' => 33, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 4],
+                ['id' => 37, 'discussion_id' => 33, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 5],
+                ['id' => 38, 'discussion_id' => 33, 'created_at' => Carbon::createFromDate(1975, 5, 21)->toDateTimeString(), 'user_id' => 1, 'type' => 'comment', 'content' => '<t><p>foo bar</p></t>', 'number' => 6],
             ],
             'discussion_user' => [
                 ['discussion_id' => 1, 'user_id' => 1, 'last_read_post_number' => 1, 'subscription' => 'follow'],
+                ['discussion_id' => 1, 'user_id' => 2, 'last_read_post_number' => 1, 'subscription' => 'follow'],
                 ['discussion_id' => 2, 'user_id' => 1, 'last_read_post_number' => 1, 'subscription' => 'follow'],
+
+                ['discussion_id' => 33, 'user_id' => 2, 'last_read_post_number' => 1, 'subscription' => 'follow'],
+                ['discussion_id' => 33, 'user_id' => 3, 'last_read_post_number' => 1, 'subscription' => 'follow'],
             ]
         ]);
     }
 
-    /** @test */
+    /**
-    public function replying_to_a_discussion_with_comment_post_as_last_post_sends_reply_notification()
+     * @dataProvider replyingSendsNotificationsDataProvider
+     * @test
+     */
+    public function replying_to_a_discussion_with_comment_post_as_last_post_sends_reply_notification(int $userId, int $discussionId, int $newNotificationCount)
     {
         $this->app();
 
         /** @var User $mainUser */
-        $mainUser = User::query()->find(1);
+        $mainUser = User::query()->find($userId);
 
         $this->assertEquals(0, $mainUser->getUnreadNotificationCount());
 
+        for ($i = 0; $i < 5; $i++) {
             $this->send(
                 $this->request('POST', '/api/posts', [
-                'authenticatedAs' => 2,
+                    'authenticatedAs' => 4,
                     'json' => [
                         'data' => [
                             'attributes' => [
                                 'content' => 'reply with predetermined content for automated testing - too-obscure',
                             ],
                             'relationships' => [
-                            'discussion' => ['data' => ['id' => 1]],
+                                'discussion' => ['data' => ['id' => $discussionId]],
                             ],
                         ],
                     ],
-            ])
+                ])->withAttribute('bypassThrottling', true)
             );
+        }
 
-        $this->assertEquals(1, $mainUser->getUnreadNotificationCount());
+        $this->assertEquals($newNotificationCount, $mainUser->getUnreadNotificationCount());
+    }
+
+    public function replyingSendsNotificationsDataProvider(): array
+    {
+        return [
+            'admin receives a notification when another replies to a discussion they are following and caught up to' => [1, 1, 1],
+            'user receives a notification when another replies to a discussion they are following and caught up to' => [2, 1, 1],
+
+            'user receives notification for every new post to a discussion they are following when preference is on' => [3, 33, 5],
+        ];
     }
 
     /** @test */
@@ -136,9 +166,6 @@ class ReplyNotificationTest extends TestCase
     public function deleting_last_posts_then_posting_new_one_sends_reply_notification(array $postIds)
     {
         $this->prepareDatabase([
-            'users' => [
-                ['id' => 3, 'username' => 'acme', 'email' => 'acme@machine.local', 'is_email_confirmed' => 1],
-            ],
             'discussions' => [
                 ['id' => 3, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 5, 'last_post_number' => 5, 'last_post_id' => 10],
             ],

yarn.lock

@@ -1799,9 +1799,9 @@ enhanced-resolve@^5.9.2:
     tapable "^2.2.0"
 
 enhanced-resolve@^5.9.3:
-  version "5.10.0"
+  version "5.9.3"
-  resolved "https://registry.yarnpkg.com/enhanced-resolve/-/enhanced-resolve-5.10.0.tgz#0dc579c3bb2a1032e357ac45b8f3a6f3ad4fb1e6"
+  resolved "https://registry.yarnpkg.com/enhanced-resolve/-/enhanced-resolve-5.9.3.tgz#44a342c012cbc473254af5cc6ae20ebd0aae5d88"
-  integrity sha512-T0yTFjdpldGY8PmuXXR0PyQ1ufZpEGiHVrp7zHKB7jdR4qlmZHhONVM5AQOAWXuF/w3dnHbEQVrNptJgt7F+cQ==
+  integrity sha512-Bq9VSor+kjvW3f9/MiiR4eE3XYgOl7/rS8lnSxbRbF3kS0B2r+Y9w5krBWxZgDxASVZbdYrn5wT4j/Wb0J9qow==
   dependencies:
     graceful-fs "^4.2.4"
     tapable "^2.2.0"