From 6456a441fe3009bca4c4e52f8d461fd31daf507a Mon Sep 17 00:00:00 2001 From: jaideng123 Date: Sat, 16 Aug 2014 16:01:10 -0500 Subject: [PATCH 1/3] Modified behavior of mark all as read button --- app/controllers/notifications_controller.rb | 19 +++-- app/views/notifications/index.html.haml | 2 +- app/views/notifications/index.mobile.haml | 2 +- config/routes.rb | 5 +- .../notifications_controller_spec.rb | 74 ++++++++++++------- 5 files changed, 67 insertions(+), 35 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 85e951436..033e68c2f 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -65,12 +65,21 @@ class NotificationsController < ApplicationController end def read_all - Notification.where(:recipient_id => current_user.id).update_all(:unread => false) + if params[:type] + Notification.where(:recipient_id => current_user.id, :type => Notification.types[params[:type]]).update_all(:unread => false) + else + Notification.where(:recipient_id => current_user.id).update_all(:unread => false) + end respond_to do |format| - format.html { redirect_to stream_path } - format.mobile{ redirect_to stream_path} - format.xml { render :xml => {}.to_xml } - format.json { render :json => {}.to_json } + if current_user.unread_notifications.count > 0 + format.html { redirect_to notifications_path} + format.mobile{ redirect_to notifications_path} + else + format.html { redirect_to stream_path } + format.mobile{ redirect_to stream_path} + end + format.xml { render :xml => {}.to_xml } + format.json { render :json => {}.to_json } end end diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 9f91e6946..9d0c894dd 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -36,7 +36,7 @@ = t('.show_all') %a.btn.btn-default{ :class => ('active' if params[:show] == 'unread'), :href => '/notifications?show=unread' + (params[:type] ? '&type=' + params[:type] : '') } = t('.show_unread') - %a.btn.btn-default{:href => notifications_read_all_path, :class => ('disabled' unless @unread_notification_count > 0)} + %a.btn.btn-default{:href => read_all_notifications_path(:type => params[:type] ), :class => ('disabled' unless @unread_notification_count > 0)} = t('.mark_all_as_read') - @group_days.each do |day, notes| .day_group.row-fluid diff --git a/app/views/notifications/index.mobile.haml b/app/views/notifications/index.mobile.haml index cee20a748..4aa88bca2 100644 --- a/app/views/notifications/index.mobile.haml +++ b/app/views/notifications/index.mobile.haml @@ -2,7 +2,7 @@ = t('.notifications') .right - = link_to t('.mark_all_as_read'), notifications_read_all_path, :class => 'btn' + = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'btn' %ul.notifications - @group_days.each do |day, notes| diff --git a/config/routes.rb b/config/routes.rb index 0469aed85..f7954d617 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -79,9 +79,12 @@ Diaspora::Application.routes.draw do delete 'visibility' => 'conversation_visibilities#destroy' end - get 'notifications/read_all' => 'notifications#read_all' resources :notifications, :only => [:index, :update] do + collection do + get :read_all + end end + resources :tags, :only => [:index] diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index 5b2e37452..adb739df5 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -42,33 +42,6 @@ describe NotificationsController do end end - describe "#read_all" do - it 'marks all notifications as read' do - request.env["HTTP_REFERER"] = "I wish I were spelled right" - FactoryGirl.create(:notification, :recipient => alice) - FactoryGirl.create(:notification, :recipient => alice) - - Notification.where(:unread => true).count.should == 2 - get :read_all - Notification.where(:unread => true).count.should == 0 - end - it "should redirect to the stream in the html version" do - FactoryGirl.create(:notification, :recipient => alice) - get :read_all, :format => :html - response.should redirect_to(stream_path) - end - it "should redirect to the stream in the mobile version" do - FactoryGirl.create(:notification, :recipient => alice) - get :read_all, :format => :mobile - response.should redirect_to(stream_path) - end - it "should return a dummy value in the json version" do - FactoryGirl.create(:notification, :recipient => alice) - get :read_all, :format => :json - response.should_not be_redirect - end - end - describe '#index' do before do @post = FactoryGirl.create(:status_message) @@ -137,4 +110,51 @@ describe NotificationsController do end end end + + describe "#read_all" do + it 'marks all notifications as read' do + request.env["HTTP_REFERER"] = "I wish I were spelled right" + FactoryGirl.create(:notification, :recipient => alice) + FactoryGirl.create(:notification, :recipient => alice) + + Notification.where(:unread => true).count.should == 2 + get :read_all + Notification.where(:unread => true).count.should == 0 + end + it 'marks all notifications in the current filter as read' do + request.env["HTTP_REFERER"] = "I wish I were spelled right" + FactoryGirl.create(:notification, :recipient => alice) + eve.share_with(alice.person, eve.aspects.first) + Notification.where(:unread => true).count.should == 2 + get :read_all, "type" => "started_sharing" + Notification.where(:unread => true).count.should == 1 + end + it "should redirect back in the html version if it has > 0 notifications" do + FactoryGirl.create(:notification, :recipient => alice) + eve.share_with(alice.person, eve.aspects.first) + get :read_all, :format => :html, "type" => "started_sharing" + response.should redirect_to(notifications_path) + end + it "should redirect back in the mobile version if it has > 0 notifications" do + FactoryGirl.create(:notification, :recipient => alice) + eve.share_with(alice.person, eve.aspects.first) + get :read_all, :format => :mobile, "type" => "started_sharing" + response.should redirect_to(notifications_path) + end + it "should redirect to stream in the html version if it has 0 notifications" do + FactoryGirl.create(:notification, :recipient => alice) + get :read_all, :format => :html + response.should redirect_to(stream_path) + end + it "should redirect back in the mobile version if it has 0 notifications" do + FactoryGirl.create(:notification, :recipient => alice) + get :read_all, :format => :mobile + response.should redirect_to(stream_path) + end + it "should return a dummy value in the json version" do + FactoryGirl.create(:notification, :recipient => alice) + get :read_all, :format => :json + response.should_not be_redirect + end + end end From 406397988bc3f0fb31726465343f06031ff9f292 Mon Sep 17 00:00:00 2001 From: jaideng123 Date: Sun, 17 Aug 2014 11:16:30 -0500 Subject: [PATCH 2/3] Made wording more clear, refactored read_all --- app/controllers/notifications_controller.rb | 20 ++++++++++---------- app/views/notifications/index.html.haml | 5 ++++- config/locales/diaspora/en.yml | 1 + 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 033e68c2f..4354093bc 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -65,22 +65,22 @@ class NotificationsController < ApplicationController end def read_all - if params[:type] - Notification.where(:recipient_id => current_user.id, :type => Notification.types[params[:type]]).update_all(:unread => false) - else - Notification.where(:recipient_id => current_user.id).update_all(:unread => false) - end + current_type = Notification.types[params[:type]] + notifications = Notification.where(:recipient_id => current_user.id) + notifications = notifications.where(:type => current_type) if params[:type] + notifications.update_all(:unread => false) respond_to do |format| if current_user.unread_notifications.count > 0 - format.html { redirect_to notifications_path} - format.mobile{ redirect_to notifications_path} + format.html { redirect_to notifications_path } + format.mobile { redirect_to notifications_path } else format.html { redirect_to stream_path } - format.mobile{ redirect_to stream_path} + format.mobile { redirect_to stream_path } end - format.xml { render :xml => {}.to_xml } - format.json { render :json => {}.to_json } + format.xml { render :xml => {}.to_xml } + format.json { render :json => {}.to_json } end + end end diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 9d0c894dd..19033fe7a 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -37,7 +37,10 @@ %a.btn.btn-default{ :class => ('active' if params[:show] == 'unread'), :href => '/notifications?show=unread' + (params[:type] ? '&type=' + params[:type] : '') } = t('.show_unread') %a.btn.btn-default{:href => read_all_notifications_path(:type => params[:type] ), :class => ('disabled' unless @unread_notification_count > 0)} - = t('.mark_all_as_read') + -if params[:type] + = t('.mark_all_shown_as_read') + -else + = t('.mark_all_as_read') - @group_days.each do |day, notes| .day_group.row-fluid .date.span2 diff --git a/config/locales/diaspora/en.yml b/config/locales/diaspora/en.yml index 851d01e37..ea4049660 100644 --- a/config/locales/diaspora/en.yml +++ b/config/locales/diaspora/en.yml @@ -708,6 +708,7 @@ en: index: notifications: "Notifications" mark_all_as_read: "Mark all as read" + mark_all_shown_as_read: "Mark all shown as read" mark_read: "Mark read" mark_unread: "Mark unread" show_all: "show all" From c68eeb70fb2b151be4d45f186f916571381f5197 Mon Sep 17 00:00:00 2001 From: jaideng123 Date: Sun, 17 Aug 2014 15:51:37 -0500 Subject: [PATCH 3/3] Added the button changing effect + the ability to read all of a type to mobile --- app/views/notifications/index.mobile.haml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/notifications/index.mobile.haml b/app/views/notifications/index.mobile.haml index 4aa88bca2..498ee29a4 100644 --- a/app/views/notifications/index.mobile.haml +++ b/app/views/notifications/index.mobile.haml @@ -2,8 +2,10 @@ = t('.notifications') .right - = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'btn' - + -if params[:type] + = link_to t('.mark_all_shown_as_read'), read_all_notifications_path(:type => params[:type] ), :class => 'btn' + -else + = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'btn' %ul.notifications - @group_days.each do |day, notes| %li