diff --git a/app/views/layouts/_notification_item.html.erb b/app/views/layouts/_notification_item.html.erb
new file mode 100644
index 000000000..034726ea9
--- /dev/null
+++ b/app/views/layouts/_notification_item.html.erb
@@ -0,0 +1,30 @@
+<% if user_signed_in? %>
+
+ <%= link_to notifications_path, rel: "nofollow",
+ class: "notifications" do %>
+
+ <%= t("layouts.header.notification_item.notifications") %>
+
+
+ <% if current_user.notifications.unread.count > 0 %>
+
+
+
+
+ <%= t('layouts.header.notification_item.new_notifications',
+ count: current_user.notifications_count).html_safe %>
+
+ <% else %>
+
+
+
+ <%= t('layouts.header.notification_item.no_notifications') %>
+
+ <% end %>
+
+ <% end %>
+
+<% end %>
\ No newline at end of file
diff --git a/app/views/notifications/_notification.html.erb b/app/views/notifications/_notification.html.erb
index a5c69550c..654006fad 100644
--- a/app/views/notifications/_notification.html.erb
+++ b/app/views/notifications/_notification.html.erb
@@ -3,19 +3,37 @@
<%= link_to notification do %>
- <%= t("notifications.index.#{notification.notifiable_action}",
+ <%= t("notifications.notification.action.#{notification.notifiable_action}",
count: notification.counter) %>
- <%= notification.notifiable_title %>
+
+ <%= notification.notifiable_title %>
+
-
<%= l notification.timestamp, format: :datetime %>
+
+ <%= l notification.timestamp, format: :datetime %>
+
<% end %>
<% else %>
- <%= t("notifications.index.notifiable_hidden") %>
+ <%= t("notifications.notification.notifiable_hidden") %>
<% end %>
+
+
+ <% 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 %>
+
\ No newline at end of file
diff --git a/app/views/notifications/index.html.erb b/app/views/notifications/index.html.erb
index 889e5395d..646b0c7a0 100644
--- a/app/views/notifications/index.html.erb
+++ b/app/views/notifications/index.html.erb
@@ -3,15 +3,22 @@
<%= t("notifications.index.title") %>
+
+ <%= link_to t("notifications.index.unread"), notifications_path %>
+ <%= link_to t("notifications.index.read"), read_notifications_path %>
+
+
<% 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 %>
-
+ <% if action_name == "index" %>
+
+ <%= link_to t("notifications.index.mark_all_as_read"),
+ mark_all_as_read_notifications_path, method: :put %>
+
+ <% end %>
<%= render @notifications %>
diff --git a/app/views/notifications/mark_as_read.js.erb b/app/views/notifications/mark_as_read.js.erb
new file mode 100644
index 000000000..6e6cb3785
--- /dev/null
+++ b/app/views/notifications/mark_as_read.js.erb
@@ -0,0 +1 @@
+$("#notification_<%= @notification.id %>").hide()
\ No newline at end of file
diff --git a/app/views/notifications/mark_as_unread.js.erb b/app/views/notifications/mark_as_unread.js.erb
new file mode 100644
index 000000000..6e6cb3785
--- /dev/null
+++ b/app/views/notifications/mark_as_unread.js.erb
@@ -0,0 +1 @@
+$("#notification_<%= @notification.id %>").hide()
\ No newline at end of file
diff --git a/app/views/notifications/read.html.erb b/app/views/notifications/read.html.erb
new file mode 100644
index 000000000..3a02d4246
--- /dev/null
+++ b/app/views/notifications/read.html.erb
@@ -0,0 +1 @@
+<%= render template: "notifications/index" %>
diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml
index 628e2a15b..f2a7c41a3 100644
--- a/config/i18n-tasks.yml
+++ b/config/i18n-tasks.yml
@@ -164,9 +164,7 @@ ignore_unused:
- 'proposals.index.section_header.*'
- 'spending_proposals.index.search_form.*'
- '*.index.search_form.*'
- - 'notifications.index.comments_on*'
- - 'notifications.index.replies_to*'
- - 'notifications.index.proposal_notification*'
+ - 'notifications.notification.action.*'
- 'legislation.processes.index.filter*'
- 'legislation.processes.index.section_header.*'
- 'helpers.page_entries_info.*' # kaminari
diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml
index 66f392921..bc37884d6 100644
--- a/config/locales/en/general.yml
+++ b/config/locales/en/general.yml
@@ -234,11 +234,6 @@ en:
help: Help
my_account_link: My account
my_activity_link: My activity
- notifications: Notifications
- new_notifications:
- one: You have a new notification
- other: You have %{count} new notifications
- no_notifications: "You don't have new notifications"
open: open
open_city_slogan_html: There are cities that are governed directly by their inhabitants, who discuss the topics they are concerned about, propose ideas to improve their lives and decide among themselves which ones will be carried out.
open_city_title: Love the city, and it will become a city you love
@@ -247,6 +242,12 @@ en:
poll_questions: Voting
budgets: Participatory budgeting
spending_proposals: Spending Proposals
+ notification_item:
+ new_notifications:
+ one: You have a new notification
+ other: You have %{count} new notifications
+ notifications: Notifications
+ no_notifications: "You don't have new notifications"
admin:
watch_form_message: 'You have unsaved changes. Do you confirm to leave the page?'
legacy_legislation:
@@ -259,19 +260,25 @@ en:
locale: English
notifications:
index:
- comments_on:
- one: Someone commented on
- other: There are %{count} new comments on
empty_notifications: You don't have new notifications.
- notifiable_hidden: This resource is not available anymore.
mark_all_as_read: Mark all as read
- proposal_notification:
- one: There is one new notification on
- other: There are %{count} new notifications on
- replies_to:
- one: Someone replied to your comment on
- other: There are %{count} new replies to your comment on
+ read: All notifications
title: Notifications
+ unread: Unread
+ notification:
+ action:
+ comments_on:
+ one: Someone commented on
+ other: There are %{count} new comments on
+ proposal_notification:
+ one: There is one new notification on
+ other: There are %{count} new notifications on
+ replies_to:
+ one: Someone replied to your comment on
+ other: There are %{count} new replies to your comment on
+ mark_as_read: Mark as read
+ mark_as_unread: Mark as unread
+ notifiable_hidden: This resource is not available anymore.
map:
title: "Districts"
proposal_for_district: "Start a proposal for your district"
diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml
index 08bbbdba6..738d73ed0 100644
--- a/config/locales/es/general.yml
+++ b/config/locales/es/general.yml
@@ -234,11 +234,6 @@ es:
help: Ayuda
my_account_link: Mi cuenta
my_activity_link: Mi actividad
- notifications: Notificaciones
- new_notifications:
- one: Tienes una nueva notificación
- other: Tienes %{count} notificaciones nuevas
- no_notifications: "No tienes notificaciones nuevas"
open: abierto
open_city_slogan_html: Existen ciudades gobernadas directamente por sus habitantes, que debaten sobre temas que les preocupan, proponen ideas para mejorar sus vidas y deciden entre todas y todos las que se llevan a cabo.
open_city_title: La ciudad que quieres será la ciudad que quieras
@@ -247,6 +242,12 @@ es:
poll_questions: Votaciones
budgets: Presupuestos participativos
spending_proposals: Propuestas de inversión
+ notification_item:
+ new_notifications:
+ one: Tienes una nueva notificación
+ other: Tienes %{count} notificaciones nuevas
+ notifications: Notificaciones
+ no_notifications: "No tienes notificaciones nuevas"
admin:
watch_form_message: 'Has realizado cambios que no han sido guardados. ¿Seguro que quieres abandonar la página?'
legacy_legislation:
@@ -259,19 +260,25 @@ es:
locale: Español
notifications:
index:
- comments_on:
- one: Hay un nuevo comentario en
- other: Hay %{count} comentarios nuevos en
empty_notifications: No tienes notificaciones nuevas.
- notifiable_hidden: Este elemento ya no está disponible.
mark_all_as_read: Marcar todas como leídas
- proposal_notification:
- one: Hay una nueva notificación en
- other: Hay %{count} nuevas notificaciones en
- replies_to:
- one: Hay una respuesta nueva a tu comentario en
- other: Hay %{count} nuevas respuestas a tu comentario en
+ read: Todas
title: Notificaciones
+ unread: Nuevas
+ notification:
+ action:
+ comments_on:
+ one: Hay un nuevo comentario en
+ other: Hay %{count} comentarios nuevos en
+ proposal_notification:
+ one: Hay una nueva notificación en
+ other: Hay %{count} nuevas notificaciones en
+ replies_to:
+ one: Hay una respuesta nueva a tu comentario en
+ other: Hay %{count} nuevas respuestas a tu comentario en
+ mark_as_read: Marcar como leída
+ mark_as_unread: Marcar como no leída
+ notifiable_hidden: Este elemento ya no está disponible.
map:
title: "Distritos"
proposal_for_district: "Crea una propuesta para tu distrito"
diff --git a/config/routes/notification.rb b/config/routes/notification.rb
index 18936ff59..b9d8cb3a4 100644
--- a/config/routes/notification.rb
+++ b/config/routes/notification.rb
@@ -1,5 +1,8 @@
resources :notifications, only: [:index, :show] do
+ put :mark_as_read, on: :member
put :mark_all_as_read, on: :collection
+ put :mark_as_unread, on: :member
+ get :read, on: :collection
end
resources :proposal_notifications, only: [:new, :create, :show]
diff --git a/db/migrate/20180222120017_add_read_at_to_notifications.rb b/db/migrate/20180222120017_add_read_at_to_notifications.rb
new file mode 100644
index 000000000..c65186340
--- /dev/null
+++ b/db/migrate/20180222120017_add_read_at_to_notifications.rb
@@ -0,0 +1,5 @@
+class AddReadAtToNotifications < ActiveRecord::Migration
+ def change
+ add_column :notifications, :read_at, :timestamp
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 945f0d52e..28794ecaf 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -12,7 +12,6 @@
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 20180320104823) do
-
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
enable_extension "unaccent"
@@ -640,6 +639,7 @@ ActiveRecord::Schema.define(version: 20180320104823) do
t.string "notifiable_type"
t.integer "counter", default: 1
t.datetime "emailed_at"
+ t.datetime "read_at"
end
add_index "notifications", ["user_id"], name: "index_notifications_on_user_id", using: :btree
diff --git a/spec/factories.rb b/spec/factories.rb
index 5019f326a..2f9cc4acc 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -705,6 +705,10 @@ FactoryBot.define do
factory :notification do
user
association :notifiable, factory: :proposal
+
+ trait :read do
+ read_at Time.current
+ end
end
factory :geozone do
diff --git a/spec/features/notifications_spec.rb b/spec/features/notifications_spec.rb
index c413f3fd2..5b979d5ec 100644
--- a/spec/features/notifications_spec.rb
+++ b/spec/features/notifications_spec.rb
@@ -4,42 +4,128 @@ feature "Notifications" do
let(:user) { create :user }
- context "mark as read" do
-
- scenario "mark a single notification as read" do
- 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
- 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(page).to have_current_path(notifications_path)
- end
-
+ background do
+ login_as(user)
+ visit root_path
end
- scenario "no notifications" do
- login_as user
- visit notifications_path
+ scenario "View all" do
+ read1 = create(:notification, :read, user: user)
+ read2 = create(:notification, :read, user: user)
+ unread = create(:notification, user: user)
- expect(page).to have_content "You don't have new notifications"
+ click_notifications_icon
+ click_link "All notifications"
+
+ expect(page).to have_css(".notification", count: 2)
+ expect(page).to have_content(read1.notifiable_title)
+ expect(page).to have_content(read2.notifiable_title)
+ expect(page).to_not have_content(unread.notifiable_title)
+ end
+
+ scenario "View unread" do
+ unread1 = create(:notification, user: user)
+ unread2 = create(:notification, user: user)
+ read = create(:notification, :read, user: user)
+
+ click_notifications_icon
+ click_link "Unread"
+
+ expect(page).to have_css(".notification", count: 2)
+ expect(page).to have_content(unread1.notifiable_title)
+ expect(page).to have_content(unread2.notifiable_title)
+ expect(page).to_not have_content(read.notifiable_title)
+ end
+
+ scenario "View single notification" do
+ proposal = create(:proposal)
+ notification = create(:notification, user: user, notifiable: proposal)
+
+ click_notifications_icon
+
+ first(".notification a").click
+ expect(page).to have_current_path(proposal_path(proposal))
+
+ visit notifications_path
+ expect(page).to have_css ".notification", count: 0
+
+ visit read_notifications_path
+ expect(page).to have_css ".notification", count: 1
+ end
+
+ scenario "Mark as read", :js do
+ notification1 = create(:notification, user: user)
+ notification2 = create(:notification, user: user)
+
+ click_notifications_icon
+
+ within("#notification_#{notification1.id}") do
+ click_link "Mark as read"
+ end
+
+ expect(page).to have_css(".notification", count: 1)
+ expect(page).to have_content(notification2.notifiable_title)
+ expect(page).to_not have_content(notification1.notifiable_title)
+ end
+
+ scenario "Mark all as read" do
+ notification1 = create(:notification, user: user)
+ notification2 = create(:notification, user: user)
+
+ click_notifications_icon
+
+ expect(page).to have_css(".notification", count: 2)
+ click_link "Mark all as read"
+
+ expect(page).to have_css(".notification", count: 0)
+ end
+
+ scenario "Mark as unread", :js do
+ notification1 = create(:notification, :read, user: user)
+ notification2 = create(:notification, user: user)
+
+ click_notifications_icon
+ click_link "All notifications"
+
+ expect(page).to have_css(".notification", count: 1)
+ within("#notification_#{notification1.id}") do
+ click_link "Mark as unread"
+ end
+
+ expect(page).to have_css(".notification", count: 0)
+
+ visit notifications_path
+ expect(page).to have_css(".notification", count: 2)
+ expect(page).to have_content(notification1.notifiable_title)
+ expect(page).to have_content(notification2.notifiable_title)
+ end
+
+ scenario "Bell" do
+ create(:notification, user: user)
+ visit root_path
+
+ within("#notifications") do
+ expect(page).to have_css(".icon-circle")
+ end
+
+ click_notifications_icon
+ first(".notification a").click
+
+ within("#notifications") do
+ expect(page).to_not have_css(".icon-circle")
+ end
+ end
+
+ scenario "No notifications" do
+ click_notifications_icon
+ expect(page).to have_content "You don't have new notifications."
+ end
+
+ scenario "User not logged in" do
+ logout
+ visit root_path
+
+ expect(page).to_not have_css("#notifications")
end
end
diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index 014e8342f..d3fc91b37 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -2,29 +2,89 @@ require 'rails_helper'
describe Notification do
- describe "#unread (scope)" do
- it "returns only unread notifications" do
- 2.times { create :notification }
- expect(described_class.unread.size).to be 2
+ let(:notification) { build(:notification) }
+
+ context "validations" do
+
+ it "should be valid" do
+ expect(notification).to be_valid
+ end
+
+ it "should not be valid without a user" do
+ notification.user = nil
+ expect(notification).to_not be_valid
+ end
+
+ end
+
+ context "scopes" do
+
+ describe "#read" do
+ it "returns only read notifications" do
+ read_notification1 = create(:notification, :read)
+ read_notification2 = create(:notification, :read)
+ unread_notification = create(:notification)
+
+ expect(described_class.read).to include read_notification1
+ expect(described_class.read).to include read_notification2
+ expect(described_class.read).not_to include unread_notification
+ end
+ end
+
+ describe "#unread" do
+ it "returns only unread notifications" do
+ read_notification = create(:notification, :read)
+ unread_notification1 = create(:notification)
+ unread_notification2 = create(:notification)
+
+ expect(described_class.unread).to include unread_notification1
+ expect(described_class.unread).to include unread_notification2
+ expect(described_class.unread).not_to include read_notification
+ end
+ end
+
+ describe "#recent" do
+ it "returns notifications sorted by id descendant" do
+ old_notification = create :notification
+ new_notification = create :notification
+
+ sorted_notifications = described_class.recent
+ expect(sorted_notifications.size).to be 2
+ expect(sorted_notifications.first).to eq new_notification
+ expect(sorted_notifications.last).to eq old_notification
+ end
+ end
+
+ describe "#for_render" do
+ it "returns notifications including notifiable and user" do
+ allow(described_class).to receive(:includes).with(:notifiable).exactly(:once)
+ described_class.for_render
+ end
+ end
+
+ end
+
+ describe "#mark_as_read" do
+ it "destroys notification" do
+ notification = create(:notification)
+ expect(described_class.read.size).to eq 0
+ expect(described_class.unread.size).to eq 1
+
+ notification.mark_as_read
+ expect(described_class.read.size).to eq 1
+ expect(described_class.unread.size).to eq 0
end
end
- describe "#recent (scope)" do
- it "returns notifications sorted by id descendant" do
- old_notification = create :notification
- new_notification = create :notification
+ describe "#mark_as_unread" do
+ it "destroys notification" do
+ notification = create(:notification, :read)
+ expect(described_class.unread.size).to eq 0
+ expect(described_class.read.size).to eq 1
- sorted_notifications = described_class.recent
- expect(sorted_notifications.size).to be 2
- expect(sorted_notifications.first).to eq new_notification
- expect(sorted_notifications.last).to eq old_notification
- end
- end
-
- describe "#for_render (scope)" do
- it "returns notifications including notifiable and user" do
- allow(described_class).to receive(:includes).with(:notifiable).exactly(:once)
- described_class.for_render
+ notification.mark_as_unread
+ expect(described_class.unread.size).to eq 1
+ expect(described_class.read.size).to eq 0
end
end
@@ -37,13 +97,66 @@ describe Notification do
end
end
- describe "#mark_as_read" do
- it "destroys notification" do
- notification = create :notification
- expect(described_class.unread.size).to eq 1
+ describe "#existent" do
+ it "returns the notifiable when there is an existent notification of that notifiable" do
+ user = create(:user)
+ comment = create(:comment)
+ notification = create(:notification, user: user, notifiable: comment)
- notification.mark_as_read
- expect(described_class.unread.size).to eq 0
+ expect(described_class.existent(user, comment)).to eq(notification)
+ end
+
+ it "returns nil when there are no notifications of that notifiable for a user" do
+ user = create(:user)
+ comment1 = create(:comment)
+ comment2 = create(:comment)
+ create(:notification, user: user, notifiable: comment1)
+
+ expect(described_class.existent(user, comment2)).to eq(nil)
+ end
+
+ it "returns nil when there are notifications of a notifiable for another user" do
+ user1 = create(:user)
+ user2 = create(:user)
+ comment = create(:comment)
+ notification = create(:notification, user: user1, notifiable: comment)
+
+ expect(described_class.existent(user2, comment)).to eq(nil)
+ end
+ end
+
+ describe "#add" do
+ it "creates a new notification" do
+ user = create(:user)
+ comment = create(:comment)
+
+ described_class.add(user, comment)
+ expect(user.notifications.count).to eq(1)
+ end
+
+ it "increments the notification counter for an unread notification of the same notifiable" do
+ user = create(:user)
+ comment = create(:comment)
+
+ described_class.add(user, comment)
+ described_class.add(user, comment)
+
+ expect(user.notifications.count).to eq(1)
+ expect(user.notifications.first.counter).to eq(2)
+ end
+
+ it "creates a new notification for a read notification of the same notifiable" do
+ user = create(:user)
+ comment = create(:comment)
+
+ first_notification = described_class.add(user, comment)
+ first_notification.update(read_at: Time.current)
+
+ second_notification = described_class.add(user, comment)
+
+ expect(user.notifications.count).to eq(2)
+ expect(first_notification.counter).to eq(1)
+ expect(second_notification.counter).to eq(1)
end
end
diff --git a/spec/support/common_actions.rb b/spec/support/common_actions.rb
index 0e1ced904..81d831f5f 100644
--- a/spec/support/common_actions.rb
+++ b/spec/support/common_actions.rb
@@ -353,4 +353,7 @@ module CommonActions
fill_in "newsletter_body", with: (options[:body] || "This is a different body")
end
+ def click_notifications_icon
+ find("#notifications a").click
+ end
end