grammer nazi cleanup of notification_people_link and refactor + tests

app/helpers/notifications_helper.rb

@@ -49,11 +49,20 @@ module NotificationsHelper
   def notification_people_link(note)
     actors = note.actors
     number_of_actors = actors.count
-    actor_links = actors.collect{ |person| link_to("#{h(person.name.titlecase)}", person_path(person))}
+    sentence_translations = {:two_words_connector => " #{t('notifications.index.and')} ", :last_word_connector => ", #{t('notifications.index.and')} " }
+    actor_links = actors.collect{ |person| link_to("#{h(person.name.titlecase.strip)}", person_path(person))}
+    
     if number_of_actors < 4
-      message =  actor_links.join(', ')
+      message = actor_links.to_sentence(sentence_translations)
     else
-      message  = actor_links[0..2].join(', ') << "<a class='more' href='#'> #{t('.and_others', :number =>(number_of_actors - 3))}</a><span class='hidden'>, " << actor_links[3..(number_of_actors-2)].join(', ')<< " #{t('.and')} "<< actor_links.last << '</span>'
+      first, second, third, *others = actor_links
+      others_sentence = others.to_sentence(sentence_translations)
+      if others.count == 1
+        others_sentence = " #{t('notifications.index.and')} " + others_sentence
+      end
+      message = "#{first}, #{second}, #{third},"
+      message += "<a class='more' href='#'> #{t('notifications.index.and_others', :number =>(number_of_actors - 3))}</a>"
+      message += "<span class='hidden'> #{others_sentence} </span>"
     end
     message.html_safe
   end

spec/helpers/notifications_helper_spec.rb

@@ -13,6 +13,48 @@ describe NotificationsHelper do
     
   end
   describe '#notification_people_link' do
+    context 'formatting' do
+      include ActionView::Helpers::SanitizeHelper
+      let(:output){ strip_tags(notification_people_link(@note)) }
+
+      before do
+        @max = Factory(:person)
+        @max.profile.first_name = 'max'
+        @max.profile.last_name = 'salzberg'
+        @sarah = Factory(:person)
+        @sarah.profile.first_name = 'sarah'
+        @sarah.profile.last_name = 'mei'
+
+
+        @daniel = Factory(:person)
+        @daniel.profile.first_name = 'daniel'
+        @daniel.profile.last_name = 'grippi'
+
+        @ilya = Factory(:person)
+        @ilya.profile.first_name = 'ilya'
+        @ilya.profile.last_name = 'zhit'
+        @note = mock()
+      end
+
+      it 'with two, does not comma seperate two actors' do
+        @note.stub!(:actors).and_return([@max, @sarah])
+        output.scan(/,/).should be_empty
+        output.scan(/and/).count.should be 1
+      end
+
+      it 'with three, comma seperates the first two, and and the last actor' do
+        @note.stub!(:actors).and_return([@max, @sarah, @daniel])
+        output.scan(/,/).count.should be 2
+        output.scan(/and/).count.should be 1
+      end
+
+      it 'with more than three, lists the first three, then the others tag' do
+        @note.stub!(:actors).and_return([@max, @sarah, @daniel, @ilya])
+        puts output
+        output.scan(/,/).count.should be 3
+        output.scan(/and/).count.should be 2
+      end
+    end
     describe 'for a like' do
       it 'displays #{list of actors}' do
         output = notification_people_link(@notification)