destroy notifications when marked as read

app/controllers/notifications_controller.rb

@@ -1,9 +1,26 @@
 class NotificationsController < ApplicationController
   before_action :authenticate_user!
-  load_and_authorize_resource class: "User"
+  after_action :mark_as_read, only: :show
+  skip_authorization_check
 
   def index
-    @notifications = current_user.notifications.recent.for_render
+    @notifications = current_user.notifications.unread.recent.for_render
+  end
+
+  def show
+    @notification = current_user.notifications.find(params[:id])
+    redirect_to url_for(@notification.notifiable.commentable)
+  end
+
+  def mark_all_as_read
+    current_user.notifications.each { |notification| notification.mark_as_read }
+    redirect_to notifications_path
+  end
+
+  private
+
+    def mark_as_read
+      @notification.mark_as_read
     end
 
 end

app/helpers/notifications_helper.rb

@@ -1,11 +1,7 @@
 module NotificationsHelper
 
-  def notification_text_for(notification)
+  def notification_action(notification)
-    if notification.notifiable.reply?
+    notification.notifiable.reply? ? "replied_to_your_comment" : "commented_on_your_debate"
-      t("comments.notifications.replied_to_your_comment")
-    else
-      t("comments.notifications.commented_on_your_debate")
-    end
   end
 
   def notifications_class_for(user)

app/models/notification.rb

@@ -14,6 +14,7 @@ class Notification < ActiveRecord::Base
     notifiable.created_at
   end
 
-  def mark_as_read!
+  def mark_as_read
+    self.destroy
   end
 end

app/views/notifications/_notification.html.erb

@@ -0,0 +1,8 @@
+<li id="<%= dom_id(notification) %>" class="notification">
+  <%= link_to notification do %>
+    <time><%= l notification.timestamp, format: :datetime %></time>&nbsp;&bull;&nbsp;
+    <span><%= notification.username %> </span>
+    <span><%= t("notifications.index.#{notification_action(notification)}") %></span>
+    <span><%= notification.notifiable.commentable.title %></span>
+  <% end %>
+</li>

app/views/notifications/index.html.erb

@@ -1,11 +1,14 @@
+<% if @notifications.empty? %>
+  <div><%= t("notifications.index.empty_notifications") %></div>
+<% else %>
   <div class="row">
     <ul>
-    <% @notifications.each do |notification| %>
+      <%= render @notifications %>
-    <li>
-      <time><%= l notification.timestamp, format: :datetime %></time>&nbsp;&bull;&nbsp;
-      <%= notification.username %> <%= notification_text_for(notification) %>
-      <%= link_to notification.notifiable.commentable.title, notification.notifiable.commentable %>
-    </li>
-    <% end %>
     </ul>
+
+    <div>
+      <%= link_to t("notifications.index.mark_all_as_read"),
+                  mark_all_as_read_notifications_path, method: :put %>
     </div>
+  </div>
+<% end %>

config/locales/en.yml

@@ -267,9 +267,6 @@ en:
         zero: "No votes"
         one: "1 vote"
         other: "%{count} votes"
-    notifications:
-      commented_on_your_debate: "commented on your debate"
-      replied_to_your_comment: "replied to your comment on"
   comments_helper:
     comment_link: "Comment"
     comment_button: "Publish comment"
@@ -313,6 +310,12 @@ en:
       user_permission_votes: "Participate on final voting"
       user_permission_verify: "To perform all the actions verify your account."
       user_permission_verify_info: "* Only for users on Madrid City Census."
+  notifications:
+    index:
+      mark_all_as_read: "Mark all as read"
+      empty_notifications: "There are no new notifications."
+      commented_on_your_debate: "commented on your debate"
+      replied_to_your_comment: "replied to your comment on"
   simple_captcha:
     placeholder: "Enter the text from the image"
     label: "Enter the text from the image in the box below"

config/locales/es.yml

@@ -267,9 +267,6 @@ es:
         zero: "Sin votos"
         one: "1 voto"
         other: "%{count} votos"
