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(); }); });