FIX: Filter read/unread notifications on the server side (#10152)

https://meta.discourse.org/t/notifications-unread-only-filter/37621/32

app/assets/javascripts/discourse/app/components/user-notifications-large.js

@@ -4,20 +4,18 @@ import { observes } from "discourse-common/utils/decorators";
 export default MountWidget.extend({
   widget: "user-notifications-large",
   notifications: null,
-  filter: null,
   args: null,
 
   init() {
     this._super(...arguments);
 
-    this.args = { notifications: this.notifications, filter: this.filter };
+    this.args = { notifications: this.notifications };
   },
 
-  @observes("notifications.length", "notifications.@each.read", "filter")
+  @observes("notifications.length", "notifications.@each.read")
   _triggerRefresh() {
     this.set("args", {
-      notifications: this.notifications,
+      notifications: this.notifications
-      filter: this.filter
     });
 
     this.queueRerender();

app/assets/javascripts/discourse/app/controllers/user-notifications.js

@@ -6,6 +6,7 @@ import { inject as service } from "@ember/service";
 
 export default Controller.extend({
   application: controller(),
+  queryParams: ["filter"],
   router: service(),
   currentPath: readOnly("router._router.currentPath"),
   filter: "all",
@@ -15,13 +16,8 @@ export default Controller.extend({
     this.set("application.showFooter", !this.get("model.canLoadMore"));
   },
 
-  @discourseComputed("model.content.length", "filter")
+  @discourseComputed("model.content.length")
-  hasFilteredNotifications(length, filter) {
+  hasFilteredNotifications(length) {
-    if (filter === "read") {
-      return this.model.filterBy("read", true).length > 0;
-    } else if (filter === "unread") {
-      return this.model.filterBy("read", false).length > 0;
-    }
     return length > 0;
   },
 
@@ -41,10 +37,6 @@ export default Controller.extend({
 
     loadMore() {
       this.model.loadMore();
-    },
-
-    filterNotifications(value) {
-      this.set("filter", value);
     }
   }
 });

app/assets/javascripts/discourse/app/routes/user-notifications.js

@@ -2,6 +2,9 @@ import DiscourseRoute from "discourse/routes/discourse";
 import ViewingActionType from "discourse/mixins/viewing-action-type";
 
 export default DiscourseRoute.extend(ViewingActionType, {
+  controllerName: "user-notifications",
+  queryParams: { filter: { refreshModel: true } },
+
   renderTemplate() {
     this.render("user/notifications");
   },
@@ -13,14 +16,17 @@ export default DiscourseRoute.extend(ViewingActionType, {
     }
   },
 
-  model() {
+  model(params) {
     const username = this.modelFor("user").get("username");
 
     if (
       this.get("currentUser.username") === username ||
       this.get("currentUser.admin")
     ) {
-      return this.store.find("notification", { username });
+      return this.store.find("notification", {
+        username: username,
+        filter: params.filter
+      });
     }
   },
 

app/assets/javascripts/discourse/app/templates/user/notifications-index.hbs

@@ -8,8 +8,9 @@
   </div>
 {{/if}}
 
-{{notifications-filter value=filter onChange=(action "filterNotifications")}}
+<div class="user-notifications-filter-select-kit">
-<span class="user-notifications-filter-separator"></span>
+  {{notifications-filter value=filter onChange=(action (mut filter))}}
+</div>
 
 {{#if hasFilteredNotifications}}
   {{user-notifications-large notifications=model filter=filter}}

app/assets/javascripts/discourse/app/widgets/user-notifications-large.js

@@ -30,13 +30,9 @@ createWidget("large-notification-item", {
 
 export default createWidget("user-notifications-large", {
   html(attrs) {
-    let notifications = attrs.notifications;
+    const notifications = attrs.notifications;
     const username = notifications.findArgs.username;
-    if (attrs.filter === "read") {
+
-      notifications = notifications.filterBy("read", true);
-    } else if (attrs.filter === "unread") {
-      notifications = notifications.filterBy("read", false);
-    }
     return notifications.map(n => {
       n.username = username;
       return this.attach("large-notification-item", n);

app/assets/stylesheets/common/base/user.scss

@@ -65,10 +65,10 @@
     color: $love;
   }
 
-  .user-notifications-filter-separator {
+  .user-notifications-filter-select-kit {
     display: block;
     width: 100%;
-    border: 0.5px solid $primary-low;
+    border-bottom: 0.5px solid $primary-low;
   }
 }
 

app/controllers/notifications_controller.rb

@@ -44,12 +44,16 @@ class NotificationsController < ApplicationController
         .includes(:topic)
         .order(created_at: :desc)
 
+      notifications = notifications.where(read: true) if params[:filter] == "read"
+
+      notifications = notifications.where(read: false) if params[:filter] == "unread"
+
       total_rows = notifications.dup.count
       notifications = notifications.offset(offset).limit(60)
       render_json_dump(notifications: serialize_data(notifications, NotificationSerializer),
                        total_rows_notifications: total_rows,
                        seen_notification_id: user.seen_notification_id,
-                       load_more_notifications: notifications_path(username: user.username, offset: offset + 60))
+                       load_more_notifications: notifications_path(username: user.username, offset: offset + 60, filter: params[:filter]))
     end
 
   end

spec/requests/notifications_controller_spec.rb

@@ -74,6 +74,24 @@ describe NotificationsController do
           Discourse.clear_redis_readonly!
         end
 
+        it "get notifications with all filters" do
+          notification = Fabricate(:notification, user: user)
+          notification2 = Fabricate(:notification, user: user)
+          put "/notifications/mark-read.json", params: { id: notification.id }
+          expect(response.status).to eq(200)
+
+          get "/notifications.json"
+          expect(JSON.parse(response.body)['notifications'].length).to be >= 2
+
+          get "/notifications.json", params: { filter: "read" }
+          expect(JSON.parse(response.body)['notifications'].length).to be >= 1
+          expect(JSON.parse(response.body)['notifications'][0]['read']).to eq(true)
+
+          get "/notifications.json", params: { filter: "unread" }
+          expect(JSON.parse(response.body)['notifications'].length).to be >= 1
+          expect(JSON.parse(response.body)['notifications'][0]['read']).to eq(false)
+        end
+
         context 'when username params is not valid' do
           it 'should raise the right error' do
             get "/notifications.json", params: { username: 'somedude' }

test/javascripts/acceptance/notifications-filter-test.js

@@ -0,0 +1,32 @@
+import { acceptance } from "helpers/qunit-helpers";
+import selectKit from "helpers/select-kit-helper";
+
+acceptance("NotificationsFilter", {
+  loggedIn: true
+});
+
+test("Notifications filter true", async assert => {
+  await visit("/u/eviltrout/notifications");
+
+  assert.ok(find(".large-notification").length >= 0);
+});
+
+test("Notifications filter read", async assert => {
+  await visit("/u/eviltrout/notifications");
+
+  const dropdown = selectKit(".notifications-filter");
+  await dropdown.expand();
+  await dropdown.selectRowByValue("read");
+
+  assert.ok(find(".large-notification").length >= 0);
+});
+
+test("Notifications filter unread", async assert => {
+  await visit("/u/eviltrout/notifications");
+
+  const dropdown = selectKit(".notifications-filter");
+  await dropdown.expand();
+  await dropdown.selectRowByValue("unread");
+
+  assert.ok(find(".large-notification").length >= 0);
+});