-    notifications:
-      commented_on_your_debate: "ha comentado en tu debate"
-      replied_to_your_comment: "ha respondido a tu comentario en"
   comments_helper:
     comment_link: "Comentar"
     comment_button: "Publicar comentario"
@@ -313,6 +310,12 @@ es:
       user_permission_votes: "Participar en las votaciones finales*"
       user_permission_verify: "Para poder realizar todas las acciones verifica tu cuenta."
       user_permission_verify_info: "* Sólo usuarios empadronados en el municipio de Madrid."
+  notifications:
+    index:
+      mark_all_as_read: "Marcar todas como leídas"
+      empty_notifications: "No hay notificaciones nuevas."
+      commented_on_your_debate: "ha comentado en tu debate"
+      replied_to_your_comment: "ha respondido a tu comentario en"
   simple_captcha:
     placeholder: "Introduce el texto de la imagen"
     label: "Introduce el texto de la imagen en la siguiente caja"

config/routes.rb

@@ -75,7 +75,9 @@ Rails.application.routes.draw do
     collection { get :erase }
   end
 
-  resources :notifications, only: :index
+  resources :notifications, only: [:index, :show] do
+    collection { put :mark_all_as_read }
+  end
 
   resource :verification, controller: "verification", only: [:show]
 

spec/controllers/notifications_controller_spec.rb

@@ -1,21 +0,0 @@
-require 'rails_helper'
-
-describe NotificationsController do
-
-  describe "#index" do
-    let(:user) { create :user }
-
-    it "mark all notifications as read" do
-      notifications = [create(:notification, user: user), create(:notification, user: user)]
-      Notification.all.each do |notification|
-        expect(notification.read).to be false
-      end
-
-      sign_in user
-      get :index
-      Notification.all.each do |notification|
-        expect(notification.read).to be true
-      end
-    end
-  end
-end

spec/features/notifications_spec.rb

@@ -21,10 +21,10 @@ feature "Notifications" do
     expect(page).to have_xpath "//a[@class='with_notifications' and @href='#{notifications_path}' and text()='Notificaciones']"
 
     click_link "Notificaciones"
+    expect(page).to have_css ".notification", count: 1
     expect(page).to have_content user.username
-    expect(page).to have_content I18n.t("comments.notifications.commented_on_your_debate")
+    expect(page).to have_content "commented on your debate"
-    expect(page).to_not have_content I18n.t("comments.notifications.replied_to_your_comment")
+    expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']"
-    expect(page).to have_link debate.title, href: debate_path(debate)
   end
 
   scenario "User replied to my comment", :js do
@@ -48,10 +48,10 @@ feature "Notifications" do
     expect(page).to have_xpath "//a[@class='with_notifications' and @href='#{notifications_path}' and text()='Notificaciones']"
 
     visit notifications_path
+    expect(page).to have_css ".notification", count: 1
     expect(page).to have_content user.username
-    expect(page).to have_content I18n.t("comments.notifications.replied_to_your_comment")
+    expect(page).to have_content "replied to your comment on"
-    expect(page).to_not have_content I18n.t("comments.notifications.commented_on_your_debate")
+    expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']"
-    expect(page).to have_link debate.title, href: debate_path(debate)
   end
 
   scenario "Author commented on his own debate", :js do
@@ -66,10 +66,7 @@ feature "Notifications" do
     expect(page).to have_xpath "//a[@class='without_notifications' and @href='#{notifications_path}' and text()='Notificaciones']"
 
     click_link "Notificaciones"
-    expect(page).to_not have_content user.username
+    expect(page).to have_css ".notification", count: 0
-    expect(page).to_not have_content I18n.t("comments.notifications.commented_on_your_debate")
-    expect(page).to_not have_content I18n.t("comments.notifications.replied_to_your_comment")
-    expect(page).to_not have_link debate.title, href: debate_path(debate)
   end
 
   scenario "Author replied to his own comment", :js do
@@ -89,9 +86,47 @@ feature "Notifications" do
     expect(page).to have_xpath "//a[@class='without_notifications' and @href='#{notifications_path}' and text()='Notificaciones']"
 
     visit notifications_path
