Extend notifications to be marked as read and unread

app/controllers/comments_controller.rb

@@ -86,7 +86,7 @@ class CommentsController < ApplicationController
       notifiable = comment.reply? ? comment.parent : comment.commentable
       notifiable_author_id = notifiable.try(:author_id)
       if notifiable_author_id.present? && notifiable_author_id != comment.author_id
-        Notification.add(notifiable.author_id, notifiable)
+        Notification.add(notifiable.author, notifiable)
       end
     end
 

app/controllers/notifications_controller.rb

@@ -2,28 +2,40 @@ class NotificationsController < ApplicationController
   include CustomUrlsHelper
 
   before_action :authenticate_user!
-  after_action :mark_as_read, only: :show
   skip_authorization_check
 
+  respond_to :html, :js
+
   def index
-    @notifications = current_user.notifications.unread.recent.for_render
+    @notifications = current_user.notifications.unread
   end
 
   def show
     @notification = current_user.notifications.find(params[:id])
+    @notification.mark_as_read
     redirect_to linkable_resource_path(@notification)
   end
 
+  def read
+    @notifications = current_user.notifications.read
+  end
+
   def mark_all_as_read
-    current_user.notifications.each { |notification| notification.mark_as_read }
+    current_user.notifications.unread.each { |notification| notification.mark_as_read }
     redirect_to notifications_path
   end
 
-  private
+  def mark_as_read
+    @notification = current_user.notifications.find(params[:id])
+    @notification.mark_as_read
+  end
 
-    def mark_as_read
-      @notification.mark_as_read
-    end
+  def mark_as_unread
+    @notification = current_user.notifications.find(params[:id])
+    @notification.mark_as_unread
+  end
+
+  private
 
     def linkable_resource_path(notification)
       case notification.linkable_resource.class.name

app/controllers/proposal_notifications_controller.rb

@@ -12,7 +12,7 @@ class ProposalNotificationsController < ApplicationController
     @proposal = Proposal.find(proposal_notification_params[:proposal_id])
     if @notification.save
       @proposal.users_to_notify.each do |user|
-        Notification.add(user.id, @notification)
+        Notification.add(user, @notification)
       end
       redirect_to @notification, notice: I18n.t("flash.actions.create.proposal_notification")
     else

app/models/notification.rb

@@ -3,30 +3,48 @@ class Notification < ActiveRecord::Base
   belongs_to :user, counter_cache: true
   belongs_to :notifiable, polymorphic: true
 
-  scope :unread,      -> { all }
-  scope :recent,      -> { order(id: :desc) }
+  validates :user, presence: true
+
+  scope :read,        -> { where.not(read_at: nil).recent.for_render }
+  scope :unread,      -> { where(read_at: nil).recent.for_render }
   scope :not_emailed, -> { where(emailed_at: nil) }
+  scope :recent,      -> { order(id: :desc) }
   scope :for_render,  -> { includes(:notifiable) }
 
-  delegate :notifiable_title, :notifiable_available?, :check_availability, :linkable_resource,
-           to: :notifiable, allow_nil: true
+  delegate :notifiable_title, :notifiable_available?, :check_availability,
+           :linkable_resource, to: :notifiable, allow_nil: true
+
+  def mark_as_read
+    update(read_at: Time.current)
+  end
+
+  def mark_as_unread
+    update(read_at: nil)
+  end
+
+  def read?
+    read_at.present?
+  end
+
+  def unread?
+    read_at.nil?
+  end
 
   def timestamp
     notifiable.created_at
   end
 
-  def mark_as_read
-    destroy
+  def self.add(user, notifiable)
+    notification = Notification.existent(user, notifiable)
+    if notification.present?
+      increment_counter(:counter, notification.id)
+    else
+      create!(user: user, notifiable: notifiable)
+    end
   end
 
-  def self.add(user_id, notifiable)
-    notification = Notification.find_by(user_id: user_id, notifiable: notifiable)
-
-    if notification.present?
-      Notification.increment_counter(:counter, notification.id)
-    else
-      Notification.create!(user_id: user_id, notifiable: notifiable)
-    end
+  def self.existent(user, notifiable)
+    unread.where(user: user, notifiable: notifiable).first
   end
 
   def notifiable_action

app/views/devise/menu/_login_items.html.erb

@@ -1,22 +1,11 @@
 <% if user_signed_in? %>
-  <li id="notifications">
-    <%= link_to notifications_path, rel: "nofollow", class: "notifications" do %>
-      <span class="show-for-sr"><%= t("layouts.header.notifications") %></span>
-      <% if current_user.notifications_count > 0 %>
-        <span class="icon-circle" aria-hidden="true"></span>
-        <span class="icon-notification" aria-hidden="true" title="<%= t('layouts.header.new_notifications', count: current_user.notifications_count).html_safe %>">
-        </span>
-        <small class="show-for-small-only">
-          <%= t('layouts.header.new_notifications', count: current_user.notifications_count).html_safe %>
-        </small>
-      <% else %>
-        <span class="icon-no-notification" aria-hidden="true" title="<%= t('layouts.header.no_notifications') %>"></span>
-        <small class="show-for-small-only">
-          <%= t('layouts.header.no_notifications') %>
-        </small>
-      <% end %>
-    <% end %>
-  </li>
+  <% if current_budget %>
+    <li>
+      <%= link_to(t("layouts.header.delegation"), 
+          new_budget_recommendation_path(budget_id: current_budget.slug), 
+                                         accesskey: "d") %>
+    </li>
+  <% end %>
   <li>
     <%= layout_menu_link_to t("layouts.header.my_activity_link"),
                               user_path(current_user),

