From a11c8d8a964e94a0546ed5117b1a0e970bd2b811 Mon Sep 17 00:00:00 2001 From: Steven Fuchs Date: Tue, 17 Jan 2012 19:20:42 -0500 Subject: [PATCH 1/6] javascripts for marking notifications items as unread on both the notifications page and popup menu. --- public/javascripts/widgets/header.js | 4 +- .../widgets/notifications-badge.js | 13 +- public/javascripts/widgets/notifications.js | 114 +++++++++++++++--- .../javascripts/widgets/notifications-spec.js | 45 ++++++- spec/models/notification_spec.rb | 15 +++ 5 files changed, 155 insertions(+), 36 deletions(-) diff --git a/public/javascripts/widgets/header.js b/public/javascripts/widgets/header.js index 208c5cd84..10fd0d996 100644 --- a/public/javascripts/widgets/header.js +++ b/public/javascripts/widgets/header.js @@ -4,8 +4,8 @@ this.subscribe("widget/ready", function(evt, header) { self.notifications = self.instantiate("Notifications", - header.find("#notifications"), - header.find("#notification_badge .badge_count") + header.find("#notification_badge .badge_count"), + header.find(".notifications") ); self.notificationsDropdown = self.instantiate("NotificationsDropdown", diff --git a/public/javascripts/widgets/notifications-badge.js b/public/javascripts/widgets/notifications-badge.js index e64d5da39..d69666fe3 100644 --- a/public/javascripts/widgets/notifications-badge.js +++ b/public/javascripts/widgets/notifications-badge.js @@ -63,6 +63,7 @@ $.each(notifications, function(index, notification) { var notificationElement = $("
") .addClass("notification_element") + .data( "guid", notification.id ) .html(notification.translation) .prepend($("", { src: notification.actors[0].avatar })) .append("
") @@ -70,19 +71,15 @@ "class": "timeago", "title": notification.created_at })) + .append('mark unread') .appendTo(self.dropdownNotifications); notificationElement.find("abbr.timeago").timeago(); if(notification.unread) { - notificationElement.addClass("unread"); - $.ajax({ - url: "/notifications/" + notification.id, - type: "PUT", - success: function() { - Diaspora.page.header.notifications.decrementCount(); - } - }); + Diaspora.page.header.notifications.setUpUnread( notificationElement ); + }else{ + Diaspora.page.header.notifications.setUpRead( notificationElement ); } }); }); diff --git a/public/javascripts/widgets/notifications.js b/public/javascripts/widgets/notifications.js index fff80b404..ce59e0895 100644 --- a/public/javascripts/widgets/notifications.js +++ b/public/javascripts/widgets/notifications.js @@ -8,32 +8,99 @@ var Notifications = function() { var self = this; - this.subscribe("widget/ready", function(evt, notificationArea, badge) { + this.subscribe("widget/ready", function(evt, badge, notificationMenu) { $.extend(self, { badge: badge, count: parseInt(badge.html()) || 0, - notificationArea: notificationArea + notificationArea: null, + notificationMenu: notificationMenu }); - $(".stream_element.unread").live("mousedown", function() { - self.decrementCount(); - - $.ajax({ - url: "notifications/" + $(this).removeClass("unread").data("guid"), - type: "PUT" - }); - }); - - $("a.more").live("click", function(evt) { + $("a.more").click( function(evt) { evt.preventDefault(); $(this).hide() .next(".hidden") .removeClass("hidden"); }); }); - + this.setUpNotificationPage = function( contentArea ) { + self.notificationArea = contentArea; + contentArea.find(".unread,.read").each(function(index) { + if ( $(this).hasClass("unread") ) { + self.setUpUnread( $(this) ); + } else { + self.setUpRead( $(this) ); + } + }); + } + this.unreadClick = function() { + $.ajax({ + url: "/notifications/" + $(this).parent().data("guid"), + data: { set_unread: true }, + type: "PUT", + success: self.clickSuccess + }); + }; + this.readClick = function() { + $.ajax({ + url: "/notifications/" + $(this).data("guid"), + data: { set_unread: false }, + type: "PUT", + success: self.clickSuccess + }); + }; + this.setUpUnread = function( an_obj ) { + an_obj.removeClass("read").addClass( "unread" ); + an_obj.find('.unread-setter').hide(); + an_obj.find('.unread-setter').unbind("click"); + an_obj.unbind( "mouseenter mouseleave" ); + an_obj.click(self.readClick); + } + this.setUpRead = function( an_obj ) { + an_obj.removeClass("unread").addClass( "read" ); + an_obj.unbind( "click" ); + an_obj.find(".unread-setter").click(Diaspora.page.header.notifications.unreadClick); + an_obj.hover( + function () { + $(this).find(".unread-setter").show(); + }, + function () { + $(this).find(".unread-setter").hide(); + } + ); + } + this.clickSuccess = function( data ) { + var jsList = jQuery.parseJSON(data); + var itemID = jsList["guid"] + var isUnread = jsList["unread"] + if ( isUnread ) { + self.incrementCount(); + }else{ + self.decrementCount(); + } + self.notificationMenu.find('.read,.unread').each(function(index) { + if ( $(this).data("guid") == itemID ) { + if ( isUnread ) { + self.setUpUnread( $(this) ) + } else { + self.setUpRead( $(this) ) + } + } + }); + if ( self.notificationArea ) { + self.notificationArea.find('.read,.unread').each(function(index) { + if ( $(this).data("guid") == itemID ) { + if ( isUnread ) { + self.setUpUnread( $(this) ) + } else { + self.setUpRead( $(this) ) + } + } + }); + } + }; this.showNotification = function(notification) { - $(notification.html).prependTo(this.notificationArea) + $(notification.html).prependTo(this.notificationMenu) .fadeIn(200) .delay(8000) .fadeOut(200, function() { @@ -50,13 +117,20 @@ if(self.badge.text() !== "") { self.badge.text(self.count); + if ( self.notificationArea ) { + self.notificationArea.find( ".notification_count" ).text(self.count); - if(self.count === 0) { - self.badge.addClass("hidden"); - } - else if(self.count === 1) { - self.badge.removeClass("hidden"); - } + if(self.count === 0) { + self.badge.addClass("hidden"); + if ( self.notificationArea ) + self.notificationArea.find( ".notification_count" ).removeClass("unread"); + } + else if(self.count === 1) { + self.badge.removeClass("hidden"); + if ( self.notificationArea ) + self.notificationArea.find( ".notification_count" ).addClass("unread"); + } + } } }; diff --git a/spec/javascripts/widgets/notifications-spec.js b/spec/javascripts/widgets/notifications-spec.js index cbb962180..628bc413b 100644 --- a/spec/javascripts/widgets/notifications-spec.js +++ b/spec/javascripts/widgets/notifications-spec.js @@ -3,13 +3,46 @@ * the COPYRIGHT file. */ describe("Diaspora.Widgets.Notifications", function() { - var changeNotificationCountSpy, notifications; + var changeNotificationCountSpy, notifications, incrementCountSpy, decrementCountSpy; beforeEach(function() { spec.loadFixture("aspects_index"); - notifications = Diaspora.BaseWidget.instantiate("Notifications", $("#notifications"), $("#notification_badge .badge_count")); + this.view = new app.views.Header().render(); + + notifications = Diaspora.BaseWidget.instantiate("Notifications", this.view.$("#notification_badge .badge_count"), this.view.$(".notifications")); changeNotificationCountSpy = spyOn(notifications, "changeNotificationCount").andCallThrough(); + incrementCountSpy = spyOn(notifications, "incrementCount").andCallThrough(); + decrementCountSpy = spyOn(notifications, "decrementCount").andCallThrough(); + }); + + describe("clickSuccess", function(){ + it("changes the css to a read cell", function() { + this.view.$(".notifications").html( + '
' + + '
' + ); + notifications.clickSuccess(JSON.stringify({guid:2,unread:false})); + expect( this.view.$('.stream_element#2')).toHaveClass("read"); + }); + it("changes the css to an unread cell", function() { + this.view.$(".notifications").html( + '
' + + '
' + ); + notifications.clickSuccess(JSON.stringify({guid:1,unread:true})); + expect( this.view.$('.stream_element#1')).toHaveClass("unread"); + }); + + + it("calls Notifications.decrementCount on a read cell", function() { + notifications.clickSuccess(JSON.stringify({guid:1,unread:false})); + expect(notifications.decrementCount).toHaveBeenCalled(); + }); + it("calls Notifications.incrementCount on a unread cell", function() { + notifications.clickSuccess(JSON.stringify({guid:1,unread:true})); + expect(notifications.incrementCount).toHaveBeenCalled(); + }); }); describe("decrementCount", function() { @@ -40,13 +73,13 @@ describe("Diaspora.Widgets.Notifications", function() { describe("showNotification", function() { it("prepends a div to div#notifications", function() { - expect($("#notifications div").length).toEqual(0); + expect(this.view.$(".notifications div").length).toEqual(1); notifications.showNotification({ - html: '
' + html: '
' }); - expect($("#notifications div").length).toEqual(1); + expect(this.view.$(".notifications div").length).toEqual(2); }); it("only increments the notification count if specified to do so", function() { @@ -61,4 +94,4 @@ describe("Diaspora.Widgets.Notifications", function() { }); }); -}); +}); \ No newline at end of file diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index a619bb7ef..eb478784d 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -40,6 +40,21 @@ describe Notification do end end + describe 'set_read_state method' do + it "should set an unread notification to read" do + @note.unread = true + @note.set_read_state( true ) + @note.unread.should == false + end + it "should set an read notification to unread" do + @note.unread = false + @note.set_read_state( false ) + @note.unread.should == true + end + + end + + describe '.concatenate_or_create' do it 'creates a new notificiation if the notification does not exist, or if it is unread' do @note.unread = false From aae5fe04ceadc30c58698ff3891f99c272f4d096 Mon Sep 17 00:00:00 2001 From: Steven Fuchs Date: Tue, 17 Jan 2012 19:21:25 -0500 Subject: [PATCH 2/6] css for marking as unread, and hook for notification page to be part of it when called. --- app/views/notifications/index.html.haml | 61 +++++++++++++----------- public/stylesheets/sass/application.sass | 9 +++- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 88a098ab2..0e07e31ac 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -2,33 +2,40 @@ -self.extend AspectsHelper -self.extend ApplicationHelper -self.extend ErrorMessagesHelper -.span-13 - %h2 - %span.notification_count{:class => ('unread' if @notification_count > 0)} - = @notification_count - = t('.notifications') -.span-8.last - = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'button' +#notifications_content + .span-13 + %h2 + %span.notification_count{:class => ('unread' if @notification_count > 0)} + = @notification_count + = t('.notifications') + .span-8.last + = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'button' + .span-24.last + .stream.notifications + - group_days.each do |day, notes| + .day_group.span-24.last + .span-3 + .date + .day= the_day(day.split(' ')) + .month= the_month(day.split(' ')) -.span-24.last - .stream.notifications - - group_days.each do |day, notes| - .day_group.span-24.last - .span-3 - .date - .day= the_day(day.split(' ')) - .month= the_month(day.split(' ')) + .span-8.notifications_for_day + - notes.each do |note| + .stream_element{:data=>{:guid => note.id}, :class => "#{note.unread ? 'unread' : 'read'}"} + - if note.type == "Notifications::StartedSharing" && contact = current_user.contact_for(note[:target]) + .right + = aspect_membership_dropdown(contact, note[:target], 'left') - .span-8.notifications_for_day - - notes.each do |note| - .stream_element{:data=>{:guid => note.id}, :class => "#{note.unread ? 'unread' : ''}"} - - if note.type == "Notifications::StartedSharing" && contact = current_user.contact_for(note[:target]) - .right - = aspect_membership_dropdown(contact, note[:target], 'left') + %span.from + = notification_message_for(note) + %br + %time= timeago(note.created_at) + %a{:class => 'unread-setter'} + mark unread + .span-13.last + = will_paginate notifications - %span.from - = notification_message_for(note) - %br - %time= timeago(note.created_at) - .span-13.last - = will_paginate notifications +:javascript + $(document).ready(function(){ + Diaspora.page.header.notifications.setUpNotificationPage( $("#notifications_content" ) ); + }); diff --git a/public/stylesheets/sass/application.sass b/public/stylesheets/sass/application.sass index 047586227..20145c001 100644 --- a/public/stylesheets/sass/application.sass +++ b/public/stylesheets/sass/application.sass @@ -2987,6 +2987,12 @@ ul.left_nav .user_card :margin-left 8px +.unread-setter + :float right + :display none + :cursor pointer + :margin-top 8px + .stream_container :min-height 500px @@ -3139,7 +3145,8 @@ a.toggle_selector .notification_element :padding 10px :min-height 30px - + &:hover + :background-color #FAFAFA > img :height 30px :width 30px From 36fd43db4c04f574cfd03ef1e5612862832e16ef Mon Sep 17 00:00:00 2001 From: Steven Fuchs Date: Tue, 17 Jan 2012 19:27:48 -0500 Subject: [PATCH 3/6] incorporated improvements from @maxwell from last pull request. --- app/controllers/notifications_controller.rb | 4 ++-- app/models/notification.rb | 3 +++ .../notifications_controller_spec.rb | 20 ++++++++++++++++--- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index c27df96c7..2e8109db6 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -11,8 +11,8 @@ class NotificationsController < VannaController def update(opts=params) note = Notification.where(:recipient_id => current_user.id, :id => opts[:id]).first if note - note.update_attributes(:unread => false) - {} + note.set_read_state(opts[:set_unread] != "true" ) + { :guid => note.id, :unread => note.unread } else Response.new :status => 404 end diff --git a/app/models/notification.rb b/app/models/notification.rb index 547899379..0e8dfe013 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -36,6 +36,9 @@ class Notification < ActiveRecord::Base self.recipient.mail(self.mail_job, self.recipient_id, actor.id, target.id) end + def set_read_state( read_state ) + self.update_attributes( :unread => !read_state ) + end def mail_job raise NotImplementedError.new('Subclass this.') diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index 3f1e64b1f..ffa54f8a8 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -14,10 +14,24 @@ describe NotificationsController do end describe '#update' do - it 'marks a notification as read' do - note = Factory(:notification, :recipient => @user) + it 'marks a notification as read if it gets no other information' do + note = mock_model( Notification ) + Notification.should_receive( :where ).and_return( [note] ) + note.should_receive( :set_read_state ).with( true ) @controller.update :id => note.id - Notification.first.unread.should == false + end + it 'marks a notification as read if it is told to' do + note = mock_model( Notification ) + Notification.should_receive( :where ).and_return( [note] ) + note.should_receive( :set_read_state ).with( true ) + @controller.update :id => note.id, :set_unread => "false" + end + + it 'marks a notification as unread if it is told to' do + note = mock_model( Notification ) + Notification.should_receive( :where ).and_return( [note] ) + note.should_receive( :set_read_state ).with( false ) + @controller.update :id => note.id, :set_unread => "true" end it 'only lets you read your own notifications' do From e882dedd333dc45dafb6ba6a48fad73149bb02e9 Mon Sep 17 00:00:00 2001 From: Steven Fuchs Date: Sun, 22 Jan 2012 17:39:35 -0500 Subject: [PATCH 4/6] remove vanna controller from app, adjust scripts, specs, views and controllers to work. --- Gemfile | 1 - Gemfile.lock | 9 -- app/controllers/application_controller.rb | 10 +- app/controllers/notifications_controller.rb | 61 ++++----- app/controllers/vanna_controller.rb | 116 ------------------ app/helpers/application_helper.rb | 8 ++ app/helpers/notifications_helper.rb | 6 - .../notifications/_notify_popup_item.haml | 7 ++ app/views/notifications/index.html.haml | 6 +- app/views/templates/header.jst | 2 +- config/routes.rb | 2 +- public/javascripts/widgets/header.js | 2 +- .../widgets/notifications-badge.js | 31 ++--- public/javascripts/widgets/notifications.js | 55 ++++++--- .../notifications_controller_spec.rb | 32 ++--- .../javascripts/widgets/notifications-spec.js | 6 +- 16 files changed, 114 insertions(+), 240 deletions(-) delete mode 100644 app/controllers/vanna_controller.rb create mode 100644 app/views/notifications/_notify_popup_item.haml diff --git a/Gemfile b/Gemfile index 546de70bc..87a9bffb1 100644 --- a/Gemfile +++ b/Gemfile @@ -63,7 +63,6 @@ gem 'jammit', '0.6.5' # JSON and API gem 'json' -gem 'vanna', :git => 'git://github.com/MikeSofaer/vanna.git' gem 'acts_as_api' # localization diff --git a/Gemfile.lock b/Gemfile.lock index a7c4f983b..cbb7a7b77 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,14 +7,6 @@ GIT activesupport (>= 2.3.0) nokogiri (>= 1.3.3) -GIT - remote: git://github.com/MikeSofaer/vanna.git - revision: 334eec220dbfddcc6bd3108e6e6c77fec8484dc4 - specs: - vanna (0.1.1) - json - rails (>= 3.0.0) - GIT remote: git://github.com/binarylogic/settingslogic.git revision: 4884d455bf18d92723cb8190cfd2dbf87f3aafd5 @@ -499,7 +491,6 @@ DEPENDENCIES timecop twitter (= 2.0.2) typhoeus - vanna! webmock whenever will_paginate diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index cce1e7772..6639409e0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,6 +4,7 @@ class ApplicationController < ActionController::Base has_mobile_fu + include ApplicationHelper protect_from_forgery :except => :receive @@ -27,15 +28,6 @@ class ApplicationController < ActionController::Base request.env['HTTP_REFERER'] ||= '/aspects' end - # we need to do this for vanna controller. these should really be controller - # helper methods instead - def set_header_data - if user_signed_in? && request.format.html? && !params[:only_posts] - @notification_count = Notification.for(current_user, :unread =>true).count - @unread_message_count = ConversationVisibility.sum(:unread, :conditions => "person_id = #{current_user.person.id}") - end - end - # Overwriting the sign_out redirect path method def after_sign_out_path_for(resource_or_scope) # mobile_fu's is_mobile_device? wasn't working here for some reason... diff --git a/app/controllers/notifications_controller.rb b/app/controllers/notifications_controller.rb index 49f0db571..0673b8338 100644 --- a/app/controllers/notifications_controller.rb +++ b/app/controllers/notifications_controller.rb @@ -2,28 +2,30 @@ # licensed under the Affero General Public License version 3 or later. See # the COPYRIGHT file. -class NotificationsController < VannaController +class NotificationsController < ApplicationController include NotificationsHelper - include ActionController::MobileFu - has_mobile_fu - - def update(opts=params) - note = Notification.where(:recipient_id => current_user.id, :id => opts[:id]).first + def update + note = Notification.where(:recipient_id => current_user.id, :id => params[:id]).first if note - note.set_read_state(opts[:set_unread] != "true" ) - { :guid => note.id, :unread => note.unread } + note.set_read_state(params[:set_unread] != "true" ) + + respond_to do |format| + format.json { render :json => { :guid => note.id, :unread => note.unread } } + end + else - Response.new :status => 404 + respond_to do |format| + format.json { render :json => {}.to_json } + end end end - def index(opts=params) - @aspect = :notification + def index conditions = {:recipient_id => current_user.id} - page = opts[:page] || 1 - per_page = opts[:per_page] || 25 - notifications = WillPaginate::Collection.create(page, per_page, Notification.where(conditions).count ) do |pager| + page = params[:page] || 1 + per_page = params[:per_page] || 25 + @notifications = WillPaginate::Collection.create(page, per_page, Notification.where(conditions).count ) do |pager| result = Notification.find(:all, :conditions => conditions, :order => 'created_at desc', @@ -34,27 +36,26 @@ class NotificationsController < VannaController pager.replace(result) end - notifications.each do |n| - n[:actors] = n.actors - n[:translation] = notification_message_for(n) - n[:translation_key] = n.popup_translation_key - n[:target] = n.translation_key == "notifications.mentioned" ? n.target.post : n.target + @notifications.each do |n| + n[:note_html] = render_to_string( :partial => 'notify_popup_item', :locals => { :n => n } ) end - group_days = notifications.group_by{|note| I18n.l(note.created_at, :format => I18n.t('date.formats.fullmonth_day')) } - {:group_days => group_days, :notifications => notifications} + @group_days = @notifications.group_by{|note| I18n.l(note.created_at, :format => I18n.t('date.formats.fullmonth_day')) } + + respond_to do |format| + format.html + format.xml { render :xml => @notifications.to_xml } + format.json { render :json => @notifications.to_json } + end + end - def read_all(opts=params) + def read_all Notification.where(:recipient_id => current_user.id).update_all(:unread => false) - end - - post_process :html do - def post_read_all(json) - Response.new(:status => 302, :location => multi_stream_path) + respond_to do |format| + format.html { redirect_to notifications_path } + format.xml { render :xml => {}.to_xml } + format.json { render :json => {}.to_json } end end - def controller - Object.new - end end diff --git a/app/controllers/vanna_controller.rb b/app/controllers/vanna_controller.rb deleted file mode 100644 index 693f10dc9..000000000 --- a/app/controllers/vanna_controller.rb +++ /dev/null @@ -1,116 +0,0 @@ -# Copyright (c) 2010-2011, Diaspora Inc. This file is -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. - -class VannaController < Vanna::Base - include Devise::Controllers::Helpers - include AspectGlobalHelper - include PeopleHelper - include UsersHelper - - helper :layout - helper_method :current_user - helper_method :all_aspects - helper_method :flash - config.stylesheets_dir = "public/stylesheets" - layout "application" - include ActionController::Flash - default_url_options[:host] = "localhost" - include ActionController::MobileFu::InstanceMethods - helper_method :is_mobile_device? - - protect_from_forgery :except => :receive - - before_filter :authenticate_user! - before_filter :ensure_http_referer_is_set - before_filter :set_header_data, :except => [:create, :update] - before_filter :set_locale - before_filter :set_git_header if (AppConfig[:git_update] && AppConfig[:git_revision]) - before_filter :which_action_and_user - before_filter :all_aspects - before_filter :set_grammatical_gender - - def ensure_http_referer_is_set - request.env['HTTP_REFERER'] ||= '/aspects' - end - - def set_header_data - if user_signed_in? - if request.format.html? && !params[:only_posts] - @aspect = nil - @notification_count = Notification.for(current_user, :unread =>true).count - @unread_message_count = ConversationVisibility.sum(:unread, :conditions => "person_id = #{current_user.person.id}") - end - @all_aspects = current_user.aspects - end - end - - def ensure_page - params[:page] = params[:page] ? params[:page].to_i : 1 - end - - def all_aspects - @all_aspects ||= current_user.aspects - end - - def set_git_header - headers['X-Git-Update'] = AppConfig[:git_update] - headers['X-Git-Revision'] = AppConfig[:git_revision] - end - - def which_action_and_user - str = "event=request_with_user controller=#{self.class} action=#{self.action_name} " - if current_user - str << "uid=#{current_user.id} " - str << "user_created_at='#{current_user.created_at.to_date.to_s}' user_created_at_unix=#{current_user.created_at.to_i} " if current_user.created_at - str << "user_non_pending_contact_count=#{current_user.contacts.size} user_contact_count=#{Contact.unscoped.where(:user_id => current_user.id).size} " - else - str << 'uid=nil' - end - Rails.logger.info str - end - - def set_locale - if user_signed_in? - I18n.locale = current_user.language - else - I18n.locale = request.compatible_language_from AVAILABLE_LANGUAGE_CODES - end - - WillPaginate::ViewHelpers.pagination_options[:previous_label] = "« #{I18n.t('previous')}" - WillPaginate::ViewHelpers.pagination_options[:next_label] = "#{I18n.t('next')} »" - end - - def redirect_unless_admin - unless current_user.admin? - redirect_to multi_stream_path, :notice => 'you need to be an admin to do that' - return - end - end - - def set_grammatical_gender - if (user_signed_in? && I18n.inflector.inflected_locale?) - gender = current_user.profile.gender.to_s.tr('!()[]"\'`*=|/\#.,-:', '').downcase - unless gender.empty? - i_langs = I18n.inflector.inflected_locales(:gender) - i_langs.delete I18n.locale - i_langs.unshift I18n.locale - i_langs.each do |lang| - token = I18n.inflector.true_token(gender, :gender, lang) - unless token.nil? - @grammatical_gender = token - break - end - end - end - end - end - - def grammatical_gender - @grammatical_gender || nil - end - - def after_sign_in_path_for(resource) - stored_location_for(:user) || aspects_path(:a_ids => current_user.aspects.where(:open => true).select(:id).all.map{|a| a.id}) - end -end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 629b6cabc..9601b2c98 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -3,6 +3,14 @@ # the COPYRIGHT file. module ApplicationHelper + + def set_header_data + if user_signed_in? && request.format.html? && !params[:only_posts] + @notification_count = Notification.for(current_user, :unread =>true).count + @unread_message_count = ConversationVisibility.sum(:unread, :conditions => "person_id = #{current_user.person.id}") + end + end + def how_long_ago(obj) timeago(obj.created_at) end diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index fdcd24d6e..6f25e8cc8 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -1,11 +1,5 @@ module NotificationsHelper - include ERB::Util - include ActionView::Helpers::TranslationHelper - include ActionView::Helpers::UrlHelper include PeopleHelper - include UsersHelper - include ApplicationHelper - include LanguageHelper def object_link(note, actors) target_type = note.popup_translation_key diff --git a/app/views/notifications/_notify_popup_item.haml b/app/views/notifications/_notify_popup_item.haml new file mode 100644 index 000000000..7478eef96 --- /dev/null +++ b/app/views/notifications/_notify_popup_item.haml @@ -0,0 +1,7 @@ +.notification_element{:data=>{:guid => n.id},:class => (n.unread ? "unread" : "read")} + %img{:src => n.actors.first.profile.image_url(:thumb_medium)} + = notification_message_for(n) + %br/ + %abbr.timeago{:title=>n.created_at.iso8601} + %a{:class => 'unread-setter'} + mark unread diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 0e07e31ac..ab096ece9 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -9,10 +9,10 @@ = @notification_count = t('.notifications') .span-8.last - = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'button' + = link_to t('.mark_all_as_read'), notifications_read_all_path, :class => 'button' .span-24.last .stream.notifications - - group_days.each do |day, notes| + - @group_days.each do |day, notes| .day_group.span-24.last .span-3 .date @@ -33,7 +33,7 @@ %a{:class => 'unread-setter'} mark unread .span-13.last - = will_paginate notifications + = will_paginate @notifications :javascript $(document).ready(function(){ diff --git a/app/views/templates/header.jst b/app/views/templates/header.jst index fe26bf528..4b11290a8 100644 --- a/app/views/templates/header.jst +++ b/app/views/templates/header.jst @@ -39,7 +39,7 @@
- + <%= Diaspora.I18n.t("header.mark_all_as_read") %> | diff --git a/config/routes.rb b/config/routes.rb index de5a45cfa..260610739 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,8 +49,8 @@ Diaspora::Application.routes.draw do delete 'visibility' => 'conversation_visibilities#destroy' end + get 'notifications/read_all' => 'notifications#read_all' resources :notifications, :only => [:index, :update] do - get :read_all, :on => :collection end resources :tags, :only => [:index] diff --git a/public/javascripts/widgets/header.js b/public/javascripts/widgets/header.js index 10fd0d996..a1fa4ec88 100644 --- a/public/javascripts/widgets/header.js +++ b/public/javascripts/widgets/header.js @@ -5,7 +5,7 @@ this.subscribe("widget/ready", function(evt, header) { self.notifications = self.instantiate("Notifications", header.find("#notification_badge .badge_count"), - header.find(".notifications") + header.find("#notification_dropdown") ); self.notificationsDropdown = self.instantiate("NotificationsDropdown", diff --git a/public/javascripts/widgets/notifications-badge.js b/public/javascripts/widgets/notifications-badge.js index d69666fe3..cf589eeb4 100644 --- a/public/javascripts/widgets/notifications-badge.js +++ b/public/javascripts/widgets/notifications-badge.js @@ -59,30 +59,19 @@ this.renderNotifications = function() { self.dropdownNotifications.empty(); - $.each(self.notifications.notifications, function(index, notifications) { + $.each(self.notifications, function(index, notifications) { $.each(notifications, function(index, notification) { - var notificationElement = $("
") - .addClass("notification_element") - .data( "guid", notification.id ) - .html(notification.translation) - .prepend($("", { src: notification.actors[0].avatar })) - .append("
") - .append($("", { - "class": "timeago", - "title": notification.created_at - })) - .append('mark unread') - .appendTo(self.dropdownNotifications); - - notificationElement.find("abbr.timeago").timeago(); - - if(notification.unread) { - Diaspora.page.header.notifications.setUpUnread( notificationElement ); - }else{ - Diaspora.page.header.notifications.setUpRead( notificationElement ); - } + self.dropdownNotifications.append(notification.note_html); }); }); + self.dropdownNotifications.find("abbr.timeago").timeago(); + + self.dropdownNotifications.find('.unread').each(function(index) { + Diaspora.page.header.notifications.setUpUnread( $(this) ); + }); + self.dropdownNotifications.find('.read').each(function(index) { + Diaspora.page.header.notifications.setUpRead( $(this) ); + }); self.ajaxLoader.hide(); }; diff --git a/public/javascripts/widgets/notifications.js b/public/javascripts/widgets/notifications.js index ce59e0895..eddfdc9de 100644 --- a/public/javascripts/widgets/notifications.js +++ b/public/javascripts/widgets/notifications.js @@ -22,6 +22,25 @@ .next(".hidden") .removeClass("hidden"); }); + self.notificationMenu.find('a#mark_all_read_link').click(function() { + $.ajax({ + url: "/notifications/read_all", + type: "GET", + dataType:'json', + success: function(){ + self.notificationMenu.find('.unread').each(function(index) { + self.setUpRead( $(this) ); + }); + if ( self.notificationArea ) { + self.notificationArea.find('.unread').each(function(index) { + self.setUpRead( $(this) ); + }); + } + self.resetCount(); + } + }); + return false; + }); }); this.setUpNotificationPage = function( contentArea ) { self.notificationArea = contentArea; @@ -70,9 +89,8 @@ ); } this.clickSuccess = function( data ) { - var jsList = jQuery.parseJSON(data); - var itemID = jsList["guid"] - var isUnread = jsList["unread"] + var itemID = data["guid"] + var isUnread = data["unread"] if ( isUnread ) { self.incrementCount(); }else{ @@ -115,23 +133,24 @@ this.changeNotificationCount = function(change) { self.count += change; - if(self.badge.text() !== "") { - self.badge.text(self.count); - if ( self.notificationArea ) { - self.notificationArea.find( ".notification_count" ).text(self.count); + self.badge.text(self.count); + if ( self.notificationArea ) + self.notificationArea.find( ".notification_count" ).text(self.count); - if(self.count === 0) { - self.badge.addClass("hidden"); - if ( self.notificationArea ) - self.notificationArea.find( ".notification_count" ).removeClass("unread"); - } - else if(self.count === 1) { - self.badge.removeClass("hidden"); - if ( self.notificationArea ) - self.notificationArea.find( ".notification_count" ).addClass("unread"); - } - } + if(self.count === 0) { + self.badge.addClass("hidden"); + if ( self.notificationArea ) + self.notificationArea.find( ".notification_count" ).removeClass("unread"); } + else if(self.count === 1) { + self.badge.removeClass("hidden"); + if ( self.notificationArea ) + self.notificationArea.find( ".notification_count" ).addClass("unread"); + } + }; + this.resetCount = function(change) { + self.count = 0; + this.changeNotificationCount(0); }; this.decrementCount = function() { diff --git a/spec/controllers/notifications_controller_spec.rb b/spec/controllers/notifications_controller_spec.rb index ffa54f8a8..eedb697f4 100644 --- a/spec/controllers/notifications_controller_spec.rb +++ b/spec/controllers/notifications_controller_spec.rb @@ -18,20 +18,20 @@ describe NotificationsController do note = mock_model( Notification ) Notification.should_receive( :where ).and_return( [note] ) note.should_receive( :set_read_state ).with( true ) - @controller.update :id => note.id + get :update, "id" => note.id end it 'marks a notification as read if it is told to' do note = mock_model( Notification ) Notification.should_receive( :where ).and_return( [note] ) note.should_receive( :set_read_state ).with( true ) - @controller.update :id => note.id, :set_unread => "false" + get :update, "id" => note.id, :set_unread => "false" end it 'marks a notification as unread if it is told to' do note = mock_model( Notification ) Notification.should_receive( :where ).and_return( [note] ) note.should_receive( :set_read_state ).with( false ) - @controller.update :id => note.id, :set_unread => "true" + get :update, "id" => note.id, :set_unread => "true" end it 'only lets you read your own notifications' do @@ -40,7 +40,7 @@ describe NotificationsController do Factory(:notification, :recipient => @user) note = Factory(:notification, :recipient => user2) - @controller.update :id => note.id + get :update, "id" => note.id, :set_unread => "false" Notification.find(note.id).unread.should == true end @@ -53,7 +53,7 @@ describe NotificationsController do Factory(:notification, :recipient => @user) Notification.where(:unread => true).count.should == 2 - @controller.read_all({}) + get :read_all Notification.where(:unread => true).count.should == 0 end end @@ -66,26 +66,16 @@ describe NotificationsController do it 'paginates the notifications' do 25.times { Factory(:notification, :recipient => @user, :target => @post) } - - @controller.index({})[:notifications].count.should == 25 - @controller.index(:page => 2)[:notifications].count.should == 1 - end - - it "includes the actors" do - Factory(:notification, :recipient => @user, :target => @post) - response = @controller.index({}) - response[:notifications].first[:actors].first.should be_a(Person) - end - - it 'eager loads the target' do - response = @controller.index({}) - response[:notifications].each { |note| note[:target].should be } + get :index + assigns[:notifications].count.should == 25 + get :index, "page" => 2 + assigns[:notifications].count.should == 1 end it "supports a limit per_page parameter" do 5.times { Factory(:notification, :recipient => @user, :target => @post) } - response = @controller.index({:per_page => 5}) - response[:notifications].length.should == 5 + get :index, "per_page" => 5 + assigns[:notifications].count.should == 5 end end end diff --git a/spec/javascripts/widgets/notifications-spec.js b/spec/javascripts/widgets/notifications-spec.js index 628bc413b..6f5e84dce 100644 --- a/spec/javascripts/widgets/notifications-spec.js +++ b/spec/javascripts/widgets/notifications-spec.js @@ -22,7 +22,7 @@ describe("Diaspora.Widgets.Notifications", function() { '
' + '
' ); - notifications.clickSuccess(JSON.stringify({guid:2,unread:false})); + notifications.clickSuccess({guid:2,unread:false}); expect( this.view.$('.stream_element#2')).toHaveClass("read"); }); it("changes the css to an unread cell", function() { @@ -30,7 +30,7 @@ describe("Diaspora.Widgets.Notifications", function() { '
' + '
' ); - notifications.clickSuccess(JSON.stringify({guid:1,unread:true})); + notifications.clickSuccess({guid:1,unread:true}); expect( this.view.$('.stream_element#1')).toHaveClass("unread"); }); @@ -40,7 +40,7 @@ describe("Diaspora.Widgets.Notifications", function() { expect(notifications.decrementCount).toHaveBeenCalled(); }); it("calls Notifications.incrementCount on a unread cell", function() { - notifications.clickSuccess(JSON.stringify({guid:1,unread:true})); + notifications.clickSuccess({guid:1,unread:true}); expect(notifications.incrementCount).toHaveBeenCalled(); }); }); From 052690941bc85a279cb637ff0075ce0a46f86028 Mon Sep 17 00:00:00 2001 From: Steven Fuchs Date: Sun, 22 Jan 2012 23:23:21 -0500 Subject: [PATCH 5/6] move set_header_data back into application_controller --- app/controllers/application_controller.rb | 7 +++++++ app/helpers/application_helper.rb | 7 ------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6639409e0..726d7f3d6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,6 +24,13 @@ class ApplicationController < ActionController::Base :tags, :open_publisher + def set_header_data + if user_signed_in? && request.format.html? && !params[:only_posts] + @notification_count = Notification.for(current_user, :unread =>true).count + @unread_message_count = ConversationVisibility.sum(:unread, :conditions => "person_id = #{current_user.person.id}") + end + end + def ensure_http_referer_is_set request.env['HTTP_REFERER'] ||= '/aspects' end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 9601b2c98..a32d76a4b 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -4,13 +4,6 @@ module ApplicationHelper - def set_header_data - if user_signed_in? && request.format.html? && !params[:only_posts] - @notification_count = Notification.for(current_user, :unread =>true).count - @unread_message_count = ConversationVisibility.sum(:unread, :conditions => "person_id = #{current_user.person.id}") - end - end - def how_long_ago(obj) timeago(obj.created_at) end From 7cbcaafbeb3582437b5cbd96d6c4df97de9e7532 Mon Sep 17 00:00:00 2001 From: Steven Fuchs Date: Sun, 22 Jan 2012 23:28:55 -0500 Subject: [PATCH 6/6] oops, application_helper gone from application_controller. --- app/controllers/application_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 726d7f3d6..b34d46fbc 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,8 +4,6 @@ class ApplicationController < ActionController::Base has_mobile_fu - include ApplicationHelper - protect_from_forgery :except => :receive before_filter :ensure_http_referer_is_set