From 17a2c9e311b1dfa53a527a3a3c3a4d37b185f062 Mon Sep 17 00:00:00 2001 From: zhitomirskiyi Date: Sat, 29 Jan 2011 23:34:11 -0800 Subject: [PATCH] moving over some code migration successful --- app/helpers/notifications_helper.rb | 20 ++++++--- app/helpers/sockets_helper.rb | 2 +- app/models/notification.rb | 45 ++++++++++++++----- app/models/person.rb | 2 +- app/views/notifications/_popup.haml | 2 +- app/views/notifications/index.html.haml | 2 +- app/views/shared/_notification.haml | 2 +- ...130072907_notification_multiple_people.rb} | 6 +-- db/schema.rb | 13 +++++- spec/models/notification_spec.rb | 22 ++++++--- spec/models/person_spec.rb | 2 +- 11 files changed, 84 insertions(+), 34 deletions(-) rename db/migrate/{20110110023610_notification_multiple_people.rb => 20110130072907_notification_multiple_people.rb} (97%) diff --git a/app/helpers/notifications_helper.rb b/app/helpers/notifications_helper.rb index e5c739667..5c97f7dd1 100644 --- a/app/helpers/notifications_helper.rb +++ b/app/helpers/notifications_helper.rb @@ -8,16 +8,16 @@ module NotificationsHelper when 'new_request' translation when 'comment_on_post' - comment = Comment.where(:id => note.target_id).first - if comment - "#{translation} #{link_to t('notifications.post'), object_path(comment.post)}".html_safe + post = Post.where(:id => note.target_id).first + if post + "#{translation} #{link_to t('notifications.post'), object_path(post)}".html_safe else "#{translation} #{t('notifications.deleted')} #{t('notifications.post')}" end when 'also_commented' - comment = Comment.where(:id => note.target_id).first - if comment - "#{translation} #{link_to t('notifications.post'), object_path(comment.post)}".html_safe + post = Post.where(:id => note.target_id).first + if post + "#{translation} #{link_to t('notifications.post'), object_path(post)}".html_safe else "#{translation} #{t('notifications.deleted')} #{t('notifications.post')}" end @@ -38,4 +38,12 @@ module NotificationsHelper link_to new_notification_text(count), notifications_path end end + + def notification_people_link(note) + note.actors.collect{ |person| link_to("#{person.name.titlecase}", person_path(person))}.join(" , ").html_safe + end + + def peoples_names(note) + note.actors.map{|p| p.name}.join(",") + end end diff --git a/app/helpers/sockets_helper.rb b/app/helpers/sockets_helper.rb index ac8ef29e0..1c6ee13ac 100644 --- a/app/helpers/sockets_helper.rb +++ b/app/helpers/sockets_helper.rb @@ -51,7 +51,7 @@ module SocketsHelper v = render_to_string(:partial => 'comments/comment', :locals => {:comment => object, :person => object.person}) elsif object.is_a? Notification - v = render_to_string(:partial => 'notifications/popup', :locals => {:note => object, :person => object.actor}) + v = render_to_string(:partial => 'notifications/popup', :locals => {:note => object, :person => opts[:actor]}) else raise "#{object.inspect} with class #{object.class} is not actionhashable." unless object.is_a? Retraction diff --git a/app/models/notification.rb b/app/models/notification.rb index e07ec0bf5..c34a1b3e9 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -18,27 +18,50 @@ class Notification < ActiveRecord::Base def self.notify(recipient, target, actor) if target.respond_to? :notification_type if action = target.notification_type(recipient, actor) - n = create(:target => target, - :action => action, - :actor => actor, - :recipient => recipient) - n.email_the_user if n - n.socket_to_user(recipient) if n + if target.is_a? Comment + n = concatenate_or_create(recipient, target.post, actor, action) + else + n = make_notification(recipient, target, actor, action) + end + n.email_the_user(actor) if n + n.socket_to_uid(user, :actor => person) if n n end end end - def email_the_user + def email_the_user(comment, actor) case self.action when "new_request" - self.recipient.mail(Job::MailRequestReceived, self.recipient_id, self.actor_id) + self.recipient.mail(Job::MailRequestReceived, self.recipient_id, actor.id) when "request_accepted" - self.recipient.mail(Job::MailRequestAcceptance, self.recipient_id, self.actor_id) + self.recipient.mail(Job::MailRequestAcceptance, self.recipient_id, actor.id) when "comment_on_post" - self.recipient.mail(Job::MailCommentOnPost, self.recipient_id, self.actor_id, target.id) + self.recipient.mail(Job::MailCommentOnPost, self.recipient_id, actor.id, comment.id) when "also_commented" - self.recipient.mail(Job::MailAlsoCommented, self.recipient_id, self.actor_id, target.id) + self.recipient.mail(Job::MailAlsoCommented, self.recipient_id, actor.id, comment.id) end end + +private + def self.concatenate_or_create(recipient, target, actor, action) + if n = Notification.where(:target_id => target.id, + :action => action, + :recipient_id => recipient.id).first + n.actors << NotificationActor.new(:actor => actor) + n.save! + n + else + n = make_notification(recipient, target, actor, action) + end + end + + def self.make_notification(recipient, target, actor, action) + n = Notification.new(:target_id => target.id, + :action => action, + :recipient_id => recipient.id) + n.actors << NotificationActor.new(:actor => actor) + n.save! + n + end end diff --git a/app/models/person.rb b/app/models/person.rb index d75191d04..01d0474fb 100644 --- a/app/models/person.rb +++ b/app/models/person.rb @@ -186,6 +186,6 @@ class Person < ActiveRecord::Base Post.where(:person_id => id).delete_all Comment.where(:person_id => id).delete_all Contact.where(:person_id => id).delete_all - Notification.where(:actor_id => id).delete_all + Notification.joins(:notification_actors).where(:notification_actors => {:actor_id => self}).delete_all end end diff --git a/app/views/notifications/_popup.haml b/app/views/notifications/_popup.haml index 8293a4af9..5f4a17a6d 100644 --- a/app/views/notifications/_popup.haml +++ b/app/views/notifications/_popup.haml @@ -2,5 +2,5 @@ -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. -= link_to "#{person.name.titleize}", person_path(person) += link_to "#{person.name.titleize}", person_path(person.id) = object_link(note) diff --git a/app/views/notifications/index.html.haml b/app/views/notifications/index.html.haml index 3b5d52bad..c7163b635 100644 --- a/app/views/notifications/index.html.haml +++ b/app/views/notifications/index.html.haml @@ -49,7 +49,7 @@ - notes.each do |note| .stream_element{:data=>{:guid => note.id}, :class => "#{note.unread ? 'unread' : ''}"} %span.from - = link_to "#{note.actor.name.titleize}", person_path(note.actor) + = notification_people_link(note) = object_link(note) %span.time= timeago(note.created_at) diff --git a/app/views/shared/_notification.haml b/app/views/shared/_notification.haml index cb68338c5..5a6d7699e 100644 --- a/app/views/shared/_notification.haml +++ b/app/views/shared/_notification.haml @@ -2,7 +2,7 @@ -# licensed under the Affero General Public License version 3 or later. See -# the COPYRIGHT file. -= link_to t('.new', :type => object.class.to_s, :from => object.person.name), object_path(object.post) += link_to t('.new', :type => object.class.to_s, :from => peoples_names(note)), object_path(object.post) = link_to "#{note.actor.name.titelize}", person_path(note.actor) diff --git a/db/migrate/20110110023610_notification_multiple_people.rb b/db/migrate/20110130072907_notification_multiple_people.rb similarity index 97% rename from db/migrate/20110110023610_notification_multiple_people.rb rename to db/migrate/20110130072907_notification_multiple_people.rb index f1f670c2a..11e42c434 100644 --- a/db/migrate/20110110023610_notification_multiple_people.rb +++ b/db/migrate/20110130072907_notification_multiple_people.rb @@ -36,8 +36,8 @@ class NotificationMultiplePeople < ActiveRecord::Migration "n1.target_type, n1.target_id, n1.recipient_id, n1.action " + "FROM keep_table n1, notifications n2 " + "WHERE n1.keep_id != n2.id " + - "AND n1.target_type = n2.target_type AND n1.target_id = n2.target_id" + - "AND n1.recipient_id = n2.recipient_id AND n1.action = n2.action " + "AND n1.target_type = n2.target_type AND n1.target_id = n2.target_id " + + "AND n1.recipient_id = n2.recipient_id AND n1.action = n2.action " + "AND (n1.action = 'comment_on_post' OR n1.action = 'also_commented'))" #have the notifications actors reference the notifications that need to be kept @@ -51,7 +51,7 @@ class NotificationMultiplePeople < ActiveRecord::Migration "WHERE notifications.id = keep_delete.delete_id " - remove_column :notification, :actor_id + remove_column :notifications, :actor_id end def self.down diff --git a/db/schema.rb b/db/schema.rb index 2fa0551f1..78e1b15fe 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20110127000953) do +ActiveRecord::Schema.define(:version => 20110130072907) do create_table "aspect_memberships", :force => true do |t| t.integer "aspect_id", :null => false @@ -47,6 +47,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.text "youtube_titles" t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "comments", ["guid"], :name => "index_comments_on_guid", :unique => true @@ -60,6 +61,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.boolean "pending", :default => true, :null => false t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "contacts", ["mongo_id"], :name => "index_contacts_on_mongo_id" @@ -84,6 +86,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.integer "aspect_id" t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "invitations", ["aspect_id"], :name => "index_invitations_on_aspect_id" @@ -305,11 +308,11 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.string "target_type" t.integer "target_id" t.integer "recipient_id", :null => false - t.integer "actor_id", :null => false t.string "action", :null => false t.boolean "unread", :default => true, :null => false t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "notifications", ["mongo_id"], :name => "index_notifications_on_mongo_id" @@ -325,6 +328,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.integer "owner_id" t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "people", ["diaspora_handle"], :name => "index_people_on_diaspora_handle", :unique => true @@ -360,6 +364,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.text "youtube_titles" t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "posts", ["guid"], :name => "index_posts_on_guid" @@ -384,6 +389,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.integer "person_id", :null => false t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "profiles", ["first_name", "last_name", "searchable"], :name => "index_profiles_on_first_name_and_last_name_and_searchable" @@ -398,6 +404,7 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.integer "aspect_id" t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" end add_index "requests", ["mongo_id"], :name => "index_requests_on_mongo_id" @@ -414,6 +421,8 @@ ActiveRecord::Schema.define(:version => 20110127000953) do t.string "nickname" t.datetime "created_at" t.datetime "updated_at" + t.string "mongo_id" + t.string "user_mongo_id" end add_index "services", ["mongo_id"], :name => "index_services_on_mongo_id" diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 1e15ce06e..61af92feb 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -14,9 +14,10 @@ describe Notification do @opts = {:target_id => @sm.id, :target_type => @sm.class.name, :action => "comment_on_post", - :actor_id => @person.id, + :actors => [@person], :recipient_id => @user.id} @note = Notification.new(@opts) + @note.actors =[ @person] end it 'contains a type' do @@ -27,8 +28,9 @@ describe Notification do @note.target_id.should == @sm.id end - it 'contains a person_id' do - @note.actor_id == @person.id + + it 'has many people' do + @note.associations[:people].type.should == :many end describe '.for' do @@ -47,7 +49,7 @@ describe Notification do describe '.notify' do it 'does not call Notification.create if the object does not have a notification_type' do - Notification.should_not_receive(:create) + Notification.should_not_receive(:make_notificatin) Notification.notify(@user, @sm, @person) end context 'with a request' do @@ -55,7 +57,7 @@ describe Notification do @request = Request.diaspora_initialize(:from => @user.person, :to => @user2.person, :into => @aspect) end it 'calls Notification.create if the object has a notification_type' do - Notification.should_receive(:create).once + Notification.should_receive(:make_notification).once Notification.notify(@user, @request, @person) end @@ -84,9 +86,17 @@ describe Notification do n.stub!(:recipient).and_return @user @user.should_receive(:mail) - n.email_the_user + n.email_the_user("mock", @person) end end + + it "updates the notification with a more people if one already exists" do + @user3 = bob + sm = @user3.post(:status_message, :message => "comment!", :to => :all) + @user3.receive_object(@user2.reload.comment("hey", :on => sm), @user2.person) + @user3.receive_object(@user.reload.comment("way", :on => sm), @user.person) + Notification.where(:user_id => @user.id,:target_id => sm.id).first.people.count.should == 2 + end end end end diff --git a/spec/models/person_spec.rb b/spec/models/person_spec.rb index 2756cd618..0a80469f4 100644 --- a/spec/models/person_spec.rb +++ b/spec/models/person_spec.rb @@ -106,7 +106,7 @@ describe Person do end it "deletes all notifications from a person's actions" do - note = Factory(:notification, :actor => @deleter, :recipient => @user) + note = Factory(:notification, :actors => [@deleter], :recipient => @user) @deleter.destroy Notification.where(:id => note.id).first.should be_nil end