allowed inbox notifications to be filtered by status

Author: Oaphi

app/assets/stylesheets/notifications.scss

@@ -1,5 +1,10 @@
 @import 'variables';
 
+.notifications-list-header {
+  display: flex;
+  justify-content: space-between;
+}
+
 .notification {
   border-top: 1px solid $muted-graphic;
   padding: 0.5em;

app/controllers/notifications_controller.rb

@@ -10,6 +10,10 @@ def index
                                  .paginate(page: params[:page], per_page: 100)
                                  .order(Arel.sql('is_read ASC, created_at DESC'))
 
+    if params[:status].present?
+      @notifications = @notifications.where(is_read: params[:status] == 'read')
+    end
+
     if stale?(@notifications)
       respond_to do |format|
         format.html { render :index, layout: 'without_sidebar' }

app/models/notification.rb

@@ -3,4 +3,16 @@ class Notification < ApplicationRecord
   belongs_to :user
 
   delegate :name, to: :community, prefix: true
+
+  # Is the notification marked as read?
+  # @return [Boolean] check result
+  def read?
+    is_read
+  end
+
+  # Is the notification not marked as read? The inverse of +read?+
+  # @return [Boolean] check result
+  def unread?
+    !read?
+  end
 end

app/views/notifications/index.html.erb

@@ -1,8 +1,23 @@
 <%= render 'users/tabs', user: current_user %>
 
+<%
+  status = params[:status].presence || 'all'
+%>
+
 <h1>Your Inbox</h1>
 <p>You'll find all your inbox messages here, as far back as your account goes.</p>
 
+<div class="notifications-list-header has-margin-bottom-2">
+  <div class="button-list is-gutterless">
+    <%= link_to 'All', notifications_path,
+                class: "button is-muted is-outlined #{'is-active' if status == 'all'}" %>
+    <%= link_to 'Read', query_url(status: 'read'),
+                class: "button is-muted is-outlined #{'is-active' if status == 'read'}" %>
+    <%= link_to 'Unread', query_url(status: 'unread'),
+                class: "button is-muted is-outlined #{'is-active' if status == 'unread'}" %>
+  </div>
+</div>
+
 <% @notifications.each do |notif| %>
   <div class="notification">
     <div class="form-caption"><%= CGI.unescape_html(notif.created_at.strftime('%b&nbsp;%-d,&nbsp;%Y&nbsp;at&nbsp;%H:%M')).html_safe %></div>

test/controllers/notifications_controller_test.rb

@@ -7,7 +7,7 @@ class NotificationsControllerTest < ActionController::TestCase
     sign_in users(:standard_user)
 
     [:json, :html].each do |format|
-      try_index(format: format)
+      try_notifications(format: format)
 
       assert_response(:success)
 
@@ -27,9 +27,30 @@ class NotificationsControllerTest < ActionController::TestCase
     end
   end
 
+  test ':index should correctly filter notifications' do
+    sign_in users(:standard_user)
+
+    ['any', 'read', 'unread'].each do |status|
+      try_notifications(format: :html, status: status)
+
+      assert_response(:success)
+
+      @notifications = assigns(:notifications)
+      assert @notifications&.any?, "Expected at least one #{status} notification"
+
+      if status == 'read'
+        assert @notifications.none?(&:unread?)
+      elsif status == 'unread'
+        assert @notifications.none?(&:read?)
+      end
+    end
+  end
+
   test 'should mark notification as read' do
     sign_in users(:standard_user)
-    post :read, params: { id: notifications(:one).id, format: :json }
+
+    post :read, params: { id: notifications(:unread).id, format: :json }
+
     assert_not_nil assigns(:notification)
     assert_equal true, assigns(:notification).is_read
     assert_equal 'success', JSON.parse(response.body)['status']
@@ -38,7 +59,9 @@ class NotificationsControllerTest < ActionController::TestCase
 
   test 'should mark all notifications as read' do
     sign_in users(:standard_user)
+
     post :read_all, params: { format: :json }
+
     assert_not_nil assigns(:notifications)
     assigns(:notifications).each do |notification|
       assert_equal true, notification.is_read
@@ -49,19 +72,19 @@ class NotificationsControllerTest < ActionController::TestCase
 
   test 'should prevent users marking others notifications read' do
     sign_in users(:editor)
-    post :read, params: { id: notifications(:one).id, format: :json }
+    post :read, params: { id: notifications(:unread).id, format: :json }
     assert_response(:forbidden)
   end
 
   test 'should require authentication to get index' do
     sign_out :user
-    get :index, params: { format: :json }
+    try_notifications
     assert_response(:unauthorized) # Devise seems to respond 401 for JSON requests.
   end
 
   private
 
-  def try_index(format: :json)
-    get :index, params: { format: format }
+  def try_notifications(**opts)
+    get :index, params: { format: :json }.merge(opts)
   end
 end

test/fixtures/notifications.yml

@@ -1,11 +1,13 @@
-one:
-  content: ABCDEF GHIJKL MNOPQR STUVWX YZ
+read:
+  content: This is a test notification that is marked as read
   link: /
   user: standard_user
   community: sample
+  is_read: true
 
-two:
-  content: ABCDEF GHIJKL MNOPQR STUVWX
+unread:
+  content: This is a test notification that is NOT marked as read
   link: /
   user: standard_user
   community: sample
+  is_read: false