Merge pull request #2714 from stwf/im-sorry-bye-vanna

Remove Vanna Controller from app and fix up notifications controller, views, scripts, specs

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

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

app/controllers/application_controller.rb

@@ -4,7 +4,6 @@
 
 class ApplicationController < ActionController::Base
   has_mobile_fu
-
   protect_from_forgery :except => :receive
 
   before_filter :ensure_http_referer_is_set
@@ -23,12 +22,6 @@ class ApplicationController < ActionController::Base
                 :tags,
                 :open_publisher
 
-  def ensure_http_referer_is_set
-    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
@@ -36,6 +29,10 @@ class ApplicationController < ActionController::Base
     end
   end
 
+  def ensure_http_referer_is_set
+    request.env['HTTP_REFERER'] ||= '/aspects'
+  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...

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
+  def update
-  has_mobile_fu
+    note = Notification.where(:recipient_id => current_user.id, :id => params[:id]).first
-
-  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(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)
+  def index
-    @aspect = :notification
     conditions = {:recipient_id => current_user.id}
-    page = opts[:page] || 1
+    page = params[:page] || 1
-    per_page = opts[:per_page] || 25
+    per_page = params[:per_page] || 25
-    notifications = WillPaginate::Collection.create(page, per_page, Notification.where(conditions).count ) do |pager|
+    @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|
+    @notifications.each do |n|
-      n[:actors] = n.actors
+      n[:note_html] = render_to_string( :partial => 'notify_popup_item', :locals => { :n => n } )
-      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
     end
-    group_days = notifications.group_by{|note| I18n.l(note.created_at, :format => I18n.t('date.formats.fullmonth_day')) }
+    @group_days = @notifications.group_by{|note| I18n.l(note.created_at, :format => I18n.t('date.formats.fullmonth_day')) }
-    {:group_days => group_days, :notifications => notifications}
+
+    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
+    respond_to do |format|
-
+      format.html { redirect_to notifications_path }
-  post_process :html do
+      format.xml { render :xml => {}.to_xml }
-    def post_read_all(json)
+      format.json { render :json => {}.to_json }
-      Response.new(:status => 302, :location => multi_stream_path)
     end
   end
 
-  def controller
-    Object.new
-  end
 end

app/controllers/vanna_controller.rb

@@ -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] = "&laquo; #{I18n.t('previous')}"
-    WillPaginate::ViewHelpers.pagination_options[:next_label] = "#{I18n.t('next')} &raquo;"
-  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

app/helpers/application_helper.rb

@@ -3,6 +3,7 @@
 #   the COPYRIGHT file.
 
 module ApplicationHelper
+
   def how_long_ago(obj)
     timeago(obj.created_at)
   end

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

app/models/notification.rb

@@ -37,6 +37,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.')

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

app/views/notifications/index.html.haml

@@ -2,33 +2,40 @@
 -self.extend AspectsHelper
 -self.extend ApplicationHelper
 -self.extend ErrorMessagesHelper
-.span-13
+#notifications_content
-  %h2
+  .span-13
-    %span.notification_count{:class => ('unread' if @notification_count > 0)}
+    %h2
-      = @notification_count
+      %span.notification_count{:class => ('unread' if @notification_count > 0)}
-    = t('.notifications')
+        = @notification_count
-.span-8.last
+      = t('.notifications')
-  = link_to t('.mark_all_as_read'), read_all_notifications_path, :class => 'button'
+  .span-8.last
+    = link_to t('.mark_all_as_read'), notifications_read_all_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
+          .span-8.notifications_for_day
-  .stream.notifications
+            - notes.each do |note|
-    - group_days.each do |day, notes|
+              .stream_element{:data=>{:guid => note.id}, :class => "#{note.unread ? 'unread' : 'read'}"}
-      .day_group.span-24.last
+                - if note.type == "Notifications::StartedSharing" && contact = current_user.contact_for(note[:target])
-        .span-3
+                  .right
-          .date
+                    = aspect_membership_dropdown(contact, note[:target], 'left')
-            .day= the_day(day.split(' '))
-            .month= the_month(day.split(' '))
 
-        .span-8.notifications_for_day
+                %span.from
-          - notes.each do |note|
+                  = notification_message_for(note)
-            .stream_element{:data=>{:guid => note.id}, :class => "#{note.unread ? 'unread' : ''}"}
+                %br
-              - if note.type == "Notifications::StartedSharing" && contact = current_user.contact_for(note[:target])
+                %time= timeago(note.created_at)
-                .right
+                %a{:class => 'unread-setter'}
-                  = aspect_membership_dropdown(contact, note[:target], 'left')
+                  mark unread
+          .span-13.last
+      = will_paginate @notifications
 
