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

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

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

diff --git a/app/assets/javascripts/discourse/app/components/user-notifications-large.js b/app/assets/javascripts/discourse/app/components/user-notifications-large.js
index 5f50878..a814122 100644
--- a/app/assets/javascripts/discourse/app/components/user-notifications-large.js
+++ b/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,
-      filter: this.filter
+      notifications: this.notifications
     });
 
     this.queueRerender();
diff --git a/app/assets/javascripts/discourse/app/controllers/user-notifications.js b/app/assets/javascripts/discourse/app/controllers/user-notifications.js
index c0761e7..6f6012c 100644
--- a/app/assets/javascripts/discourse/app/controllers/user-notifications.js
+++ b/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")
-  hasFilteredNotifications(length, filter) {
-    if (filter === "read") {
-      return this.model.filterBy("read", true).length > 0;
-    } else if (filter === "unread") {
-      return this.model.filterBy("read", false).length > 0;
-    }
+  @discourseComputed("model.content.length")
+  hasFilteredNotifications(length) {
     return length > 0;
   },
 
@@ -41,10 +37,6 @@ export default Controller.extend({
 
     loadMore() {
       this.model.loadMore();
-    },
-
-    filterNotifications(value) {
-      this.set("filter", value);
     }
   }
 });
diff --git a/app/assets/javascripts/discourse/app/routes/user-notifications.js b/app/assets/javascripts/discourse/app/routes/user-notifications.js
index 3feb837..040cacb 100644
--- a/app/assets/javascripts/discourse/app/routes/user-notifications.js
+++ b/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
+      });
     }
   },
 
diff --git a/app/assets/javascripts/discourse/app/templates/user/notifications-index.hbs b/app/assets/javascripts/discourse/app/templates/user/notifications-index.hbs
index b2c9185..14638f5 100644
--- a/app/assets/javascripts/discourse/app/templates/user/notifications-index.hbs
+++ b/app/assets/javascripts/discourse/app/templates/user/notifications-index.hbs
@@ -8,8 +8,9 @@
   </div>
 {{/if}}
 
-{{notifications-filter value=filter onChange=(action "filterNotifications")}}
-<span class="user-notifications-filter-separator"></span>
+<div class="user-notifications-filter-select-kit">
+  {{notifications-filter value=filter onChange=(action (mut filter))}}
+</div>
 
 {{#if hasFilteredNotifications}}
   {{user-notifications-large notifications=model filter=filter}}
diff --git a/app/assets/javascripts/discourse/app/widgets/user-notifications-large.js b/app/assets/javascripts/discourse/app/widgets/user-notifications-large.js
index 53a4d3a..76bf1da 100644
--- a/app/assets/javascripts/discourse/app/widgets/user-notifications-large.js
+++ b/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);
diff --git a/app/assets/stylesheets/common/base/user.scss b/app/assets/stylesheets/common/base/user.scss
index 4d5e9f4..c42df5e 100644
--- a/app/assets/stylesheets/common/base/user.scss
+++ b/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;
   }
 }
 
diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb
index a928b49..1d28fb4 100644
--- a/app/controllers/notifications_controller.rb
+++ b/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
diff --git a/spec/requests/notifications_controller_spec.rb b/spec/requests/notifications_controller_spec.rb
index 4270afb..495ae5a 100644
--- a/spec/requests/notifications_controller_spec.rb
+++ b/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

[... diff too long, it was truncated ...]

GitHub sha: 04d76933

This commit appears in #10152 which was approved by davidtaylorhq and ZogStriP. It was merged by davidtaylorhq.