From e2f419e625f1996cbe14e461ce487e221ca5f2ce Mon Sep 17 00:00:00 2001 From: rgarcia Date: Thu, 7 Jan 2016 12:02:01 +0100 Subject: [PATCH] destroy notifications when marked as read --- app/controllers/notifications_controller.rb | 21 ++++++- app/helpers/notifications_helper.rb | 8 +-- app/models/notification.rb | 3 +- .../notifications/_notification.html.erb | 8 +++ app/views/notifications/index.html.erb | 25 ++++---- config/locales/en.yml | 9 ++- config/locales/es.yml | 9 ++- config/routes.rb | 4 +- .../notifications_controller_spec.rb | 21 ------- spec/features/notifications_spec.rb | 63 ++++++++++++++----- spec/helpers/notifications_helper_spec.rb | 17 ++--- spec/models/notification_spec.rb | 16 ++--- 12 files changed, 120 insertions(+), 84 deletions(-) create mode 100644 app/views/notifications/_notification.html.erb delete mode 100644 spec/controllers/notifications_controller_spec.rb diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 8ecb16104..3820d7e36 100644 --- a/app/controllers/notifications_controller.rb +++ b/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 diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index 45ef6422f..1945d7c5b 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,11 +1,7 @@ module NotificationsHelper - def notification_text_for(notification) - if notification.notifiable.reply? - t("comments.notifications.replied_to_your_comment") - else - t("comments.notifications.commented_on_your_debate") - end + def notification_action(notification) + notification.notifiable.reply? ? "replied_to_your_comment" : "commented_on_your_debate" end def notifications_class_for(user) diff --git a/app/models/notification.rb b/app/models/notification.rb index 63f44dedc..e22e68ad2 100644 --- a/app/models/notification.rb +++ b/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 \ No newline at end of file diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb new file mode 100644 index 000000000..313a5458c --- /dev/null +++ b/app/views/notifications/_notification.html.erb @@ -0,0 +1,8 @@ +
  • + <%= link_to notification do %> +  •  + <%= notification.username %> + <%= t("notifications.index.#{notification_action(notification)}") %> + <%= notification.notifiable.commentable.title %> + <% end %> +
    • \ No newline at end of file diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb index 0155f0521..5eaebefb9 100644 --- a/app/views/notifications/index.html.erb +++ b/app/views/notifications/index.html.erb @@ -1,11 +1,14 @@ -
    - -
    +<% if @notifications.empty? %> +
    <%= t("notifications.index.empty_notifications") %>
    +<% else %> +
    + + +
    + <%= link_to t("notifications.index.mark_all_as_read"), + mark_all_as_read_notifications_path, method: :put %> +
    +
    +<% end %> \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index 35af31bbc..683db0454 100755 --- a/config/locales/en.yml +++ b/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" diff --git a/config/locales/es.yml b/config/locales/es.yml index c10d8a493..f6fd907de 100644 --- a/config/locales/es.yml +++ b/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" diff --git a/config/routes.rb b/config/routes.rb index 681eff89b..29b63e91b 100644 --- a/config/routes.rb +++ b/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] diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb deleted file mode 100644 index fa6104a9c..000000000 --- a/spec/controllers/notifications_controller_spec.rb +++ /dev/null @@ -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 diff --git a/spec/features/notifications_spec.rb b/spec/features/notifications_spec.rb index 4b5b48f0e..22e4b320d 100644 --- a/spec/features/notifications_spec.rb +++ b/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_not have_content I18n.t("comments.notifications.replied_to_your_comment") - expect(page).to have_link debate.title, href: debate_path(debate) + expect(page).to have_content "commented on your debate" + expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']" 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_not have_content I18n.t("comments.notifications.commented_on_your_debate") - expect(page).to have_link debate.title, href: debate_path(debate) + expect(page).to have_content "replied to your comment on" + expect(page).to have_xpath "//a[@href='#{notification_path(Notification.last)}']" 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_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) + expect(page).to have_css ".notification", count: 0 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_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) + expect(page).to have_css ".notification", count: 0 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 diff --git a/spec/helpers/notifications_helper_spec.rb b/spec/helpers/notifications_helper_spec.rb index dd5cd1b78..fa378b2ec 100644 --- a/spec/helpers/notifications_helper_spec.rb +++ b/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 - 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 + context "when user has notifications" do it "returns class 'with_notifications'" do notification = create :notification, user: user expect(notifications_class_for(user)).to eq "with_notifications" diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index d93dcf6f3..0b8354e83 100644 --- a/spec/models/notification_spec.rb +++ b/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 - read_notification = create :notification, read: true - - unread_notifications = Notification.unread - expect(unread_notifications.size).to be 1 - expect(unread_notifications.first).to eq unread_notification + 2.times { create :notification } + expect(Notification.unread.size).to be 2 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! - expect(notification.read).to be true + notification.mark_as_read + expect(Notification.unread.size).to eq 0 end end