-              %span.from
+:javascript
-                = notification_message_for(note)
+  $(document).ready(function(){
-              %br
+    Diaspora.page.header.notifications.setUpNotificationPage( $("#notifications_content" ) );
-              %time= timeago(note.created_at)
+  });
-        .span-13.last
-    = will_paginate notifications

app/views/templates/header.jst

@@ -39,7 +39,7 @@
   <div id="notification_dropdown">
     <div class="header">
       <div class="right">
-        <a href="/notifications/read_all">
+        <a href="#" id="mark_all_read_link">
           <%= Diaspora.I18n.t("header.mark_all_as_read") %>
         </a>
         |

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]

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("#notification_dropdown")
       );
 
       self.notificationsDropdown = self.instantiate("NotificationsDropdown",

public/javascripts/widgets/notifications-badge.js

@@ -59,33 +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 = $("<div/>")
+          self.dropdownNotifications.append(notification.note_html);
-            .addClass("notification_element")
-            .html(notification.translation)
-            .prepend($("<img/>", { src: notification.actors[0].avatar }))
-            .append("<br />")
-            .append($("<abbr/>", {
-              "class": "timeago",
-              "title": notification.created_at
-            }))
-            .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();
-              }
-            });
-          }
         });
       });
+      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();
     };

public/javascripts/widgets/notifications.js

@@ -8,32 +8,117 @@
   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() {
+      $("a.more").click( function(evt) {
-        self.decrementCount();
-
-        $.ajax({
-          url: "notifications/" + $(this).removeClass("unread").data("guid"),
-          type: "PUT"
-        });
-      });
-
-      $("a.more").live("click", function(evt) {
         evt.preventDefault();
         $(this).hide()
           .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;
+      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 itemID = data["guid"]
+      var isUnread = data["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() {
@@ -48,16 +133,24 @@
     this.changeNotificationCount = function(change) {
       self.count += change;
 
-      if(self.badge.text() !== "") {
+      self.badge.text(self.count);
-				self.badge.text(self.count);
+      if ( self.notificationArea )
+        self.notificationArea.find( ".notification_count" ).text(self.count);
 
-				if(self.count === 0) {
+      if(self.count === 0) {
-	  			self.badge.addClass("hidden");
+        self.badge.addClass("hidden");
-				}
+        if ( self.notificationArea )
-				else if(self.count === 1) {
+          self.notificationArea.find( ".notification_count" ).removeClass("unread");
-	  			self.badge.removeClass("hidden");
-				}
       }
+      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() {

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

spec/controllers/notifications_controller_spec.rb

@@ -14,10 +14,24 @@ describe NotificationsController do
   end
 
   describe '#update' do
-    it 'marks a notification as read' do
+    it 'marks a notification as read if it gets no other information' do
-      note = Factory(:notification, :recipient => @user)
+      note = mock_model( Notification )
-      @controller.update :id => note.id
+      Notification.should_receive( :where ).and_return( [note] )
-      Notification.first.unread.should == false
+      note.should_receive( :set_read_state ).with( true )
+      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 )
+      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 )
+      get :update, "id" => note.id, :set_unread => "true"
     end
 
     it 'only lets you read your own notifications' do
@@ -26,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
@@ -39,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
@@ -52,26 +66,16 @@ describe NotificationsController do
 
     it 'paginates the notifications' do
       25.times { Factory(:notification, :recipient => @user, :target => @post) }
-
+      get :index
-      @controller.index({})[:notifications].count.should == 25
+      assigns[:notifications].count.should == 25
-      @controller.index(:page => 2)[:notifications].count.should == 1
+      get :index, "page" => 2
-    end
+      assigns[:notifications].count.should == 1
-
-    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 }
     end
 
     it "supports a limit per_page parameter" do
       5.times { Factory(:notification, :recipient => @user, :target => @post) }
-      response = @controller.index({:per_page => 5})
+      get :index, "per_page" => 5
-      response[:notifications].length.should == 5
+      assigns[:notifications].count.should == 5 
     end
   end
 end

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(
+        '<div id="1" class="stream_element read" data-guid=1></div>' +
+        '<div id="2" class="stream_element unread" data-guid=2></div>'
+      );
+      notifications.clickSuccess({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(
+        '<div id="1" class="stream_element read" data-guid=1></div>' +
+        '<div id="2" class="stream_element unread" data-guid=2></div>'
+      );
+      notifications.clickSuccess({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({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: '<div class="notification"></div>'
+        html: '<div class="notification_element"></div>'
       });
 
-      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() {
 
     });
   });
-});
+});

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