-    expect(page).to_not have_content user.username
+    expect(page).to have_css ".notification", count: 0
-    expect(page).to_not have_content I18n.t("comments.notifications.replied_to_your_comment")
-    expect(page).to_not have_content I18n.t("comments.notifications.commented_on_your_debate")
-    expect(page).to_not have_link debate.title, href: debate_path(debate)
   end
+
+  context "mark as read" do
+
+    scenario "mark a single notification as read" do
+      user = create :user
+      notification = create :notification, user: user
+
+      login_as user
+      visit notifications_path
+
+      expect(page).to have_css ".notification", count: 1
+
+      first(".notification a").click
+      visit notifications_path
+
+      expect(page).to have_css ".notification", count: 0
+    end
+
+    scenario "mark all notifications as read" do
+      user = create :user
+      2.times { create :notification, user: user }
+
+      login_as user
+      visit notifications_path
+
+      expect(page).to have_css ".notification", count: 2
+      click_link "Mark all as read"
+
+      expect(page).to have_css ".notification", count: 0
+      expect(current_path).to eq(notifications_path)
+    end
+
+  end
+
+  scenario "no notifications" do
+    login_as user
+    visit notifications_path
+
+    expect(page).to have_content "There are no new notifications"
+  end
+
 end

spec/helpers/notifications_helper_spec.rb

@@ -2,7 +2,7 @@ require 'rails_helper'
 
 describe NotificationsHelper do
 
-  describe "#notification_text_for" do
+  describe "#notification_action" do
     let(:debate) { create :debate }
     let(:debate_comment) { create :comment, commentable: debate }
     let(:comment_reply)  { create :comment, commentable: debate, parent: debate_comment }
@@ -10,14 +10,14 @@ describe NotificationsHelper do
     context "when action was comment on a debate" do
       it "returns correct text when someone comments on your debate" do
         notification = create :notification, notifiable: debate_comment
-        expect(notification_text_for(notification)).to eq "commented on your debate"
+        expect(notification_action(notification)).to eq "commented_on_your_debate"
       end
     end
 
     context "when action was comment on a debate" do
       it "returns correct text when someone replies to your comment" do
         notification = create :notification, notifiable: comment_reply
-        expect(notification_text_for(notification)).to eq "replied to your comment on"
+        expect(notification_action(notification)).to eq "replied_to_your_comment"
       end
     end
   end
@@ -25,20 +25,13 @@ describe NotificationsHelper do
   describe "#notifications_class_for" do
     let(:user) { create :user }
 
-    context "when user doesn't have any notification" do
+    context "when user doesn't have notifications" do
       it "returns class 'without_notifications'" do
         expect(notifications_class_for(user)).to eq "without_notifications"
       end
     end
 
-    context "when user doesn't have unread notifications" do
+    context "when user has notifications" do
-      it "returns class 'without_notifications'" do
-        notification = create :notification, user: user, read: true
-        expect(notifications_class_for(user)).to eq "without_notifications"
-      end
-    end
-
-    context "when user has unread notifications" do
       it "returns class 'with_notifications'" do
         notification = create :notification, user: user
         expect(notifications_class_for(user)).to eq "with_notifications"

spec/models/notification_spec.rb

@@ -4,12 +4,8 @@ describe Notification do
 
   describe "#unread (scope)" do
     it "returns only unread notifications" do
-      unread_notification = create :notification
+      2.times { create :notification }
-      read_notification = create :notification, read: true
+      expect(Notification.unread.size).to be 2
-
-      unread_notifications = Notification.unread
-      expect(unread_notifications.size).to be 1
-      expect(unread_notifications.first).to eq unread_notification
     end
   end
 
@@ -42,12 +38,12 @@ describe Notification do
   end
 
   describe "#mark_as_read" do
-    it "set up read flag to true" do
+    it "destroys notification" do
       notification = create :notification
-      expect(notification.read).to be false
+      expect(Notification.unread.size).to eq 1
 
-      notification.mark_as_read!
+      notification.mark_as_read
-      expect(notification.read).to be true
+      expect(Notification.unread.size).to eq 0
     end
   end