From aab0a438fd2db192da0645355e8c31da4fda104e Mon Sep 17 00:00:00 2001
From: Miles Gould <miles@assyrian.org.uk>
Date: Tue, 30 Apr 2013 12:39:03 +0100
Subject: [PATCH] Replace blank/nil notification subjects with (no subject)

---
 app/models/notification.rb                    |  7 +++
 spec/models/notification_spec.rb              | 10 ++++
 .../notifications/index.html.haml_spec.rb     | 46 +++++++++++++++----
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/app/models/notification.rb b/app/models/notification.rb
index 1be9da637..3fc48a0c7 100644
--- a/app/models/notification.rb
+++ b/app/models/notification.rb
@@ -9,12 +9,19 @@ class Notification < ActiveRecord::Base
   default_scope order('created_at DESC')
   scope :unread, where(:read => false)
 
+  before_create :replace_blank_subject
   after_create :send_email
 
   def self.unread_count
     self.unread.count
   end
 
+  def replace_blank_subject
+    if self.subject.nil? or self.subject =~ /^\s*$/
+      self.subject = "(no subject)"
+    end
+  end
+
   def send_email
     if self.recipient.send_notification_email
       Notifier.notify(self).deliver
diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb
index 8ebaba010..a8bddb9fa 100644
--- a/spec/models/notification_spec.rb
+++ b/spec/models/notification_spec.rb
@@ -48,4 +48,14 @@ describe Notification do
     ActionMailer::Base.deliveries.last.to.should == [@notification2.recipient.email]
   end
 
+  it "replaces missing subjects with (no subject)" do
+    @notification = FactoryGirl.create(:notification, :subject => nil)
+    @notification.subject.should == "(no subject)"
+  end
+
+  it "replaces whitespace-only subjects with (no subject)" do
+    @notification = FactoryGirl.create(:notification, :subject => "    ")
+    @notification.subject.should == "(no subject)"
+  end
+
 end
diff --git a/spec/views/notifications/index.html.haml_spec.rb b/spec/views/notifications/index.html.haml_spec.rb
index 0e65dc93f..351bbb44f 100644
--- a/spec/views/notifications/index.html.haml_spec.rb
+++ b/spec/views/notifications/index.html.haml_spec.rb
@@ -4,19 +4,45 @@ describe "notifications/index" do
   before(:each) do
     @member = FactoryGirl.create(:member)
     controller.stub(:current_user) { @member }
-    @notification = FactoryGirl.create(:notification, :sender => @member, :recipient => @member)
-    assign(:notifications, [ @notification, @notification ])
-    render
   end
 
-  it "renders a list of notifications" do
-    # Run the generator again with the --webrat flag if you want to use webrat matchers
-    assert_select "table"
-    assert_select "tr>td", :text => @notification.sender.to_s, :count => 2
-    assert_select "tr>td", :text => @notification.subject, :count => 2
+  context "ordinary notifications" do
+    before(:each) do
+      @notification = FactoryGirl.create(:notification, :sender => @member,
+                                         :recipient => @member)
+      assign(:notifications, [ @notification, @notification ])
+      render
+    end
+
+    it "renders a list of notifications" do
+      assert_select "table"
+      assert_select "tr>td", :text => @notification.sender.to_s, :count => 2
+      assert_select "tr>td", :text => @notification.subject, :count => 2
+    end
+
+    it "links to sender's profile" do
+      assert_select "a", :href => member_path(@notification.sender)
+    end
   end
 
-  it "links to sender's profile" do
-    assert_select "a", :href => member_path(@notification.sender)
+  context "no subject" do
+    it "shows (no subject)" do
+      @notification = FactoryGirl.create(:notification,
+         :sender => @member, :recipient => @member, :subject => nil)
+      assign(:notifications, [@notification])
+      render
+      rendered.should contain "(no subject)"
+    end
   end
+
+  context "whitespace-only subject" do
+    it "shows (no subject)" do
+      @notification = FactoryGirl.create(:notification,
+         :sender => @member, :recipient => @member, :subject => "   ")
+      assign(:notifications, [@notification])
+      render
+      rendered.should contain "(no subject)"
+    end
+  end
+
 end