app/views/layouts/_header.html.erb

@@ -30,6 +30,7 @@
         <div class="top-bar-right">
           <ul class="menu" data-responsive-menu="medium-dropdown">
             <%= render "shared/admin_login_items" %>
+            <%= render "layouts/notification_item" %>
             <%= render "devise/menu/login_items" %>
           </ul>
 

app/views/layouts/_notification_item.html.erb

@@ -0,0 +1,30 @@
+<% if user_signed_in? %>
+  <li id="notifications">
+    <%= link_to notifications_path, rel: "nofollow", 
+                class: "notifications" do %>
+      <span class="show-for-sr">
+        <%= t("layouts.header.notification_item.notifications") %>
+      </span>
+      
+      <% if current_user.notifications.unread.count > 0 %>
+        <span class="icon-circle" aria-hidden="true"></span>
+        <span class="icon-notification" aria-hidden="true" 
+              title="<%= t('layouts.header.notification_item.new_notifications', 
+                         count: current_user.notifications_count).html_safe %>">
+        </span>
+        <small class="show-for-small-only">
+          <%= t('layouts.header.notification_item.new_notifications', 
+              count: current_user.notifications_count).html_safe %>
+        </small>
+      <% else %>
+        <span class="icon-no-notification" aria-hidden="true" 
+              title="<%= t('layouts.header.notification_item.no_notifications') %>">
+        </span>
+        <small class="show-for-small-only">
+          <%= t('layouts.header.notification_item.no_notifications') %>
+        </small>
+      <% end %>
+
+    <% end %>
+  </li>
+<% end %>

app/views/notifications/_notification.html.erb

@@ -3,19 +3,37 @@
     <%= link_to notification do %>
       <p>
         <em>
-          <%= t("notifications.index.#{notification.notifiable_action}",
+          <%= t("notifications.notification.action.#{notification.notifiable_action}",
                 count: notification.counter) %>
         </em>
-        <strong><%= notification.notifiable_title %></strong>
+        <strong id="<%= dom_id(notification) %>_title">
+          <%= notification.notifiable_title %>
+        </strong>
       </p>
 
-      <p class="time"><%= l notification.timestamp, format: :datetime %></p>
+      <p class="time">
+        <%= l notification.timestamp, format: :datetime %>
+      </p>
     <% end %>
   <% else %>
     <p>
       <strong>
-        <%= t("notifications.index.notifiable_hidden") %>
+        <%= t("notifications.notification.notifiable_hidden") %>
       </strong>
     </p>
   <% end %>
+
+  <div>
+    <% if notification.unread? %>
+      <%= link_to t("notifications.notification.mark_as_read"),
+                mark_as_read_notification_path(notification),
+                method: :put,
+                remote: true %>
+    <% else %>
+      <%= link_to t("notifications.notification.mark_as_unread"), 
+                mark_as_unread_notification_path(notification),
+                method: :put,
+                remote: true %>
+    <% end %>
+  </div>
 </li>

app/views/notifications/index.html.erb

@@ -3,15 +3,22 @@
 
     <h1 class="float-left"><%= t("notifications.index.title") %></h1>
 
+    <h4>
+      <%= link_to t("notifications.index.unread"), notifications_path %>
+      <%= link_to t("notifications.index.read"), read_notifications_path %>
+    </h4>
+
     <% if @notifications.empty? %>
       <div data-alert class="callout primary margin-top clear">
         <%= t("notifications.index.empty_notifications") %>
       </div>
     <% else %>
-      <div class="float-right margin-top">
-        <%= link_to t("notifications.index.mark_all_as_read"),
-                    mark_all_as_read_notifications_path, method: :put %>
-      </div>
+      <% if action_name == "index" %>
+        <div class="float-right margin-top">
+          <%= link_to t("notifications.index.mark_all_as_read"),
+                      mark_all_as_read_notifications_path, method: :put %>
+        </div>
+      <% end %>
 
       <ul class="no-bullet clear notifications-list">
         <%= render @notifications %>

app/views/notifications/mark_as_read.js.erb

@@ -0,0 +1 @@
+$("#notification_<%= @notification.id %>").hide()

app/views/notifications/mark_as_unread.js.erb

@@ -0,0 +1 @@
+$("#notification_<%= @notification.id %>").hide()

app/views/notifications/read.html.erb

@@ -0,0 +1 @@
+<%= render template: "notifications/index" %>